Re: [PATCH 1/2] sequencer: truncate labels to accommodate loose refs

> "Mark Ruvald Pedersen via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > +/*
> > + * To accommodate common filesystem limitations, where the loose refs' file
> > + * names must not exceed `NAME_MAX`, the labels generated by `git rebase
> > + * --rebase-merges` need to be truncated if the corresponding commit subjects
> > + * are too long.
> > + * Add some margin to stay clear from reaching `NAME_MAX`.
> > + */
> > +#define GIT_MAX_LABEL_LENGTH ((NAME_MAX) - (LOCK_SUFFIX_LEN) - 16)
>
> OK.  Hopefully no systems defien NAME_MAX shorter than 20 bytes ;-).

> We may suffix "-%d" to make it unique after this truncation, so
> there definitely is a need for some slop, and 16-bytes should
> sufficiently be long.
>
>
> > @@ -5404,14 +5415,34 @@ static const char *label_oid(struct object_id *oid, const char *label,
> >  		 *
> >  		 * 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.
> > +		 * in file names.
> > +		 *
> > +		 * As we will use the labels as names of (loose) refs, it is
> > +		 * vital that the name not be longer than the maximum component
> > +		 * size of the file system (`NAME_MAX`). We are careful to
> > +		 * truncate the label accordingly, allowing for the `.lock`
> > +		 * suffix and for the label to be UTF-8 encoded (i.e. we avoid
> > +		 * truncating in the middle of a character).
> >  		 */
> > -		for (; *label; label++)
> > -			if ((*label & 0x80) || isalnum(*label))
> > +		for (; *label && buf->len + 1 < max_len; label++)
> > +			if (isalnum(*label) ||
> > +			    (!label_is_utf8 && (*label & 0x80)))
> >  				strbuf_addch(buf, *label);
> > +			else if (*label & 0x80) {
> > +				const char *p = label;
> > +
> > +				utf8_width(&p, NULL);
> > +				if (p) {
> > +					if (buf->len + (p - label) > max_len)
> > +						break;
> > +					strbuf_add(buf, label, p - label);
> > +					label = p - 1;
> > +				} else {
> > +					label_is_utf8 = 0;
> > +					strbuf_addch(buf, *label);
> > +				}
>
> Utf8_width() does let you advance one unicode character at a time as
> its side effect, but it may be a bit overkill, as its primary
> function is to compute the display width of that character.
>
> We could take advantage of the fact that the first byte of a UTF-8
> character has two high-bits set (i.e. 11xxxxxx) while the second and
> subsequent bytes have only the top-bit set and the second highest
> bit clear (i.e. 10xxxxxx) to simplify/optimize it.  If this were in
> a performance sensitive codepath, that is.