print an error when remote helpers die during capabilities
The transport-helper code generally relies on the remote-helper to provide an informative message to the user when it encounters an error. In the rare cases where the helper does not do so, the output can be quite confusing. E.g.: $ git clone https://example.com/foo.git Cloning into 'foo'... $ echo $? 128 $ ls foo /bin/ls: cannot access foo: No such file or directory We tried to address this withmaint81d340d(transport-helper: report errors properly, 2013-04-10). But that makes the common case much more confusing. The remote helper protocol's method for signaling normal errors is to simply hang up. So when the helper does encounter a routine error and prints something to stderr, the extra error message is redundant and misleading. So we dropped it again in266f1fd(transport-helper: be quiet on read errors from helpers, 2013-06-21). This puts the uncommon case right back where it started. We may be able to do a little better, though. It is common for the helper to die during a "real" command, like fetching the list of remote refs. It is not common for it to die during the initial "capabilities" negotiation, right after we start. Reporting failure here is likely to catch fundamental problems that prevent the helper from running (and reporting errors) at all. Anything after that is the responsibility of the helper itself to report. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
parent
dbecc617f7
commit
6e7fac9bca
|
|
@ -321,4 +321,15 @@ test_expect_success 'fetch tag' '
|
|||
compare_refs local v1.0 server v1.0
|
||||
'
|
||||
|
||||
test_expect_success 'totally broken helper reports failure message' '
|
||||
write_script git-remote-broken <<-\EOF &&
|
||||
read cap_cmd
|
||||
exit 1
|
||||
EOF
|
||||
test_must_fail \
|
||||
env PATH="$PWD:$PATH" \
|
||||
git clone broken://example.com/foo.git 2>stderr &&
|
||||
grep aborted stderr
|
||||
'
|
||||
|
||||
test_done
|
||||
|
|
|
|||
|
|
@ -83,11 +83,18 @@ static int recvline(struct helper_data *helper, struct strbuf *buffer)
|
|||
return recvline_fh(helper->out, buffer);
|
||||
}
|
||||
|
||||
static void write_constant(int fd, const char *str)
|
||||
static int write_constant_gently(int fd, const char *str)
|
||||
{
|
||||
if (debug)
|
||||
fprintf(stderr, "Debug: Remote helper: -> %s", str);
|
||||
if (write_in_full(fd, str, strlen(str)) < 0)
|
||||
return -1;
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void write_constant(int fd, const char *str)
|
||||
{
|
||||
if (write_constant_gently(fd, str) < 0)
|
||||
die_errno(_("full write to remote helper failed"));
|
||||
}
|
||||
|
||||
|
|
@ -161,13 +168,16 @@ static struct child_process *get_helper(struct transport *transport)
|
|||
die_errno(_("can't dup helper output fd"));
|
||||
data->out = xfdopen(duped, "r");
|
||||
|
||||
write_constant(helper->in, "capabilities\n");
|
||||
sigchain_push(SIGPIPE, SIG_IGN);
|
||||
if (write_constant_gently(helper->in, "capabilities\n") < 0)
|
||||
die("remote helper '%s' aborted session", data->name);
|
||||
sigchain_pop(SIGPIPE);
|
||||
|
||||
while (1) {
|
||||
const char *capname, *arg;
|
||||
int mandatory = 0;
|
||||
if (recvline(data, &buf))
|
||||
exit(128);
|
||||
die("remote helper '%s' aborted session", data->name);
|
||||
|
||||
if (!*buf.buf)
|
||||
break;
|
||||
|
|
|
|||
Loading…
Reference in New Issue