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>

Thomas Gummerer committed Jun 15, 2019 at 12:26 UTC 63b50c8ffe7133d2ec7d243dd9240c14ddfe8a26
2 files changed +23 -4
builtin/stash.c
+5 -4
@@ -713,11 +713,11 @@ static int git_stash_config(const char *var, const char *value, void *cb)
713 static int show_stash(int argc, const char **argv, const char *prefix)
714 {
715 int i;
716 - int opts = 0;
716 int ret = 0;
717 struct stash_info info;
718 struct rev_info rev;
719 struct argv_array stash_args = ARGV_ARRAY_INIT;
720 + struct argv_array revision_args = ARGV_ARRAY_INIT;
721 struct option options[] = {
722 OPT_END()
723 };
@@ -726,11 +726,12 @@ static int show_stash(int argc, const char **argv, const char *prefix)
726 git_config(git_diff_ui_config, NULL);
727 init_revisions(&rev, prefix);
728
729 + argv_array_push(&revision_args, argv[0]);
730 for (i = 1; i < argc; i++) {
731 if (argv[i][0] != '-')
732 argv_array_push(&stash_args, argv[i]);
733 else
733 - opts++;
734 + argv_array_push(&revision_args, argv[i]);
735 }
736
737 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)
743 * The config settings are applied only if there are not passed
744 * any options.
745 */
745 - if (!opts) {
746 + if (revision_args.argc == 1) {
747 git_config(git_stash_config, NULL);
748 if (show_stat)
749 rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
@@ -756,7 +757,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)
757 }
758 }
759
759 - argc = setup_revisions(argc, argv, &rev, NULL);
760 + argc = setup_revisions(revision_args.argc, revision_args.argv, &rev, NULL);
761 if (argc > 1) {
762 free_stash_info(&info);
763 usage_with_options(git_stash_show_usage, options);
t/t3903-stash.sh
+18
@@ -708,6 +708,24 @@ test_expect_success 'invalid ref of the form "n", n >= N' '
708 git stash drop
709 '
710
711 +test_expect_success 'valid ref of the form "n", n < N' '
712 + git stash clear &&
713 + echo bar5 >file &&
714 + echo bar6 >file2 &&
715 + git add file2 &&
716 + git stash &&
717 + git stash show 0 &&
718 + git stash branch tmp 0 &&
719 + git checkout master &&
720 + git stash &&
721 + git stash apply 0 &&
722 + git reset --hard &&
723 + git stash pop 0 &&
724 + git stash &&
725 + git stash drop 0 &&
726 + test_must_fail git stash drop
727 +'
728 +
729 test_expect_success 'branch: do not drop the stash if the branch exists' '
730 git stash clear &&
731 echo foo >file &&