From: Junio C Hamano <firstname.lastname@example.org> To: Jeff King <email@example.com> Cc: firstname.lastname@example.org, Damien Marié <email@example.com> Subject: Re: [PATCH v3] run-command: add hint when a hook is ignored Date: Fri, 05 Jan 2018 11:36:39 -0800 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <20180103083145.GA7049@sigill.intra.peff.net> Jeff King <email@example.com> 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.
next prev parent reply index Thread overview: 15+ 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 publically 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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
email@example.com mailing list mirror (one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.org/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ or Tor2web: https://www.tor2web.org/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox