Skip to content

Commit

Permalink
receive-pack: send pack-processing stderr over sideband
Browse files Browse the repository at this point in the history
Receive-pack invokes either unpack-objects or index-pack to
handle the incoming pack. However, we do not redirect the
stderr of the sub-processes at all, so it is never seen by
the client. From the initial thread adding sideband support,
which is here:

  http://thread.gmane.org/gmane.comp.version-control.git/139471

it is clear that some messages are specifically kept off the
sideband (with the assumption that they are of interest only
to an administrator, not the client). The stderr of the
subprocesses is mentioned in the thread, but it's unclear if
they are included in that group, or were simply forgotten.

However, there are a few good reasons to show them to the
client:

  1. In many cases, they are directly about the incoming
     packfile (e.g., fsck warnings with --strict, corruption
     in the packfile, etc). Without these messages, the
     client just gets "unpacker error" with no extra useful
     diagnosis.

  2. No matter what the cause, we are probably better off
     showing the errors to the client. If the client and the
     server admin are not the same entity, it is probably
     much easier for the client to cut-and-paste the errors
     they see than for the admin to try to dig them out of a
     log and correlate them with a particular session.

  3. Users of the ssh transport typically already see these
     stderr messages, as the remote's stderr is copied
     literally by ssh. This brings other transports (http,
     and push-over-git if you are crazy enough to enable it)
     more in line with ssh. As a bonus for ssh users,
     because the messages are now fed through the sideband
     and printed by the local git, they will have "remote:"
     prepended and be properly interleaved with any local
     output to stderr.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Sep 21, 2012
1 parent 59bfdfb commit a22e6f8
Showing 1 changed file with 24 additions and 2 deletions.
26 changes: 24 additions & 2 deletions builtin/receive-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ static const char *parse_pack_header(struct pack_header *hdr)

static const char *pack_lockfile;

static const char *unpack(void)
static const char *unpack(int err_fd)
{
struct pack_header hdr;
const char *hdr_err;
Expand Down Expand Up @@ -833,6 +833,7 @@ static const char *unpack(void)
memset(&child, 0, sizeof(child));
child.argv = unpacker;
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
code = run_command(&child);
if (!code)
Expand All @@ -859,6 +860,7 @@ static const char *unpack(void)
memset(&ip, 0, sizeof(ip));
ip.argv = keeper;
ip.out = -1;
ip.err = err_fd;
ip.git_cmd = 1;
status = start_command(&ip);
if (status) {
Expand All @@ -875,6 +877,26 @@ static const char *unpack(void)
}
}

static const char *unpack_with_sideband(void)
{
struct async muxer;
const char *ret;

if (!use_sideband)
return unpack(0);

memset(&muxer, 0, sizeof(muxer));
muxer.proc = copy_to_sideband;
muxer.in = -1;
if (start_async(&muxer))
return NULL;

ret = unpack(muxer.in);

finish_async(&muxer);
return ret;
}

static void report(struct command *commands, const char *unpack_status)
{
struct command *cmd;
Expand Down Expand Up @@ -972,7 +994,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
const char *unpack_status = NULL;

if (!delete_only(commands))
unpack_status = unpack();
unpack_status = unpack_with_sideband();
execute_commands(commands, unpack_status);
if (pack_lockfile)
unlink_or_warn(pack_lockfile);
Expand Down

0 comments on commit a22e6f8

Please sign in to comment.