Re: [PATCH 1/8] odb: refactor parsing of alternates to be self-contained
From: Justin Tobler <hidden>
Date: 2025-12-08 22:37:52
On 25/12/08 09:04AM, Patrick Steinhardt wrote:
Parsing of the alternates file and environment variable is currently
split up across multiple different functions and is entangled with
`link_alt_odb_entries()`, which is responsible for linking the parsed
object database sources. This results in two downsides:
- We have mutual recursion between parsing alternates and linking them
into the object database. This is because we also parse alternates
that the newly added sources may have.
- We mix up the actual logic to parse the data and to link them into
place.
Refactor the logic so that parsing of the alternates file is entirely
self-contained. Note that this doesn't yet fix the above two issues, but
it is a necessary step to get there.Looking at the existing code, parse_alt_odb_entry() only reads a single entry at a time and relies on link_alt_odb_entries() to call it in a look to get all alternate entries. I agree that handling alternates parsing on a single file in one place is a bit nicer.
quoted hunk ↗ jump to hunk
Signed-off-by: Patrick Steinhardt <redacted> --- odb.c | 70 ++++++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 40 insertions(+), 30 deletions(-)diff --git a/odb.c b/odb.c index dc8f292f3d..9785f62cb6 100644 --- a/odb.c +++ b/odb.c@@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb, return alternate; } -static const char *parse_alt_odb_entry(const char *string, - int sep, - struct strbuf *out) +static void parse_alternates(const char *string, + int sep, + struct strvec *out) { - const char *end; + struct strbuf buf = STRBUF_INIT; - strbuf_reset(out); + while (*string) { + const char *end; + + strbuf_reset(&buf); + + if (*string == '#') { + /* comment; consume up to next separator */ + end = strchrnul(string, sep); + } else if (*string == '"' && !unquote_c_style(&buf, string, &end)) { + /* + * quoted path; unquote_c_style has copied the + * data for us and set "end". Broken quoting (e.g., + * an entry that doesn't end with a quote) falls + * back to the unquoted case below. + */ + } else { + /* normal, unquoted path */ + end = strchrnul(string, sep); + strbuf_add(&buf, string, end - string); + } - if (*string == '#') { - /* comment; consume up to next separator */ - end = strchrnul(string, sep); - } else if (*string == '"' && !unquote_c_style(out, string, &end)) { - /* - * quoted path; unquote_c_style has copied the - * data for us and set "end". Broken quoting (e.g., - * an entry that doesn't end with a quote) falls - * back to the unquoted case below. - */ - } else { - /* normal, unquoted path */ - end = strchrnul(string, sep); - strbuf_add(out, string, end - string); + if (*end) + end++; + string = end; + + if (!buf.len) + continue; + + strvec_push(out, buf.buf);
We parse entries in the exact same way as before, but now we read all entries into a strvec up front. Nice.
quoted hunk ↗ jump to hunk
} - if (*end) - end++; - return end; + strbuf_release(&buf); } static void link_alt_odb_entries(struct object_database *odb, const char *alt, int sep, const char *relative_base, int depth) { - struct strbuf dir = STRBUF_INIT; + struct strvec alternates = STRVEC_INIT; if (!alt || !*alt) return;@@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt, return; } - while (*alt) { - alt = parse_alt_odb_entry(alt, sep, &dir); - if (!dir.len) - continue; - link_alt_odb_entry(odb, dir.buf, relative_base, depth); - } - strbuf_release(&dir); + parse_alternates(alt, sep, &alternates); + + for (size_t i = 0; i < alternates.nr; i++) + link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
Now with this impletation we parse alternate entries up front and then iterate through each of them to link. Linking may still result in recursive alternate parsing if further alternates file are defined. Looks good so far. -Justin