Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec

From: Thomas Gummerer <t.gummerer@gmail.com>
Date: Feb 27, 2017, 21:53 UTC
Message-ID: <20170227215351.GA17065@hank>
In-Reply-To: <xmqqwpcbuyrd.fsf@gitster.mtv.corp.google.com>

On 02/27, Junio C Hamano wrote:

Show 24 quoted lines

> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> >  	if test -z "$patch_mode"
> >  	then
> > -		git reset --hard ${GIT_QUIET:+-q}
> > +		if test $# != 0
> > +		then
> > +			git reset ${GIT_QUIET:+-q} -- "$@"
> > +			git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
> 
> "ls-files -z" on the command line?  
> 
> Apparently new tests do not cover the correctness of this codepath.
> 
> I wonder if this
> 
> 	git ls-files -z --modified "$@" |
> 	git checkout-index -z --force --stdin
> 
> is what the above "checkout" wanted to do.  The "reset" in the
> previous step presumably updated the index entries that match
> specified pathspec to those of the HEAD, so checking out the paths
> that match "$@" from the index would be the same as checking them
> out from the HEAD (while updating the index with them).

Yes, you're completely right, this is exactly what it should have done. Sorry for being slow on getting this right.

> Perhaps squash the following into an appropriate patch in the
> series?

Thanks, I'll squash this in and re-roll.

Show 43 quoted lines

>  git-stash.sh     |  3 ++-
>  t/t3903-stash.sh | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index 28d0624c75..9c70662cc8 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -300,7 +300,8 @@ push_stash () {
>  		if test $# != 0
>  		then
>  			git reset ${GIT_QUIET:+-q} -- "$@"
> -			git checkout ${GIT_QUIET:+-q} HEAD -- $(git ls-files -z --modified "$@")
> +			git ls-files -z --modified -- "$@" |
> +			git checkout-index -z --force --stdin
>  			git clean --force ${GIT_QUIET:+-q} -d -- "$@"
>  		else
>  			git reset --hard ${GIT_QUIET:+-q}
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index f7733b4dd4..e868aafab2 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -891,4 +891,20 @@ test_expect_success 'stash without verb with pathspec' '
>  	test_path_is_file bar
>  '
>  
> +test_expect_success 'stash with pathspec matching multiple paths' '
> +	echo original >file &&
> +	echo original >other-file &&
> +	git commit -m "two" file other-file &&
> +	echo modified >file &&
> +	echo modified >other-file &&
> +	git stash -- "*file" &&
> +	echo original >expect &&
> +	test_cmp expect file &&
> +	test_cmp expect other-file &&
> +	git stash pop &&
> +	echo modified >expect &&
> +	test_cmp expect file &&
> +	test_cmp expect other-file
> +'
> +
>  test_done