From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 12 Apr 2012 19:30:20 GMT
Subject: Re: [PATCH] stash: don't leak underlying error messages
Message-ID: <7vaa2g694j.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1334169379-20827-1-git-send-email-rosslagerwall@gmail.com>

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