receive-pack: send pack-processing stderr over sideband

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>

Jeff King committed Sep 21, 2012 at 01:34 UTC a22e6f85470f47d33ce392a11a5f9e2bb9a5c308
1 file changed +24 -2
builtin/receive-pack.c
+24 -2
@@ -801,7 +801,7 @@ static const char *parse_pack_header(struct pack_header *hdr)
801
802 static const char *pack_lockfile;
803
804 -static const char *unpack(void)
804 +static const char *unpack(int err_fd)
805 {
806 struct pack_header hdr;
807 const char *hdr_err;
@@ -833,6 +833,7 @@ static const char *unpack(void)
833 memset(&child, 0, sizeof(child));
834 child.argv = unpacker;
835 child.no_stdout = 1;
836 + child.err = err_fd;
837 child.git_cmd = 1;
838 code = run_command(&child);
839 if (!code)
@@ -859,6 +860,7 @@ static const char *unpack(void)
860 memset(&ip, 0, sizeof(ip));
861 ip.argv = keeper;
862 ip.out = -1;
863 + ip.err = err_fd;
864 ip.git_cmd = 1;
865 status = start_command(&ip);
866 if (status) {
@@ -875,6 +877,26 @@ static const char *unpack(void)
877 }
878 }
879
880 +static const char *unpack_with_sideband(void)
881 +{
882 + struct async muxer;
883 + const char *ret;
884 +
885 + if (!use_sideband)
886 + return unpack(0);
887 +
888 + memset(&muxer, 0, sizeof(muxer));
889 + muxer.proc = copy_to_sideband;
890 + muxer.in = -1;
891 + if (start_async(&muxer))
892 + return NULL;
893 +
894 + ret = unpack(muxer.in);
895 +
896 + finish_async(&muxer);
897 + return ret;
898 +}
899 +
900 static void report(struct command *commands, const char *unpack_status)
901 {
902 struct command *cmd;
@@ -972,7 +994,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
994 const char *unpack_status = NULL;
995
996 if (!delete_only(commands))
975 - unpack_status = unpack();
997 + unpack_status = unpack_with_sideband();
998 execute_commands(commands, unpack_status);
999 if (pack_lockfile)
1000 unlink_or_warn(pack_lockfile);