Re: Bugreport: Git responds with stderr instead of stdout

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sun, Apr 25, 2010 at 19:32, Jeff King <peff@peff.net> wrote:
> On Sun, Apr 25, 2010 at 07:32:00PM +0000, Ævar Arnfjörð Bjarmason wrote:
>
>> >> I've had some issues scripting `git fetch` because on error it'll
>> >> print to stdout and not stderr.
>> >
>> > Errors should go to stderr, so I imagine patches would be welcome. Which
>> > messages went to stdout?
>>
>> I can't recall exactly now. Looking at fetch.c I can't see anything
>> obvious, I'll report anything if I spot it in the future.
>
> Thanks. As I mentioned, we've been fixing little things like this as
> time goes on, so it may well have been fixed already.

Actually here's an example with Git 1.7.1:

    # time /etc/github-backup/github-backup
    remote: Counting objects: 76, done.
    remote: Compressing objects: 100% (43/43), done.
    remote: Total 47 (delta 26), reused 18 (delta 4)
    Unpacking objects: 100% (47/47), done.
    From github.com:avar/linode-etc
       75a27cf..09d5ff7  master     -> origin/master
    From github.com:avar/svn-dump-fast-export
     * [new branch]      gh-pages   -> origin/gh-pages
     * [new branch]      git-merge  -> origin/git-merge
     * [new branch]      master     -> origin/master
     * [new branch]      rollout    -> origin/rollout

The script I'm running is github-backup
(http://github.com/avar/github-backup) which just outputs `git fetch`
output as-is.

Looking at the source the problematic code is in builtin/fetch.c's
update_local_ref. That function takes a char *display which it writes
to things that are both errors and just status messages:

Error:

		sprintf(display, "! %-*s %-*s -> %s  (can't fetch in current branch)",
			TRANSPORT_SUMMARY_WIDTH, "[rejected]", REFCOL_WIDTH, remote,
			pretty_ref);

Just a status message (in my case):

		else {
			msg = "storing head";
			what = "[new branch]";
		}

		r = s_update_ref(msg, ref, 0);
		sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '*',
			TRANSPORT_SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,
			r ? "  (unable to update local ref)" : "");

That function is then called as:

		if (ref) {
			rc |= update_local_ref(ref, what, note);
			free(ref);
		} else
			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
				TRANSPORT_SUMMARY_WIDTH, *kind ? kind : "branch",
				 REFCOL_WIDTH, *what ? what : "HEAD");
		if (*note) {
			if (verbosity >= 0 && !shown_url) {
				fprintf(stderr, "From %.*s\n",
						url_len, url);
				shown_url = 1;
			}
			if (verbosity >= 0)
				fprintf(stderr, " %s\n", note);
		}

Shouldn't that fprintf() be called as:

    fprintf((rc ? stderr : stdout), ...)

?