From: Sverre Rabbelier Date: Wed, 10 Apr 2013 21:22:44 GMT Subject: Re: [PATCH 1/2] transport-helper: report errors properly Message-ID: In-Reply-To: <20130410211552.GA3256@sigill.intra.peff.net> On Wed, Apr 10, 2013 at 2:15 PM, Jeff King wrote: > From: Felipe Contreras > > If a push fails because the remote-helper died (with > fast-export), the user does not see any error message. We do > correctly die with a failed exit code, as we notice that the > helper has died while reading back the ref status from the > helper. However, we don't print any message. This is OK if > the helper itself printed a useful error message, but we > cannot count on that; let's let the user know that the > helper failed. > > In the long run, it may make more sense to propagate the > error back up to push, so that it can present the usual > status table and give a nicer message. But this is a much > simpler fix that can help immediately. > > While we're adding tests, let's also confirm that the > remote-helper dying is also detect when importing refs. We > currently do so robustly when the helper uses the "done" > feature (and that is what we test). We cannot do so > reliably when the helper does not use the "done" feature, > but it is not even worth testing; the right solution is for > the helper to start using "done". > > Suggested-by: Jeff King > Signed-off-by: Felipe Contreras > Signed-off-by: Jeff King The fixes you made to this patch make a lot of sense, glad to not have a 'sleep 1' in our tests. Acked-by: Sverre Rabbelier -- Cheers, Sverre Rabbelier