Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rebase autostash #52

Closed
wants to merge 3 commits into from
Closed

Conversation

dscho
Copy link
Member

@dscho dscho commented Oct 22, 2018

Gábor reported in https://public-inbox.org/git/20181019124625.GB30222@szeder.dev/ that t5520-pull.sh fails from time to time, and Alban root-caused this to a bug in the built-in rebase.

This patch series fixes that, and while at it also fixes an oversight of yours truly when helping Pratik with his GSoC project, and it also adds a change on top that makes really, really certain that git stash apply interprets the OID passed to it correctly (as opposed to an insanely large number for the stash reflog).

Please note that I based these patches on top of next (they might be most appropriately applied on top of rebase-in-c-6-final, though).

(Sorry for the v2, v1 did not send due to an UTF-8 character in the Cc: list, a bug that still needs to be fixed in GitGitGadget.)

Cc: SZEDER Gabor szeder.dev@gmail.com, Alban Gruin alban.gruin@gmail.com

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>
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>
@dscho
Copy link
Member Author

dscho commented Oct 22, 2018

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 22, 2018

Submitted as pull.52.git.gitgitgadget@gmail.com

@dscho
Copy link
Member Author

dscho commented Oct 22, 2018

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 22, 2018

An error occurred while submitting:

Error: d3b47a4 was already submitted

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>
@dscho
Copy link
Member Author

dscho commented Oct 22, 2018

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 22, 2018

Submitted as pull.52.v2.git.gitgitgadget@gmail.com

Copy link
Member Author

dscho commented Nov 2, 2018

This patch series was integrated into pu via git@498042f.

Copy link
Member Author

dscho commented Nov 2, 2018

This patch series was integrated into next via git@680e648.

Copy link
Member Author

dscho commented Nov 5, 2018

This patch series was integrated into pu via git@b78c5fe.

Copy link
Member Author

dscho commented Nov 5, 2018

This patch series was integrated into next via git@b78c5fe.

Copy link
Member Author

dscho commented Nov 5, 2018

This patch series was integrated into master via git@b78c5fe.

@dscho dscho added the master label Nov 5, 2018 — with GitGitGadget
@dscho dscho closed this Nov 5, 2018
Copy link
Member Author

dscho commented Nov 5, 2018

Closed via b78c5fe.

@dscho dscho deleted the fix-rebase-autostash branch November 5, 2018 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant