* Bugreport: Git responds with stderr instead of stdout @ 2010-04-25 18:06 Jack Desert 2010-04-25 18:10 ` Jacob Helwig 0 siblings, 1 reply; 19+ messages in thread From: Jack Desert @ 2010-04-25 18:06 UTC (permalink / raw) To: git I think I found a bug in Git. When I run the command git checkout -b new_branch Git does exactly what I've asked, except that Git's response: Switched to a new branch 'new_branch' comes through the stderr pipe instead of through the stdout pipe. Where do I file a bug report for this? I am using Git 1.6.3.3, Ubuntu 9.10 -Jack -- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Jack Desert -- Writer, Entrepeneur Author and Spokesman: www.LetsEATalready.com Software Developer: http://GrooveTask.org Email: JackDesert556@gmail.com ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Bugreport: Git responds with stderr instead of stdout 2010-04-25 18:06 Bugreport: Git responds with stderr instead of stdout Jack Desert @ 2010-04-25 18:10 ` Jacob Helwig 2010-04-25 18:24 ` Ævar Arnfjörð Bjarmason 2010-04-25 18:34 ` Bugreport: Git responds with stderr instead of stdout Jack Desert 0 siblings, 2 replies; 19+ messages in thread From: Jacob Helwig @ 2010-04-25 18:10 UTC (permalink / raw) To: Jack Desert; +Cc: git On Sun, Apr 25, 2010 at 11:06, Jack Desert <jackdesert556@gmail.com> wrote: > I think I found a bug in Git. When I run the command > > git checkout -b new_branch > > Git does exactly what I've asked, except that Git's response: > > Switched to a new branch 'new_branch' > > comes through the stderr pipe instead of through the stdout pipe. Where do I file a bug report for this? > > I am using Git 1.6.3.3, Ubuntu 9.10 > > -Jack > > I can't really say if it's actually a bug, or not, but as to your question about where to file a bug report: You just did. This mailing list is the correct place. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Bugreport: Git responds with stderr instead of stdout 2010-04-25 18:10 ` Jacob Helwig @ 2010-04-25 18:24 ` Ævar Arnfjörð Bjarmason 2010-04-25 19:22 ` Jeff King 2010-04-25 18:34 ` Bugreport: Git responds with stderr instead of stdout Jack Desert 1 sibling, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-04-25 18:24 UTC (permalink / raw) To: Jacob Helwig; +Cc: Jack Desert, git On Sun, Apr 25, 2010 at 18:10, Jacob Helwig <jacob.helwig@gmail.com> wrote: > I can't really say if it's actually a bug, or not, but as to your > question about where to file a bug report: You just did. This mailing > list is the correct place. I've had some issues scripting `git fetch` because on error it'll print to stdout and not stderr. Are there some general guidelines for git's utilities that they follow in this regard or does each tool just do its own thing? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Bugreport: Git responds with stderr instead of stdout 2010-04-25 18:24 ` Ævar Arnfjörð Bjarmason @ 2010-04-25 19:22 ` Jeff King 2010-04-25 19:32 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2010-04-25 19:22 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Jacob Helwig, Jack Desert, git On Sun, Apr 25, 2010 at 06:24:43PM +0000, Ævar Arnfjörð Bjarmason wrote: > On Sun, Apr 25, 2010 at 18:10, Jacob Helwig <jacob.helwig@gmail.com> wrote: > > I can't really say if it's actually a bug, or not, but as to your > > question about where to file a bug report: You just did. This mailing > > list is the correct place. > > 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? > Are there some general guidelines for git's utilities that they follow > in this regard or does each tool just do its own thing? In practice, each tool does its own thing because they evolved differently and from different authors. I think we are slowly converging on similar behavior, though, as people fix warts. As to exactly what that behavior is, I don't know that anybody has ever enumerated it exactly. Verbose status and progress reports, especially human readable ones, should probably always go to stderr. The "Switched to a new branch" message that started this thread is correct to go to stderr. If you want to silence the message but keep stderr open for actual errors, the right way is to use "-q". I tend to think the only thing that should go to stdout is the "main" output of a command. For something like "ls-files", that is obviously the list of files. For something like "checkout", which is about changing the repository and not about querying it, I think there is probably nothing that makes sense on stdout. -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Bugreport: Git responds with stderr instead of stdout 2010-04-25 19:22 ` Jeff King @ 2010-04-25 19:32 ` Ævar Arnfjörð Bjarmason 2010-04-25 19:32 ` Jeff King 0 siblings, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-04-25 19:32 UTC (permalink / raw) To: Jeff King; +Cc: Jacob Helwig, Jack Desert, git On Sun, Apr 25, 2010 at 19:22, Jeff King <peff@peff.net> wrote: > On Sun, Apr 25, 2010 at 06:24:43PM +0000, Ævar Arnfjörð Bjarmason wrote: > >> On Sun, Apr 25, 2010 at 18:10, Jacob Helwig <jacob.helwig@gmail.com> wrote: >> > I can't really say if it's actually a bug, or not, but as to your >> > question about where to file a bug report: You just did. This mailing >> > list is the correct place. >> >> 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Bugreport: Git responds with stderr instead of stdout 2010-04-25 19:32 ` Ævar Arnfjörð Bjarmason @ 2010-04-25 19:32 ` Jeff King 2010-06-12 16:52 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2010-04-25 19:32 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Jacob Helwig, Jack Desert, git 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. -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Bugreport: Git responds with stderr instead of stdout 2010-04-25 19:32 ` Jeff King @ 2010-06-12 16:52 ` Ævar Arnfjörð Bjarmason 2010-06-24 22:34 ` [PATCH] fetch: don't output non-errors on stderr Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-06-12 16:52 UTC (permalink / raw) To: Jeff King; +Cc: Jacob Helwig, Jack Desert, git 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), ...) ? ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] fetch: don't output non-errors on stderr 2010-06-12 16:52 ` Ævar Arnfjörð Bjarmason @ 2010-06-24 22:34 ` Ævar Arnfjörð Bjarmason 2010-06-25 13:30 ` [PATCH v2] " Ævar Arnfjörð Bjarmason 2010-06-25 17:25 ` [PATCH] " Junio C Hamano 0 siblings, 2 replies; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-06-24 22:34 UTC (permalink / raw) To: git; +Cc: Jeff King, Junio C Hamano, Ævar Arnfjörð Bjarmason Change git-fetch to only print to stderr if it has encountered an error. A normal branch update (like "* branch HEAD -> FETCH_HEAD") is no longer output to stderr but on stdout. Genuine errors (like "[rejected]" messages) still go to stderr. With this change I can run a cron script I've been developing (http://github.com/avar/github-backup) without redirecting stderr to /dev/null. Before the change error messages were drowned out by git-fetch's non-error update notices, which didn't need my attention. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- On Sat, Jun 12, 2010 at 16:52, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > Shouldn't that fprintf() be called as: > > fprintf((rc ? stderr : stdout), ...) To answer my own question: Yes it should. This patch fixes git-fetch so that it doesn't taint stderr with non-error messages. The small changes to the test suite that this requires is a testiment to how bad our test coverage is in this area. As far as I can see the error messages that update_local_ref can emit aren't being tested for. Fixing that is outside the scope of this patch, however. builtin/fetch.c | 5 +++-- t/t5521-pull-options.sh | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 5cb369c..116b322 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -397,13 +397,14 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, TRANSPORT_SUMMARY_WIDTH, *kind ? kind : "branch", REFCOL_WIDTH, *what ? what : "HEAD"); if (*note) { + FILE *fout = rc ? stderr : stdout; if (verbosity >= 0 && !shown_url) { - fprintf(stderr, "From %.*s\n", + fprintf(fout, "From %.*s\n", url_len, url); shown_url = 1; } if (verbosity >= 0) - fprintf(stderr, " %s\n", note); + fprintf(fout, " %s\n", note); } } free(url); diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index 1b06691..7ec36e7 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -23,16 +23,16 @@ test_expect_success 'git pull' ' mkdir cloned && (cd cloned && git init && git pull "../parent" >out 2>err && - test -s err && - test ! -s out) + test ! -s err && + test -s out) ' test_expect_success 'git pull -v' ' mkdir clonedv && (cd clonedv && git init && git pull -v "../parent" >out 2>err && - test -s err && - test ! -s out) + test ! -s err && + test -s out) ' test_expect_success 'git pull -v -q' ' @@ -47,8 +47,8 @@ test_expect_success 'git pull -q -v' ' mkdir clonedqv && (cd clonedqv && git init && git pull -q -v "../parent" >out 2>err && - test ! -s out && - test -s err) + test ! -s err && + test -s out) ' test_expect_success 'git pull --force' ' -- 1.7.1.251.g92a7 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2] fetch: don't output non-errors on stderr 2010-06-24 22:34 ` [PATCH] fetch: don't output non-errors on stderr Ævar Arnfjörð Bjarmason @ 2010-06-25 13:30 ` Ævar Arnfjörð Bjarmason 2010-06-25 17:25 ` [PATCH] " Junio C Hamano 1 sibling, 0 replies; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-06-25 13:30 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason Change git-fetch to only print to stderr if it has encountered an error. A normal branch update (like "* branch HEAD -> FETCH_HEAD") is no longer output to stderr but on stdout. Genuine errors (like "[rejected]" messages) still go to stderr. With this change I can run a cron script I've been developing (http://github.com/avar/github-backup) without redirecting stderr to /dev/null. Before the change error messages were drowned out by git-fetch's non-error update notices, which didn't need my attention. The changes in t/t5521-pull-options.sh invert the previously tested for behavior of checking if normal messages are output on stderr. The changes in t/t5510-fetch.sh however contain no behavioral changes, just assertions that will break if git fetch's behavior is changed again. There still aren't tests for some of the errors output by builtin/fetch.c's update_local_ref function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- On Thu, Jun 24, 2010 at 22:34, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > The small changes to the test suite that this requires is a testiment > to how bad our test coverage is in this area. As far as I can see the > error messages that update_local_ref can emit aren't being tested > for. Fixing that is outside the scope of this patch, however. Here's an updated patch that has a some of those supposedly out of scope tests. The new tests have the same behavior, but will start breaking if this behavior is changed again. builtin/fetch.c | 5 ++- t/t5510-fetch.sh | 57 +++++++++++++++++++++++++++++++++------------- t/t5521-pull-options.sh | 12 +++++----- 3 files changed, 50 insertions(+), 24 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 5cb369c..116b322 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -397,13 +397,14 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, TRANSPORT_SUMMARY_WIDTH, *kind ? kind : "branch", REFCOL_WIDTH, *what ? what : "HEAD"); if (*note) { + FILE *fout = rc ? stderr : stdout; if (verbosity >= 0 && !shown_url) { - fprintf(stderr, "From %.*s\n", + fprintf(fout, "From %.*s\n", url_len, url); shown_url = 1; } if (verbosity >= 0) - fprintf(stderr, " %s\n", note); + fprintf(fout, " %s\n", note); } } free(url); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 4eb10f6..808b256 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -51,7 +51,9 @@ test_expect_success "fetch test" ' echo >file updated by origin && git commit -a -m "updated by origin" && cd two && - git fetch && + git fetch >out 2>err && + test ! -s err && + test -s out && test -f .git/refs/heads/one && mine=`git rev-parse refs/heads/one` && his=`cd ../one && git rev-parse refs/heads/master` && @@ -61,7 +63,9 @@ test_expect_success "fetch test" ' test_expect_success "fetch test for-merge" ' cd "$D" && cd three && - git fetch && + git fetch >out 2>err && + test ! -s err && + test -s out && test -f .git/refs/heads/two && test -f .git/refs/heads/one && master_in_two=`cd ../two && git rev-parse master` && @@ -81,7 +85,9 @@ test_expect_success 'fetch tags when there is no tags' ' cd notags && git init && - git fetch -t .. + git fetch -t .. >out 2>err && + test ! -s err && + test ! -s out ' @@ -95,7 +101,10 @@ test_expect_success 'fetch following tags' ' cd four && git init && - git fetch .. :track && + git fetch .. :track >out 2>err && + test ! -s err && + test -s out && + git show-ref --verify refs/tags/anno && git show-ref --verify refs/tags/light @@ -109,8 +118,9 @@ test_expect_success 'fetch must not resolve short tag name' ' cd five && git init && - test_must_fail git fetch .. anno:five - + ! git fetch .. anno:five >out 2>err && + test -s err && + test ! -s out ' test_expect_success 'fetch must not resolve short remote name' ' @@ -122,7 +132,9 @@ test_expect_success 'fetch must not resolve short remote name' ' cd six && git init && - test_must_fail git fetch .. six:six + ! git fetch .. six:six >out 2>err && + test -s err && + test ! -s out ' @@ -148,7 +160,9 @@ test_expect_success 'create bundle 2' ' test_expect_success 'unbundle 1' ' cd "$D/bundle" && git checkout -b some-branch && - test_must_fail git fetch "$D/bundle1" master:master + ! git fetch "$D/bundle1" master:master >out 2>err && + test -s err && + test ! -s out ' @@ -167,7 +181,9 @@ test_expect_success 'bundle 1 has only 3 files ' ' test_expect_success 'unbundle 2' ' cd "$D/bundle" && - git fetch ../bundle2 master:master && + git fetch ../bundle2 master:master >out 2>err && + test ! -s err && + test -s out && test "tip" = "$(git log -1 --pretty=oneline master | cut -b42-)" ' @@ -203,7 +219,9 @@ test_expect_success 'fetch via rsync' ' mkdir rsynced && (cd rsynced && git init --bare && - git fetch "rsync:$(pwd)/../.git" master:refs/heads/master && + git fetch "rsync:$(pwd)/../.git" master:refs/heads/master >out 2>err && + test ! -s err && + test -s out && git gc --prune && test $(git rev-parse master) = $(cd .. && git rev-parse master) && git fsck --full) @@ -237,14 +255,17 @@ test_expect_success 'fetch with a non-applying branch.<name>.merge' ' git config branch.master.merge refs/heads/bigfoot && git config remote.blub.url one && git config remote.blub.fetch "refs/heads/*:refs/remotes/one/*" && - git fetch blub + git fetch blub >out 2>err && + test ! -s err && + test -s out ' # the strange name is: a\!'b test_expect_success 'quoting of a strangely named repo' ' - test_must_fail git fetch "a\\!'\''b" > result 2>&1 && - cat result && - grep "fatal: '\''a\\\\!'\''b'\''" result + test_must_fail git fetch "a\\!'\''b" >out 2>err && + test -s err && + test ! -s out && + grep "fatal: '\''a\\\\!'\''b'\''" err ' test_expect_success 'bundle should record HEAD correctly' ' @@ -267,7 +288,9 @@ test_expect_success 'explicit fetch should not update tracking' ' ( cd three && o=$(git rev-parse --verify refs/remotes/origin/master) && - git fetch origin master && + git fetch origin master >out 2>err && + test ! -s err && + test -s out && n=$(git rev-parse --verify refs/remotes/origin/master) && test "$o" = "$n" && test_must_fail git rev-parse --verify refs/remotes/origin/side @@ -295,7 +318,9 @@ test_expect_success 'configured fetch updates tracking' ' ( cd three && o=$(git rev-parse --verify refs/remotes/origin/master) && - git fetch origin && + git fetch origin >out 2>err && + test ! -s err && + test -s out && n=$(git rev-parse --verify refs/remotes/origin/master) && test "$o" != "$n" && git rev-parse --verify refs/remotes/origin/side diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index 1b06691..7ec36e7 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -23,16 +23,16 @@ test_expect_success 'git pull' ' mkdir cloned && (cd cloned && git init && git pull "../parent" >out 2>err && - test -s err && - test ! -s out) + test ! -s err && + test -s out) ' test_expect_success 'git pull -v' ' mkdir clonedv && (cd clonedv && git init && git pull -v "../parent" >out 2>err && - test -s err && - test ! -s out) + test ! -s err && + test -s out) ' test_expect_success 'git pull -v -q' ' @@ -47,8 +47,8 @@ test_expect_success 'git pull -q -v' ' mkdir clonedqv && (cd clonedqv && git init && git pull -q -v "../parent" >out 2>err && - test ! -s out && - test -s err) + test ! -s err && + test -s out) ' test_expect_success 'git pull --force' ' -- 1.7.1.251.g92a7 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-24 22:34 ` [PATCH] fetch: don't output non-errors on stderr Ævar Arnfjörð Bjarmason 2010-06-25 13:30 ` [PATCH v2] " Ævar Arnfjörð Bjarmason @ 2010-06-25 17:25 ` Junio C Hamano 2010-06-25 21:28 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2010-06-25 17:25 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Before the change error messages were drowned out by git-fetch's > non-error update notices, which didn't need my attention. I don't understand this part; care to elaborate? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-25 17:25 ` [PATCH] " Junio C Hamano @ 2010-06-25 21:28 ` Ævar Arnfjörð Bjarmason 2010-06-25 21:43 ` Junio C Hamano 0 siblings, 1 reply; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-06-25 21:28 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jeff King On Fri, Jun 25, 2010 at 17:25, Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Before the change error messages were drowned out by git-fetch's >> non-error update notices, which didn't need my attention. > > I don't understand this part; care to elaborate? I have a cron job (github-backup) that calls git fetch. Without this patch I have to run it as '> /dev/null 2>&1' and just rely on the exit code, with it I can just do '> /dev/null' and not ignore stderr, because non-error output isn't being sent there anymore. In short, the current git-fetch breaks the conventional *nix assumption that stderr only contains errors. With this patch errors go to stderr and normal output to stdout. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-25 21:28 ` Ævar Arnfjörð Bjarmason @ 2010-06-25 21:43 ` Junio C Hamano 2010-06-26 6:13 ` Jeff King 0 siblings, 1 reply; 19+ messages in thread From: Junio C Hamano @ 2010-06-25 21:43 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Fri, Jun 25, 2010 at 17:25, Junio C Hamano <gitster@pobox.com> wrote: >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> Before the change error messages were drowned out by git-fetch's >>> non-error update notices, which didn't need my attention. >> >> I don't understand this part; care to elaborate? > > I have a cron job (github-backup) that calls git fetch. Without this > patch I have to run it as '> /dev/null 2>&1' and just rely on the exit > code, Signaling failure with exit code is _the_ standard practice, no? Some people seem to think unclean standard error means some error (most notably tcl ;-), but I think they are mistaken. Not that I care too much about this issue, though. I might end up queuing it, but we need to think about things like advice messages and such (e.g. 011fe98 (git-push: fix an advice message so it goes to stderr, 2010-02-26)). ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-25 21:43 ` Junio C Hamano @ 2010-06-26 6:13 ` Jeff King 2010-06-26 12:14 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 19+ messages in thread From: Jeff King @ 2010-06-26 6:13 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git On Fri, Jun 25, 2010 at 02:43:27PM -0700, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > > On Fri, Jun 25, 2010 at 17:25, Junio C Hamano <gitster@pobox.com> wrote: > >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> > >>> Before the change error messages were drowned out by git-fetch's > >>> non-error update notices, which didn't need my attention. > >> > >> I don't understand this part; care to elaborate? > > > > I have a cron job (github-backup) that calls git fetch. Without this > > patch I have to run it as '> /dev/null 2>&1' and just rely on the exit > > code, > > Signaling failure with exit code is _the_ standard practice, no? > > Some people seem to think unclean standard error means some error (most > notably tcl ;-), but I think they are mistaken. Agreed. I thought it was intentional for any human-readable progress messages go to stderr. It is true in the case of git-fetch that stdout is not being used for anything, but: 1. Git should be consistent about where output goes. And other programs may actually produce useful output on stdout, which should not be mixed with human-readable verbose messages. For the sake of those programs, we should be consistent about sending the output to stderr. 2. Fetch may be combined with other operations in script. Consider this toy script to print the latest origin/master to stdout: #!/bin/sh git fetch && git rev-parse origin/master The user sees verbose cruft on stderr, but the interesting part is on stdout. With your patch, the script is now broken. Yes, it's obviously a toy, but I don't think it inconceivable that somebody would not want fetch unexpectedly polluting their stdout. Even if it would have been a better behavior in the first place (which I don't agree with), changing it now means breaking scripts. The real problem is that git is very chatty compared to other unix programs. Most produce no output at all unless there is an error, or human-readable non-error output on stderr only if it is a tty. We do this already with progress meters, and this output is just another form of progress update. So I think a patch to quell the status table when stderr is not a tty would be better (that still can break some scripts, too, but I am less sympathetic to people trying to save and parse human-readable stderr messages). Or even easier: is there a reason that "git fetch -q" would not do what you (Ævar) want? -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-26 6:13 ` Jeff King @ 2010-06-26 12:14 ` Ævar Arnfjörð Bjarmason 2010-06-26 13:40 ` Tay Ray Chuan 2010-06-26 16:46 ` Jeff King 0 siblings, 2 replies; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-06-26 12:14 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, git On Sat, Jun 26, 2010 at 06:13, Jeff King <peff@peff.net> wrote: > Or even easier: is there a reason that "git fetch -q" would not do what > you (Ævar) want? That'd reduce the verbosity level, which'd skip some messages that I might want. E.g.: if (verbosity >= 0) { fprintf(stderr, " x %-*s %-*s -> %s\n", TRANSPORT_SUMMARY_WIDTH, "[deleted]", REFCOL_WIDTH, "(none)", prettify_refname(ref->name)); Anyway, it looks like the only correct way to do this with Git in general is to: 1. Capture stderr and stdout 2. Check the exit code, and if it's non-zero print both But it sounds like we need some general discussion on what stdout and stderr should be used for in Git with regards to progress messages, errors and other similar things. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-26 12:14 ` Ævar Arnfjörð Bjarmason @ 2010-06-26 13:40 ` Tay Ray Chuan 2010-06-26 15:07 ` Ævar Arnfjörð Bjarmason 2010-06-26 16:46 ` Jeff King 1 sibling, 1 reply; 19+ messages in thread From: Tay Ray Chuan @ 2010-06-26 13:40 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, Junio C Hamano, git Hi, On Sat, Jun 26, 2010 at 8:14 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > But it sounds like we need some general discussion on what stdout and > stderr should be used for in Git with regards to progress messages, > errors and other similar things. I'd say - submit a patch for the style guide with your proposed guidelines on stdout and stderr usage. -- Cheers, Ray Chuan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-26 13:40 ` Tay Ray Chuan @ 2010-06-26 15:07 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-06-26 15:07 UTC (permalink / raw) To: Tay Ray Chuan; +Cc: Jeff King, Junio C Hamano, git On Sat, Jun 26, 2010 at 13:40, Tay Ray Chuan <rctay89@gmail.com> wrote: > On Sat, Jun 26, 2010 at 8:14 PM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> But it sounds like we need some general discussion on what stdout and >> stderr should be used for in Git with regards to progress messages, >> errors and other similar things. > > I'd say - submit a patch for the style guide with your proposed > guidelines on stdout and stderr usage. I'm not sure how such a guide should be like for the general case, but at least making commands internally consistent (as this patch does) is one step in that direction. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-26 12:14 ` Ævar Arnfjörð Bjarmason 2010-06-26 13:40 ` Tay Ray Chuan @ 2010-06-26 16:46 ` Jeff King 2010-06-26 16:50 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 19+ messages in thread From: Jeff King @ 2010-06-26 16:46 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git On Sat, Jun 26, 2010 at 12:14:59PM +0000, Ævar Arnfjörð Bjarmason wrote: > On Sat, Jun 26, 2010 at 06:13, Jeff King <peff@peff.net> wrote: > > > Or even easier: is there a reason that "git fetch -q" would not do what > > you (Ævar) want? > > That'd reduce the verbosity level, which'd skip some messages that I > might want. E.g.: > > if (verbosity >= 0) { > fprintf(stderr, " x %-*s %-*s -> %s\n", > TRANSPORT_SUMMARY_WIDTH, "[deleted]", > REFCOL_WIDTH, "(none)", prettify_refname(ref->name)); Wait, what? Isn't that line also part of the same human-readable status table? What makes the status of a pruned ref any different than the status of an updated ref? I don't see how that is an error message, but the other lines are not. -Peff ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] fetch: don't output non-errors on stderr 2010-06-26 16:46 ` Jeff King @ 2010-06-26 16:50 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 19+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2010-06-26 16:50 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, git On Sat, Jun 26, 2010 at 16:46, Jeff King <peff@peff.net> wrote: > On Sat, Jun 26, 2010 at 12:14:59PM +0000, Ævar Arnfjörð Bjarmason wrote: > >> On Sat, Jun 26, 2010 at 06:13, Jeff King <peff@peff.net> wrote: >> >> > Or even easier: is there a reason that "git fetch -q" would not do what >> > you (Ævar) want? >> >> That'd reduce the verbosity level, which'd skip some messages that I >> might want. E.g.: >> >> if (verbosity >= 0) { >> fprintf(stderr, " x %-*s %-*s -> %s\n", >> TRANSPORT_SUMMARY_WIDTH, "[deleted]", >> REFCOL_WIDTH, "(none)", prettify_refname(ref->name)); > > Wait, what? Isn't that line also part of the same human-readable status > table? What makes the status of a pruned ref any different than the > status of an updated ref? I don't see how that is an error message, but > the other lines are not. I misread that and picked the wrong example, sorry for the noise. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Bugreport: Git responds with stderr instead of stdout 2010-04-25 18:10 ` Jacob Helwig 2010-04-25 18:24 ` Ævar Arnfjörð Bjarmason @ 2010-04-25 18:34 ` Jack Desert 1 sibling, 0 replies; 19+ messages in thread From: Jack Desert @ 2010-04-25 18:34 UTC (permalink / raw) To: Jacob Helwig; +Cc: git El Sun, 25 Apr 2010 11:10:47 -0700 Jacob Helwig <jacob.helwig@gmail.com> escribió: > On Sun, Apr 25, 2010 at 11:06, Jack Desert <jackdesert556@gmail.com> wrote: > > I think I found a bug in Git. When I run the command > > > > git checkout -b new_branch > > > > Git does exactly what I've asked, except that Git's response: > > > > Switched to a new branch 'new_branch' > > > > comes through the stderr pipe instead of through the stdout pipe. Where do I file a bug report for this? > > > > I am using Git 1.6.3.3, Ubuntu 9.10 > > > > -Jack > > > > > > I can't really say if it's actually a bug, or not, but as to your > question about where to file a bug report: You just did. This mailing > list is the correct place. I just finished testing with the latest development version and it has the same issue that 1.6.3.3 has in this regard. -- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Jack Desert -- Writer, Entrepeneur Author and Spokesman: www.LetsEATalready.com Software Developer: http://GrooveTask.org Email: JackDesert556@gmail.com ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-06-26 16:50 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-04-25 18:06 Bugreport: Git responds with stderr instead of stdout Jack Desert 2010-04-25 18:10 ` Jacob Helwig 2010-04-25 18:24 ` Ævar Arnfjörð Bjarmason 2010-04-25 19:22 ` Jeff King 2010-04-25 19:32 ` Ævar Arnfjörð Bjarmason 2010-04-25 19:32 ` Jeff King 2010-06-12 16:52 ` Ævar Arnfjörð Bjarmason 2010-06-24 22:34 ` [PATCH] fetch: don't output non-errors on stderr Ævar Arnfjörð Bjarmason 2010-06-25 13:30 ` [PATCH v2] " Ævar Arnfjörð Bjarmason 2010-06-25 17:25 ` [PATCH] " Junio C Hamano 2010-06-25 21:28 ` Ævar Arnfjörð Bjarmason 2010-06-25 21:43 ` Junio C Hamano 2010-06-26 6:13 ` Jeff King 2010-06-26 12:14 ` Ævar Arnfjörð Bjarmason 2010-06-26 13:40 ` Tay Ray Chuan 2010-06-26 15:07 ` Ævar Arnfjörð Bjarmason 2010-06-26 16:46 ` Jeff King 2010-06-26 16:50 ` Ævar Arnfjörð Bjarmason 2010-04-25 18:34 ` Bugreport: Git responds with stderr instead of stdout Jack Desert
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.