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 <mh@glandium.org>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/stash.c b/builtin/stash.c
index 2a8e6d09b406345519201ef64f572197a409e7a4..fde6397caa16326912087215383506026d00ab79 100644 (file)
--- 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;
 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;
        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()
        };
        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);
 
        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
        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);
        }
 
        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.
         */
         * 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;
                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);
        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 ea30d5f6a0f228971d29e257a89fca6ea594cecb..b22e671608119d37dd73bf6b0df41ce62e1a26f9 100755 (executable)
--- 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
 '
 
        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 &&
 test_expect_success 'branch: do not drop the stash if the branch exists' '
        git stash clear &&
        echo foo >file &&