Re: [PATCH v2] receive-pack: ignore SIGPIPE while reporting status to client

["Robin Jarry" <robin@jarry.cc>]

Hi Junio,

Junio C Hamano, Nov 09, 2021 at 22:10:
> All of the above talks about the pre-receive hook, but it is unclear
> how that is relevant to this change.  The first paragraph says
> "... is not killed", and if that was a bad thing (in other words, it
> should be killed but is not, and that is a bug worth fixing), and if
> this patch changes the behaviour, then that paragraph is worth
> saying.  Similarly for the other two.
>
> > Before running the post-receive hook, status info is reported back to
> > the client. Since the client has died, receive-pack is killed by SIGPIPE
> > and post-receive is never executed.
>
> In other words, regardless of what happens (or does not happen) to
> the pre-receive hook, which may not even exist, if "git push" dies
> before the post-receive hook runs, this paragraph applies, no?  
>
> What I am getting at is that this can (and should) be the first
> paragraph of the description without losing clarity.

You're right. I wanted to give context to better explain why
receive-pack is not killed while running the pre-receive hook but
afterwards. This should go into another commit which fixes that issue.

I will reword accordingly.

> >  		execute_commands(commands, unpack_status, &si,
> >  				 &push_options);
> > +		sigchain_push(SIGPIPE, SIG_IGN);
> >  		if (pack_lockfile)
> >  			unlink_or_warn(pack_lockfile);
>
> Shouldn't we start ignoring SIGPIPE here, not before we try to
> unlink the lockfile?

I initially wanted to avoid getting SIGPIPE'd while printing a warning
if the lockfile cannot be unlinked. Maybe this means the repository
integrity is compromised and we are well beyond ensuring post-receive is
executed or not. I do not know git internals well enough to be sure.

What do you think?

-- 
Robin