On 06/14, Mike Hommey wrote:
> Hi,
> 
> `git stash <command> <n>` where n is a number used to work until 2.21.*.
> It doesn't work in 2.22.0.
> 
> Bisection points to:
> 
> dc7bd382b1063303f4f45d243bff371899285acb is the first bad commit
> commit dc7bd382b1063303f4f45d243bff371899285acb
> Author: Paul-Sebastian Ungureanu <ungureanupaulsebast...@gmail.com>
> Date:   Mon Feb 25 23:16:20 2019 +0000
> 
>     stash: convert show to builtin
> 
> which I guess makes sense :)

Yup, this is definitely a bug.  I think it only affected 'git stash
show' however, and not other stash subcommands.  If not, could you
point me to where else you saw this bug?

Below is a patch that should fix it.

--- >8 ---
Subject: [PATCH] stash: fix show referencing stash index

In the conversion of 'stash show' to C in dc7bd382b1 ("stash: convert
show to builtin", 2019-02-25), 'git stash show <n>', where n is the
index of a stash got broken, if n is not a file or a valid revision by
itself.

'stash show' accepts any flag 'git diff' accepts for changing the
output format.  Internally we use 'setup_revisions()' to parse these
command line flags.  Currently we pass the whole argv through to
'setup_revisions()', which includes the stash index.

As the stash index is not a valid revision or a file in the working
tree in most cases however, this 'setup_revisions()' call (and thus
the whole command) ends up failing if we use this form of 'git stash
show'.

Instead of passing the whole argv to 'setup_revisions()', only pass
the flags (and the command name) through, while excluding the stash
reference.  The stash reference is parsed (and validated) in
'get_stash_info()' already.

This separate parsing also means that we currently do produce the
correct output if the command succeeds.

Reported-by: Mike Hommey <m...@glandium.org>
Signed-off-by: Thomas Gummerer <t.gumme...@gmail.com>
---
 builtin/stash.c  |  9 +++++----
 t/t3903-stash.sh | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 2a8e6d09b4..fde6397caa 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -713,11 +713,11 @@ static int git_stash_config(const char *var, const char 
*value, void *cb)
 static int show_stash(int argc, const char **argv, const char *prefix)
 {
        int i;
-       int opts = 0;
        int ret = 0;
        struct stash_info info;
        struct rev_info rev;
        struct argv_array stash_args = ARGV_ARRAY_INIT;
+       struct argv_array revision_args = ARGV_ARRAY_INIT;
        struct option options[] = {
                OPT_END()
        };
@@ -726,11 +726,12 @@ static int show_stash(int argc, const char **argv, const 
char *prefix)
        git_config(git_diff_ui_config, NULL);
        init_revisions(&rev, prefix);
 
+       argv_array_push(&revision_args, argv[0]);
        for (i = 1; i < argc; i++) {
                if (argv[i][0] != '-')
                        argv_array_push(&stash_args, argv[i]);
                else
-                       opts++;
+                       argv_array_push(&revision_args, argv[i]);
        }
 
        ret = get_stash_info(&info, stash_args.argc, stash_args.argv);
@@ -742,7 +743,7 @@ static int show_stash(int argc, const char **argv, const 
char *prefix)
         * The config settings are applied only if there are not passed
         * any options.
         */
-       if (!opts) {
+       if (revision_args.argc == 1) {
                git_config(git_stash_config, NULL);
                if (show_stat)
                        rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
@@ -756,7 +757,7 @@ static int show_stash(int argc, const char **argv, const 
char *prefix)
                }
        }
 
-       argc = setup_revisions(argc, argv, &rev, NULL);
+       argc = setup_revisions(revision_args.argc, revision_args.argv, &rev, 
NULL);
        if (argc > 1) {
                free_stash_info(&info);
                usage_with_options(git_stash_show_usage, options);
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index ea30d5f6a0..3973cbda0e 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -708,6 +708,24 @@ test_expect_success 'invalid ref of the form "n", n >= N' '
        git stash drop
 '
 
+test_expect_success 'valid ref of the form "n", n >= N' '
+       git stash clear &&
+       echo bar5 >file &&
+       echo bar6 >file2 &&
+       git add file2 &&
+       git stash &&
+       git stash show 0 &&
+       git stash branch tmp 0 &&
+       git checkout master &&
+       git stash &&
+       git stash apply 0 &&
+       git reset --hard &&
+       git stash pop 0 &&
+       git stash &&
+       git stash drop 0 &&
+       test_must_fail git stash drop
+'
+
 test_expect_success 'branch: do not drop the stash if the branch exists' '
        git stash clear &&
        echo foo >file &&
-- 
2.22.0.rc2

Reply via email to