From: Jeff King Date: Wed, 22 Feb 2012 20:40:50 GMT Subject: Re: [PATCH] remote-curl: Fix push status report when all branches fail Message-ID: <20120222204050.GB6781@sigill.intra.peff.net> In-Reply-To: On Wed, Feb 22, 2012 at 07:22:10AM -0800, Shawn O. Pearce wrote: > > +                       /* > > +                        * Ignore write errors; there's nothing we can do, > > +                        * since we're about to close the pipe anyway. And the > > +                        * most likely error is EPIPE due to the helper dying > > +                        * to report an error itself. > > +                        */ > > +                       sigchain_push(SIGPIPE, SIG_IGN); > > +                       xwrite(data->helper->in, "\n", 1); > > +                       sigchain_pop(SIGPIPE); > [...] > > This sounds right to me. Its unfortunate that we missed the error > status output when we built the remote helper protocol, but your patch > above might be the best we can do now. > > Eh, well, actually we could have the helper advertise a new capability > that can be enabled to return exit status. That is a much bigger > change, and even if we do it for remote-curl (since that is in tree > and easy to update) we still need your patch for the same race > condition for out of tree helpers (which Google actually has so I care > about out of tree helpers too). I don't think it's worth a new capability. This is one of those "it would be nice if it were designed that way from day one" cases, but it wasn't. And while this is a minor hack, I don't think it has any functional downsides. So adding a new capability on top of the hack just makes things more complex. I'll re-send the patch with a stand-alone commit message. -Peff