Re: [RFC PATCH 3/5] pack-objects: add delta-islands support

From: Christian Couder <christian.couder@gmail.com>
Date: Aug 5, 2018, 17:40 UTC
Message-ID: <CAP8UFD2Ri6N33UhZSHOvxtxiCBC13-1rpa_ZByY9dsyPjmaC8A@mail.gmail.com>
In-Reply-To: <xmqqfu08zhh7.fsf@gitster-ct.c.googlers.com>

On Tue, Jul 24, 2018 at 7:11 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 6 quoted lines

>
> Another thing I noticed from 2/5 is that you can have up to 7 such
> capture groups.  I do not have any opinion if 7 is too few or too
> many, but we would want the number to be documented, and end-user
> input diagnosed when it requires more captures than we support (if
> we are not already checking, that is).

This is the new documentation:

-> Refs are grouped into islands based on their "names", and two regexes -> that produce the same name are considered to be in the same -> island. The names are computed from the regexes by concatenating any -> capture groups from the regex, with a '-' dash in between. (And if -> there are no capture groups, then the name is the empty string, as in -> the above example.) This allows you to create arbitrary numbers of -> islands. Only up to 7 such capture groups are supported though.

I added the last sentence above, but I wonder if it is 7 or 8. The code is the following:

-> static int find_island_for_ref(const char *refname, const struct object_id *oid, -> int flags, void *data) -> { -> int i, m; -> regmatch_t matches[8]; -> struct strbuf island_name = STRBUF_INIT; -> -> /* walk backwards to get last-one-wins ordering */ -> for (i = island_regexes_nr - 1; i >= 0; i--) { -> if (!regexec(&island_regexes[i], refname, -> ARRAY_SIZE(matches), matches, 0)) -> break; -> }

I also wonder if the above is enough to diagnose end-user input when it requires more captures than we support. A quick look at the man page of the regex functions wasn't enough to enlighten me. Any input on these issues is very welcome!