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

Re: [PATCH 2/4] pull: use --quiet rather than 2>/dev/null

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Mar 21, 2010, 04:33 UTC
Message-ID
<20100321043330.GA9803@progeny.tock>
In-Reply-To
<818FE4EC-5AE9-4F05-B9AD-E1753F7ED9E6@meyerhome.net>
Hi again,
Benjamin Meyer wrote:
> re-checking the other patches I think they are correct in their
> usage.
Thanks for checking.
Show 5 quoted lines
> I ran the matching tests, the rebase ones passed,
> 3903-stash.sh is already failing before my patch and
> t3904-stash-patch.sh continues passing with the patch.  Other then
> running the tests in t/ is there anything I should do to verify
> patches?

Regarding testing: the best thing to do IMHO is to explain in the commit message what effect the patch will have. Then reviewers (including you) can read it and try it out to make sure what you say is true.

The test suite is not very good for checking that a patch does what it’s meant to do, but that’s okay, since it has a different purpose [1].

As an example of what I mean, here is your patch 1/4 again (untested, though). I didn’t add any tests since it would be kind of hard to provoke address space exhaustion at just the right moment.

[1] See http://thread.gmane.org/gmane.comp.version-control.git/142480/focus=142745
-- %< --
From: Benjamin C Meyer <bmeyer@rim.com>
Subject: scripts: use rev-parse --verify -q rather than using 2>/dev/null

b1b3596 (2008-04-26) taught rev-parse --verify a --quiet option to suppress the “fatal: Needed a single revision” message when its argument is not a valid object. Use it; this simplifies scripts somewhat, and it allows other errors (such as address space exhaustion) to be reported properly.

Since --quiet does not suppress warnings about ambiguous ref names and inadequate reflogs, the redirection has been kept for call sites that might involve either of these phenomena.

Signed-off-by: Benjamin C Meyer <bmeyer@rim.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
t3903-stash.sh fails for me with your original patch.  Maybe you
forgot to run 'make' before running tests?  I fixed it by putting
back in the 2>&1 on the ... || clear_stash line.
 contrib/completion/git-completion.bash |    2 +-
 contrib/examples/git-checkout.sh       |   10 +++++-----
 contrib/examples/git-commit.sh         |    2 +-
 contrib/examples/git-fetch.sh          |    4 ++--
 contrib/examples/git-merge.sh          |   10 +++++-----
 contrib/examples/git-reset.sh          |    2 +-
 contrib/examples/git-revert.sh         |    4 ++--
 git-cvsimport.perl                     |    2 +-
 git-stash.sh                           |   13 +++++++------
 templates/hooks--pre-commit.sample     |    2 +-
 10 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 733ac39..5b287e1 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -151,7 +151,7 @@ __git_ps1 ()
 				fi
 			fi
 			if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ]; then
-			        git rev-parse --verify refs/stash >/dev/null 2>&1 && s="$"
+				git rev-parse --verify -q refs/stash >/dev/null && s="$"
 			fi
 
 			if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
diff --git a/contrib/examples/git-checkout.sh b/contrib/examples/git-checkout.sh
index 1a7689a..f735209 100755
--- a/contrib/examples/git-checkout.sh
+++ b/contrib/examples/git-checkout.sh
@@ -16,7 +16,7 @@ SUBDIRECTORY_OK=Sometimes
 require_work_tree
 
 old_name=HEAD
-old=$(git rev-parse --verify $old_name 2>/dev/null)
+old=$(git rev-parse --verify -q $old_name)
 oldbranch=$(git symbolic-ref $old_name 2>/dev/null)
 new=
 new_name=
@@ -71,8 +71,8 @@ while test $# != 0; do
 done
 
 arg="$1"
-rev=$(git rev-parse --verify "$arg" 2>/dev/null)
-if rev=$(git rev-parse --verify "$rev^0" 2>/dev/null)
+rev=$(git rev-parse --verify -q "$arg" 2>/dev/null)
+if rev=$(git rev-parse --verify -q "$rev^0" 2>/dev/null)
 then
 	[ -z "$rev" ] && die "unknown flag $arg"
 	new_name="$arg"
@@ -83,7 +83,7 @@ then
 	fi
 	new="$rev"
 	shift
-elif rev=$(git rev-parse --verify "$rev^{tree}" 2>/dev/null)
+elif rev=$(git rev-parse --verify -q "$rev^{tree}" 2>/dev/null)
 then
 	# checking out selected paths from a tree-ish.
 	new="$rev"
@@ -151,7 +151,7 @@ else
 	# but switching branches.
 	if test '' != "$new"
 	then
-		git rev-parse --verify "$new^{commit}" >/dev/null 2>&1 ||
+		git rev-parse --verify -q "$new^{commit}" >/dev/null 2>&1 ||
 		die "Cannot switch branch to a non-commit."
 	fi
 fi
diff --git a/contrib/examples/git-commit.sh b/contrib/examples/git-commit.sh
index 5c72f65..35626c5 100755
--- a/contrib/examples/git-commit.sh
+++ b/contrib/examples/git-commit.sh
@@ -9,7 +9,7 @@ OPTIONS_SPEC=
 . git-sh-setup
 require_work_tree
 
-git rev-parse --verify HEAD >/dev/null 2>&1 || initial_commit=t
+git rev-parse --verify -q HEAD >/dev/null || initial_commit=t
 
 case "$0" in
 *status)
diff --git a/contrib/examples/git-fetch.sh b/contrib/examples/git-fetch.sh
index e44af2c..a35f997 100755
--- a/contrib/examples/git-fetch.sh
+++ b/contrib/examples/git-fetch.sh
@@ -124,7 +124,7 @@ append_fetch_head () {
 # repository is always fine.
 if test -z "$update_head_ok" && test $(is_bare_repository) = false
 then
-	orig_head=$(git rev-parse --verify HEAD 2>/dev/null)
+	orig_head=$(git rev-parse --verify -q HEAD)
 fi
 
 # Allow --notags from remote.$1.tagopt
@@ -365,7 +365,7 @@ case "$orig_head" in
 '')
 	;;
 ?*)
-	curr_head=$(git rev-parse --verify HEAD 2>/dev/null)
+	curr_head=$(git rev-parse --verify -q HEAD)
 	if test "$curr_head" != "$orig_head"
 	then
 	    git update-ref \
diff --git a/contrib/examples/git-merge.sh b/contrib/examples/git-merge.sh
index 8f617fc..5ff2999 100755
--- a/contrib/examples/git-merge.sh
+++ b/contrib/examples/git-merge.sh
@@ -130,7 +130,7 @@ finish () {
 
 merge_name () {
 	remote="$1"
-	rh=$(git rev-parse --verify "$remote^0" 2>/dev/null) || return
+	rh=$(git rev-parse --verify -q "$remote^0" 2>/dev/null) || return
 	bh=$(git show-ref -s --verify "refs/heads/$remote" 2>/dev/null)
 	if test "$rh" = "$bh"
 	then
@@ -226,15 +226,15 @@ fi
 # have "-m" so it is an additional safety measure to check for it.
 
 if test -z "$have_message" &&
-	second_token=$(git rev-parse --verify "$2^0" 2>/dev/null) &&
-	head_commit=$(git rev-parse --verify "HEAD" 2>/dev/null) &&
+	second_token=$(git rev-parse --verify -q "$2^0" 2>/dev/null) &&
+	head_commit=$(git rev-parse --verify -q "HEAD") &&
 	test "$second_token" = "$head_commit"
 then
 	merge_msg="$1"
 	shift
 	head_arg="$1"
 	shift
-elif ! git rev-parse --verify HEAD >/dev/null 2>&1
+elif ! git rev-parse --verify -q HEAD >/dev/null
 then
 	# If the merged head is a valid one there is no reason to
 	# forbid "git merge" into a branch yet to be born.  We do
@@ -277,7 +277,7 @@ set_reflog_action "merge $*"
 remoteheads=
 for remote
 do
-	remotehead=$(git rev-parse --verify "$remote"^0 2>/dev/null) ||
+	remotehead=$(git rev-parse --verify -q "$remote"^0 2>/dev/null) ||
 	    die "$remote - not something we can merge"
 	remoteheads="${remoteheads}$remotehead "
 	eval GITHEAD_$remotehead='"$remote"'
diff --git a/contrib/examples/git-reset.sh b/contrib/examples/git-reset.sh
index bafeb52..e09a9c4 100755
--- a/contrib/examples/git-reset.sh
+++ b/contrib/examples/git-reset.sh
@@ -75,7 +75,7 @@ else
 fi
 
 # Any resets update HEAD to the head being switched to.
-if orig=$(git rev-parse --verify HEAD 2>/dev/null)
+if orig=$(git rev-parse --verify -q HEAD)
 then
 	echo "$orig" >"$GIT_DIR/ORIG_HEAD"
 else
diff --git a/contrib/examples/git-revert.sh b/contrib/examples/git-revert.sh
index 49f0032..5672b4f 100755
--- a/contrib/examples/git-revert.sh
+++ b/contrib/examples/git-revert.sh
@@ -78,9 +78,9 @@ esac
 rev=$(git-rev-parse --verify "$@") &&
 commit=$(git-rev-parse --verify "$rev^0") ||
 	die "Not a single commit $@"
-prev=$(git-rev-parse --verify "$commit^1" 2>/dev/null) ||
+prev=$(git rev-parse --verify -q "$commit^1") ||
 	die "Cannot run $me a root commit"
-git-rev-parse --verify "$commit^2" >/dev/null 2>&1 &&
+git rev-parse --verify -q "$commit^2" >/dev/null &&
 	die "Cannot run $me a multi-parent commit."
 
 encoding=$(git config i18n.commitencoding || echo UTF-8)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 9e03eee..0b13d23 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -573,7 +573,7 @@ sub is_sha1 {
 
 sub get_headref ($) {
 	my $name = shift;
-	my $r = `git rev-parse --verify '$name' 2>/dev/null`;
+	my $r = `git rev-parse --verify -q '$name' 2>/dev/null`;
 	return undef unless $? == 0;
 	chomp $r;
 	return $r;
diff --git a/git-stash.sh b/git-stash.sh
index aa47e54..f6bd623 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -39,7 +39,7 @@ clear_stash () {
 	then
 		die "git stash clear with parameters is unimplemented"
 	fi
-	if current=$(git rev-parse --verify $ref_stash 2>/dev/null)
+	if current=$(git rev-parse --verify -q $ref_stash)
 	then
 		git update-ref -d $ref_stash $current
 	fi
@@ -201,7 +201,7 @@ save_stash () {
 }
 
 have_stash () {
-	git rev-parse --verify $ref_stash >/dev/null 2>&1
+	git rev-parse --verify -q $ref_stash >/dev/null
 }
 
 list_stash () {
@@ -342,16 +342,17 @@ drop_stash () {
 	fi
 	# Verify supplied argument looks like a stash entry
 	s=$(git rev-parse --verify "$@") &&
-	git rev-parse --verify "$s:"   > /dev/null 2>&1 &&
-	git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
-	git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
+	git rev-parse --verify -q "$s:"   > /dev/null &&
+	git rev-parse --verify -q "$s^1:" > /dev/null &&
+	git rev-parse --verify -q "$s^2:" > /dev/null ||
 		die "$*: not a valid stashed state"
 
 	git reflog delete --updateref --rewrite "$@" &&
 		say "Dropped $* ($s)" || die "$*: Could not drop stash entry"
 
 	# clear_stash if we just dropped the last stash entry
-	git rev-parse --verify "$ref_stash@{0}" > /dev/null 2>&1 || clear_stash
+	git rev-parse --verify -q "$ref_stash@{0}" > /dev/null 2>&1 ||
+		clear_stash
 }
 
 apply_to_branch () {
diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index 439eefd..a1ebe75 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -7,7 +7,7 @@
 #
 # To enable this hook, rename this file to "pre-commit".
 
-if git-rev-parse --verify HEAD >/dev/null 2>&1
+if git rev-parse --verify -q HEAD >/dev/null
 then
 	against=HEAD
 else

-- 
1.7.0.2
Previous: Benjamin MeyerNext: Benjamin Meyer
Message 5 of 9 in “stash: use --quiet rather than using 2>/dev/null”
  1. 1/4 stash: use --quiet rather than using 2>/dev/nullBenjamin C Meyer, Mar 20, 2010
  2. 2/4 pull: use --quiet rather than 2>/dev/nullBenjamin C Meyer, Mar 20, 2010
  3. Jonathan NiederMar 20, 2010
  4. Benjamin MeyerMar 20, 2010
  5. Jonathan NiederMar 21, 2010
  6. Benjamin MeyerMar 21, 2010
  7. Jonathan NiederMar 23, 2010
  8. 3/4 rebase: use --quiet rather than 2>/dev/nullBenjamin C Meyer, Mar 20, 2010
  9. 4/4 rebase-interactive: use --quiet rather than 2>/dev/nullBenjamin C Meyer, Mar 20, 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.