git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Damien Marié" <damien@dam.io>
Subject: Re: [PATCH v3] run-command: add hint when a hook is ignored
Date: Fri, 05 Jan 2018 11:36:39 -0800	[thread overview]
Message-ID: <xmqqk1wwcd2w.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20180103083145.GA7049@sigill.intra.peff.net> (Jeff King's message of "Wed, 3 Jan 2018 03:31:46 -0500")

Jeff King <peff@peff.net> writes:

> The problem is that I was bisecting an unrelated change in old code, and
> I built a commit which predates f98f8cbac0 (Ship sample hooks with .sample
> suffix, 2008-06-24). That wrote a bare "update" file into templates/blt
> (without the ".sample" suffix). After that, the cruft is forever in my
> build directory until I "git clean -x".
>
> The t5523 script tries to run "git push --quiet" and make sure that it
> produces no output, but with this setup it produces a round of "hint"
> lines (actually, they come from receive-pack on the remote end but still
> end up on stderr).
>
> So that raises a few interesting questions to me:
>
>   1. Should receive-pack generate these hints? They get propagated by
>      the client, but there's a reasonable chance that the user can't
>      actually fix the situation.

True.  The user could tell the server operator to rename them or
remove them because they are not doing anything useful, but then
as long as everybody knows they are not doing anything, it is OK
to leave that sleeping dog lie, as they are not doing anything
harmful anyway.

That brings us one step further back to question if the hints are
useful in the first place, though ;-).

>   2. Should these hints be suppressed with --quiet? I can see an
>      argument that "--quiet" only applies to non-errors. And while these
>      are not fatal errors, they're outside the realm of the usual
>      chattiness.

I do not think having a non-executable file whose name happens to be
the same as a hook is an error in the first place [*1*], so I do not
think it is unreasonable for --quiet to squelch.

	side note [*1*]: we need to make sure that it is clearly
	documented that such a file is not a hook, of course.

>   3. Should our tests be more careful about not looking at the
>      template hooks? I think test_create_repo already disables the hooks
>      directory manually, but many repos will be created by "git clone"
>      during the tests.

This probably is much deeper issue and templates/hooks--* is merely
a tip of iceberg, I suspect.  In general build artifacts from
different versions of Git in the same build area could interfere the
tests in other ways (e.g. a test for "git help" output could try to
grab a deprecated command left in bin-wrappers/ from previous build
of an ancient version), and could potentially interfere things other
than the tests (e.g. the 'install' target of the Makefile may be
written loosely to install everything in a directory).

In general, I am a bit pessimistic and suspect that the best we
could say is "if you want to absolutely avoid such interference,
'make distclean' before switching to test another revision".

>   4. Should our build system be more clever about dropping non-existent
>      files from templates/blt?

It would be a reasonable mitigation that is specific to templates/,
I suspect.

  reply	other threads:[~2018-01-05 19:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-03 10:59 [PATCH] run-command.c: add hint when hook is not executable Damien
2017-10-04  4:40 ` Junio C Hamano
2017-10-05 20:48   ` Damien
2017-10-05 20:53 ` [PATCH v2] run-command: add hint when a hook is ignored Damien Marié
2017-10-06  4:52   ` Junio C Hamano
2017-10-06  5:25     ` Junio C Hamano
2017-10-06  5:43     ` Junio C Hamano
2017-10-06  5:53     ` Junio C Hamano
2017-10-06  8:04       ` Damien
2017-10-06  8:07       ` [PATCH v3] " Damien Marié
2017-10-10  4:21         ` Junio C Hamano
2017-10-11  6:26           ` Junio C Hamano
2018-01-03  8:31             ` Jeff King
2018-01-05 19:36               ` Junio C Hamano [this message]
2018-03-03  7:16                 ` Jeff King
2018-03-05 21:19                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk1wwcd2w.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=damien@dam.io \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).