{"thread":{"id":"51311","subject":"`git stash <command> <n>` stopped working in 2.22.0","startedAt":"2019-06-14T07:42:12Z","lastAt":"2019-06-24T20:29:13Z","messageCount":5,"participants":["Mike Hommey","Thomas Gummerer","Andrei Rybak"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"377222","messageId":"20190614074207.mxidz3h573mtd43x@glandium.org","threadId":"51311","inReplyTo":null,"subject":"`git stash <command> <n>` stopped working in 2.22.0","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2019-06-14T07:42:07Z","receivedAt":"2019-06-14T07:42:12Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"Hi,\n\n`git stash <command> <n>` where n is a number used to work until 2.21.*.\nIt doesn't work in 2.22.0.\n\nBisection points to:\n\ndc7bd382b1063303f4f45d243bff371899285acb is the first bad commit\ncommit dc7bd382b1063303f4f45d243bff371899285acb\nAuthor: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>\nDate:   Mon Feb 25 23:16:20 2019 +0000\n\n    stash: convert show to builtin\n\nwhich I guess makes sense :)\n\nMike\n"},{"id":"377301","messageId":"20190615112618.GC11340@hank.intra.tgummerer.com","threadId":"51311","inReplyTo":"20190614074207.mxidz3h573mtd43x@glandium.org","subject":"[PATCH] stash: fix show referencing stash index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-06-15T11:26:18Z","receivedAt":"2019-06-15T11:26:25Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 06/14, Mike Hommey wrote:\n> Hi,\n> \n> `git stash <command> <n>` where n is a number used to work until 2.21.*.\n> It doesn't work in 2.22.0.\n> \n> Bisection points to:\n> \n> dc7bd382b1063303f4f45d243bff371899285acb is the first bad commit\n> commit dc7bd382b1063303f4f45d243bff371899285acb\n> Author: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>\n> Date:   Mon Feb 25 23:16:20 2019 +0000\n> \n>     stash: convert show to builtin\n> \n> which I guess makes sense :)\n\nYup, this is definitely a bug.  I think it only affected 'git stash\nshow' however, and not other stash subcommands.  If not, could you\npoint me to where else you saw this bug?\n\nBelow is a patch that should fix it.\n\n--- >8 ---\nSubject: [PATCH] stash: fix show referencing stash index\n\nIn the conversion of 'stash show' to C in dc7bd382b1 (\"stash: convert\nshow to builtin\", 2019-02-25), 'git stash show <n>', where n is the\nindex of a stash got broken, if n is not a file or a valid revision by\nitself.\n\n'stash show' accepts any flag 'git diff' accepts for changing the\noutput format.  Internally we use 'setup_revisions()' to parse these\ncommand line flags.  Currently we pass the whole argv through to\n'setup_revisions()', which includes the stash index.\n\nAs the stash index is not a valid revision or a file in the working\ntree in most cases however, this 'setup_revisions()' call (and thus\nthe whole command) ends up failing if we use this form of 'git stash\nshow'.\n\nInstead of passing the whole argv to 'setup_revisions()', only pass\nthe flags (and the command name) through, while excluding the stash\nreference.  The stash reference is parsed (and validated) in\n'get_stash_info()' already.\n\nThis separate parsing also means that we currently do produce the\ncorrect output if the command succeeds.\n\nReported-by: Mike Hommey <mh@glandium.org>\nSigned-off-by: Thomas Gummerer <t.gummerer@gmail.com>\n---\n builtin/stash.c  |  9 +++++----\n t/t3903-stash.sh | 18 ++++++++++++++++++\n 2 files changed, 23 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 2a8e6d09b4..fde6397caa 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -713,11 +713,11 @@ static int git_stash_config(const char *var, const char *value, void *cb)\n static int show_stash(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n-\tint opts = 0;\n \tint ret = 0;\n \tstruct stash_info info;\n \tstruct rev_info rev;\n \tstruct argv_array stash_args = ARGV_ARRAY_INIT;\n+\tstruct argv_array revision_args = ARGV_ARRAY_INIT;\n \tstruct option options[] = {\n \t\tOPT_END()\n \t};\n@@ -726,11 +726,12 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n \tgit_config(git_diff_ui_config, NULL);\n \tinit_revisions(&rev, prefix);\n \n+\targv_array_push(&revision_args, argv[0]);\n \tfor (i = 1; i < argc; i++) {\n \t\tif (argv[i][0] != '-')\n \t\t\targv_array_push(&stash_args, argv[i]);\n \t\telse\n-\t\t\topts++;\n+\t\t\targv_array_push(&revision_args, argv[i]);\n \t}\n \n \tret = get_stash_info(&info, stash_args.argc, stash_args.argv);\n@@ -742,7 +743,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n \t * The config settings are applied only if there are not passed\n \t * any options.\n \t */\n-\tif (!opts) {\n+\tif (revision_args.argc == 1) {\n \t\tgit_config(git_stash_config, NULL);\n \t\tif (show_stat)\n \t\t\trev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;\n@@ -756,7 +757,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\targc = setup_revisions(argc, argv, &rev, NULL);\n+\targc = setup_revisions(revision_args.argc, revision_args.argv, &rev, NULL);\n \tif (argc > 1) {\n \t\tfree_stash_info(&info);\n \t\tusage_with_options(git_stash_show_usage, options);\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex ea30d5f6a0..3973cbda0e 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -708,6 +708,24 @@ test_expect_success 'invalid ref of the form \"n\", n >= N' '\n \tgit stash drop\n '\n \n+test_expect_success 'valid ref of the form \"n\", n >= N' '\n+\tgit stash clear &&\n+\techo bar5 >file &&\n+\techo bar6 >file2 &&\n+\tgit add file2 &&\n+\tgit stash &&\n+\tgit stash show 0 &&\n+\tgit stash branch tmp 0 &&\n+\tgit checkout master &&\n+\tgit stash &&\n+\tgit stash apply 0 &&\n+\tgit reset --hard &&\n+\tgit stash pop 0 &&\n+\tgit stash &&\n+\tgit stash drop 0 &&\n+\ttest_must_fail git stash drop\n+'\n+\n test_expect_success 'branch: do not drop the stash if the branch exists' '\n \tgit stash clear &&\n \techo foo >file &&\n-- \n2.22.0.rc2\n\n"},{"id":"377302","messageId":"78f730f1-d40e-1415-b6a6-4a1b224e7818@gmail.com","threadId":"51311","inReplyTo":"20190615112618.GC11340@hank.intra.tgummerer.com","subject":"Re: [PATCH] stash: fix show referencing stash index","fromName":"Andrei Rybak","fromEmail":"rybak.a.v@gmail.com","sentAt":"2019-06-15T13:02:57Z","receivedAt":"2019-06-15T13:03:04Z","isPatch":true,"sender":{"key":"rybak.a.v@gmail.com","avatar":"https://avatars.githubusercontent.com/u/624072?v=4"},"body":"On 6/15/19 1:26 PM, Thomas Gummerer wrote:\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index ea30d5f6a0..3973cbda0e 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -708,6 +708,24 @@ test_expect_success 'invalid ref of the form \"n\", n >= N' '\n>  \tgit stash drop\n>  '\n>  \n> +test_expect_success 'valid ref of the form \"n\", n >= N' '\n\n\nIf ref is valid, 'n < N' was probably meant here.\n"},{"id":"377347","messageId":"20190617060311.GE28007@hank.intra.tgummerer.com","threadId":"51311","inReplyTo":"78f730f1-d40e-1415-b6a6-4a1b224e7818@gmail.com","subject":"Re: [PATCH] stash: fix show referencing stash index","fromName":"Thomas Gummerer","fromEmail":"t.gummerer@gmail.com","sentAt":"2019-06-17T06:03:11Z","receivedAt":"2019-06-17T06:03:17Z","isPatch":true,"sender":{"key":"t.gummerer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/191004?v=4"},"body":"On 06/15, Andrei Rybak wrote:\n> On 6/15/19 1:26 PM, Thomas Gummerer wrote:\n> > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> > index ea30d5f6a0..3973cbda0e 100755\n> > --- a/t/t3903-stash.sh\n> > +++ b/t/t3903-stash.sh\n> > @@ -708,6 +708,24 @@ test_expect_success 'invalid ref of the form \"n\", n >= N' '\n> >  \tgit stash drop\n> >  '\n> >  \n> > +test_expect_success 'valid ref of the form \"n\", n >= N' '\n> \n> If ref is valid, 'n < N' was probably meant here.\n\nYes, indeed.  Thanks!\n"},{"id":"377931","messageId":"20190624202908.atlphbwai7miwl3u@glandium.org","threadId":"51311","inReplyTo":"20190615112618.GC11340@hank.intra.tgummerer.com","subject":"Re: [PATCH] stash: fix show referencing stash index","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2019-06-24T20:29:08Z","receivedAt":"2019-06-24T20:29:13Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Sat, Jun 15, 2019 at 12:26:18PM +0100, Thomas Gummerer wrote:\n> On 06/14, Mike Hommey wrote:\n> > Hi,\n> > \n> > `git stash <command> <n>` where n is a number used to work until 2.21.*.\n> > It doesn't work in 2.22.0.\n> > \n> > Bisection points to:\n> > \n> > dc7bd382b1063303f4f45d243bff371899285acb is the first bad commit\n> > commit dc7bd382b1063303f4f45d243bff371899285acb\n> > Author: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>\n> > Date:   Mon Feb 25 23:16:20 2019 +0000\n> > \n> >     stash: convert show to builtin\n> > \n> > which I guess makes sense :)\n> \n> Yup, this is definitely a bug.  I think it only affected 'git stash\n> show' however, and not other stash subcommands.  If not, could you\n> point me to where else you saw this bug?\n\nI confirmed pop, apply, branch, and drop are not affected.\n\nMike\n"}]}