{"thread":{"id":"46167","subject":"[PATCH] git-stash: fix pushing stash with pathspec from subdir","startedAt":"2017-06-13T11:38:28Z","lastAt":"2017-06-13T15:33:34Z","messageCount":2,"participants":["Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"322074","messageId":"6200f6bf1a9ac81d02391b898d203d650c6af962.1497353866.git.ps@pks.im","threadId":"46167","inReplyTo":null,"subject":"[PATCH] git-stash: fix pushing stash with pathspec from subdir","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2017-06-13T11:38:34Z","receivedAt":"2017-06-13T11:38:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `git stash push` command recently gained the ability to get a\npathspec as its argument to only stash matching files. Calling this\ncommand from a subdirectory does not work, though, as one of the first\nthings we do is changing to the top level directory without keeping\ntrack of the prefix from which the command is being run.\n\nFix the shortcoming by storing the prefix previous to the call to\n`cd_to_toplevel` and then subsequently using `git rev-parse --prefix` to\ncorrectly resolve the pathspec. Add a test to catch future breakage of\nthis usecase.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n git-stash.sh     |  3 +++\n t/t3903-stash.sh | 16 ++++++++++++++++\n 2 files changed, 19 insertions(+)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 2fb651b2b..e7b85932d 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -19,6 +19,7 @@ OPTIONS_SPEC=\n START_DIR=$(pwd)\n . git-sh-setup\n require_work_tree\n+prefix=$(git rev-parse --show-prefix) || exit 1\n cd_to_toplevel\n \n TMP=\"$GIT_DIR/.git-stash.$$\"\n@@ -273,6 +274,8 @@ push_stash () {\n \t\tshift\n \tdone\n \n+\teval \"set $(git rev-parse --sq --prefix \"$prefix\" -- \"$@\")\"\n+\n \tif test -n \"$patch_mode\" && test -n \"$untracked\"\n \tthen\n \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 3b4bed5c9..4046817d7 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -812,6 +812,22 @@ test_expect_success 'stash -- <pathspec> stashes and restores the file' '\n \ttest_path_is_file bar\n '\n \n+test_expect_success 'stash -- <pathspec> stashes in subdirectory' '\n+\tmkdir sub &&\n+\t>foo &&\n+\t>bar &&\n+\tgit add foo bar &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit stash push -- ../foo\n+\t) &&\n+\ttest_path_is_file bar &&\n+\ttest_path_is_missing foo &&\n+\tgit stash pop &&\n+\ttest_path_is_file foo &&\n+\ttest_path_is_file bar\n+'\n+\n test_expect_success 'stash with multiple pathspec arguments' '\n \t>foo &&\n \t>bar &&\n-- \n2.13.1\n\n"},{"id":"322093","messageId":"xmqqr2yn9a2h.fsf@gitster.mtv.corp.google.com","threadId":"46167","inReplyTo":"6200f6bf1a9ac81d02391b898d203d650c6af962.1497353866.git.ps@pks.im","subject":"Re: [PATCH] git-stash: fix pushing stash with pathspec from subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-06-13T15:33:26Z","receivedAt":"2017-06-13T15:33:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The `git stash push` command recently gained the ability to get a\n> pathspec as its argument to only stash matching files. Calling this\n> command from a subdirectory does not work, though, as one of the first\n> things we do is changing to the top level directory without keeping\n> track of the prefix from which the command is being run.\n>\n> Fix the shortcoming by storing the prefix previous to the call to\n> `cd_to_toplevel` and then subsequently using `git rev-parse --prefix` to\n> correctly resolve the pathspec. Add a test to catch future breakage of\n> this usecase.\n\nIt sounds more like a simple bug than \"shortcoming\" ;-), and the\nright approach to fix it is to add the original prefix before the\npathspecs before using them, which is exactly what your patch does.\nLooks good.\n\nI suspect that \"rev-parse --prefix\" needs a bit of tweak to make it\ntruly usable for pathspecs with magic, but that is a totally\nseparate issue.\n\nThanks.\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  git-stash.sh     |  3 +++\n>  t/t3903-stash.sh | 16 ++++++++++++++++\n>  2 files changed, 19 insertions(+)\n>\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 2fb651b2b..e7b85932d 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -19,6 +19,7 @@ OPTIONS_SPEC=\n>  START_DIR=$(pwd)\n>  . git-sh-setup\n>  require_work_tree\n> +prefix=$(git rev-parse --show-prefix) || exit 1\n>  cd_to_toplevel\n>  \n>  TMP=\"$GIT_DIR/.git-stash.$$\"\n> @@ -273,6 +274,8 @@ push_stash () {\n>  \t\tshift\n>  \tdone\n>  \n> +\teval \"set $(git rev-parse --sq --prefix \"$prefix\" -- \"$@\")\"\n> +\n>  \tif test -n \"$patch_mode\" && test -n \"$untracked\"\n>  \tthen\n>  \t\tdie \"$(gettext \"Can't use --patch and --include-untracked or --all at the same time\")\"\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index 3b4bed5c9..4046817d7 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -812,6 +812,22 @@ test_expect_success 'stash -- <pathspec> stashes and restores the file' '\n>  \ttest_path_is_file bar\n>  '\n>  \n> +test_expect_success 'stash -- <pathspec> stashes in subdirectory' '\n> +\tmkdir sub &&\n> +\t>foo &&\n> +\t>bar &&\n> +\tgit add foo bar &&\n> +\t(\n> +\t\tcd sub &&\n> +\t\tgit stash push -- ../foo\n> +\t) &&\n> +\ttest_path_is_file bar &&\n> +\ttest_path_is_missing foo &&\n> +\tgit stash pop &&\n> +\ttest_path_is_file foo &&\n> +\ttest_path_is_file bar\n> +'\n> +\n>  test_expect_success 'stash with multiple pathspec arguments' '\n>  \t>foo &&\n>  \t>bar &&\n"}]}