Re: [PATCH 0/3] quarantine-push loose ends
- Date: Tue, 11 Apr 2017 00:51:39 +0200
- From: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
- Subject: Re: [PATCH 0/3] quarantine-push loose ends
On Tue, Apr 11, 2017 at 12:10 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Mon, Apr 10, 2017 at 05:14:24PM -0400, Jeff King wrote:
>> > Also, I think this whole thing could really do with some documentation
>> > in githooks(5). E.g. what hooks does it apply for? The test is just
>> > for pre-receive but the patch changes run_update_hook(), does it also
>> > take effect for update? Ditto the caveat about update-ref.
>> My thinking was that the cases where the effects were user-visible were
>> sufficiently insane that nobody would need to know or care when the
>> feature was in use.
> I guess it can't hurt to write about it, though. Here's that, plus a
> cleanup on the stray tmp_objdir_env() call you noticed. The final patch
> provides some safety for the ref-update case. My assumption all along
> has been that nobody would want to update random refs from inside the
> pre-receive hook. This doubles down on that by making it forbidden. I
> don't think that's a big loss, because doing so now is extremely
> dangerous. If that assumption is wrong, the correct path forward is to
> make the quarantining configurable.
Thanks a lot. Cleared up all the questions I have, and now we have
permanent docs & more testing for them.
> [1/3]: receive-pack: drop tmp_objdir_env from run_update_hook
> [2/3]: receive-pack: document user-visible quarantine effects
> [3/3]: refs: reject ref updates while GIT_QUARANTINE_PATH is set
> Documentation/git-receive-pack.txt | 29 +++++++++++++++++++++++++++++
> Documentation/githooks.txt | 3 +++
> builtin/receive-pack.c | 1 -
> refs.c | 6 ++++++
> t/t5547-push-quarantine.sh | 11 +++++++++++
> 5 files changed, 49 insertions(+), 1 deletion(-)