* [PATCH] stash: fix show referencing stash index
2019-06-14 7:42 `git stash <command> <n>` stopped working in 2.22.0 Mike Hommey
@ 2019-06-15 11:26 ` Thomas Gummerer
2019-06-15 13:02 ` Andrei Rybak
2019-06-24 20:29 ` Mike Hommey
0 siblings, 2 replies; 5+ messages in thread
From: Thomas Gummerer @ 2019-06-15 11:26 UTC (permalink / raw)
To: Mike Hommey; +Cc: git, ungureanupaulsebastian
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 <ungureanupaulsebastian@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 <mh@glandium.org>
Signed-off-by: Thomas Gummerer <t.gummerer@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
^ permalink raw reply related [flat|nested] 5+ messages in thread