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

Update ROGERSM94/fix-rebase to the version that made it upstream #2428

Merged
merged 3 commits into from
Dec 13, 2019

Conversation

dscho
Copy link
Member

@dscho dscho commented Dec 11, 2019

As with many contributions accepted in Git for Windows' fork of Git, I shepherded this patch upstream, and happens frequently, I had to change a couple things in response to reviewer suggestions.

Let's "accept back" the version of the patch that made it already into master (see gitgitgadget#327 for details).

Let's revert that Windows-only patch in favor of the version that made
it upstream.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
One of the trickier aspects of the design of `git rebase
--rebase-merges` is the way labels are generated for the initial todo
list: those labels are supposed to be intuitive and first and foremost
unique.

To that end, `label_oid()` appends a unique suffix when necessary.

Those labels not only need to be unique, but they also need to be valid
refs. To make sure of that, `make_script_with_merges()` replaces
whitespace by dashes.

That would appear to be the wrong layer for that sanitizing step,
though: all callers of `label_oid()` should get that same benefit.

Even if it does not make a difference currently (the only called of
`label_oid()` that passes a label that might need to be sanitized _is_
`make_script_with_merges()`), let's move the responsibility for
sanitizing labels into the `label_oid()` function.

This commit is best viewed with `-w` because it unfortunately needs to
change the indentation of a large block of code in `label_oid()`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The `label` todo command in interactive rebases creates temporary refs
in the `refs/rewritten/` namespace. These refs are stored as loose refs,
i.e. as files in `.git/refs/rewritten/`, therefore they have to conform
with file name limitations on the current filesystem in addition to the
accepted ref format.

This poses a problem in particular on NTFS/FAT, where e.g. the colon,
double-quote and pipe characters are disallowed as part of a file name.

Let's safeguard against this by replacing not only white-space
characters by dashes, but all non-alpha-numeric ones.

However, we exempt non-ASCII UTF-8 characters from that, as it should be
quite possible to reflect branch names such as `↯↯↯` in refs/file names.

Signed-off-by: Matthew Rogers <mattr94@gmail.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho merged commit b5deb24 into git-for-windows:master Dec 13, 2019
@dscho dscho deleted the sync-with-upstream branch December 13, 2019 13:47
git-for-windows-ci pushed a commit that referenced this pull request Dec 13, 2019
Update ROGERSM94/fix-rebase to the version that made it upstream
dscho added a commit that referenced this pull request Dec 29, 2019
Update ROGERSM94/fix-rebase to the version that made it upstream
dscho added a commit that referenced this pull request Jan 4, 2020
Update ROGERSM94/fix-rebase to the version that made it upstream
dscho added a commit that referenced this pull request Jan 13, 2020
Update ROGERSM94/fix-rebase to the version that made it upstream
git-for-windows-ci pushed a commit that referenced this pull request Jan 16, 2020
Update ROGERSM94/fix-rebase to the version that made it upstream
git-for-windows-ci pushed a commit that referenced this pull request Jan 17, 2020
Update ROGERSM94/fix-rebase to the version that made it upstream
git-for-windows-ci pushed a commit that referenced this pull request Jan 22, 2020
Update ROGERSM94/fix-rebase to the version that made it upstream
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant