Re: Scripted clone generating an incomplete, unusable .git/config

From: Jeff King <peff@peff.net>
Date: Nov 11, 2010, 19:05 UTC
Message-ID: <20101111190508.GA3038@sigill.intra.peff.net>
In-Reply-To: <20101111184829.GG16972@burratino>

On Thu, Nov 11, 2010 at 12:48:29PM -0600, Jonathan Nieder wrote:

> So why does git clone keep running after the first remove_junk() call?
> It seems that the signal is initially set up (by Python's popen()?) as
> SIG_IGN.  I guess "git clone" should explicitly override that to be
> SIG_DFL?

I was tracing this earlier today, too, and got sidetracked. But I got to the same confusing point: why doesn't it die after cleaning up? It looks like we inherit SIG_IGN for SIGPIPE from the parent python process.

I don't think it makes sense for git-clone to do this itself. If we are going to say "SIGPIPE should default to SIG_DFL on startup" then we should do it as the very first thing in the git wrapper, not just for git-clone. That gives each git program a known starting point of behavior.

But I wonder if we should perhaps just be ignoring SIGPIPE in this instance instead. There isn't a real error here; we just ended up not being able to write some useless progress report to stdout. There's no reason to fail.

Note that we probably don't want to ignore SIGPIPE for all of git; many of the output-producing programs rely on it for early termination when the user closes the pager. But for clone, it makes sense.

Show 6 quoted lines

> Here's a proof of concept.  It is not very good because it overrides
> any previously set sigchain handlers (in case the "git" wrappers
> start to use one) and because using SIG_DFL as a sigchain_fun feels
> like violating an abstraction.
> [...]
> +	sigchain_push_common(SIG_DFL);

I don't think your patch is the right solution, but FWIW, sigchain was explicitly intended to be able to take SIG_DFL and SIG_IGN. Probably sigchain_fun should be removed and we should just use sighandler_t explicitly (I think in initial versions they were not the same thing, and I realized the error of my ways, but the sigchain_fun typedef hung around anyway).

-Peff