[PATCH] git-stash: fix pushing stash with pathspec from subdir

Patrick Steinhardt· Jun 13, 2017, 11:38 UTC · lore

The `git stash push` command recently gained the ability to get a pathspec as its argument to only stash matching files. Calling this command from a subdirectory does not work, though, as one of the first things we do is changing to the top level directory without keeping track of the prefix from which the command is being run.

Fix the shortcoming by storing the prefix previous to the call to `cd_to_toplevel` and then subsequently using `git rev-parse --prefix` to correctly resolve the pathspec. Add a test to catch future breakage of this usecase.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 git-stash.sh     |  3 +++
 t/t3903-stash.sh | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

Show changes to 2 files +19 −0

git-stash.sh, t/t3903-stash.sh

diff --git a/git-stash.sh b/git-stash.sh
index 2fb651b2b..e7b85932d 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -19,6 +19,7 @@ OPTIONS_SPEC=
 START_DIR=$(pwd)
 . git-sh-setup
 require_work_tree
+prefix=$(git rev-parse --show-prefix) || exit 1
 cd_to_toplevel
 
 TMP="$GIT_DIR/.git-stash.$$"
@@ -273,6 +274,8 @@ push_stash () {
 		shift
 	done
 
+	eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
+
 	if test -n "$patch_mode" && test -n "$untracked"
 	then
 		die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3b4bed5c9..4046817d7 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -812,6 +812,22 @@ test_expect_success 'stash -- <pathspec> stashes and restores the file' '
 	test_path_is_file bar
 '
 
+test_expect_success 'stash -- <pathspec> stashes in subdirectory' '
+	mkdir sub &&
+	>foo &&
+	>bar &&
+	git add foo bar &&
+	(
+		cd sub &&
+		git stash push -- ../foo
+	) &&
+	test_path_is_file bar &&
+	test_path_is_missing foo &&
+	git stash pop &&
+	test_path_is_file foo &&
+	test_path_is_file bar
+'
+
 test_expect_success 'stash with multiple pathspec arguments' '
 	>foo &&
 	>bar &&

-- 
2.13.1

Junio C Hamano· Jun 13, 2017, 15:33 UTC · re: Patrick Steinhardt · lore

Re: [PATCH] git-stash: fix pushing stash with pathspec from subdir

Patrick Steinhardt <ps@pks.im> writes:

Show 10 quoted lines

> The `git stash push` command recently gained the ability to get a
> pathspec as its argument to only stash matching files. Calling this
> command from a subdirectory does not work, though, as one of the first
> things we do is changing to the top level directory without keeping
> track of the prefix from which the command is being run.
>
> Fix the shortcoming by storing the prefix previous to the call to
> `cd_to_toplevel` and then subsequently using `git rev-parse --prefix` to
> correctly resolve the pathspec. Add a test to catch future breakage of
> this usecase.

It sounds more like a simple bug than "shortcoming" ;-), and the right approach to fix it is to add the original prefix before the pathspecs before using them, which is exactly what your patch does. Looks good.

I suspect that "rev-parse --prefix" needs a bit of tweak to make it truly usable for pathspecs with magic, but that is a totally separate issue.

Thanks.

Show 54 quoted lines

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  git-stash.sh     |  3 +++
>  t/t3903-stash.sh | 16 ++++++++++++++++
>  2 files changed, 19 insertions(+)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 2fb651b2b..e7b85932d 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -19,6 +19,7 @@ OPTIONS_SPEC=
>  START_DIR=$(pwd)
>  . git-sh-setup
>  require_work_tree
> +prefix=$(git rev-parse --show-prefix) || exit 1
>  cd_to_toplevel
>  
>  TMP="$GIT_DIR/.git-stash.$$"
> @@ -273,6 +274,8 @@ push_stash () {
>  		shift
>  	done
>  
> +	eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
> +
>  	if test -n "$patch_mode" && test -n "$untracked"
>  	then
>  		die "$(gettext "Can't use --patch and --include-untracked or --all at the same time")"
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 3b4bed5c9..4046817d7 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -812,6 +812,22 @@ test_expect_success 'stash -- <pathspec> stashes and restores the file' '
>  	test_path_is_file bar
>  '
>  
> +test_expect_success 'stash -- <pathspec> stashes in subdirectory' '
> +	mkdir sub &&
> +	>foo &&
> +	>bar &&
> +	git add foo bar &&
> +	(
> +		cd sub &&
> +		git stash push -- ../foo
> +	) &&
> +	test_path_is_file bar &&
> +	test_path_is_missing foo &&
> +	git stash pop &&
> +	test_path_is_file foo &&
> +	test_path_is_file bar
> +'
> +
>  test_expect_success 'stash with multiple pathspec arguments' '
>  	>foo &&
>  	>bar &&

patchgit-stash: fix pushing stash with pathspec from subdir