* [PATCH v2 1/3] rebase (autostash): avoid duplicate call to state_dir_path()
2018-10-22 22:15 ` [PATCH v2 0/3] Fix rebase autostash Johannes Schindelin via GitGitGadget
@ 2018-10-22 22:15 ` Johannes Schindelin via GitGitGadget
2018-10-22 22:15 ` [PATCH v2 2/3] rebase (autostash): store the full OID in <state-dir>/autostash Johannes Schindelin via GitGitGadget
2018-10-22 22:15 ` [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash Johannes Schindelin via GitGitGadget
2 siblings, 0 replies; 9+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2018-10-22 22:15 UTC (permalink / raw)
To: git; +Cc: SZEDER Gabor, Alban Gruin, Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
We already called that function at this point, and stored the result in
the `path` variable. We might just as well use it ;-)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 313a8263d..d21504d9f 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -251,7 +251,7 @@ static int apply_autostash(struct rebase_options *opts)
if (!file_exists(path))
return 0;
- if (read_one(state_dir_path("autostash", opts), &autostash))
+ if (read_one(path, &autostash))
return error(_("Could not read '%s'"), path);
argv_array_pushl(&stash_apply.args,
"stash", "apply", autostash.buf, NULL);
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] rebase (autostash): store the full OID in <state-dir>/autostash
2018-10-22 22:15 ` [PATCH v2 0/3] Fix rebase autostash Johannes Schindelin via GitGitGadget
2018-10-22 22:15 ` [PATCH v2 1/3] rebase (autostash): avoid duplicate call to state_dir_path() Johannes Schindelin via GitGitGadget
@ 2018-10-22 22:15 ` Johannes Schindelin via GitGitGadget
2018-10-22 22:15 ` [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash Johannes Schindelin via GitGitGadget
2 siblings, 0 replies; 9+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2018-10-22 22:15 UTC (permalink / raw)
To: git; +Cc: SZEDER Gabor, Alban Gruin, Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
It was reported by Gábor Szeder and analyzed by Alban Gruin that the
built-in rebase stores only abbreviated stash hashes in the `autostash`
file.
This is problematic e.g. in t5520-pull.sh, where the abbreviated hash is
so short that it sometimes consists only of digits, which are
subsequently mistaken ("DWIMmed") for numbers by `git stash apply`.
Let's align the behavior of the built-in rebase with the scripted rebase
and store the full stash hash instead. That makes it a lot less likely
that it consists only of digits.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index d21504d9f..418624837 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1375,7 +1375,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
if (safe_create_leading_directories_const(autostash))
die(_("Could not create directory for '%s'"),
options.state_dir);
- write_file(autostash, "%s", buf.buf);
+ write_file(autostash, "%s", oid_to_hex(&oid));
printf(_("Created autostash: %s\n"), buf.buf);
if (reset_head(&head->object.oid, "reset --hard",
NULL, 0, NULL, NULL) < 0)
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash
2018-10-22 22:15 ` [PATCH v2 0/3] Fix rebase autostash Johannes Schindelin via GitGitGadget
2018-10-22 22:15 ` [PATCH v2 1/3] rebase (autostash): avoid duplicate call to state_dir_path() Johannes Schindelin via GitGitGadget
2018-10-22 22:15 ` [PATCH v2 2/3] rebase (autostash): store the full OID in <state-dir>/autostash Johannes Schindelin via GitGitGadget
@ 2018-10-22 22:15 ` Johannes Schindelin via GitGitGadget
2018-10-22 22:25 ` Eric Sunshine
` (2 more replies)
2 siblings, 3 replies; 9+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2018-10-22 22:15 UTC (permalink / raw)
To: git; +Cc: SZEDER Gabor, Alban Gruin, Junio C Hamano, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
When `git stash apply <argument>` sees an argument that consists only of
digits, it tries to be smart and interpret it as `stash@{<number>}`.
Unfortunately, an all-digit hash (which is unlikely but still possible)
is therefore misinterpreted as `stash@{<n>}` reflog.
To prevent that from happening, let's append `^0` after the stash hash,
to make sure that it is interpreted as an OID rather than as a number.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/rebase.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 418624837..30d58118c 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -253,6 +253,8 @@ static int apply_autostash(struct rebase_options *opts)
if (read_one(path, &autostash))
return error(_("Could not read '%s'"), path);
+ /* Ensure that the hash is not mistake for a number */
+ strbuf_addstr(&autostash, "^0");
argv_array_pushl(&stash_apply.args,
"stash", "apply", autostash.buf, NULL);
stash_apply.git_cmd = 1;
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash
2018-10-22 22:15 ` [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash Johannes Schindelin via GitGitGadget
@ 2018-10-22 22:25 ` Eric Sunshine
2018-10-22 22:32 ` SZEDER Gábor
2018-10-23 17:07 ` Alban Gruin
2 siblings, 0 replies; 9+ messages in thread
From: Eric Sunshine @ 2018-10-22 22:25 UTC (permalink / raw)
To: gitgitgadget
Cc: Git List, SZEDER Gábor, Alban Gruin, Junio C Hamano,
Johannes Schindelin
On Mon, Oct 22, 2018 at 6:15 PM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> When `git stash apply <argument>` sees an argument that consists only of
> digits, it tries to be smart and interpret it as `stash@{<number>}`.
>
> Unfortunately, an all-digit hash (which is unlikely but still possible)
> is therefore misinterpreted as `stash@{<n>}` reflog.
>
> To prevent that from happening, let's append `^0` after the stash hash,
> to make sure that it is interpreted as an OID rather than as a number.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> @@ -253,6 +253,8 @@ static int apply_autostash(struct rebase_options *opts)
>
> if (read_one(path, &autostash))
> return error(_("Could not read '%s'"), path);
> + /* Ensure that the hash is not mistake for a number */
s/mistake/mistaken/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash
2018-10-22 22:15 ` [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash Johannes Schindelin via GitGitGadget
2018-10-22 22:25 ` Eric Sunshine
@ 2018-10-22 22:32 ` SZEDER Gábor
2018-10-23 4:13 ` Junio C Hamano
2018-10-23 17:07 ` Alban Gruin
2 siblings, 1 reply; 9+ messages in thread
From: SZEDER Gábor @ 2018-10-22 22:32 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Alban Gruin, Junio C Hamano, Johannes Schindelin
On Mon, Oct 22, 2018 at 03:15:05PM -0700, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> When `git stash apply <argument>` sees an argument that consists only of
> digits, it tries to be smart and interpret it as `stash@{<number>}`.
>
> Unfortunately, an all-digit hash (which is unlikely but still possible)
> is therefore misinterpreted as `stash@{<n>}` reflog.
>
> To prevent that from happening, let's append `^0` after the stash hash,
> to make sure that it is interpreted as an OID rather than as a number.
Oh, this is clever.
FWIW, all patches look good to me, barring the typo below.
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> builtin/rebase.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 418624837..30d58118c 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -253,6 +253,8 @@ static int apply_autostash(struct rebase_options *opts)
>
> if (read_one(path, &autostash))
> return error(_("Could not read '%s'"), path);
> + /* Ensure that the hash is not mistake for a number */
s/mistake/mistaken/
> + strbuf_addstr(&autostash, "^0");
> argv_array_pushl(&stash_apply.args,
> "stash", "apply", autostash.buf, NULL);
> stash_apply.git_cmd = 1;
> --
> gitgitgadget
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash
2018-10-22 22:32 ` SZEDER Gábor
@ 2018-10-23 4:13 ` Junio C Hamano
2018-10-23 9:34 ` Johannes Schindelin
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2018-10-23 4:13 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Johannes Schindelin via GitGitGadget, git, Alban Gruin,
Johannes Schindelin
SZEDER Gábor <szeder.dev@gmail.com> writes:
>> To prevent that from happening, let's append `^0` after the stash hash,
>> to make sure that it is interpreted as an OID rather than as a number.
>
> Oh, this is clever.
Yeah, we can do this as we know we'd be dealing with a commit-ish.
If we made a mistake to use a tree object to represent a stash, this
trick wouldn't have been possible ;-)
> FWIW, all patches look good to me, barring the typo below.
> ...
>> + /* Ensure that the hash is not mistake for a number */
>
> s/mistake/mistaken/
Will squash this in and add your reviewed-by before queuing.
Thanks, both.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash
2018-10-23 4:13 ` Junio C Hamano
@ 2018-10-23 9:34 ` Johannes Schindelin
0 siblings, 0 replies; 9+ messages in thread
From: Johannes Schindelin @ 2018-10-23 9:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, Johannes Schindelin via GitGitGadget, git,
Alban Gruin
[-- Attachment #1: Type: text/plain, Size: 733 bytes --]
Hi,
On Tue, 23 Oct 2018, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
> >> To prevent that from happening, let's append `^0` after the stash hash,
> >> to make sure that it is interpreted as an OID rather than as a number.
> >
> > Oh, this is clever.
>
> Yeah, we can do this as we know we'd be dealing with a commit-ish.
> If we made a mistake to use a tree object to represent a stash, this
> trick wouldn't have been possible ;-)
>
> > FWIW, all patches look good to me, barring the typo below.
> > ...
> >> + /* Ensure that the hash is not mistake for a number */
> >
> > s/mistake/mistaken/
>
> Will squash this in and add your reviewed-by before queuing.
>
> Thanks, both.
Thank you all!
Dscho
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash
2018-10-22 22:15 ` [PATCH v2 3/3] rebase (autostash): use an explicit OID to apply the stash Johannes Schindelin via GitGitGadget
2018-10-22 22:25 ` Eric Sunshine
2018-10-22 22:32 ` SZEDER Gábor
@ 2018-10-23 17:07 ` Alban Gruin
2 siblings, 0 replies; 9+ messages in thread
From: Alban Gruin @ 2018-10-23 17:07 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget, git
Cc: SZEDER Gabor, Junio C Hamano, Johannes Schindelin
Hi Johannes,
this looks good to me, too!
Cheers,
Alban
^ permalink raw reply [flat|nested] 9+ messages in thread