All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] stash: don't leak underlying error messages
@ 2012-04-11 18:36 Ross Lagerwall
  2012-04-12 19:30 ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Ross Lagerwall @ 2012-04-11 18:36 UTC (permalink / raw)
  To: git; +Cc: Ross Lagerwall

When running git-stash on an empty repository, don't let the underlying
error messages leak through to the surface; instead, redirect them to
/dev/null.
---
 git-stash.sh |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index fe4ab28..5c72d1b 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -34,8 +34,8 @@ else
 fi
 
 no_changes () {
-	git diff-index --quiet --cached HEAD --ignore-submodules -- &&
-	git diff-files --quiet --ignore-submodules &&
+	git diff-index --quiet --cached HEAD --ignore-submodules -- 2>/dev/null &&
+	git diff-files --quiet --ignore-submodules 2>/dev/null &&
 	(test -z "$untracked" || test -z "$(untracked_files)")
 }
 
@@ -67,7 +67,7 @@ create_stash () {
 	fi
 
 	# state of the base commit
-	if b_commit=$(git rev-parse --verify HEAD)
+	if b_commit=$(git rev-parse --verify HEAD 2>/dev/null)
 	then
 		head=$(git rev-list --oneline -n 1 HEAD --)
 	else
-- 
1.7.10

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

* Re: [PATCH] stash: don't leak underlying error messages
  2012-04-11 18:36 [PATCH] stash: don't leak underlying error messages Ross Lagerwall
@ 2012-04-12 19:30 ` Junio C Hamano
  2012-04-13  3:59   ` Ross Lagerwall
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2012-04-12 19:30 UTC (permalink / raw)
  To: Ross Lagerwall; +Cc: git

Ross Lagerwall <rosslagerwall@gmail.com> writes:

> When running git-stash on an empty repository, don't let the underlying
> error messages leak through to the surface; instead, redirect them to
> /dev/null.
> ---

Sign-off?

Is create_stash (hence save_stash) the only operation that do not make
sense when HEAD is not born yet?  I am wondering if it makes more sense to
either:

 (1) catch the case where HEAD is not born yet a lot earlier and do not
     let the control even reach these functions (i.e. die inside the
     case/esac statement at the end of the script); or

 (2) pretend as if HEAD is a commit that records an empty tree, and not
     error out to begin with.

If either one of the above turns out to make sense, then the issue this
patch addresses becomes irrelevant, so...

>  git-stash.sh |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index fe4ab28..5c72d1b 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -34,8 +34,8 @@ else
>  fi
>  
>  no_changes () {
> -	git diff-index --quiet --cached HEAD --ignore-submodules -- &&
> -	git diff-files --quiet --ignore-submodules &&
> +	git diff-index --quiet --cached HEAD --ignore-submodules -- 2>/dev/null &&
> +	git diff-files --quiet --ignore-submodules 2>/dev/null &&
>  	(test -z "$untracked" || test -z "$(untracked_files)")
>  }
>  
> @@ -67,7 +67,7 @@ create_stash () {
>  	fi
>  
>  	# state of the base commit
> -	if b_commit=$(git rev-parse --verify HEAD)
> +	if b_commit=$(git rev-parse --verify HEAD 2>/dev/null)
>  	then
>  		head=$(git rev-list --oneline -n 1 HEAD --)
>  	else

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

* Re: [PATCH] stash: don't leak underlying error messages
  2012-04-12 19:30 ` Junio C Hamano
@ 2012-04-13  3:59   ` Ross Lagerwall
  0 siblings, 0 replies; 3+ messages in thread
From: Ross Lagerwall @ 2012-04-13  3:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 867 bytes --]

On 04/12/2012 09:30 PM, Junio C Hamano wrote:
> Sign-off?

Oops, I'm not really used to the whole sign-off thing.

> Is create_stash (hence save_stash) the only operation that do not make
> sense when HEAD is not born yet?  I am wondering if it makes more sense to
> either:
> 
>  (1) catch the case where HEAD is not born yet a lot earlier and do not
>      let the control even reach these functions (i.e. die inside the
>      case/esac statement at the end of the script); or
> 
>  (2) pretend as if HEAD is a commit that records an empty tree, and not
>      error out to begin with.
> 
> If either one of the above turns out to make sense, then the issue this
> patch addresses becomes irrelevant, so...

I think it would be more consistent if stash worked without any commits
having taken place so I'll look at (2).
-- 
Ross Lagerwall


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

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

end of thread, other threads:[~2012-04-13  4:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-11 18:36 [PATCH] stash: don't leak underlying error messages Ross Lagerwall
2012-04-12 19:30 ` Junio C Hamano
2012-04-13  3:59   ` Ross Lagerwall

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.