Skip to content

Make git rebase -r's label generation more resilient

Johannes Schindelin via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest2 patches 2 repliesIn review
  2. v11 patch 11 repliesSuperseded

Changes in v2 from the cover letter

* moved the entire sanitizing logic to label_oid(), as a preparatory step.

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2rebase-merges: move labels' whitespace mangling into `label_oid()`sequencer.c+28 -280 replies
2/2rebase -r: let `label` generate safer labelssequencer.c t/t3430-rebase-merges.sh+25 -10 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.

Read the v2 thread

Cover letter [PATCH v2 0/2] Make git rebase -r's label generation more resilient

Those labels must be valid ref names, and therefore valid file names. The initial patch came in via Git for Windows.

Change since v1:

* moved the entire sanitizing logic to label_oid(), as a preparatory step.

Johannes Schindelin (1):
rebase-merges: move labels' whitespace mangling into label_oid()

Matthew Rogers (1):
rebase -r: let label generate safer labels

 sequencer.c              | 72 +++++++++++++++++++++++++---------------
 t/t3430-rebase-merges.sh |  6 ++++
 2 files changed, 51 insertions(+), 27 deletions(-)

base-commit: d9f6f3b6195a0ca35642561e530798ad1469bd41
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-327%2Fdscho%2Ffix-rebase-r-labels-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-327/dscho/fix-rebase-r-labels-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/327

Range-diff vs v1:

 -:  ---------- > 1:  3f4d086e80 rebase-merges: move labels' whitespace mangling into `label_oid()`
 1:  4a02c38442 ! 2:  adc22c8ba2 rebase -r: let `label` generate safer labels

@@ -1,14 +1,15 @@
-Author: Matt R <[email protected]>
+Author: Matthew Rogers <[email protected]>

rebase -r: let label generate safer labels

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.
+ 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
- character is not a valid part of a file name.
+ 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.
@@ -23,8 +24,8 @@
--- a/sequencer.c
+++ b/sequencer.c
@@
- else
- strbuf_addbuf(&label, &oneline);
+ } else {
+ struct strbuf *buf = &state->buf;

+ /*
+ * Sanitize labels by replacing non-alpha-numeric characters
@@ -36,18 +37,26 @@
+ * in file names. We do not validate the UTF-8 here, that's not
+ * the job of this function.
+ */
- for (p1 = label.buf; *p1; p1++)
-- if (isspace(*p1))
-+ if (!(*p1 & 0x80) && !isalnum(*p1))
- *(char *)p1 = '-';
+ for (; *label; label++)
+- strbuf_addch(buf, isspace(*label) ? '-' : *label);
++ if ((*label & 0x80) || isalnum(*label))
++ strbuf_addch(buf, *label);
++ /* avoid leading dash and double-dashes */
++ else if (buf->len && buf->buf[buf->len - 1] != '-')
++ strbuf_addch(buf, '-');
++ if (!buf->len) {
++ strbuf_addstr(buf, "rev-");
++ strbuf_add_unique_abbrev(buf, oid, default_abbrev);
++ }
+ label = buf->buf;

- strbuf_reset(&buf);
+ if ((buf->len == the_hash_algo->hexsz &&

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh --- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@
- test_path_is_missing .git/MERGE_HEAD
+ test_cmp expect G.t
'

+test_expect_success '--rebase-merges with commit that can generate bad characters for filename' '
@@ -55,4 +64,5 @@
+ git merge -m "colon: this should work" G &&
+ git rebase --rebase-merges --force-rebase E
+'
++
test_done

Signature · 1 line
gitgitgadget