Merge branch 'js/rebase-r-safer-label'

A label used in the todo list that are generated by "git rebase
--rebase-merges" is used as a part of a refname; the logic to come
up with the label has been tightened to avoid names that cannot be
used as such.

* js/rebase-r-safer-label:
  rebase -r: let `label` generate safer labels
  rebase-merges: move labels' whitespace mangling into `label_oid()`

diff --git a/sequencer.c b/sequencer.c
index 2c6fbcb41ae30b1186dbef073d29e732dac0c722..115d5bd4b635e83534cbfb3b5082bbb9c57b313f 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -4430,7 +4430,6 @@ static const char *label_oid(struct object_id *oid, const char *label,
        struct labels_entry *labels_entry;
        struct string_entry *string_entry;
        struct object_id dummy;
-       size_t len;
        int i;
 
        string_entry = oidmap_get(&state->commit2label, oid);
@@ -4450,10 +4449,10 @@ static const char *label_oid(struct object_id *oid, const char *label,
         * abbreviation for any uninteresting commit's names that does not
         * clash with any other label.
         */
+       strbuf_reset(&state->buf);
        if (!label) {
                char *p;
 
-               strbuf_reset(&state->buf);
                strbuf_grow(&state->buf, GIT_MAX_HEXSZ);
                label = p = state->buf.buf;
 
@@ -4476,32 +4475,55 @@ static const char *label_oid(struct object_id *oid, const char *label,
                                p[i] = save;
                        }
                }
-       } else if (((len = strlen(label)) == the_hash_algo->hexsz &&
-                   !get_oid_hex(label, &dummy)) ||
-                  (len == 1 && *label == '#') ||
-                  hashmap_get_from_hash(&state->labels,
-                                        strihash(label), label)) {
+       } else {
+               struct strbuf *buf = &state->buf;
+
                /*
-                * If the label already exists, or if the label is a valid full
-                * OID, or the label is a '#' (which we use as a separator
-                * between merge heads and oneline), we append a dash and a
-                * number to make it unique.
+                * Sanitize labels by replacing non-alpha-numeric characters
+                * (including white-space ones) by dashes, as they might be
+                * illegal in file names (and hence in ref names).
+                *
+                * Note that we retain non-ASCII UTF-8 characters (identified
+                * via the most significant bit). They should be all acceptable
+                * in file names. We do not validate the UTF-8 here, that's not
+                * the job of this function.
                 */
-               struct strbuf *buf = &state->buf;
+               for (; *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);
-               strbuf_add(buf, label, len);
+               if ((buf->len == the_hash_algo->hexsz &&
+                    !get_oid_hex(label, &dummy)) ||
+                   (buf->len == 1 && *label == '#') ||
+                   hashmap_get_from_hash(&state->labels,
+                                         strihash(label), label)) {
+                       /*
+                        * If the label already exists, or if the label is a
+                        * valid full OID, or the label is a '#' (which we use
+                        * as a separator between merge heads and oneline), we
+                        * append a dash and a number to make it unique.
+                        */
+                       size_t len = buf->len;
 
-               for (i = 2; ; i++) {
-                       strbuf_setlen(buf, len);
-                       strbuf_addf(buf, "-%d", i);
-                       if (!hashmap_get_from_hash(&state->labels,
-                                                  strihash(buf->buf),
-                                                  buf->buf))
-                               break;
-               }
+                       for (i = 2; ; i++) {
+                               strbuf_setlen(buf, len);
+                               strbuf_addf(buf, "-%d", i);
+                               if (!hashmap_get_from_hash(&state->labels,
+                                                          strihash(buf->buf),
+                                                          buf->buf))
+                                       break;
+                       }
 
-               label = buf->buf;
+                       label = buf->buf;
+               }
        }
 
        FLEX_ALLOC_STR(labels_entry, label, label);
@@ -4603,10 +4625,6 @@ static int make_script_with_merges(struct pretty_print_context *pp,
                else
                        strbuf_addbuf(&label, &oneline);
 
-               for (p1 = label.buf; *p1; p1++)
-                       if (isspace(*p1))
-                               *(char *)p1 = '-';
-
                strbuf_reset(&buf);
                strbuf_addf(&buf, "%s -C %s",
                            cmd_merge, oid_to_hex(&commit->object.oid));

diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index a30d27e9f366853f6ddca1817088f4aa68ed9291..bd701458ea504d16bb198bec77246fb694572a13 100755 (executable)
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -468,4 +468,10 @@ test_expect_success '--rebase-merges with strategies' '
        test_cmp expect G.t
 '
 
+test_expect_success '--rebase-merges with commit that can generate bad characters for filename' '
+       git checkout -b colon-in-label E &&
+       git merge -m "colon: this should work" G &&
+       git rebase --rebase-merges --force-rebase E
+'
+
 test_done