git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] fetch: don't output non-errors on stderr

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jun 25, 2010, 13:30 UTC
Message-ID
<1277472641-18148-1-git-send-email-avarab@gmail.com>
In-Reply-To
<1277418881-11286-1-git-send-email-avarab@gmail.com>

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
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 9 of 19 in “Bugreport: Git responds with stderr instead of stdout”
  1. Jack DesertApr 25, 2010
  2. Jacob HelwigApr 25, 2010
  3. Ævar Arnfjörð BjarmasonApr 25, 2010
  4. Jeff KingApr 25, 2010
  5. Ævar Arnfjörð BjarmasonApr 25, 2010
  6. Jeff KingApr 25, 2010
  7. Ævar Arnfjörð BjarmasonJun 12, 2010
  8. fetch: don't output non-errors on stderrÆvar Arnfjörð Bjarmason, Jun 24, 2010
  9. fetch: don't output non-errors on stderrÆvar Arnfjörð Bjarmason, Jun 25, 2010
  10. Junio C HamanoJun 25, 2010
  11. Ævar Arnfjörð BjarmasonJun 25, 2010
  12. Junio C HamanoJun 25, 2010
  13. Jeff KingJun 26, 2010
  14. Ævar Arnfjörð BjarmasonJun 26, 2010
  15. Tay Ray ChuanJun 26, 2010
  16. Ævar Arnfjörð BjarmasonJun 26, 2010
  17. Jeff KingJun 26, 2010
  18. Ævar Arnfjörð BjarmasonJun 26, 2010
  19. Jack DesertApr 25, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.