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

From: Robin Jarry <robin@jarry.cc>
Date: Nov 9, 2021, 21:38 UTC
Message-ID: <CFLKOIVJ8EX0.2PWQ6PCXZ340A@diabtop>
In-Reply-To: <xmqqzgqd11dp.fsf@gitster.g>

Hi Junio,

Junio C Hamano, Nov 09, 2021 at 22:10:

Show 17 quoted lines

> 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.

Show 8 quoted lines

> >  		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