Patrick Steinhardt <p...@pks.im> writes:

> 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.

> Signed-off-by: Patrick Steinhardt <p...@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 &&

Reply via email to