git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2] stash: prefer --quiet over shell redirection
@ 2014-08-31  7:42 David Aguilar
  2014-09-12 19:05 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: David Aguilar @ 2014-08-31  7:42 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Sixt, Christian Couder,
	Ævar Arnfjörð Bjarmason, Jon Seymour

Use `git rev-parse --verify --quiet` instead of redirecting
stderr to /dev/null.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-stash.sh | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index bcc757b..2ff8b94 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -50,7 +50,7 @@ clear_stash () {
 	then
 		die "$(gettext "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 --quiet $ref_stash)
 	then
 		git update-ref -d $ref_stash $current
 	fi
@@ -292,7 +292,7 @@ save_stash () {
 }
 
 have_stash () {
-	git rev-parse --verify $ref_stash >/dev/null 2>&1
+	git rev-parse --verify --quiet $ref_stash >/dev/null
 }
 
 list_stash () {
@@ -392,12 +392,12 @@ parse_flags_and_rev()
 		;;
 	esac
 
-	REV=$(git rev-parse --quiet --symbolic --verify "$1" 2>/dev/null) || {
+	REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
 		reference="$1"
 		die "$(eval_gettext "\$reference is not valid reference")"
 	}
 
-	i_commit=$(git rev-parse --quiet --verify "$REV^2" 2>/dev/null) &&
+	i_commit=$(git rev-parse --verify --quiet "$REV^2") &&
 	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&
 	s=$1 &&
 	w_commit=$1 &&
@@ -409,7 +409,7 @@ parse_flags_and_rev()
 	test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" &&
 	IS_STASH_REF=t
 
-	u_commit=$(git rev-parse --quiet --verify "$REV^3" 2>/dev/null) &&
+	u_commit=$(git rev-parse --verify --quiet "$REV^3") &&
 	u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)
 }
 
@@ -531,7 +531,8 @@ drop_stash () {
 		die "$(eval_gettext "\${REV}: 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 --quiet "$ref_stash@{0}" >/dev/null ||
+	clear_stash
 }
 
 apply_to_branch () {
-- 
2.1.0.29.g9b751c4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] stash: prefer --quiet over shell redirection
  2014-08-31  7:42 [PATCH v2] stash: prefer --quiet over shell redirection David Aguilar
@ 2014-09-12 19:05 ` Junio C Hamano
  2014-09-13 21:57   ` David Aguilar
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2014-09-12 19:05 UTC (permalink / raw)
  To: David Aguilar
  Cc: git, Johannes Sixt, Christian Couder,
	Ævar Arnfjörð Bjarmason, Jon Seymour

David Aguilar <davvid@gmail.com> writes:

> Use `git rev-parse --verify --quiet` instead of redirecting
> stderr to /dev/null.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---

Has this patch ever been tested?  t3903 seems to break with this at
least for me.

>  git-stash.sh | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index bcc757b..2ff8b94 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -50,7 +50,7 @@ clear_stash () {
>  	then
>  		die "$(gettext "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 --quiet $ref_stash)
>  	then
>  		git update-ref -d $ref_stash $current
>  	fi
> @@ -292,7 +292,7 @@ save_stash () {
>  }
>  
>  have_stash () {
> -	git rev-parse --verify $ref_stash >/dev/null 2>&1
> +	git rev-parse --verify --quiet $ref_stash >/dev/null
>  }
>  
>  list_stash () {
> @@ -392,12 +392,12 @@ parse_flags_and_rev()
>  		;;
>  	esac
>  
> -	REV=$(git rev-parse --quiet --symbolic --verify "$1" 2>/dev/null) || {
> +	REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
>  		reference="$1"
>  		die "$(eval_gettext "\$reference is not valid reference")"
>  	}
>  
> -	i_commit=$(git rev-parse --quiet --verify "$REV^2" 2>/dev/null) &&
> +	i_commit=$(git rev-parse --verify --quiet "$REV^2") &&
>  	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&
>  	s=$1 &&
>  	w_commit=$1 &&
> @@ -409,7 +409,7 @@ parse_flags_and_rev()
>  	test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" &&
>  	IS_STASH_REF=t
>  
> -	u_commit=$(git rev-parse --quiet --verify "$REV^3" 2>/dev/null) &&
> +	u_commit=$(git rev-parse --verify --quiet "$REV^3") &&
>  	u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)
>  }
>  
> @@ -531,7 +531,8 @@ drop_stash () {
>  		die "$(eval_gettext "\${REV}: 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 --quiet "$ref_stash@{0}" >/dev/null ||
> +	clear_stash
>  }
>  
>  apply_to_branch () {

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] stash: prefer --quiet over shell redirection
  2014-09-12 19:05 ` Junio C Hamano
@ 2014-09-13 21:57   ` David Aguilar
  0 siblings, 0 replies; 3+ messages in thread
From: David Aguilar @ 2014-09-13 21:57 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Johannes Sixt, Christian Couder,
	Ævar Arnfjörð Bjarmason, Jon Seymour,
	Ronnie Sahlberg, Michael Haggerty

On Fri, Sep 12, 2014 at 12:05:48PM -0700, Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
> 
> > Use `git rev-parse --verify --quiet` instead of redirecting
> > stderr to /dev/null.
> >
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> 
> Has this patch ever been tested?  t3903 seems to break with this at
> least for me.

Oops, I thought I did but it's definitely failing in pu.

The root cause is that "git rev-parse --verify --quiet" is not
actually quiet.

I'll send a patch to fix this shortly.

It touches the ref machinery, which I know Ronnie has been improving,
so I hope this doesn't cause any conflicts with other in-flight topics.


> >  git-stash.sh | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/git-stash.sh b/git-stash.sh
> > index bcc757b..2ff8b94 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -50,7 +50,7 @@ clear_stash () {
> >  	then
> >  		die "$(gettext "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 --quiet $ref_stash)
> >  	then
> >  		git update-ref -d $ref_stash $current
> >  	fi
> > @@ -292,7 +292,7 @@ save_stash () {
> >  }
> >  
> >  have_stash () {
> > -	git rev-parse --verify $ref_stash >/dev/null 2>&1
> > +	git rev-parse --verify --quiet $ref_stash >/dev/null
> >  }
> >  
> >  list_stash () {
> > @@ -392,12 +392,12 @@ parse_flags_and_rev()
> >  		;;
> >  	esac
> >  
> > -	REV=$(git rev-parse --quiet --symbolic --verify "$1" 2>/dev/null) || {
> > +	REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
> >  		reference="$1"
> >  		die "$(eval_gettext "\$reference is not valid reference")"
> >  	}
> >  
> > -	i_commit=$(git rev-parse --quiet --verify "$REV^2" 2>/dev/null) &&
> > +	i_commit=$(git rev-parse --verify --quiet "$REV^2") &&
> >  	set -- $(git rev-parse "$REV" "$REV^1" "$REV:" "$REV^1:" "$REV^2:" 2>/dev/null) &&
> >  	s=$1 &&
> >  	w_commit=$1 &&
> > @@ -409,7 +409,7 @@ parse_flags_and_rev()
> >  	test "$ref_stash" = "$(git rev-parse --symbolic-full-name "${REV%@*}")" &&
> >  	IS_STASH_REF=t
> >  
> > -	u_commit=$(git rev-parse --quiet --verify "$REV^3" 2>/dev/null) &&
> > +	u_commit=$(git rev-parse --verify --quiet "$REV^3") &&
> >  	u_tree=$(git rev-parse "$REV^3:" 2>/dev/null)
> >  }
> >  
> > @@ -531,7 +531,8 @@ drop_stash () {
> >  		die "$(eval_gettext "\${REV}: 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 --quiet "$ref_stash@{0}" >/dev/null ||
> > +	clear_stash
> >  }
> >  
> >  apply_to_branch () {

-- 
David

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2014-09-13 21:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-31  7:42 [PATCH v2] stash: prefer --quiet over shell redirection David Aguilar
2014-09-12 19:05 ` Junio C Hamano
2014-09-13 21:57   ` David Aguilar

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).