{"thread":{"id":"64595","subject":"[PATCH 0/8] Refactor handling of alternates to work via sources","startedAt":"2025-12-08T08:04:32Z","lastAt":"2025-12-11T09:30:42Z","messageCount":41,"participants":["Patrick Steinhardt","Justin Tobler","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"531813","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":null,"subject":"[PATCH 0/8] Refactor handling of alternates to work via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:17Z","receivedAt":"2025-12-08T08:04:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series refactors how we handle alternate object directories\nso that the interface is structured around the object database source.\n\nNext to being simpler to reason about, it also allows us to eventually\nabstract handling of alternates to use different mechanisms based on the\nspecific backend used. In a world of pluggable object databases not\nevery backend may use a physical directory, so it may not be possible to\nread alternates via \"objects/info/alternates\". Consequently, formats may\nneed a different mechanism entirely to make this list available.\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (8):\n      odb: refactor parsing of alternates to be self-contained\n      odb: resolve relative alternative paths when parsing\n      odb: move computation of normalized objdir into `alt_odb_usable()`\n      odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`\n      odb: remove mutual recursion when parsing alternates\n      odb: drop forward declaration of `read_info_alternates()`\n      odb: read alternates via sources\n      odb: write alternates via sources\n\n odb.c | 307 ++++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 158 insertions(+), 149 deletions(-)\n\n\n---\nbase-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06\nchange-id: 20251206-b4-pks-odb-alternates-via-source-802d87cbbda5\n\n"},{"id":"531814","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-1-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 1/8] odb: refactor parsing of alternates to be self-contained","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:18Z","receivedAt":"2025-12-08T08:04:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Parsing of the alternates file and environment variable is currently\nsplit up across multiple different functions and is entangled with\n`link_alt_odb_entries()`, which is responsible for linking the parsed\nobject database sources. This results in two downsides:\n\n  - We have mutual recursion between parsing alternates and linking them\n    into the object database. This is because we also parse alternates\n    that the newly added sources may have.\n\n  - We mix up the actual logic to parse the data and to link them into\n    place.\n\nRefactor the logic so that parsing of the alternates file is entirely\nself-contained. Note that this doesn't yet fix the above two issues, but\nit is a necessary step to get there.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 40 insertions(+), 30 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex dc8f292f3d..9785f62cb6 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n \treturn alternate;\n }\n \n-static const char *parse_alt_odb_entry(const char *string,\n-\t\t\t\t       int sep,\n-\t\t\t\t       struct strbuf *out)\n+static void parse_alternates(const char *string,\n+\t\t\t     int sep,\n+\t\t\t     struct strvec *out)\n {\n-\tconst char *end;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n-\tstrbuf_reset(out);\n+\twhile (*string) {\n+\t\tconst char *end;\n+\n+\t\tstrbuf_reset(&buf);\n+\n+\t\tif (*string == '#') {\n+\t\t\t/* comment; consume up to next separator */\n+\t\t\tend = strchrnul(string, sep);\n+\t\t} else if (*string == '\"' && !unquote_c_style(&buf, string, &end)) {\n+\t\t\t/*\n+\t\t\t * quoted path; unquote_c_style has copied the\n+\t\t\t * data for us and set \"end\". Broken quoting (e.g.,\n+\t\t\t * an entry that doesn't end with a quote) falls\n+\t\t\t * back to the unquoted case below.\n+\t\t\t */\n+\t\t} else {\n+\t\t\t/* normal, unquoted path */\n+\t\t\tend = strchrnul(string, sep);\n+\t\t\tstrbuf_add(&buf, string, end - string);\n+\t\t}\n \n-\tif (*string == '#') {\n-\t\t/* comment; consume up to next separator */\n-\t\tend = strchrnul(string, sep);\n-\t} else if (*string == '\"' && !unquote_c_style(out, string, &end)) {\n-\t\t/*\n-\t\t * quoted path; unquote_c_style has copied the\n-\t\t * data for us and set \"end\". Broken quoting (e.g.,\n-\t\t * an entry that doesn't end with a quote) falls\n-\t\t * back to the unquoted case below.\n-\t\t */\n-\t} else {\n-\t\t/* normal, unquoted path */\n-\t\tend = strchrnul(string, sep);\n-\t\tstrbuf_add(out, string, end - string);\n+\t\tif (*end)\n+\t\t\tend++;\n+\t\tstring = end;\n+\n+\t\tif (!buf.len)\n+\t\t\tcontinue;\n+\n+\t\tstrvec_push(out, buf.buf);\n \t}\n \n-\tif (*end)\n-\t\tend++;\n-\treturn end;\n+\tstrbuf_release(&buf);\n }\n \n static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\t\t\t int sep, const char *relative_base, int depth)\n {\n-\tstruct strbuf dir = STRBUF_INIT;\n+\tstruct strvec alternates = STRVEC_INIT;\n \n \tif (!alt || !*alt)\n \t\treturn;\n@@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\treturn;\n \t}\n \n-\twhile (*alt) {\n-\t\talt = parse_alt_odb_entry(alt, sep, &dir);\n-\t\tif (!dir.len)\n-\t\t\tcontinue;\n-\t\tlink_alt_odb_entry(odb, dir.buf, relative_base, depth);\n-\t}\n-\tstrbuf_release(&dir);\n+\tparse_alternates(alt, sep, &alternates);\n+\n+\tfor (size_t i = 0; i < alternates.nr; i++)\n+\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n+\n+\tstrvec_clear(&alternates);\n }\n \n static void read_info_alternates(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531815","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-2-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 2/8] odb: resolve relative alternative paths when parsing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:19Z","receivedAt":"2025-12-08T08:04:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Parsing alternates and resolving potential relative paths is currently\nhandled in two separate steps. This has the effect that the logic to\nretrieve alternates is not entirely self-contained. We want it to be\njust that though so that we can eventually move the logic to list\nalternates into the `struct odb_source`.\n\nMove the logic to resolve relative alternative paths into\n`parse_alternates()`. Besides bringing us a step closer towards the\nabove goal, it also neatly separates concerns of generating the list of\nalternatives and linking them into the object database.\n\nNote that we ignore any errors when the relative path cannot be\nresolved. This isn't really a change in behaviour though: if the path\ncannot be resolved to a directory then `alt_odb_usable()` still knows to\nbail out.\n\nWhile at it, rename the function to `odb_add_source()` to more clearly\nindicate what its intent is and to align it with modern terminology.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 9785f62cb6..3ffeece567 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,\n \treturn source;\n }\n \n-static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n-\t\t\t\t\t     const char *dir,\n-\t\t\t\t\t     const char *relative_base,\n-\t\t\t\t\t     int depth)\n+static struct odb_source *odb_add_source(struct object_database *odb,\n+\t\t\t\t\t const char *source,\n+\t\t\t\t\t int depth)\n {\n \tstruct odb_source *alternate = NULL;\n-\tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf tmp = STRBUF_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n-\tif (!is_absolute_path(dir) && relative_base) {\n-\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n-\t\tstrbuf_addch(&pathbuf, '/');\n-\t}\n-\tstrbuf_addstr(&pathbuf, dir);\n-\n-\tif (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {\n-\t\terror(_(\"unable to normalize alternate object path: %s\"),\n-\t\t      pathbuf.buf);\n-\t\tgoto error;\n-\t}\n-\tstrbuf_swap(&pathbuf, &tmp);\n-\n-\t/*\n-\t * The trailing slash after the directory name is given by\n-\t * this function at the end. Remove duplicates.\n-\t */\n-\twhile (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')\n-\t\tstrbuf_setlen(&pathbuf, pathbuf.len - 1);\n-\n-\tstrbuf_reset(&tmp);\n \tstrbuf_realpath(&tmp, odb->sources->path, 1);\n \n-\tif (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))\n+\tif (!alt_odb_usable(odb, source, tmp.buf))\n \t\tgoto error;\n \n-\talternate = odb_source_new(odb, pathbuf.buf, false);\n+\talternate = odb_source_new(odb, source, false);\n \n \t/* add the alternate entry */\n \t*odb->sources_tail = alternate;\n@@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n \n  error:\n \tstrbuf_release(&tmp);\n-\tstrbuf_release(&pathbuf);\n \treturn alternate;\n }\n \n static void parse_alternates(const char *string,\n \t\t\t     int sep,\n+\t\t\t     const char *relative_base,\n \t\t\t     struct strvec *out)\n {\n+\tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \twhile (*string) {\n \t\tconst char *end;\n \n \t\tstrbuf_reset(&buf);\n+\t\tstrbuf_reset(&pathbuf);\n \n \t\tif (*string == '#') {\n \t\t\t/* comment; consume up to next separator */\n@@ -250,9 +229,30 @@ static void parse_alternates(const char *string,\n \t\tif (!buf.len)\n \t\t\tcontinue;\n \n+\t\tif (!is_absolute_path(buf.buf) && relative_base) {\n+\t\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n+\t\t\tstrbuf_addch(&pathbuf, '/');\n+\t\t}\n+\t\tstrbuf_addbuf(&pathbuf, &buf);\n+\n+\t\tstrbuf_reset(&buf);\n+\t\tif (!strbuf_realpath(&buf, pathbuf.buf, 0)) {\n+\t\t\terror(_(\"unable to normalize alternate object path: %s\"),\n+\t\t\t      pathbuf.buf);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * The trailing slash after the directory name is given by\n+\t\t * this function at the end. Remove duplicates.\n+\t\t */\n+\t\twhile (buf.len && buf.buf[buf.len - 1] == '/')\n+\t\t\tstrbuf_setlen(&buf, buf.len - 1);\n+\n \t\tstrvec_push(out, buf.buf);\n \t}\n \n+\tstrbuf_release(&pathbuf);\n \tstrbuf_release(&buf);\n }\n \n@@ -270,10 +270,10 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\treturn;\n \t}\n \n-\tparse_alternates(alt, sep, &alternates);\n+\tparse_alternates(alt, sep, relative_base, &alternates);\n \n \tfor (size_t i = 0; i < alternates.nr; i++)\n-\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n+\t\todb_add_source(odb, alternates.v[i], depth);\n \n \tstrvec_clear(&alternates);\n }\n@@ -348,7 +348,7 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n \t * overwritten when they are.\n \t */\n \todb_prepare_alternates(odb);\n-\treturn link_alt_odb_entry(odb, dir, NULL, 0);\n+\treturn odb_add_source(odb, dir, 0);\n }\n \n struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531816","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-3-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:20Z","receivedAt":"2025-12-08T08:04:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `alt_odb_usable()` receives as input the object database,\nthe path it's supposed to determine usability for as well as the\nnormalized path of the main object directory of the repository. The last\npart is derived by the function's caller from the object database. As we\nalready pass the object database to `alt_odb_usable()` it is redundant\ninformation.\n\nDrop the extra parameter and compute the normalized object directory in\nthe function itself.\n\nWhile at it, rename the function to `odb_is_source_usable()` to align it\nwith modern terminology.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 27 +++++++++++++++------------\n 1 file changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 3ffeece567..2513457a31 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,\n /*\n  * Return non-zero iff the path is usable as an alternate object database.\n  */\n-static int alt_odb_usable(struct object_database *o, const char *path,\n-\t\t\t  const char *normalized_objdir)\n+static bool odb_is_source_usable(struct object_database *o, const char *path)\n {\n \tint r;\n+\tstruct strbuf normalized_objdir = STRBUF_INIT;\n+\tbool usable = false;\n+\n+\tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n \n \t/* Detect cases where alternate disappeared */\n \tif (!is_directory(path)) {\n \t\terror(_(\"object directory %s does not exist; \"\n \t\t\t\"check .git/objects/info/alternates\"),\n \t\t      path);\n-\t\treturn 0;\n+\t\tgoto out;\n \t}\n \n \t/*\n@@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,\n \t\tkh_value(o->source_by_path, p) = o->sources;\n \t}\n \n-\tif (fspatheq(path, normalized_objdir))\n-\t\treturn 0;\n+\tif (fspatheq(path, normalized_objdir.buf))\n+\t\tgoto out;\n \n \tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n-\t\treturn 0;\n+\t\tgoto out;\n+\n+\tusable = true;\n \n-\treturn 1;\n+out:\n+\tstrbuf_release(&normalized_objdir);\n+\treturn usable;\n }\n \n /*\n@@ -164,13 +171,10 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n \t\t\t\t\t int depth)\n {\n \tstruct odb_source *alternate = NULL;\n-\tstruct strbuf tmp = STRBUF_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n-\tstrbuf_realpath(&tmp, odb->sources->path, 1);\n-\n-\tif (!alt_odb_usable(odb, source, tmp.buf))\n+\tif (!odb_is_source_usable(odb, source))\n \t\tgoto error;\n \n \talternate = odb_source_new(odb, source, false);\n@@ -188,7 +192,6 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n \tread_info_alternates(odb, alternate->path, depth + 1);\n \n  error:\n-\tstrbuf_release(&tmp);\n \treturn alternate;\n }\n \n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531817","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-4-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:21Z","receivedAt":"2025-12-08T08:04:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `odb_add_to_alternates_file()` we know to add the newly\nadded source to the object database in case we have already loaded\nalternates. This is done so that we can make its objects accessible\nimmediately without having to fully reload all alternates.\n\nThe way we do this though is to call `link_alt_odb_entries()`, which\nadds _multiple_ sources to the object database source in case we have\nnewline-separated entries. This behaviour is not documented in the\nfunction documentation of `odb_add_to_alternates_file()`, and all\ncallers only ever pass a single directory to it. It's thus entirely\nsurprising and a conceptual mismatch.\n\nFix this issue by directly calling `odb_add_source()` instead.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/odb.c b/odb.c\nindex 2513457a31..94cff19221 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \t\tif (commit_lock_file(&lock))\n \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n \t\tif (odb->loaded_alternates)\n-\t\t\tlink_alt_odb_entries(odb, dir, '\\n', NULL, 0);\n+\t\t\todb_add_source(odb, dir, 0);\n \t}\n \tfree(alts);\n }\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531818","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-5-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 5/8] odb: remove mutual recursion when parsing alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:22Z","receivedAt":"2025-12-08T08:04:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When adding an alternative object database source we not only have to\nconsider the added source itself, but we also have to add _its_ sources\nto our database. We implement this via mutual recursion:\n\n  1. We first call `link_alt_odb_entries()`.\n\n  2. `link_alt_odb_entries()` calls `parse_alternates()`.\n\n  3. We then add each parsed alternate via `odb_add_source()`.\n\n  4. `odb_add_source()` calls `link_alt_odb_entries()` again.\n\nThis flow is somewhat hard to follow, but more importantly it means that\nparsing of alternates is somewhat tied to the recursive behaviour.\n\nRefactor the function to remove the mutual recursion between adding\nsources and parsing alternates. The parsing step thus becomes completely\noblivious to the fact that there is recursive behaviour going on at all.\nInstead, the recursion is handled exclusively by `odb_add_source()`,\nwhich now recurses with itself.\n\nThis refactoring allows us to move parsing of alternates into object\ndatabase sources in a subsequent step.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 60 +++++++++++++++++++++++++++---------------------------------\n 1 file changed, 27 insertions(+), 33 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 94cff19221..27f3c8e263 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n  * of the object ID, an extra slash for the first level indirection, and\n  * the terminating NUL.\n  */\n-static void read_info_alternates(struct object_database *odb,\n-\t\t\t\t const char *relative_base,\n-\t\t\t\t int depth);\n+static void read_info_alternates(const char *relative_base,\n+\t\t\t\t struct strvec *out);\n \n static struct odb_source *odb_source_new(struct object_database *odb,\n \t\t\t\t\t const char *path,\n@@ -171,6 +170,7 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n \t\t\t\t\t int depth)\n {\n \tstruct odb_source *alternate = NULL;\n+\tstruct strvec sources = STRVEC_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n@@ -189,9 +189,17 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n \tkh_value(odb->source_by_path, pos) = alternate;\n \n \t/* recursively add alternates */\n-\tread_info_alternates(odb, alternate->path, depth + 1);\n+\tread_info_alternates(alternate->path, &sources);\n+\tif (sources.nr && depth + 1 > 5) {\n+\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n+\t\t      source);\n+\t} else {\n+\t\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\t\todb_add_source(odb, sources.v[i], depth + 1);\n+\t}\n \n  error:\n+\tstrvec_clear(&sources);\n \treturn alternate;\n }\n \n@@ -203,6 +211,9 @@ static void parse_alternates(const char *string,\n \tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \n+\tif (!string || !*string)\n+\t\treturn;\n+\n \twhile (*string) {\n \t\tconst char *end;\n \n@@ -259,34 +270,11 @@ static void parse_alternates(const char *string,\n \tstrbuf_release(&buf);\n }\n \n-static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n-\t\t\t\t int sep, const char *relative_base, int depth)\n+static void read_info_alternates(const char *relative_base,\n+\t\t\t\t struct strvec *out)\n {\n-\tstruct strvec alternates = STRVEC_INIT;\n-\n-\tif (!alt || !*alt)\n-\t\treturn;\n-\n-\tif (depth > 5) {\n-\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n-\t\t\t\trelative_base);\n-\t\treturn;\n-\t}\n-\n-\tparse_alternates(alt, sep, relative_base, &alternates);\n-\n-\tfor (size_t i = 0; i < alternates.nr; i++)\n-\t\todb_add_source(odb, alternates.v[i], depth);\n-\n-\tstrvec_clear(&alternates);\n-}\n-\n-static void read_info_alternates(struct object_database *odb,\n-\t\t\t\t const char *relative_base,\n-\t\t\t\t int depth)\n-{\n-\tchar *path;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tchar *path;\n \n \tpath = xstrfmt(\"%s/info/alternates\", relative_base);\n \tif (strbuf_read_file(&buf, path, 1024) < 0) {\n@@ -294,8 +282,8 @@ static void read_info_alternates(struct object_database *odb,\n \t\tfree(path);\n \t\treturn;\n \t}\n+\tparse_alternates(buf.buf, '\\n', relative_base, out);\n \n-\tlink_alt_odb_entries(odb, buf.buf, '\\n', relative_base, depth);\n \tstrbuf_release(&buf);\n \tfree(path);\n }\n@@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,\n \n void odb_prepare_alternates(struct object_database *odb)\n {\n+\tstruct strvec sources = STRVEC_INIT;\n+\n \tif (odb->loaded_alternates)\n \t\treturn;\n \n-\tlink_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);\n+\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n+\tread_info_alternates(odb->sources->path, &sources);\n+\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\todb_add_source(odb, sources.v[i], 0);\n \n-\tread_info_alternates(odb, odb->sources->path, 0);\n \todb->loaded_alternates = 1;\n+\n+\tstrvec_clear(&sources);\n }\n \n int odb_has_alternates(struct object_database *odb)\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531819","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-6-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 6/8] odb: drop forward declaration of `read_info_alternates()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:23Z","receivedAt":"2025-12-08T08:04:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Now that we have removed the mutual recursion in the preceding commit\nit is not necessary anymore to have a forward declaration of the\n`read_info_alternates()` function. Move the function and its\ndependencies further up so that we can remove it.\n\nNote that this commit also removes the function documentation of\n`read_info_alternates()`. It's unclear what it's documenting, but it for\nsure isn't documenting the modern behaviour of the function anymore.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 125 +++++++++++++++++++++++++++++-------------------------------------\n 1 file changed, 54 insertions(+), 71 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 27f3c8e263..1d83a915e3 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -132,77 +132,6 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n \treturn usable;\n }\n \n-/*\n- * Prepare alternate object database registry.\n- *\n- * The variable alt_odb_list points at the list of struct\n- * odb_source.  The elements on this list come from\n- * non-empty elements from colon separated ALTERNATE_DB_ENVIRONMENT\n- * environment variable, and $GIT_OBJECT_DIRECTORY/info/alternates,\n- * whose contents is similar to that environment variable but can be\n- * LF separated.  Its base points at a statically allocated buffer that\n- * contains \"/the/directory/corresponding/to/.git/objects/...\", while\n- * its name points just after the slash at the end of \".git/objects/\"\n- * in the example above, and has enough space to hold all hex characters\n- * of the object ID, an extra slash for the first level indirection, and\n- * the terminating NUL.\n- */\n-static void read_info_alternates(const char *relative_base,\n-\t\t\t\t struct strvec *out);\n-\n-static struct odb_source *odb_source_new(struct object_database *odb,\n-\t\t\t\t\t const char *path,\n-\t\t\t\t\t bool local)\n-{\n-\tstruct odb_source *source;\n-\n-\tCALLOC_ARRAY(source, 1);\n-\tsource->odb = odb;\n-\tsource->local = local;\n-\tsource->path = xstrdup(path);\n-\tsource->loose = odb_source_loose_new(source);\n-\n-\treturn source;\n-}\n-\n-static struct odb_source *odb_add_source(struct object_database *odb,\n-\t\t\t\t\t const char *source,\n-\t\t\t\t\t int depth)\n-{\n-\tstruct odb_source *alternate = NULL;\n-\tstruct strvec sources = STRVEC_INIT;\n-\tkhiter_t pos;\n-\tint ret;\n-\n-\tif (!odb_is_source_usable(odb, source))\n-\t\tgoto error;\n-\n-\talternate = odb_source_new(odb, source, false);\n-\n-\t/* add the alternate entry */\n-\t*odb->sources_tail = alternate;\n-\todb->sources_tail = &(alternate->next);\n-\n-\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n-\tif (!ret)\n-\t\tBUG(\"source must not yet exist\");\n-\tkh_value(odb->source_by_path, pos) = alternate;\n-\n-\t/* recursively add alternates */\n-\tread_info_alternates(alternate->path, &sources);\n-\tif (sources.nr && depth + 1 > 5) {\n-\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n-\t\t      source);\n-\t} else {\n-\t\tfor (size_t i = 0; i < sources.nr; i++)\n-\t\t\todb_add_source(odb, sources.v[i], depth + 1);\n-\t}\n-\n- error:\n-\tstrvec_clear(&sources);\n-\treturn alternate;\n-}\n-\n static void parse_alternates(const char *string,\n \t\t\t     int sep,\n \t\t\t     const char *relative_base,\n@@ -288,6 +217,60 @@ static void read_info_alternates(const char *relative_base,\n \tfree(path);\n }\n \n+\n+static struct odb_source *odb_source_new(struct object_database *odb,\n+\t\t\t\t\t const char *path,\n+\t\t\t\t\t bool local)\n+{\n+\tstruct odb_source *source;\n+\n+\tCALLOC_ARRAY(source, 1);\n+\tsource->odb = odb;\n+\tsource->local = local;\n+\tsource->path = xstrdup(path);\n+\tsource->loose = odb_source_loose_new(source);\n+\n+\treturn source;\n+}\n+\n+static struct odb_source *odb_add_source(struct object_database *odb,\n+\t\t\t\t\t const char *source,\n+\t\t\t\t\t int depth)\n+{\n+\tstruct odb_source *alternate = NULL;\n+\tstruct strvec sources = STRVEC_INIT;\n+\tkhiter_t pos;\n+\tint ret;\n+\n+\tif (!odb_is_source_usable(odb, source))\n+\t\tgoto error;\n+\n+\talternate = odb_source_new(odb, source, false);\n+\n+\t/* add the alternate entry */\n+\t*odb->sources_tail = alternate;\n+\todb->sources_tail = &(alternate->next);\n+\n+\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n+\tif (!ret)\n+\t\tBUG(\"source must not yet exist\");\n+\tkh_value(odb->source_by_path, pos) = alternate;\n+\n+\t/* recursively add alternates */\n+\tread_info_alternates(alternate->path, &sources);\n+\tif (sources.nr && depth + 1 > 5) {\n+\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n+\t\t      source);\n+\t} else {\n+\t\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\t\todb_add_source(odb, sources.v[i], depth + 1);\n+\t}\n+\n+ error:\n+\tstrvec_clear(&sources);\n+\treturn alternate;\n+}\n+\n void odb_add_to_alternates_file(struct object_database *odb,\n \t\t\t\tconst char *dir)\n {\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531820","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-7-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 7/8] odb: read alternates via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:24Z","receivedAt":"2025-12-08T08:04:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Adapt how we read alternates so that the interface is structured around\nthe object database source we're reading from. This will eventually\nallow us to abstract away this behaviour with pluggable object databases\nso that every format can have its own mechanism for listing alternates.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 1d83a915e3..bf364fe3dd 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -199,19 +199,19 @@ static void parse_alternates(const char *string,\n \tstrbuf_release(&buf);\n }\n \n-static void read_info_alternates(const char *relative_base,\n-\t\t\t\t struct strvec *out)\n+static void odb_source_read_alternates(struct odb_source *source,\n+\t\t\t\t       struct strvec *out)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tchar *path;\n \n-\tpath = xstrfmt(\"%s/info/alternates\", relative_base);\n+\tpath = xstrfmt(\"%s/info/alternates\", source->path);\n \tif (strbuf_read_file(&buf, path, 1024) < 0) {\n \t\twarn_on_fopen_errors(path);\n \t\tfree(path);\n \t\treturn;\n \t}\n-\tparse_alternates(buf.buf, '\\n', relative_base, out);\n+\tparse_alternates(buf.buf, '\\n', source->path, out);\n \n \tstrbuf_release(&buf);\n \tfree(path);\n@@ -257,7 +257,7 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n \tkh_value(odb->source_by_path, pos) = alternate;\n \n \t/* recursively add alternates */\n-\tread_info_alternates(alternate->path, &sources);\n+\todb_source_read_alternates(alternate, &sources);\n \tif (sources.nr && depth + 1 > 5) {\n \t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n \t\t      source);\n@@ -599,7 +599,7 @@ void odb_prepare_alternates(struct object_database *odb)\n \t\treturn;\n \n \tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n-\tread_info_alternates(odb->sources->path, &sources);\n+\todb_source_read_alternates(odb->sources, &sources);\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_source(odb, sources.v[i], 0);\n \n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531821","messageId":"20251208-b4-pks-odb-alternates-via-source-v1-8-e7ebb8b18c03@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH 8/8] odb: write alternates via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-08T08:04:25Z","receivedAt":"2025-12-08T08:05:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Refactor writing of alternates so that the actual business logic is\nstructured around the object database source we want to write the\nalternate to. Same as with the preceding commit, this will eventually\nallow us to have different logic for writing alternates depending on the\nbackend used.\n\nNote that after the refactoring we start to call `odb_add_source()`\nunconditionally. This is fine though as we know to skip adding sources\nthat are tracked already.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 51 +++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 35 insertions(+), 16 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex bf364fe3dd..9e7d078a46 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -271,25 +271,28 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n \treturn alternate;\n }\n \n-void odb_add_to_alternates_file(struct object_database *odb,\n-\t\t\t\tconst char *dir)\n+static int odb_source_write_alternate(struct odb_source *source,\n+\t\t\t\t      const char *alternate)\n {\n \tstruct lock_file lock = LOCK_INIT;\n-\tchar *alts = repo_git_path(odb->repo, \"objects/info/alternates\");\n+\tchar *path = xstrfmt(\"%s/%s\", source->path, \"info/alternates\");\n \tFILE *in, *out;\n \tint found = 0;\n+\tint ret;\n \n-\thold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);\n+\thold_lock_file_for_update(&lock, path, LOCK_DIE_ON_ERROR);\n \tout = fdopen_lock_file(&lock, \"w\");\n-\tif (!out)\n-\t\tdie_errno(_(\"unable to fdopen alternates lockfile\"));\n+\tif (!out) {\n+\t\tret = error_errno(_(\"unable to fdopen alternates lockfile\"));\n+\t\tgoto out;\n+\t}\n \n-\tin = fopen(alts, \"r\");\n+\tin = fopen(path, \"r\");\n \tif (in) {\n \t\tstruct strbuf line = STRBUF_INIT;\n \n \t\twhile (strbuf_getline(&line, in) != EOF) {\n-\t\t\tif (!strcmp(dir, line.buf)) {\n+\t\t\tif (!strcmp(alternate, line.buf)) {\n \t\t\t\tfound = 1;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -298,20 +301,36 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \n \t\tstrbuf_release(&line);\n \t\tfclose(in);\n+\t} else if (errno != ENOENT) {\n+\t\tret = error_errno(_(\"unable to read alternates file\"));\n+\t\tgoto out;\n \t}\n-\telse if (errno != ENOENT)\n-\t\tdie_errno(_(\"unable to read alternates file\"));\n \n \tif (found) {\n \t\trollback_lock_file(&lock);\n \t} else {\n-\t\tfprintf_or_die(out, \"%s\\n\", dir);\n-\t\tif (commit_lock_file(&lock))\n-\t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n-\t\tif (odb->loaded_alternates)\n-\t\t\todb_add_source(odb, dir, 0);\n+\t\tfprintf_or_die(out, \"%s\\n\", alternate);\n+\t\tif (commit_lock_file(&lock)) {\n+\t\t\tret = error_errno(_(\"unable to move new alternates file into place\"));\n+\t\t\tgoto out;\n+\t\t}\n \t}\n-\tfree(alts);\n+\n+\tret = 0;\n+\n+out:\n+\tfree(path);\n+\treturn ret;\n+}\n+\n+void odb_add_to_alternates_file(struct object_database *odb,\n+\t\t\t\tconst char *dir)\n+{\n+\tint ret = odb_source_write_alternate(odb->sources, dir);\n+\tif (ret < 0)\n+\t\tdie(NULL);\n+\tif (odb->loaded_alternates)\n+\t\todb_add_source(odb, dir, 0);\n }\n \n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531862","messageId":"yjpy5yitklzq5pyvrmpsd7wq3i55e53vhkt3f34bjguwbewqbz@rctteyqvvm7t","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-1-e7ebb8b18c03@pks.im","subject":"Re: [PATCH 1/8] odb: refactor parsing of alternates to be self-contained","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-12-08T22:37:47Z","receivedAt":"2025-12-08T22:37:52Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> Parsing of the alternates file and environment variable is currently\n> split up across multiple different functions and is entangled with\n> `link_alt_odb_entries()`, which is responsible for linking the parsed\n> object database sources. This results in two downsides:\n> \n>   - We have mutual recursion between parsing alternates and linking them\n>     into the object database. This is because we also parse alternates\n>     that the newly added sources may have.\n> \n>   - We mix up the actual logic to parse the data and to link them into\n>     place.\n> \n> Refactor the logic so that parsing of the alternates file is entirely\n> self-contained. Note that this doesn't yet fix the above two issues, but\n> it is a necessary step to get there.\n\nLooking at the existing code, parse_alt_odb_entry() only reads a single\nentry at a time and relies on link_alt_odb_entries() to call it in a\nlook to get all alternate entries. I agree that handling alternates\nparsing on a single file in one place is a bit nicer.\n\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------\n>  1 file changed, 40 insertions(+), 30 deletions(-)\n> \n> diff --git a/odb.c b/odb.c\n> index dc8f292f3d..9785f62cb6 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n>  \treturn alternate;\n>  }\n>  \n> -static const char *parse_alt_odb_entry(const char *string,\n> -\t\t\t\t       int sep,\n> -\t\t\t\t       struct strbuf *out)\n> +static void parse_alternates(const char *string,\n> +\t\t\t     int sep,\n> +\t\t\t     struct strvec *out)\n>  {\n> -\tconst char *end;\n> +\tstruct strbuf buf = STRBUF_INIT;\n>  \n> -\tstrbuf_reset(out);\n> +\twhile (*string) {\n> +\t\tconst char *end;\n> +\n> +\t\tstrbuf_reset(&buf);\n> +\n> +\t\tif (*string == '#') {\n> +\t\t\t/* comment; consume up to next separator */\n> +\t\t\tend = strchrnul(string, sep);\n> +\t\t} else if (*string == '\"' && !unquote_c_style(&buf, string, &end)) {\n> +\t\t\t/*\n> +\t\t\t * quoted path; unquote_c_style has copied the\n> +\t\t\t * data for us and set \"end\". Broken quoting (e.g.,\n> +\t\t\t * an entry that doesn't end with a quote) falls\n> +\t\t\t * back to the unquoted case below.\n> +\t\t\t */\n> +\t\t} else {\n> +\t\t\t/* normal, unquoted path */\n> +\t\t\tend = strchrnul(string, sep);\n> +\t\t\tstrbuf_add(&buf, string, end - string);\n> +\t\t}\n>  \n> -\tif (*string == '#') {\n> -\t\t/* comment; consume up to next separator */\n> -\t\tend = strchrnul(string, sep);\n> -\t} else if (*string == '\"' && !unquote_c_style(out, string, &end)) {\n> -\t\t/*\n> -\t\t * quoted path; unquote_c_style has copied the\n> -\t\t * data for us and set \"end\". Broken quoting (e.g.,\n> -\t\t * an entry that doesn't end with a quote) falls\n> -\t\t * back to the unquoted case below.\n> -\t\t */\n> -\t} else {\n> -\t\t/* normal, unquoted path */\n> -\t\tend = strchrnul(string, sep);\n> -\t\tstrbuf_add(out, string, end - string);\n> +\t\tif (*end)\n> +\t\t\tend++;\n> +\t\tstring = end;\n> +\n> +\t\tif (!buf.len)\n> +\t\t\tcontinue;\n> +\n> +\t\tstrvec_push(out, buf.buf);\n\nWe parse entries in the exact same way as before, but now we read all\nentries into a strvec up front. Nice.\n\n>  \t}\n>  \n> -\tif (*end)\n> -\t\tend++;\n> -\treturn end;\n> +\tstrbuf_release(&buf);\n>  }\n>  \n>  static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n>  \t\t\t\t int sep, const char *relative_base, int depth)\n>  {\n> -\tstruct strbuf dir = STRBUF_INIT;\n> +\tstruct strvec alternates = STRVEC_INIT;\n>  \n>  \tif (!alt || !*alt)\n>  \t\treturn;\n> @@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n>  \t\treturn;\n>  \t}\n>  \n> -\twhile (*alt) {\n> -\t\talt = parse_alt_odb_entry(alt, sep, &dir);\n> -\t\tif (!dir.len)\n> -\t\t\tcontinue;\n> -\t\tlink_alt_odb_entry(odb, dir.buf, relative_base, depth);\n> -\t}\n> -\tstrbuf_release(&dir);\n> +\tparse_alternates(alt, sep, &alternates);\n> +\n> +\tfor (size_t i = 0; i < alternates.nr; i++)\n> +\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n\nNow with this impletation we parse alternate entries up front and then\niterate through each of them to link. Linking may still result in\nrecursive alternate parsing if further alternates file are defined.\n\nLooks good so far.\n\n-Justin\n"},{"id":"531876","messageId":"kz2eftlrmaxpxjybhjwqlewy3dx44sdznimzs6reoqtev4qtox@hl3s2gxz3sk2","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-2-e7ebb8b18c03@pks.im","subject":"Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-12-09T02:09:30Z","receivedAt":"2025-12-09T02:09:34Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> Parsing alternates and resolving potential relative paths is currently\n> handled in two separate steps. This has the effect that the logic to\n> retrieve alternates is not entirely self-contained. We want it to be\n> just that though so that we can eventually move the logic to list\n> alternates into the `struct odb_source`.\n\nNaive question: is the intent here to eventually move alternate ODB\nsources under the primary ODB source? Or just to record the alternate\ndir info in the ODB source?\n\n> Move the logic to resolve relative alternative paths into\n> `parse_alternates()`. Besides bringing us a step closer towards the\n> above goal, it also neatly separates concerns of generating the list of\n> alternatives and linking them into the object database.\n> \n> Note that we ignore any errors when the relative path cannot be\n> resolved. This isn't really a change in behaviour though: if the path\n> cannot be resolved to a directory then `alt_odb_usable()` still knows to\n> bail out.\n> \n> While at it, rename the function to `odb_add_source()` to more clearly\n> indicate what its intent is and to align it with modern terminology.\n\nAlternates are indeed just additional ODB sources appended to the\nsources list. IIUC though, doesn't this function only add alternate\nsources? If so, maybe it would be better to use\n`odb_add_alternate_source()`?\n\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------\n>  1 file changed, 32 insertions(+), 32 deletions(-)\n> \n> diff --git a/odb.c b/odb.c\n> index 9785f62cb6..3ffeece567 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,\n>  \treturn source;\n>  }\n>  \n> -static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n> -\t\t\t\t\t     const char *dir,\n> -\t\t\t\t\t     const char *relative_base,\n> -\t\t\t\t\t     int depth)\n> +static struct odb_source *odb_add_source(struct object_database *odb,\n> +\t\t\t\t\t const char *source,\n> +\t\t\t\t\t int depth)\n>  {\n>  \tstruct odb_source *alternate = NULL;\n> -\tstruct strbuf pathbuf = STRBUF_INIT;\n>  \tstruct strbuf tmp = STRBUF_INIT;\n>  \tkhiter_t pos;\n>  \tint ret;\n>  \n> -\tif (!is_absolute_path(dir) && relative_base) {\n> -\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n> -\t\tstrbuf_addch(&pathbuf, '/');\n> -\t}\n> -\tstrbuf_addstr(&pathbuf, dir);\n> -\n> -\tif (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {\n> -\t\terror(_(\"unable to normalize alternate object path: %s\"),\n> -\t\t      pathbuf.buf);\n> -\t\tgoto error;\n> -\t}\n> -\tstrbuf_swap(&pathbuf, &tmp);\n> -\n> -\t/*\n> -\t * The trailing slash after the directory name is given by\n> -\t * this function at the end. Remove duplicates.\n> -\t */\n> -\twhile (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')\n> -\t\tstrbuf_setlen(&pathbuf, pathbuf.len - 1);\n> -\n> -\tstrbuf_reset(&tmp);\n>  \tstrbuf_realpath(&tmp, odb->sources->path, 1);\n>  \n> -\tif (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))\n> +\tif (!alt_odb_usable(odb, source, tmp.buf))\n>  \t\tgoto error;\n>  \n> -\talternate = odb_source_new(odb, pathbuf.buf, false);\n> +\talternate = odb_source_new(odb, source, false);\n>  \n>  \t/* add the alternate entry */\n>  \t*odb->sources_tail = alternate;\n> @@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n>  \n>   error:\n>  \tstrbuf_release(&tmp);\n> -\tstrbuf_release(&pathbuf);\n>  \treturn alternate;\n>  }\n>  \n>  static void parse_alternates(const char *string,\n>  \t\t\t     int sep,\n> +\t\t\t     const char *relative_base,\n>  \t\t\t     struct strvec *out)\n>  {\n> +\tstruct strbuf pathbuf = STRBUF_INIT;\n>  \tstruct strbuf buf = STRBUF_INIT;\n>  \n>  \twhile (*string) {\n>  \t\tconst char *end;\n>  \n>  \t\tstrbuf_reset(&buf);\n> +\t\tstrbuf_reset(&pathbuf);\n>  \n>  \t\tif (*string == '#') {\n>  \t\t\t/* comment; consume up to next separator */\n> @@ -250,9 +229,30 @@ static void parse_alternates(const char *string,\n>  \t\tif (!buf.len)\n>  \t\t\tcontinue;\n>  \n> +\t\tif (!is_absolute_path(buf.buf) && relative_base) {\n> +\t\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n> +\t\t\tstrbuf_addch(&pathbuf, '/');\n> +\t\t}\n> +\t\tstrbuf_addbuf(&pathbuf, &buf);\n> +\n> +\t\tstrbuf_reset(&buf);\n> +\t\tif (!strbuf_realpath(&buf, pathbuf.buf, 0)) {\n> +\t\t\terror(_(\"unable to normalize alternate object path: %s\"),\n> +\t\t\t      pathbuf.buf);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * The trailing slash after the directory name is given by\n> +\t\t * this function at the end. Remove duplicates.\n> +\t\t */\n> +\t\twhile (buf.len && buf.buf[buf.len - 1] == '/')\n> +\t\t\tstrbuf_setlen(&buf, buf.len - 1);\n> +\n\nHere we move the logic to resolve relative paths into\nparse_alternates(). This seems reasonable to me.\n\n-Justin\n"},{"id":"531881","messageId":"cqrno3lfvbfrb6ieestagbs5avshs7znoumky2plvtc4tjye2a@onwb5vmtstbx","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-3-e7ebb8b18c03@pks.im","subject":"Re: [PATCH 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-12-09T02:34:25Z","receivedAt":"2025-12-09T02:34:30Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> The function `alt_odb_usable()` receives as input the object database,\n> the path it's supposed to determine usability for as well as the\n> normalized path of the main object directory of the repository. The last\n> part is derived by the function's caller from the object database. As we\n> already pass the object database to `alt_odb_usable()` it is redundant\n> information.\n> \n> Drop the extra parameter and compute the normalized object directory in\n> the function itself.\n> \n> While at it, rename the function to `odb_is_source_usable()` to align it\n> with modern terminology.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  odb.c | 27 +++++++++++++++------------\n>  1 file changed, 15 insertions(+), 12 deletions(-)\n> \n> diff --git a/odb.c b/odb.c\n> index 3ffeece567..2513457a31 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,\n>  /*\n>   * Return non-zero iff the path is usable as an alternate object database.\n\nWhile we are here we could fix this typo: s/iff/if/\n\n>   */\n> -static int alt_odb_usable(struct object_database *o, const char *path,\n> -\t\t\t  const char *normalized_objdir)\n> +static bool odb_is_source_usable(struct object_database *o, const char *path)\n>  {\n>  \tint r;\n> +\tstruct strbuf normalized_objdir = STRBUF_INIT;\n> +\tbool usable = false;\n> +\n> +\tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n>  \n>  \t/* Detect cases where alternate disappeared */\n>  \tif (!is_directory(path)) {\n>  \t\terror(_(\"object directory %s does not exist; \"\n>  \t\t\t\"check .git/objects/info/alternates\"),\n>  \t\t      path);\n> -\t\treturn 0;\n> +\t\tgoto out;\n>  \t}\n>  \n>  \t/*\n> @@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,\n>  \t\tkh_value(o->source_by_path, p) = o->sources;\n>  \t}\n>  \n> -\tif (fspatheq(path, normalized_objdir))\n> -\t\treturn 0;\n> +\tif (fspatheq(path, normalized_objdir.buf))\n> +\t\tgoto out;\n>  \n>  \tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n> -\t\treturn 0;\n> +\t\tgoto out;\n> +\n> +\tusable = true;\n>  \n> -\treturn 1;\n> +out:\n> +\tstrbuf_release(&normalized_objdir);\n> +\treturn usable;\n>  }\n>  \n>  /*\n> @@ -164,13 +171,10 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n>  \t\t\t\t\t int depth)\n>  {\n>  \tstruct odb_source *alternate = NULL;\n> -\tstruct strbuf tmp = STRBUF_INIT;\n>  \tkhiter_t pos;\n>  \tint ret;\n>  \n> -\tstrbuf_realpath(&tmp, odb->sources->path, 1);\n> -\n> -\tif (!alt_odb_usable(odb, source, tmp.buf))\n> +\tif (!odb_is_source_usable(odb, source))\n\nThe normalized ODB path is only being used in alt_odb_usable() so\nrelocating it inside that function make sense. Looks good.\n\n-Justin\n"},{"id":"531893","messageId":"aTfYBGr-0SIDinYF@pks.im","threadId":"64595","inReplyTo":"kz2eftlrmaxpxjybhjwqlewy3dx44sdznimzs6reoqtev4qtox@hl3s2gxz3sk2","subject":"Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-09T08:04:20Z","receivedAt":"2025-12-09T08:04:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Dec 08, 2025 at 08:09:30PM -0600, Justin Tobler wrote:\n> On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> > Parsing alternates and resolving potential relative paths is currently\n> > handled in two separate steps. This has the effect that the logic to\n> > retrieve alternates is not entirely self-contained. We want it to be\n> > just that though so that we can eventually move the logic to list\n> > alternates into the `struct odb_source`.\n> \n> Naive question: is the intent here to eventually move alternate ODB\n> sources under the primary ODB source? Or just to record the alternate\n> dir info in the ODB source?\n\nNot only the primary ODB source, but into ODB sources in general as\nalternates are recursive by nature.\n\nThe problem I am trying to solve is that ODB sources may not even have a\nfilesystem-local directory, but the way we use alternates recursively\nvery much assumes they do. I don't want to treat \"files\" sources\nspecially though and only recursively add their alternates. Instead, I\nwant to move the logic of enumerating alternates into the source so that\nevery source can have a different way of enumerating them that may or\nmay not use the filesystem.\n\n> > Move the logic to resolve relative alternative paths into\n> > `parse_alternates()`. Besides bringing us a step closer towards the\n> > above goal, it also neatly separates concerns of generating the list of\n> > alternatives and linking them into the object database.\n> > \n> > Note that we ignore any errors when the relative path cannot be\n> > resolved. This isn't really a change in behaviour though: if the path\n> > cannot be resolved to a directory then `alt_odb_usable()` still knows to\n> > bail out.\n> > \n> > While at it, rename the function to `odb_add_source()` to more clearly\n> > indicate what its intent is and to align it with modern terminology.\n> \n> Alternates are indeed just additional ODB sources appended to the\n> sources list. IIUC though, doesn't this function only add alternate\n> sources? If so, maybe it would be better to use\n> `odb_add_alternate_source()`?\n\nHm, yeah, I think you're right. We still have the recursive nature at\nthe end of this series, so let's call it accordingly.\n\nPatrick\n"},{"id":"531894","messageId":"aTfYC5DnxYx2qdPG@pks.im","threadId":"64595","inReplyTo":"cqrno3lfvbfrb6ieestagbs5avshs7znoumky2plvtc4tjye2a@onwb5vmtstbx","subject":"Re: [PATCH 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-09T08:04:27Z","receivedAt":"2025-12-09T08:04:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Dec 08, 2025 at 08:34:25PM -0600, Justin Tobler wrote:\n> On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> > diff --git a/odb.c b/odb.c\n> > index 3ffeece567..2513457a31 100644\n> > --- a/odb.c\n> > +++ b/odb.c\n> > @@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,\n> >  /*\n> >   * Return non-zero iff the path is usable as an alternate object database.\n> \n> While we are here we could fix this typo: s/iff/if/\n\nThis is not a typo: \"iff\" generally means \"if and only if\".\n\nPatrick\n"},{"id":"531901","messageId":"qhwjdvcilzbd7bpj64jwmxfwldlzge5w23bsgrz3yma4rtwlw6@6becwkk4u4vj","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-5-e7ebb8b18c03@pks.im","subject":"Re: [PATCH 5/8] odb: remove mutual recursion when parsing alternates","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-12-09T17:31:05Z","receivedAt":"2025-12-09T17:31:12Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> When adding an alternative object database source we not only have to\n> consider the added source itself, but we also have to add _its_ sources\n> to our database. We implement this via mutual recursion:\n> \n>   1. We first call `link_alt_odb_entries()`.\n> \n>   2. `link_alt_odb_entries()` calls `parse_alternates()`.\n> \n>   3. We then add each parsed alternate via `odb_add_source()`.\n> \n>   4. `odb_add_source()` calls `link_alt_odb_entries()` again.\n> \n> This flow is somewhat hard to follow, but more importantly it means that\n> parsing of alternates is somewhat tied to the recursive behaviour.\n> \n> Refactor the function to remove the mutual recursion between adding\n> sources and parsing alternates. The parsing step thus becomes completely\n> oblivious to the fact that there is recursive behaviour going on at all.\n> Instead, the recursion is handled exclusively by `odb_add_source()`,\n> which now recurses with itself.\n> \n> This refactoring allows us to move parsing of alternates into object\n> database sources in a subsequent step.\n> \n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  odb.c | 60 +++++++++++++++++++++++++++---------------------------------\n>  1 file changed, 27 insertions(+), 33 deletions(-)\n> \n> diff --git a/odb.c b/odb.c\n> index 94cff19221..27f3c8e263 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n>   * of the object ID, an extra slash for the first level indirection, and\n>   * the terminating NUL.\n>   */\n> -static void read_info_alternates(struct object_database *odb,\n> -\t\t\t\t const char *relative_base,\n> -\t\t\t\t int depth);\n> +static void read_info_alternates(const char *relative_base,\n> +\t\t\t\t struct strvec *out);\n>  \n>  static struct odb_source *odb_source_new(struct object_database *odb,\n>  \t\t\t\t\t const char *path,\n> @@ -171,6 +170,7 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n>  \t\t\t\t\t int depth)\n>  {\n>  \tstruct odb_source *alternate = NULL;\n> +\tstruct strvec sources = STRVEC_INIT;\n>  \tkhiter_t pos;\n>  \tint ret;\n>  \n> @@ -189,9 +189,17 @@ static struct odb_source *odb_add_source(struct object_database *odb,\n>  \tkh_value(odb->source_by_path, pos) = alternate;\n>  \n>  \t/* recursively add alternates */\n> -\tread_info_alternates(odb, alternate->path, depth + 1);\n> +\tread_info_alternates(alternate->path, &sources);\n> +\tif (sources.nr && depth + 1 > 5) {\n> +\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n> +\t\t      source);\n> +\t} else {\n> +\t\tfor (size_t i = 0; i < sources.nr; i++)\n> +\t\t\todb_add_source(odb, sources.v[i], depth + 1);\n> +\t}\n\nOk, prior to this, read_info_alternates() would not only parse the\nalternates file for the ODB source at hand, but also recursively parse\nand add alternates of alternates. Now, read_info_alternates() is only\nresponsible for parsing a single alternates file at a time.\n\nRecursing into child alternates is now handled by odb_add_source(). IMO\nthis is much easier to reason about and ultimately matches the previous\nbehavior.\n\n>  \n>   error:\n> +\tstrvec_clear(&sources);\n>  \treturn alternate;\n>  }\n>  \n[snip]\n> @@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,\n>  \n>  void odb_prepare_alternates(struct object_database *odb)\n>  {\n> +\tstruct strvec sources = STRVEC_INIT;\n> +\n>  \tif (odb->loaded_alternates)\n>  \t\treturn;\n>  \n> -\tlink_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);\n> +\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n> +\tread_info_alternates(odb->sources->path, &sources);\n> +\tfor (size_t i = 0; i < sources.nr; i++)\n> +\t\todb_add_source(odb, sources.v[i], 0);\n\nWhen preparing alternates, sources from the environment and alternates\nfile are parsed first and then added. Adding sources is now handled\nexplicitly and is responsible for add child alternates. Looks good.\n\n-Justin\n"},{"id":"531903","messageId":"gmhqd5nkhpk5wqjnfrn6blnxo2owvfgomfbi652fi462nf3tny@eyhy6glagywx","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-7-e7ebb8b18c03@pks.im","subject":"Re: [PATCH 7/8] odb: read alternates via sources","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-12-09T17:49:55Z","receivedAt":"2025-12-09T17:49:58Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> Adapt how we read alternates so that the interface is structured around\n> the object database source we're reading from. This will eventually\n> allow us to abstract away this behaviour with pluggable object databases\n> so that every format can have its own mechanism for listing alternates.\n\nOk so IIUC, the idea here is that eventually we want each source to be\nable to define it's own way to parse its alternates. This is needed\nbecause with pluggable ODBs a given ODB source maynot even use the\nfilesystem and thus any alternates it may define would need to be parsed\nin a different manner. I suppose some future ODB source types may not\neven want to support child alternates.\n\nQuestion: the interface of odb_source_read_alternates() still expects\nparsed alternates to be written to the output strvec. The sources don't\nget added to the ODB source list until odb_add_source() is invoked on\nthe source. Does this mean odb_add_source() will have to be able to\nhandle various different types of ODB sources? If so, will these be\ndifferentiated by some sort of URI?\n\nThe patch itself here looks good.\n\n-Justin\n"},{"id":"531906","messageId":"5lkaw3kfqzjt45jhomeb34cqu6nxigapmobtqrzpyoq7mh6655@3zgqsyfui23j","threadId":"64595","inReplyTo":"aTfYBGr-0SIDinYF@pks.im","subject":"Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-12-09T18:06:09Z","receivedAt":"2025-12-09T18:06:12Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/12/09 09:04AM, Patrick Steinhardt wrote:\n> On Mon, Dec 08, 2025 at 08:09:30PM -0600, Justin Tobler wrote:\n> > On 25/12/08 09:04AM, Patrick Steinhardt wrote:\n> > > Parsing alternates and resolving potential relative paths is currently\n> > > handled in two separate steps. This has the effect that the logic to\n> > > retrieve alternates is not entirely self-contained. We want it to be\n> > > just that though so that we can eventually move the logic to list\n> > > alternates into the `struct odb_source`.\n> > \n> > Naive question: is the intent here to eventually move alternate ODB\n> > sources under the primary ODB source? Or just to record the alternate\n> > dir info in the ODB source?\n> \n> Not only the primary ODB source, but into ODB sources in general as\n> alternates are recursive by nature.\n> \n> The problem I am trying to solve is that ODB sources may not even have a\n> filesystem-local directory, but the way we use alternates recursively\n> very much assumes they do. I don't want to treat \"files\" sources\n> specially though and only recursively add their alternates. Instead, I\n> want to move the logic of enumerating alternates into the source so that\n> every source can have a different way of enumerating them that may or\n> may not use the filesystem.\n\nAh, that makes more sense now. Thanks for the explaination. :)\n\n> > > Move the logic to resolve relative alternative paths into\n> > > `parse_alternates()`. Besides bringing us a step closer towards the\n> > > above goal, it also neatly separates concerns of generating the list of\n> > > alternatives and linking them into the object database.\n> > > \n> > > Note that we ignore any errors when the relative path cannot be\n> > > resolved. This isn't really a change in behaviour though: if the path\n> > > cannot be resolved to a directory then `alt_odb_usable()` still knows to\n> > > bail out.\n> > > \n> > > While at it, rename the function to `odb_add_source()` to more clearly\n> > > indicate what its intent is and to align it with modern terminology.\n> > \n> > Alternates are indeed just additional ODB sources appended to the\n> > sources list. IIUC though, doesn't this function only add alternate\n> > sources? If so, maybe it would be better to use\n> > `odb_add_alternate_source()`?\n> \n> Hm, yeah, I think you're right. We still have the recursive nature at\n> the end of this series, so let's call it accordingly.\n\nOn a semi-related note, part of me thinks it would be nice if alternate\nsources were a bit more first class in `struct object_database`. IOW,\nexplicitly defining the primary and list of alternate sources\nseparately. From the perspective of reading objects, having a single\nlist of sources is nice, but when writing objects only the first source\nis used. This isn't too big of a deal, but certain operations like ODB\ntrasactions will reorder the source list to change where objects get\nwritten to which feels a bit fragile to me. I guess another way to\nresolve this concern could be to change ODB transactions to use a\nseparate mechanism though.\n\n-Justin\n"},{"id":"531942","messageId":"aTkK79sZPkYQ87aS@pks.im","threadId":"64595","inReplyTo":"5lkaw3kfqzjt45jhomeb34cqu6nxigapmobtqrzpyoq7mh6655@3zgqsyfui23j","subject":"Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T05:53:51Z","receivedAt":"2025-12-10T05:53:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Dec 09, 2025 at 12:06:09PM -0600, Justin Tobler wrote:\n> On a semi-related note, part of me thinks it would be nice if alternate\n> sources were a bit more first class in `struct object_database`. IOW,\n> explicitly defining the primary and list of alternate sources\n> separately. From the perspective of reading objects, having a single\n> list of sources is nice, but when writing objects only the first source\n> is used. This isn't too big of a deal, but certain operations like ODB\n> trasactions will reorder the source list to change where objects get\n> written to which feels a bit fragile to me. I guess another way to\n> resolve this concern could be to change ODB transactions to use a\n> separate mechanism though.\n\nAgreed, especially the writing side is a bit weird, and reordering\nsources when we create transactions is one of the weirdest parts. I\nthink this is out of scope for this patch series, but I certainly think\nthat we should address this by polishing the ODB transactions a bit\ngoing forward.\n\nThanks!\n\nPatrick\n"},{"id":"531943","messageId":"aTkK-ipzrsG21blw@pks.im","threadId":"64595","inReplyTo":"gmhqd5nkhpk5wqjnfrn6blnxo2owvfgomfbi652fi462nf3tny@eyhy6glagywx","subject":"Re: [PATCH 7/8] odb: read alternates via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T05:54:02Z","receivedAt":"2025-12-10T05:54:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Dec 09, 2025 at 11:49:55AM -0600, Justin Tobler wrote:\n> Question: the interface of odb_source_read_alternates() still expects\n> parsed alternates to be written to the output strvec. The sources don't\n> get added to the ODB source list until odb_add_source() is invoked on\n> the source. Does this mean odb_add_source() will have to be able to\n> handle various different types of ODB sources? If so, will these be\n> differentiated by some sort of URI?\n\nYes, exactly. The plan is to use syntax like \"files://path\" or\n\"postgres://127.0.0.1:5432?database=myrepo\". See also the discussion in\n[1].\n\nPatrick\n\n[1]: <aS2V4TKeS4V_oxAb@pks.im>\n"},{"id":"531983","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH v2 0/8] Refactor handling of alternates to work via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:33Z","receivedAt":"2025-12-10T15:32:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series refactors how we handle alternate object directories\nso that the interface is structured around the object database source.\n\nNext to being simpler to reason about, it also allows us to eventually\nabstract handling of alternates to use different mechanisms based on the\nspecific backend used. In a world of pluggable object databases not\nevery backend may use a physical directory, so it may not be possible to\nread alternates via \"objects/info/alternates\". Consequently, formats may\nneed a different mechanism entirely to make this list available.\n\nChanges in v2:\n  - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to\n    highlight that this function is recursive.\n  - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (8):\n      odb: refactor parsing of alternates to be self-contained\n      odb: resolve relative alternative paths when parsing\n      odb: move computation of normalized objdir into `alt_odb_usable()`\n      odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`\n      odb: remove mutual recursion when parsing alternates\n      odb: drop forward declaration of `read_info_alternates()`\n      odb: read alternates via sources\n      odb: write alternates via sources\n\n odb.c | 307 ++++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 158 insertions(+), 149 deletions(-)\n\nRange-diff versus v1:\n\n1:  18b0d15865 = 1:  392036039f odb: refactor parsing of alternates to be self-contained\n2:  aaf9d4e162 ! 2:  0107d40816 odb: resolve relative alternative paths when parsing\n    @@ Commit message\n         cannot be resolved to a directory then `alt_odb_usable()` still knows to\n         bail out.\n     \n    -    While at it, rename the function to `odb_add_source()` to more clearly\n    -    indicate what its intent is and to align it with modern terminology.\n    +    While at it, rename the function to `odb_add_alternate_recursively()` to\n    +    more clearly indicate what its intent is and to align it with modern\n    +    terminology.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ odb.c: static struct odb_source *odb_source_new(struct object_database *odb,\n     -\t\t\t\t\t     const char *dir,\n     -\t\t\t\t\t     const char *relative_base,\n     -\t\t\t\t\t     int depth)\n    -+static struct odb_source *odb_add_source(struct object_database *odb,\n    -+\t\t\t\t\t const char *source,\n    -+\t\t\t\t\t int depth)\n    ++static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n    ++\t\t\t\t\t\t\tconst char *source,\n    ++\t\t\t\t\t\t\tint depth)\n      {\n      \tstruct odb_source *alternate = NULL;\n     -\tstruct strbuf pathbuf = STRBUF_INIT;\n    @@ odb.c: static void link_alt_odb_entries(struct object_database *odb, const char\n      \n      \tfor (size_t i = 0; i < alternates.nr; i++)\n     -\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n    -+\t\todb_add_source(odb, alternates.v[i], depth);\n    ++\t\todb_add_alternate_recursively(odb, alternates.v[i], depth);\n      \n      \tstrvec_clear(&alternates);\n      }\n    @@ odb.c: struct odb_source *odb_add_to_alternates_memory(struct object_database *o\n      \t */\n      \todb_prepare_alternates(odb);\n     -\treturn link_alt_odb_entry(odb, dir, NULL, 0);\n    -+\treturn odb_add_source(odb, dir, 0);\n    ++\treturn odb_add_alternate_recursively(odb, dir, 0);\n      }\n      \n      struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,\n3:  077480d200 ! 3:  8b918fec33 odb: move computation of normalized objdir into `alt_odb_usable()`\n    @@ odb.c: static int alt_odb_usable(struct object_database *o, const char *path,\n      }\n      \n      /*\n    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n    - \t\t\t\t\t int depth)\n    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *\n    + \t\t\t\t\t\t\tint depth)\n      {\n      \tstruct odb_source *alternate = NULL;\n     -\tstruct strbuf tmp = STRBUF_INIT;\n    @@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n      \t\tgoto error;\n      \n      \talternate = odb_source_new(odb, source, false);\n    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *\n      \tread_info_alternates(odb, alternate->path, depth + 1);\n      \n       error:\n4:  f536d0afc3 = 4:  618bfedf22 odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`\n5:  0930371378 ! 5:  50e93145e4 odb: remove mutual recursion when parsing alternates\n    @@ Commit message\n         Refactor the function to remove the mutual recursion between adding\n         sources and parsing alternates. The parsing step thus becomes completely\n         oblivious to the fact that there is recursive behaviour going on at all.\n    -    Instead, the recursion is handled exclusively by `odb_add_source()`,\n    +    The recursion is handled by `odb_add_alternate_recursively()` instead,\n         which now recurses with itself.\n     \n         This refactoring allows us to move parsing of alternates into object\n    @@ odb.c: static bool odb_is_source_usable(struct object_database *o, const char *p\n      \n      static struct odb_source *odb_source_new(struct object_database *odb,\n      \t\t\t\t\t const char *path,\n    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n    - \t\t\t\t\t int depth)\n    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *\n    + \t\t\t\t\t\t\tint depth)\n      {\n      \tstruct odb_source *alternate = NULL;\n     +\tstruct strvec sources = STRVEC_INIT;\n      \tkhiter_t pos;\n      \tint ret;\n      \n    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *\n      \tkh_value(odb->source_by_path, pos) = alternate;\n      \n      \t/* recursively add alternates */\n    @@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n     +\t\t      source);\n     +\t} else {\n     +\t\tfor (size_t i = 0; i < sources.nr; i++)\n    -+\t\t\todb_add_source(odb, sources.v[i], depth + 1);\n    ++\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n     +\t}\n      \n       error:\n    @@ odb.c: static void parse_alternates(const char *string,\n     -\tparse_alternates(alt, sep, relative_base, &alternates);\n     -\n     -\tfor (size_t i = 0; i < alternates.nr; i++)\n    --\t\todb_add_source(odb, alternates.v[i], depth);\n    +-\t\todb_add_alternate_recursively(odb, alternates.v[i], depth);\n     -\n     -\tstrvec_clear(&alternates);\n     -}\n    @@ odb.c: static void read_info_alternates(struct object_database *odb,\n      \tstrbuf_release(&buf);\n      \tfree(path);\n      }\n    +@@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,\n    + \t\tif (commit_lock_file(&lock))\n    + \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n    + \t\tif (odb->loaded_alternates)\n    +-\t\t\todb_add_source(odb, dir, 0);\n    ++\t\t\todb_add_alternate_recursively(odb, dir, 0);\n    + \t}\n    + \tfree(alts);\n    + }\n     @@ odb.c: int odb_for_each_alternate(struct object_database *odb,\n      \n      void odb_prepare_alternates(struct object_database *odb)\n    @@ odb.c: int odb_for_each_alternate(struct object_database *odb,\n     +\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n     +\tread_info_alternates(odb->sources->path, &sources);\n     +\tfor (size_t i = 0; i < sources.nr; i++)\n    -+\t\todb_add_source(odb, sources.v[i], 0);\n    ++\t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n      \n     -\tread_info_alternates(odb, odb->sources->path, 0);\n      \todb->loaded_alternates = 1;\n6:  be857d1b09 ! 6:  d397255cdb odb: drop forward declaration of `read_info_alternates()`\n    @@ odb.c: static bool odb_is_source_usable(struct object_database *o, const char *p\n     -\treturn source;\n     -}\n     -\n    --static struct odb_source *odb_add_source(struct object_database *odb,\n    --\t\t\t\t\t const char *source,\n    --\t\t\t\t\t int depth)\n    +-static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n    +-\t\t\t\t\t\t\tconst char *source,\n    +-\t\t\t\t\t\t\tint depth)\n     -{\n     -\tstruct odb_source *alternate = NULL;\n     -\tstruct strvec sources = STRVEC_INIT;\n    @@ odb.c: static bool odb_is_source_usable(struct object_database *o, const char *p\n     -\t\t      source);\n     -\t} else {\n     -\t\tfor (size_t i = 0; i < sources.nr; i++)\n    --\t\t\todb_add_source(odb, sources.v[i], depth + 1);\n    +-\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n     -\t}\n     -\n     - error:\n    @@ odb.c: static void read_info_alternates(const char *relative_base,\n     +\treturn source;\n     +}\n     +\n    -+static struct odb_source *odb_add_source(struct object_database *odb,\n    -+\t\t\t\t\t const char *source,\n    -+\t\t\t\t\t int depth)\n    ++static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n    ++\t\t\t\t\t\t\tconst char *source,\n    ++\t\t\t\t\t\t\tint depth)\n     +{\n     +\tstruct odb_source *alternate = NULL;\n     +\tstruct strvec sources = STRVEC_INIT;\n    @@ odb.c: static void read_info_alternates(const char *relative_base,\n     +\t\t      source);\n     +\t} else {\n     +\t\tfor (size_t i = 0; i < sources.nr; i++)\n    -+\t\t\todb_add_source(odb, sources.v[i], depth + 1);\n    ++\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n     +\t}\n     +\n     + error:\n7:  a811f6abd6 ! 7:  a39997318c odb: read alternates via sources\n    @@ odb.c: static void parse_alternates(const char *string,\n      \n      \tstrbuf_release(&buf);\n      \tfree(path);\n    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *\n      \tkh_value(odb->source_by_path, pos) = alternate;\n      \n      \t/* recursively add alternates */\n    @@ odb.c: void odb_prepare_alternates(struct object_database *odb)\n     -\tread_info_alternates(odb->sources->path, &sources);\n     +\todb_source_read_alternates(odb->sources, &sources);\n      \tfor (size_t i = 0; i < sources.nr; i++)\n    - \t\todb_add_source(odb, sources.v[i], 0);\n    + \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n      \n8:  be62ab52ab ! 8:  082eb43b82 odb: write alternates via sources\n    @@ Commit message\n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n      ## odb.c ##\n    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,\n    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *\n      \treturn alternate;\n      }\n      \n    @@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,\n     -\t\tif (commit_lock_file(&lock))\n     -\t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n     -\t\tif (odb->loaded_alternates)\n    --\t\t\todb_add_source(odb, dir, 0);\n    +-\t\t\todb_add_alternate_recursively(odb, dir, 0);\n     +\t\tfprintf_or_die(out, \"%s\\n\", alternate);\n     +\t\tif (commit_lock_file(&lock)) {\n     +\t\t\tret = error_errno(_(\"unable to move new alternates file into place\"));\n    @@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,\n     +\tif (ret < 0)\n     +\t\tdie(NULL);\n     +\tif (odb->loaded_alternates)\n    -+\t\todb_add_source(odb, dir, 0);\n    ++\t\todb_add_alternate_recursively(odb, dir, 0);\n      }\n      \n      struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n\n---\nbase-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06\nchange-id: 20251206-b4-pks-odb-alternates-via-source-802d87cbbda5\n\n"},{"id":"531984","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-1-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 1/8] odb: refactor parsing of alternates to be self-contained","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:34Z","receivedAt":"2025-12-10T15:32:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Parsing of the alternates file and environment variable is currently\nsplit up across multiple different functions and is entangled with\n`link_alt_odb_entries()`, which is responsible for linking the parsed\nobject database sources. This results in two downsides:\n\n  - We have mutual recursion between parsing alternates and linking them\n    into the object database. This is because we also parse alternates\n    that the newly added sources may have.\n\n  - We mix up the actual logic to parse the data and to link them into\n    place.\n\nRefactor the logic so that parsing of the alternates file is entirely\nself-contained. Note that this doesn't yet fix the above two issues, but\nit is a necessary step to get there.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 40 insertions(+), 30 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex dc8f292f3d..9785f62cb6 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n \treturn alternate;\n }\n \n-static const char *parse_alt_odb_entry(const char *string,\n-\t\t\t\t       int sep,\n-\t\t\t\t       struct strbuf *out)\n+static void parse_alternates(const char *string,\n+\t\t\t     int sep,\n+\t\t\t     struct strvec *out)\n {\n-\tconst char *end;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n-\tstrbuf_reset(out);\n+\twhile (*string) {\n+\t\tconst char *end;\n+\n+\t\tstrbuf_reset(&buf);\n+\n+\t\tif (*string == '#') {\n+\t\t\t/* comment; consume up to next separator */\n+\t\t\tend = strchrnul(string, sep);\n+\t\t} else if (*string == '\"' && !unquote_c_style(&buf, string, &end)) {\n+\t\t\t/*\n+\t\t\t * quoted path; unquote_c_style has copied the\n+\t\t\t * data for us and set \"end\". Broken quoting (e.g.,\n+\t\t\t * an entry that doesn't end with a quote) falls\n+\t\t\t * back to the unquoted case below.\n+\t\t\t */\n+\t\t} else {\n+\t\t\t/* normal, unquoted path */\n+\t\t\tend = strchrnul(string, sep);\n+\t\t\tstrbuf_add(&buf, string, end - string);\n+\t\t}\n \n-\tif (*string == '#') {\n-\t\t/* comment; consume up to next separator */\n-\t\tend = strchrnul(string, sep);\n-\t} else if (*string == '\"' && !unquote_c_style(out, string, &end)) {\n-\t\t/*\n-\t\t * quoted path; unquote_c_style has copied the\n-\t\t * data for us and set \"end\". Broken quoting (e.g.,\n-\t\t * an entry that doesn't end with a quote) falls\n-\t\t * back to the unquoted case below.\n-\t\t */\n-\t} else {\n-\t\t/* normal, unquoted path */\n-\t\tend = strchrnul(string, sep);\n-\t\tstrbuf_add(out, string, end - string);\n+\t\tif (*end)\n+\t\t\tend++;\n+\t\tstring = end;\n+\n+\t\tif (!buf.len)\n+\t\t\tcontinue;\n+\n+\t\tstrvec_push(out, buf.buf);\n \t}\n \n-\tif (*end)\n-\t\tend++;\n-\treturn end;\n+\tstrbuf_release(&buf);\n }\n \n static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\t\t\t int sep, const char *relative_base, int depth)\n {\n-\tstruct strbuf dir = STRBUF_INIT;\n+\tstruct strvec alternates = STRVEC_INIT;\n \n \tif (!alt || !*alt)\n \t\treturn;\n@@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\treturn;\n \t}\n \n-\twhile (*alt) {\n-\t\talt = parse_alt_odb_entry(alt, sep, &dir);\n-\t\tif (!dir.len)\n-\t\t\tcontinue;\n-\t\tlink_alt_odb_entry(odb, dir.buf, relative_base, depth);\n-\t}\n-\tstrbuf_release(&dir);\n+\tparse_alternates(alt, sep, &alternates);\n+\n+\tfor (size_t i = 0; i < alternates.nr; i++)\n+\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n+\n+\tstrvec_clear(&alternates);\n }\n \n static void read_info_alternates(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531985","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-2-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 2/8] odb: resolve relative alternative paths when parsing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:35Z","receivedAt":"2025-12-10T15:32:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Parsing alternates and resolving potential relative paths is currently\nhandled in two separate steps. This has the effect that the logic to\nretrieve alternates is not entirely self-contained. We want it to be\njust that though so that we can eventually move the logic to list\nalternates into the `struct odb_source`.\n\nMove the logic to resolve relative alternative paths into\n`parse_alternates()`. Besides bringing us a step closer towards the\nabove goal, it also neatly separates concerns of generating the list of\nalternatives and linking them into the object database.\n\nNote that we ignore any errors when the relative path cannot be\nresolved. This isn't really a change in behaviour though: if the path\ncannot be resolved to a directory then `alt_odb_usable()` still knows to\nbail out.\n\nWhile at it, rename the function to `odb_add_alternate_recursively()` to\nmore clearly indicate what its intent is and to align it with modern\nterminology.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 9785f62cb6..699bdbffd1 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,\n \treturn source;\n }\n \n-static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n-\t\t\t\t\t     const char *dir,\n-\t\t\t\t\t     const char *relative_base,\n-\t\t\t\t\t     int depth)\n+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n+\t\t\t\t\t\t\tconst char *source,\n+\t\t\t\t\t\t\tint depth)\n {\n \tstruct odb_source *alternate = NULL;\n-\tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf tmp = STRBUF_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n-\tif (!is_absolute_path(dir) && relative_base) {\n-\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n-\t\tstrbuf_addch(&pathbuf, '/');\n-\t}\n-\tstrbuf_addstr(&pathbuf, dir);\n-\n-\tif (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {\n-\t\terror(_(\"unable to normalize alternate object path: %s\"),\n-\t\t      pathbuf.buf);\n-\t\tgoto error;\n-\t}\n-\tstrbuf_swap(&pathbuf, &tmp);\n-\n-\t/*\n-\t * The trailing slash after the directory name is given by\n-\t * this function at the end. Remove duplicates.\n-\t */\n-\twhile (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')\n-\t\tstrbuf_setlen(&pathbuf, pathbuf.len - 1);\n-\n-\tstrbuf_reset(&tmp);\n \tstrbuf_realpath(&tmp, odb->sources->path, 1);\n \n-\tif (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))\n+\tif (!alt_odb_usable(odb, source, tmp.buf))\n \t\tgoto error;\n \n-\talternate = odb_source_new(odb, pathbuf.buf, false);\n+\talternate = odb_source_new(odb, source, false);\n \n \t/* add the alternate entry */\n \t*odb->sources_tail = alternate;\n@@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n \n  error:\n \tstrbuf_release(&tmp);\n-\tstrbuf_release(&pathbuf);\n \treturn alternate;\n }\n \n static void parse_alternates(const char *string,\n \t\t\t     int sep,\n+\t\t\t     const char *relative_base,\n \t\t\t     struct strvec *out)\n {\n+\tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \twhile (*string) {\n \t\tconst char *end;\n \n \t\tstrbuf_reset(&buf);\n+\t\tstrbuf_reset(&pathbuf);\n \n \t\tif (*string == '#') {\n \t\t\t/* comment; consume up to next separator */\n@@ -250,9 +229,30 @@ static void parse_alternates(const char *string,\n \t\tif (!buf.len)\n \t\t\tcontinue;\n \n+\t\tif (!is_absolute_path(buf.buf) && relative_base) {\n+\t\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n+\t\t\tstrbuf_addch(&pathbuf, '/');\n+\t\t}\n+\t\tstrbuf_addbuf(&pathbuf, &buf);\n+\n+\t\tstrbuf_reset(&buf);\n+\t\tif (!strbuf_realpath(&buf, pathbuf.buf, 0)) {\n+\t\t\terror(_(\"unable to normalize alternate object path: %s\"),\n+\t\t\t      pathbuf.buf);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * The trailing slash after the directory name is given by\n+\t\t * this function at the end. Remove duplicates.\n+\t\t */\n+\t\twhile (buf.len && buf.buf[buf.len - 1] == '/')\n+\t\t\tstrbuf_setlen(&buf, buf.len - 1);\n+\n \t\tstrvec_push(out, buf.buf);\n \t}\n \n+\tstrbuf_release(&pathbuf);\n \tstrbuf_release(&buf);\n }\n \n@@ -270,10 +270,10 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\treturn;\n \t}\n \n-\tparse_alternates(alt, sep, &alternates);\n+\tparse_alternates(alt, sep, relative_base, &alternates);\n \n \tfor (size_t i = 0; i < alternates.nr; i++)\n-\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n+\t\todb_add_alternate_recursively(odb, alternates.v[i], depth);\n \n \tstrvec_clear(&alternates);\n }\n@@ -348,7 +348,7 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n \t * overwritten when they are.\n \t */\n \todb_prepare_alternates(odb);\n-\treturn link_alt_odb_entry(odb, dir, NULL, 0);\n+\treturn odb_add_alternate_recursively(odb, dir, 0);\n }\n \n struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531986","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-3-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:36Z","receivedAt":"2025-12-10T15:32:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `alt_odb_usable()` receives as input the object database,\nthe path it's supposed to determine usability for as well as the\nnormalized path of the main object directory of the repository. The last\npart is derived by the function's caller from the object database. As we\nalready pass the object database to `alt_odb_usable()` it is redundant\ninformation.\n\nDrop the extra parameter and compute the normalized object directory in\nthe function itself.\n\nWhile at it, rename the function to `odb_is_source_usable()` to align it\nwith modern terminology.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 27 +++++++++++++++------------\n 1 file changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 699bdbffd1..e314f86c3b 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,\n /*\n  * Return non-zero iff the path is usable as an alternate object database.\n  */\n-static int alt_odb_usable(struct object_database *o, const char *path,\n-\t\t\t  const char *normalized_objdir)\n+static bool odb_is_source_usable(struct object_database *o, const char *path)\n {\n \tint r;\n+\tstruct strbuf normalized_objdir = STRBUF_INIT;\n+\tbool usable = false;\n+\n+\tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n \n \t/* Detect cases where alternate disappeared */\n \tif (!is_directory(path)) {\n \t\terror(_(\"object directory %s does not exist; \"\n \t\t\t\"check .git/objects/info/alternates\"),\n \t\t      path);\n-\t\treturn 0;\n+\t\tgoto out;\n \t}\n \n \t/*\n@@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,\n \t\tkh_value(o->source_by_path, p) = o->sources;\n \t}\n \n-\tif (fspatheq(path, normalized_objdir))\n-\t\treturn 0;\n+\tif (fspatheq(path, normalized_objdir.buf))\n+\t\tgoto out;\n \n \tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n-\t\treturn 0;\n+\t\tgoto out;\n+\n+\tusable = true;\n \n-\treturn 1;\n+out:\n+\tstrbuf_release(&normalized_objdir);\n+\treturn usable;\n }\n \n /*\n@@ -164,13 +171,10 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \t\t\t\t\t\t\tint depth)\n {\n \tstruct odb_source *alternate = NULL;\n-\tstruct strbuf tmp = STRBUF_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n-\tstrbuf_realpath(&tmp, odb->sources->path, 1);\n-\n-\tif (!alt_odb_usable(odb, source, tmp.buf))\n+\tif (!odb_is_source_usable(odb, source))\n \t\tgoto error;\n \n \talternate = odb_source_new(odb, source, false);\n@@ -188,7 +192,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \tread_info_alternates(odb, alternate->path, depth + 1);\n \n  error:\n-\tstrbuf_release(&tmp);\n \treturn alternate;\n }\n \n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531987","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-4-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:37Z","receivedAt":"2025-12-10T15:32:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `odb_add_to_alternates_file()` we know to add the newly\nadded source to the object database in case we have already loaded\nalternates. This is done so that we can make its objects accessible\nimmediately without having to fully reload all alternates.\n\nThe way we do this though is to call `link_alt_odb_entries()`, which\nadds _multiple_ sources to the object database source in case we have\nnewline-separated entries. This behaviour is not documented in the\nfunction documentation of `odb_add_to_alternates_file()`, and all\ncallers only ever pass a single directory to it. It's thus entirely\nsurprising and a conceptual mismatch.\n\nFix this issue by directly calling `odb_add_source()` instead.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/odb.c b/odb.c\nindex e314f86c3b..d97e50fb61 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \t\tif (commit_lock_file(&lock))\n \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n \t\tif (odb->loaded_alternates)\n-\t\t\tlink_alt_odb_entries(odb, dir, '\\n', NULL, 0);\n+\t\t\todb_add_source(odb, dir, 0);\n \t}\n \tfree(alts);\n }\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531988","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-5-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 5/8] odb: remove mutual recursion when parsing alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:38Z","receivedAt":"2025-12-10T15:32:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When adding an alternative object database source we not only have to\nconsider the added source itself, but we also have to add _its_ sources\nto our database. We implement this via mutual recursion:\n\n  1. We first call `link_alt_odb_entries()`.\n\n  2. `link_alt_odb_entries()` calls `parse_alternates()`.\n\n  3. We then add each parsed alternate via `odb_add_source()`.\n\n  4. `odb_add_source()` calls `link_alt_odb_entries()` again.\n\nThis flow is somewhat hard to follow, but more importantly it means that\nparsing of alternates is somewhat tied to the recursive behaviour.\n\nRefactor the function to remove the mutual recursion between adding\nsources and parsing alternates. The parsing step thus becomes completely\noblivious to the fact that there is recursive behaviour going on at all.\nThe recursion is handled by `odb_add_alternate_recursively()` instead,\nwhich now recurses with itself.\n\nThis refactoring allows us to move parsing of alternates into object\ndatabase sources in a subsequent step.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 62 ++++++++++++++++++++++++++++----------------------------------\n 1 file changed, 28 insertions(+), 34 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex d97e50fb61..59944d4649 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n  * of the object ID, an extra slash for the first level indirection, and\n  * the terminating NUL.\n  */\n-static void read_info_alternates(struct object_database *odb,\n-\t\t\t\t const char *relative_base,\n-\t\t\t\t int depth);\n+static void read_info_alternates(const char *relative_base,\n+\t\t\t\t struct strvec *out);\n \n static struct odb_source *odb_source_new(struct object_database *odb,\n \t\t\t\t\t const char *path,\n@@ -171,6 +170,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \t\t\t\t\t\t\tint depth)\n {\n \tstruct odb_source *alternate = NULL;\n+\tstruct strvec sources = STRVEC_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n@@ -189,9 +189,17 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \tkh_value(odb->source_by_path, pos) = alternate;\n \n \t/* recursively add alternates */\n-\tread_info_alternates(odb, alternate->path, depth + 1);\n+\tread_info_alternates(alternate->path, &sources);\n+\tif (sources.nr && depth + 1 > 5) {\n+\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n+\t\t      source);\n+\t} else {\n+\t\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n+\t}\n \n  error:\n+\tstrvec_clear(&sources);\n \treturn alternate;\n }\n \n@@ -203,6 +211,9 @@ static void parse_alternates(const char *string,\n \tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \n+\tif (!string || !*string)\n+\t\treturn;\n+\n \twhile (*string) {\n \t\tconst char *end;\n \n@@ -259,34 +270,11 @@ static void parse_alternates(const char *string,\n \tstrbuf_release(&buf);\n }\n \n-static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n-\t\t\t\t int sep, const char *relative_base, int depth)\n+static void read_info_alternates(const char *relative_base,\n+\t\t\t\t struct strvec *out)\n {\n-\tstruct strvec alternates = STRVEC_INIT;\n-\n-\tif (!alt || !*alt)\n-\t\treturn;\n-\n-\tif (depth > 5) {\n-\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n-\t\t\t\trelative_base);\n-\t\treturn;\n-\t}\n-\n-\tparse_alternates(alt, sep, relative_base, &alternates);\n-\n-\tfor (size_t i = 0; i < alternates.nr; i++)\n-\t\todb_add_alternate_recursively(odb, alternates.v[i], depth);\n-\n-\tstrvec_clear(&alternates);\n-}\n-\n-static void read_info_alternates(struct object_database *odb,\n-\t\t\t\t const char *relative_base,\n-\t\t\t\t int depth)\n-{\n-\tchar *path;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tchar *path;\n \n \tpath = xstrfmt(\"%s/info/alternates\", relative_base);\n \tif (strbuf_read_file(&buf, path, 1024) < 0) {\n@@ -294,8 +282,8 @@ static void read_info_alternates(struct object_database *odb,\n \t\tfree(path);\n \t\treturn;\n \t}\n+\tparse_alternates(buf.buf, '\\n', relative_base, out);\n \n-\tlink_alt_odb_entries(odb, buf.buf, '\\n', relative_base, depth);\n \tstrbuf_release(&buf);\n \tfree(path);\n }\n@@ -338,7 +326,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \t\tif (commit_lock_file(&lock))\n \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n \t\tif (odb->loaded_alternates)\n-\t\t\todb_add_source(odb, dir, 0);\n+\t\t\todb_add_alternate_recursively(odb, dir, 0);\n \t}\n \tfree(alts);\n }\n@@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,\n \n void odb_prepare_alternates(struct object_database *odb)\n {\n+\tstruct strvec sources = STRVEC_INIT;\n+\n \tif (odb->loaded_alternates)\n \t\treturn;\n \n-\tlink_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);\n+\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n+\tread_info_alternates(odb->sources->path, &sources);\n+\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n-\tread_info_alternates(odb, odb->sources->path, 0);\n \todb->loaded_alternates = 1;\n+\n+\tstrvec_clear(&sources);\n }\n \n int odb_has_alternates(struct object_database *odb)\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531989","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-6-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 6/8] odb: drop forward declaration of `read_info_alternates()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:39Z","receivedAt":"2025-12-10T15:32:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Now that we have removed the mutual recursion in the preceding commit\nit is not necessary anymore to have a forward declaration of the\n`read_info_alternates()` function. Move the function and its\ndependencies further up so that we can remove it.\n\nNote that this commit also removes the function documentation of\n`read_info_alternates()`. It's unclear what it's documenting, but it for\nsure isn't documenting the modern behaviour of the function anymore.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 125 +++++++++++++++++++++++++++++-------------------------------------\n 1 file changed, 54 insertions(+), 71 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 59944d4649..dcf4a62cd2 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -132,77 +132,6 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n \treturn usable;\n }\n \n-/*\n- * Prepare alternate object database registry.\n- *\n- * The variable alt_odb_list points at the list of struct\n- * odb_source.  The elements on this list come from\n- * non-empty elements from colon separated ALTERNATE_DB_ENVIRONMENT\n- * environment variable, and $GIT_OBJECT_DIRECTORY/info/alternates,\n- * whose contents is similar to that environment variable but can be\n- * LF separated.  Its base points at a statically allocated buffer that\n- * contains \"/the/directory/corresponding/to/.git/objects/...\", while\n- * its name points just after the slash at the end of \".git/objects/\"\n- * in the example above, and has enough space to hold all hex characters\n- * of the object ID, an extra slash for the first level indirection, and\n- * the terminating NUL.\n- */\n-static void read_info_alternates(const char *relative_base,\n-\t\t\t\t struct strvec *out);\n-\n-static struct odb_source *odb_source_new(struct object_database *odb,\n-\t\t\t\t\t const char *path,\n-\t\t\t\t\t bool local)\n-{\n-\tstruct odb_source *source;\n-\n-\tCALLOC_ARRAY(source, 1);\n-\tsource->odb = odb;\n-\tsource->local = local;\n-\tsource->path = xstrdup(path);\n-\tsource->loose = odb_source_loose_new(source);\n-\n-\treturn source;\n-}\n-\n-static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n-\t\t\t\t\t\t\tconst char *source,\n-\t\t\t\t\t\t\tint depth)\n-{\n-\tstruct odb_source *alternate = NULL;\n-\tstruct strvec sources = STRVEC_INIT;\n-\tkhiter_t pos;\n-\tint ret;\n-\n-\tif (!odb_is_source_usable(odb, source))\n-\t\tgoto error;\n-\n-\talternate = odb_source_new(odb, source, false);\n-\n-\t/* add the alternate entry */\n-\t*odb->sources_tail = alternate;\n-\todb->sources_tail = &(alternate->next);\n-\n-\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n-\tif (!ret)\n-\t\tBUG(\"source must not yet exist\");\n-\tkh_value(odb->source_by_path, pos) = alternate;\n-\n-\t/* recursively add alternates */\n-\tread_info_alternates(alternate->path, &sources);\n-\tif (sources.nr && depth + 1 > 5) {\n-\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n-\t\t      source);\n-\t} else {\n-\t\tfor (size_t i = 0; i < sources.nr; i++)\n-\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n-\t}\n-\n- error:\n-\tstrvec_clear(&sources);\n-\treturn alternate;\n-}\n-\n static void parse_alternates(const char *string,\n \t\t\t     int sep,\n \t\t\t     const char *relative_base,\n@@ -288,6 +217,60 @@ static void read_info_alternates(const char *relative_base,\n \tfree(path);\n }\n \n+\n+static struct odb_source *odb_source_new(struct object_database *odb,\n+\t\t\t\t\t const char *path,\n+\t\t\t\t\t bool local)\n+{\n+\tstruct odb_source *source;\n+\n+\tCALLOC_ARRAY(source, 1);\n+\tsource->odb = odb;\n+\tsource->local = local;\n+\tsource->path = xstrdup(path);\n+\tsource->loose = odb_source_loose_new(source);\n+\n+\treturn source;\n+}\n+\n+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n+\t\t\t\t\t\t\tconst char *source,\n+\t\t\t\t\t\t\tint depth)\n+{\n+\tstruct odb_source *alternate = NULL;\n+\tstruct strvec sources = STRVEC_INIT;\n+\tkhiter_t pos;\n+\tint ret;\n+\n+\tif (!odb_is_source_usable(odb, source))\n+\t\tgoto error;\n+\n+\talternate = odb_source_new(odb, source, false);\n+\n+\t/* add the alternate entry */\n+\t*odb->sources_tail = alternate;\n+\todb->sources_tail = &(alternate->next);\n+\n+\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n+\tif (!ret)\n+\t\tBUG(\"source must not yet exist\");\n+\tkh_value(odb->source_by_path, pos) = alternate;\n+\n+\t/* recursively add alternates */\n+\tread_info_alternates(alternate->path, &sources);\n+\tif (sources.nr && depth + 1 > 5) {\n+\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n+\t\t      source);\n+\t} else {\n+\t\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n+\t}\n+\n+ error:\n+\tstrvec_clear(&sources);\n+\treturn alternate;\n+}\n+\n void odb_add_to_alternates_file(struct object_database *odb,\n \t\t\t\tconst char *dir)\n {\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531990","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-7-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 7/8] odb: read alternates via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:40Z","receivedAt":"2025-12-10T15:32:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Adapt how we read alternates so that the interface is structured around\nthe object database source we're reading from. This will eventually\nallow us to abstract away this behaviour with pluggable object databases\nso that every format can have its own mechanism for listing alternates.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex dcf4a62cd2..c5ba26b85f 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -199,19 +199,19 @@ static void parse_alternates(const char *string,\n \tstrbuf_release(&buf);\n }\n \n-static void read_info_alternates(const char *relative_base,\n-\t\t\t\t struct strvec *out)\n+static void odb_source_read_alternates(struct odb_source *source,\n+\t\t\t\t       struct strvec *out)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tchar *path;\n \n-\tpath = xstrfmt(\"%s/info/alternates\", relative_base);\n+\tpath = xstrfmt(\"%s/info/alternates\", source->path);\n \tif (strbuf_read_file(&buf, path, 1024) < 0) {\n \t\twarn_on_fopen_errors(path);\n \t\tfree(path);\n \t\treturn;\n \t}\n-\tparse_alternates(buf.buf, '\\n', relative_base, out);\n+\tparse_alternates(buf.buf, '\\n', source->path, out);\n \n \tstrbuf_release(&buf);\n \tfree(path);\n@@ -257,7 +257,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \tkh_value(odb->source_by_path, pos) = alternate;\n \n \t/* recursively add alternates */\n-\tread_info_alternates(alternate->path, &sources);\n+\todb_source_read_alternates(alternate, &sources);\n \tif (sources.nr && depth + 1 > 5) {\n \t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n \t\t      source);\n@@ -599,7 +599,7 @@ void odb_prepare_alternates(struct object_database *odb)\n \t\treturn;\n \n \tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n-\tread_info_alternates(odb->sources->path, &sources);\n+\todb_source_read_alternates(odb->sources, &sources);\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"531991","messageId":"20251210-b4-pks-odb-alternates-via-source-v2-8-eb336815f9ab@pks.im","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"[PATCH v2 8/8] odb: write alternates via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-10T15:32:41Z","receivedAt":"2025-12-10T15:33:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Refactor writing of alternates so that the actual business logic is\nstructured around the object database source we want to write the\nalternate to. Same as with the preceding commit, this will eventually\nallow us to have different logic for writing alternates depending on the\nbackend used.\n\nNote that after the refactoring we start to call `odb_add_source()`\nunconditionally. This is fine though as we know to skip adding sources\nthat are tracked already.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 51 +++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 35 insertions(+), 16 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex c5ba26b85f..cc7f832465 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -271,25 +271,28 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \treturn alternate;\n }\n \n-void odb_add_to_alternates_file(struct object_database *odb,\n-\t\t\t\tconst char *dir)\n+static int odb_source_write_alternate(struct odb_source *source,\n+\t\t\t\t      const char *alternate)\n {\n \tstruct lock_file lock = LOCK_INIT;\n-\tchar *alts = repo_git_path(odb->repo, \"objects/info/alternates\");\n+\tchar *path = xstrfmt(\"%s/%s\", source->path, \"info/alternates\");\n \tFILE *in, *out;\n \tint found = 0;\n+\tint ret;\n \n-\thold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);\n+\thold_lock_file_for_update(&lock, path, LOCK_DIE_ON_ERROR);\n \tout = fdopen_lock_file(&lock, \"w\");\n-\tif (!out)\n-\t\tdie_errno(_(\"unable to fdopen alternates lockfile\"));\n+\tif (!out) {\n+\t\tret = error_errno(_(\"unable to fdopen alternates lockfile\"));\n+\t\tgoto out;\n+\t}\n \n-\tin = fopen(alts, \"r\");\n+\tin = fopen(path, \"r\");\n \tif (in) {\n \t\tstruct strbuf line = STRBUF_INIT;\n \n \t\twhile (strbuf_getline(&line, in) != EOF) {\n-\t\t\tif (!strcmp(dir, line.buf)) {\n+\t\t\tif (!strcmp(alternate, line.buf)) {\n \t\t\t\tfound = 1;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -298,20 +301,36 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \n \t\tstrbuf_release(&line);\n \t\tfclose(in);\n+\t} else if (errno != ENOENT) {\n+\t\tret = error_errno(_(\"unable to read alternates file\"));\n+\t\tgoto out;\n \t}\n-\telse if (errno != ENOENT)\n-\t\tdie_errno(_(\"unable to read alternates file\"));\n \n \tif (found) {\n \t\trollback_lock_file(&lock);\n \t} else {\n-\t\tfprintf_or_die(out, \"%s\\n\", dir);\n-\t\tif (commit_lock_file(&lock))\n-\t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n-\t\tif (odb->loaded_alternates)\n-\t\t\todb_add_alternate_recursively(odb, dir, 0);\n+\t\tfprintf_or_die(out, \"%s\\n\", alternate);\n+\t\tif (commit_lock_file(&lock)) {\n+\t\t\tret = error_errno(_(\"unable to move new alternates file into place\"));\n+\t\t\tgoto out;\n+\t\t}\n \t}\n-\tfree(alts);\n+\n+\tret = 0;\n+\n+out:\n+\tfree(path);\n+\treturn ret;\n+}\n+\n+void odb_add_to_alternates_file(struct object_database *odb,\n+\t\t\t\tconst char *dir)\n+{\n+\tint ret = odb_source_write_alternate(odb->sources, dir);\n+\tif (ret < 0)\n+\t\tdie(NULL);\n+\tif (odb->loaded_alternates)\n+\t\todb_add_alternate_recursively(odb, dir, 0);\n }\n \n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532004","messageId":"5kulb5uk4uzn7gl4yhvnc4cnmqxzm2ngtezn5b5kkv33pgexmw@klqedekkink7","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im","subject":"Re: [PATCH v2 0/8] Refactor handling of alternates to work via sources","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-12-10T20:00:39Z","receivedAt":"2025-12-10T20:00:41Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/12/10 04:32PM, Patrick Steinhardt wrote:\n> Changes in v2:\n>   - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to\n>     highlight that this function is recursive.\n>   - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im\n\nThanks the changes in the version look good to me.\n\n-Justin\n"},{"id":"532015","messageId":"aTpQQNJyhdLpgKNg@pks.im","threadId":"64595","inReplyTo":"5kulb5uk4uzn7gl4yhvnc4cnmqxzm2ngtezn5b5kkv33pgexmw@klqedekkink7","subject":"Re: [PATCH v2 0/8] Refactor handling of alternates to work via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T05:01:52Z","receivedAt":"2025-12-11T05:01:59Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Dec 10, 2025 at 02:00:39PM -0600, Justin Tobler wrote:\n> On 25/12/10 04:32PM, Patrick Steinhardt wrote:\n> > Changes in v2:\n> >   - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to\n> >     highlight that this function is recursive.\n> >   - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im\n> \n> Thanks the changes in the version look good to me.\n\nThanks for your review!\n\nPatrick\n"},{"id":"532026","messageId":"aTpxB8gS7wG7rRJQ@szeder.dev","threadId":"64595","inReplyTo":"20251210-b4-pks-odb-alternates-via-source-v2-4-eb336815f9ab@pks.im","subject":"Re: [PATCH v2 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2025-12-11T07:21:43Z","receivedAt":"2025-12-11T07:21:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Dec 10, 2025 at 04:32:37PM +0100, Patrick Steinhardt wrote:\n> When calling `odb_add_to_alternates_file()` we know to add the newly\n> added source to the object database in case we have already loaded\n> alternates. This is done so that we can make its objects accessible\n> immediately without having to fully reload all alternates.\n> \n> The way we do this though is to call `link_alt_odb_entries()`, which\n> adds _multiple_ sources to the object database source in case we have\n> newline-separated entries. This behaviour is not documented in the\n> function documentation of `odb_add_to_alternates_file()`, and all\n> callers only ever pass a single directory to it. It's thus entirely\n> surprising and a conceptual mismatch.\n> \n> Fix this issue by directly calling `odb_add_source()` instead.\n\nOK, but:\n\n> diff --git a/odb.c b/odb.c\n> index e314f86c3b..d97e50fb61 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n>  \t\tif (commit_lock_file(&lock))\n>  \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n>  \t\tif (odb->loaded_alternates)\n> -\t\t\tlink_alt_odb_entries(odb, dir, '\\n', NULL, 0);\n> +\t\t\todb_add_source(odb, dir, 0);\n\n      CC odb.o\n  odb.c: In function ‘odb_add_to_alternates_file’:\n  odb.c:341:25: error: implicit declaration of function ‘odb_add_source’; did you mean ‘odb_find_source’? [-Werror=implicit-function-declaration]\n    341 |                         odb_add_source(odb, dir, 0);\n        |                         ^~~~~~~~~~~~~~\n        |                         odb_find_source\n  cc1: all warnings being treated as errors\n  make: *** [Makefile:2864: odb.o] Error 1\n\nNote, that several commit messages also refer to this non-existing\nfunction from the previous round.\n\n"},{"id":"532028","messageId":"aTqPBygCfm1hWtL-@pks.im","threadId":"64595","inReplyTo":"aTpxB8gS7wG7rRJQ@szeder.dev","subject":"Re: [PATCH v2 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:29:43Z","receivedAt":"2025-12-11T09:29:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 11, 2025 at 08:21:43AM +0100, SZEDER Gábor wrote:\n> On Wed, Dec 10, 2025 at 04:32:37PM +0100, Patrick Steinhardt wrote:\n> > When calling `odb_add_to_alternates_file()` we know to add the newly\n> > added source to the object database in case we have already loaded\n> > alternates. This is done so that we can make its objects accessible\n> > immediately without having to fully reload all alternates.\n> > \n> > The way we do this though is to call `link_alt_odb_entries()`, which\n> > adds _multiple_ sources to the object database source in case we have\n> > newline-separated entries. This behaviour is not documented in the\n> > function documentation of `odb_add_to_alternates_file()`, and all\n> > callers only ever pass a single directory to it. It's thus entirely\n> > surprising and a conceptual mismatch.\n> > \n> > Fix this issue by directly calling `odb_add_source()` instead.\n> \n> OK, but:\n> \n> > diff --git a/odb.c b/odb.c\n> > index e314f86c3b..d97e50fb61 100644\n> > --- a/odb.c\n> > +++ b/odb.c\n> > @@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n> >  \t\tif (commit_lock_file(&lock))\n> >  \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n> >  \t\tif (odb->loaded_alternates)\n> > -\t\t\tlink_alt_odb_entries(odb, dir, '\\n', NULL, 0);\n> > +\t\t\todb_add_source(odb, dir, 0);\n> \n>       CC odb.o\n>   odb.c: In function ‘odb_add_to_alternates_file’:\n>   odb.c:341:25: error: implicit declaration of function ‘odb_add_source’; did you mean ‘odb_find_source’? [-Werror=implicit-function-declaration]\n>     341 |                         odb_add_source(odb, dir, 0);\n>         |                         ^~~~~~~~~~~~~~\n>         |                         odb_find_source\n>   cc1: all warnings being treated as errors\n>   make: *** [Makefile:2864: odb.o] Error 1\n\nHrmpf, I only fixed this callsite in a later commit indeed.\n\n> Note, that several commit messages also refer to this non-existing\n> function from the previous round.\n\nTrue. Will fix both of these issues, thanks!\n\nPatrick\n"},{"id":"532029","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im","subject":"[PATCH v3 0/8] Refactor handling of alternates to work via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:09Z","receivedAt":"2025-12-11T09:30:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series refactors how we handle alternate object directories\nso that the interface is structured around the object database source.\n\nNext to being simpler to reason about, it also allows us to eventually\nabstract handling of alternates to use different mechanisms based on the\nspecific backend used. In a world of pluggable object databases not\nevery backend may use a physical directory, so it may not be possible to\nread alternates via \"objects/info/alternates\". Consequently, formats may\nneed a different mechanism entirely to make this list available.\n\nChanges in v3:\n  - Fix commit messages that still refer to `odb_add_source()`.\n  - Fix intermediate commit that still refers to `odb_add_source()`.\n  - Link to v2: https://lore.kernel.org/r/20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im\n\nChanges in v2:\n  - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to\n    highlight that this function is recursive.\n  - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (8):\n      odb: refactor parsing of alternates to be self-contained\n      odb: resolve relative alternative paths when parsing\n      odb: move computation of normalized objdir into `alt_odb_usable()`\n      odb: stop splitting alternate in `odb_add_to_alternates_file()`\n      odb: remove mutual recursion when parsing alternates\n      odb: drop forward declaration of `read_info_alternates()`\n      odb: read alternates via sources\n      odb: write alternates via sources\n\n odb.c | 307 ++++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 158 insertions(+), 149 deletions(-)\n\nRange-diff versus v2:\n\n1:  74d2596ef6 = 1:  4a85139a75 odb: refactor parsing of alternates to be self-contained\n2:  16d6e482d7 = 2:  1b16c0a164 odb: resolve relative alternative paths when parsing\n3:  16cce7f52e = 3:  ceb6e8494c odb: move computation of normalized objdir into `alt_odb_usable()`\n4:  b8a7138a51 ! 4:  99dbd11c48 odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`\n    @@ Metadata\n     Author: Patrick Steinhardt <ps@pks.im>\n     \n      ## Commit message ##\n    -    odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`\n    +    odb: stop splitting alternate in `odb_add_to_alternates_file()`\n     \n         When calling `odb_add_to_alternates_file()` we know to add the newly\n         added source to the object database in case we have already loaded\n    @@ Commit message\n         callers only ever pass a single directory to it. It's thus entirely\n         surprising and a conceptual mismatch.\n     \n    -    Fix this issue by directly calling `odb_add_source()` instead.\n    +    Fix this issue by directly calling `odb_add_alternate_recursively()`\n    +    instead.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,\n      \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n      \t\tif (odb->loaded_alternates)\n     -\t\t\tlink_alt_odb_entries(odb, dir, '\\n', NULL, 0);\n    -+\t\t\todb_add_source(odb, dir, 0);\n    ++\t\t\todb_add_alternate_recursively(odb, dir, 0);\n      \t}\n      \tfree(alts);\n      }\n5:  2b2d4788bf ! 5:  b9300667a6 odb: remove mutual recursion when parsing alternates\n    @@ Commit message\n     \n           2. `link_alt_odb_entries()` calls `parse_alternates()`.\n     \n    -      3. We then add each parsed alternate via `odb_add_source()`.\n    +      3. We then add each alternate via `odb_add_alternate_recursively()`.\n     \n    -      4. `odb_add_source()` calls `link_alt_odb_entries()` again.\n    +      4. `odb_add_alternate_recursively()` calls `link_alt_odb_entries()`\n    +         again.\n     \n         This flow is somewhat hard to follow, but more importantly it means that\n         parsing of alternates is somewhat tied to the recursive behaviour.\n    @@ odb.c: static void read_info_alternates(struct object_database *odb,\n      \tstrbuf_release(&buf);\n      \tfree(path);\n      }\n    -@@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,\n    - \t\tif (commit_lock_file(&lock))\n    - \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n    - \t\tif (odb->loaded_alternates)\n    --\t\t\todb_add_source(odb, dir, 0);\n    -+\t\t\todb_add_alternate_recursively(odb, dir, 0);\n    - \t}\n    - \tfree(alts);\n    - }\n     @@ odb.c: int odb_for_each_alternate(struct object_database *odb,\n      \n      void odb_prepare_alternates(struct object_database *odb)\n6:  3294336d85 = 6:  1e3a1fb081 odb: drop forward declaration of `read_info_alternates()`\n7:  55ba5815d4 = 7:  1d6a9b3c1b odb: read alternates via sources\n8:  225bcc37de ! 8:  79a053fb2b odb: write alternates via sources\n    @@ Commit message\n         allow us to have different logic for writing alternates depending on the\n         backend used.\n     \n    -    Note that after the refactoring we start to call `odb_add_source()`\n    -    unconditionally. This is fine though as we know to skip adding sources\n    -    that are tracked already.\n    +    Note that after the refactoring we start to call\n    +    `odb_add_alternate_recursively()` unconditionally. This is fine though\n    +    as we know to skip adding sources that are tracked already.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n\n---\nbase-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06\nchange-id: 20251206-b4-pks-odb-alternates-via-source-802d87cbbda5\n\n"},{"id":"532030","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-1-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 1/8] odb: refactor parsing of alternates to be self-contained","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:10Z","receivedAt":"2025-12-11T09:30:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Parsing of the alternates file and environment variable is currently\nsplit up across multiple different functions and is entangled with\n`link_alt_odb_entries()`, which is responsible for linking the parsed\nobject database sources. This results in two downsides:\n\n  - We have mutual recursion between parsing alternates and linking them\n    into the object database. This is because we also parse alternates\n    that the newly added sources may have.\n\n  - We mix up the actual logic to parse the data and to link them into\n    place.\n\nRefactor the logic so that parsing of the alternates file is entirely\nself-contained. Note that this doesn't yet fix the above two issues, but\nit is a necessary step to get there.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 40 insertions(+), 30 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex dc8f292f3d..9785f62cb6 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n \treturn alternate;\n }\n \n-static const char *parse_alt_odb_entry(const char *string,\n-\t\t\t\t       int sep,\n-\t\t\t\t       struct strbuf *out)\n+static void parse_alternates(const char *string,\n+\t\t\t     int sep,\n+\t\t\t     struct strvec *out)\n {\n-\tconst char *end;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n-\tstrbuf_reset(out);\n+\twhile (*string) {\n+\t\tconst char *end;\n+\n+\t\tstrbuf_reset(&buf);\n+\n+\t\tif (*string == '#') {\n+\t\t\t/* comment; consume up to next separator */\n+\t\t\tend = strchrnul(string, sep);\n+\t\t} else if (*string == '\"' && !unquote_c_style(&buf, string, &end)) {\n+\t\t\t/*\n+\t\t\t * quoted path; unquote_c_style has copied the\n+\t\t\t * data for us and set \"end\". Broken quoting (e.g.,\n+\t\t\t * an entry that doesn't end with a quote) falls\n+\t\t\t * back to the unquoted case below.\n+\t\t\t */\n+\t\t} else {\n+\t\t\t/* normal, unquoted path */\n+\t\t\tend = strchrnul(string, sep);\n+\t\t\tstrbuf_add(&buf, string, end - string);\n+\t\t}\n \n-\tif (*string == '#') {\n-\t\t/* comment; consume up to next separator */\n-\t\tend = strchrnul(string, sep);\n-\t} else if (*string == '\"' && !unquote_c_style(out, string, &end)) {\n-\t\t/*\n-\t\t * quoted path; unquote_c_style has copied the\n-\t\t * data for us and set \"end\". Broken quoting (e.g.,\n-\t\t * an entry that doesn't end with a quote) falls\n-\t\t * back to the unquoted case below.\n-\t\t */\n-\t} else {\n-\t\t/* normal, unquoted path */\n-\t\tend = strchrnul(string, sep);\n-\t\tstrbuf_add(out, string, end - string);\n+\t\tif (*end)\n+\t\t\tend++;\n+\t\tstring = end;\n+\n+\t\tif (!buf.len)\n+\t\t\tcontinue;\n+\n+\t\tstrvec_push(out, buf.buf);\n \t}\n \n-\tif (*end)\n-\t\tend++;\n-\treturn end;\n+\tstrbuf_release(&buf);\n }\n \n static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\t\t\t int sep, const char *relative_base, int depth)\n {\n-\tstruct strbuf dir = STRBUF_INIT;\n+\tstruct strvec alternates = STRVEC_INIT;\n \n \tif (!alt || !*alt)\n \t\treturn;\n@@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\treturn;\n \t}\n \n-\twhile (*alt) {\n-\t\talt = parse_alt_odb_entry(alt, sep, &dir);\n-\t\tif (!dir.len)\n-\t\t\tcontinue;\n-\t\tlink_alt_odb_entry(odb, dir.buf, relative_base, depth);\n-\t}\n-\tstrbuf_release(&dir);\n+\tparse_alternates(alt, sep, &alternates);\n+\n+\tfor (size_t i = 0; i < alternates.nr; i++)\n+\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n+\n+\tstrvec_clear(&alternates);\n }\n \n static void read_info_alternates(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532031","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-2-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 2/8] odb: resolve relative alternative paths when parsing","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:11Z","receivedAt":"2025-12-11T09:30:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Parsing alternates and resolving potential relative paths is currently\nhandled in two separate steps. This has the effect that the logic to\nretrieve alternates is not entirely self-contained. We want it to be\njust that though so that we can eventually move the logic to list\nalternates into the `struct odb_source`.\n\nMove the logic to resolve relative alternative paths into\n`parse_alternates()`. Besides bringing us a step closer towards the\nabove goal, it also neatly separates concerns of generating the list of\nalternatives and linking them into the object database.\n\nNote that we ignore any errors when the relative path cannot be\nresolved. This isn't really a change in behaviour though: if the path\ncannot be resolved to a directory then `alt_odb_usable()` still knows to\nbail out.\n\nWhile at it, rename the function to `odb_add_alternate_recursively()` to\nmore clearly indicate what its intent is and to align it with modern\nterminology.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------\n 1 file changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 9785f62cb6..699bdbffd1 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,\n \treturn source;\n }\n \n-static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n-\t\t\t\t\t     const char *dir,\n-\t\t\t\t\t     const char *relative_base,\n-\t\t\t\t\t     int depth)\n+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n+\t\t\t\t\t\t\tconst char *source,\n+\t\t\t\t\t\t\tint depth)\n {\n \tstruct odb_source *alternate = NULL;\n-\tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf tmp = STRBUF_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n-\tif (!is_absolute_path(dir) && relative_base) {\n-\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n-\t\tstrbuf_addch(&pathbuf, '/');\n-\t}\n-\tstrbuf_addstr(&pathbuf, dir);\n-\n-\tif (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {\n-\t\terror(_(\"unable to normalize alternate object path: %s\"),\n-\t\t      pathbuf.buf);\n-\t\tgoto error;\n-\t}\n-\tstrbuf_swap(&pathbuf, &tmp);\n-\n-\t/*\n-\t * The trailing slash after the directory name is given by\n-\t * this function at the end. Remove duplicates.\n-\t */\n-\twhile (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')\n-\t\tstrbuf_setlen(&pathbuf, pathbuf.len - 1);\n-\n-\tstrbuf_reset(&tmp);\n \tstrbuf_realpath(&tmp, odb->sources->path, 1);\n \n-\tif (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))\n+\tif (!alt_odb_usable(odb, source, tmp.buf))\n \t\tgoto error;\n \n-\talternate = odb_source_new(odb, pathbuf.buf, false);\n+\talternate = odb_source_new(odb, source, false);\n \n \t/* add the alternate entry */\n \t*odb->sources_tail = alternate;\n@@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,\n \n  error:\n \tstrbuf_release(&tmp);\n-\tstrbuf_release(&pathbuf);\n \treturn alternate;\n }\n \n static void parse_alternates(const char *string,\n \t\t\t     int sep,\n+\t\t\t     const char *relative_base,\n \t\t\t     struct strvec *out)\n {\n+\tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \twhile (*string) {\n \t\tconst char *end;\n \n \t\tstrbuf_reset(&buf);\n+\t\tstrbuf_reset(&pathbuf);\n \n \t\tif (*string == '#') {\n \t\t\t/* comment; consume up to next separator */\n@@ -250,9 +229,30 @@ static void parse_alternates(const char *string,\n \t\tif (!buf.len)\n \t\t\tcontinue;\n \n+\t\tif (!is_absolute_path(buf.buf) && relative_base) {\n+\t\t\tstrbuf_realpath(&pathbuf, relative_base, 1);\n+\t\t\tstrbuf_addch(&pathbuf, '/');\n+\t\t}\n+\t\tstrbuf_addbuf(&pathbuf, &buf);\n+\n+\t\tstrbuf_reset(&buf);\n+\t\tif (!strbuf_realpath(&buf, pathbuf.buf, 0)) {\n+\t\t\terror(_(\"unable to normalize alternate object path: %s\"),\n+\t\t\t      pathbuf.buf);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * The trailing slash after the directory name is given by\n+\t\t * this function at the end. Remove duplicates.\n+\t\t */\n+\t\twhile (buf.len && buf.buf[buf.len - 1] == '/')\n+\t\t\tstrbuf_setlen(&buf, buf.len - 1);\n+\n \t\tstrvec_push(out, buf.buf);\n \t}\n \n+\tstrbuf_release(&pathbuf);\n \tstrbuf_release(&buf);\n }\n \n@@ -270,10 +270,10 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n \t\treturn;\n \t}\n \n-\tparse_alternates(alt, sep, &alternates);\n+\tparse_alternates(alt, sep, relative_base, &alternates);\n \n \tfor (size_t i = 0; i < alternates.nr; i++)\n-\t\tlink_alt_odb_entry(odb, alternates.v[i], relative_base, depth);\n+\t\todb_add_alternate_recursively(odb, alternates.v[i], depth);\n \n \tstrvec_clear(&alternates);\n }\n@@ -348,7 +348,7 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n \t * overwritten when they are.\n \t */\n \todb_prepare_alternates(odb);\n-\treturn link_alt_odb_entry(odb, dir, NULL, 0);\n+\treturn odb_add_alternate_recursively(odb, dir, 0);\n }\n \n struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532032","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-3-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:12Z","receivedAt":"2025-12-11T09:30:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `alt_odb_usable()` receives as input the object database,\nthe path it's supposed to determine usability for as well as the\nnormalized path of the main object directory of the repository. The last\npart is derived by the function's caller from the object database. As we\nalready pass the object database to `alt_odb_usable()` it is redundant\ninformation.\n\nDrop the extra parameter and compute the normalized object directory in\nthe function itself.\n\nWhile at it, rename the function to `odb_is_source_usable()` to align it\nwith modern terminology.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 27 +++++++++++++++------------\n 1 file changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 699bdbffd1..e314f86c3b 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,\n /*\n  * Return non-zero iff the path is usable as an alternate object database.\n  */\n-static int alt_odb_usable(struct object_database *o, const char *path,\n-\t\t\t  const char *normalized_objdir)\n+static bool odb_is_source_usable(struct object_database *o, const char *path)\n {\n \tint r;\n+\tstruct strbuf normalized_objdir = STRBUF_INIT;\n+\tbool usable = false;\n+\n+\tstrbuf_realpath(&normalized_objdir, o->sources->path, 1);\n \n \t/* Detect cases where alternate disappeared */\n \tif (!is_directory(path)) {\n \t\terror(_(\"object directory %s does not exist; \"\n \t\t\t\"check .git/objects/info/alternates\"),\n \t\t      path);\n-\t\treturn 0;\n+\t\tgoto out;\n \t}\n \n \t/*\n@@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,\n \t\tkh_value(o->source_by_path, p) = o->sources;\n \t}\n \n-\tif (fspatheq(path, normalized_objdir))\n-\t\treturn 0;\n+\tif (fspatheq(path, normalized_objdir.buf))\n+\t\tgoto out;\n \n \tif (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))\n-\t\treturn 0;\n+\t\tgoto out;\n+\n+\tusable = true;\n \n-\treturn 1;\n+out:\n+\tstrbuf_release(&normalized_objdir);\n+\treturn usable;\n }\n \n /*\n@@ -164,13 +171,10 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \t\t\t\t\t\t\tint depth)\n {\n \tstruct odb_source *alternate = NULL;\n-\tstruct strbuf tmp = STRBUF_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n-\tstrbuf_realpath(&tmp, odb->sources->path, 1);\n-\n-\tif (!alt_odb_usable(odb, source, tmp.buf))\n+\tif (!odb_is_source_usable(odb, source))\n \t\tgoto error;\n \n \talternate = odb_source_new(odb, source, false);\n@@ -188,7 +192,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \tread_info_alternates(odb, alternate->path, depth + 1);\n \n  error:\n-\tstrbuf_release(&tmp);\n \treturn alternate;\n }\n \n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532033","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-4-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 4/8] odb: stop splitting alternate in `odb_add_to_alternates_file()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:13Z","receivedAt":"2025-12-11T09:30:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When calling `odb_add_to_alternates_file()` we know to add the newly\nadded source to the object database in case we have already loaded\nalternates. This is done so that we can make its objects accessible\nimmediately without having to fully reload all alternates.\n\nThe way we do this though is to call `link_alt_odb_entries()`, which\nadds _multiple_ sources to the object database source in case we have\nnewline-separated entries. This behaviour is not documented in the\nfunction documentation of `odb_add_to_alternates_file()`, and all\ncallers only ever pass a single directory to it. It's thus entirely\nsurprising and a conceptual mismatch.\n\nFix this issue by directly calling `odb_add_alternate_recursively()`\ninstead.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/odb.c b/odb.c\nindex e314f86c3b..3112eab5d0 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \t\tif (commit_lock_file(&lock))\n \t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n \t\tif (odb->loaded_alternates)\n-\t\t\tlink_alt_odb_entries(odb, dir, '\\n', NULL, 0);\n+\t\t\todb_add_alternate_recursively(odb, dir, 0);\n \t}\n \tfree(alts);\n }\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532034","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-5-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 5/8] odb: remove mutual recursion when parsing alternates","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:14Z","receivedAt":"2025-12-11T09:30:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When adding an alternative object database source we not only have to\nconsider the added source itself, but we also have to add _its_ sources\nto our database. We implement this via mutual recursion:\n\n  1. We first call `link_alt_odb_entries()`.\n\n  2. `link_alt_odb_entries()` calls `parse_alternates()`.\n\n  3. We then add each alternate via `odb_add_alternate_recursively()`.\n\n  4. `odb_add_alternate_recursively()` calls `link_alt_odb_entries()`\n     again.\n\nThis flow is somewhat hard to follow, but more importantly it means that\nparsing of alternates is somewhat tied to the recursive behaviour.\n\nRefactor the function to remove the mutual recursion between adding\nsources and parsing alternates. The parsing step thus becomes completely\noblivious to the fact that there is recursive behaviour going on at all.\nThe recursion is handled by `odb_add_alternate_recursively()` instead,\nwhich now recurses with itself.\n\nThis refactoring allows us to move parsing of alternates into object\ndatabase sources in a subsequent step.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 60 +++++++++++++++++++++++++++---------------------------------\n 1 file changed, 27 insertions(+), 33 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 3112eab5d0..59944d4649 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n  * of the object ID, an extra slash for the first level indirection, and\n  * the terminating NUL.\n  */\n-static void read_info_alternates(struct object_database *odb,\n-\t\t\t\t const char *relative_base,\n-\t\t\t\t int depth);\n+static void read_info_alternates(const char *relative_base,\n+\t\t\t\t struct strvec *out);\n \n static struct odb_source *odb_source_new(struct object_database *odb,\n \t\t\t\t\t const char *path,\n@@ -171,6 +170,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \t\t\t\t\t\t\tint depth)\n {\n \tstruct odb_source *alternate = NULL;\n+\tstruct strvec sources = STRVEC_INIT;\n \tkhiter_t pos;\n \tint ret;\n \n@@ -189,9 +189,17 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \tkh_value(odb->source_by_path, pos) = alternate;\n \n \t/* recursively add alternates */\n-\tread_info_alternates(odb, alternate->path, depth + 1);\n+\tread_info_alternates(alternate->path, &sources);\n+\tif (sources.nr && depth + 1 > 5) {\n+\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n+\t\t      source);\n+\t} else {\n+\t\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n+\t}\n \n  error:\n+\tstrvec_clear(&sources);\n \treturn alternate;\n }\n \n@@ -203,6 +211,9 @@ static void parse_alternates(const char *string,\n \tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n \n+\tif (!string || !*string)\n+\t\treturn;\n+\n \twhile (*string) {\n \t\tconst char *end;\n \n@@ -259,34 +270,11 @@ static void parse_alternates(const char *string,\n \tstrbuf_release(&buf);\n }\n \n-static void link_alt_odb_entries(struct object_database *odb, const char *alt,\n-\t\t\t\t int sep, const char *relative_base, int depth)\n+static void read_info_alternates(const char *relative_base,\n+\t\t\t\t struct strvec *out)\n {\n-\tstruct strvec alternates = STRVEC_INIT;\n-\n-\tif (!alt || !*alt)\n-\t\treturn;\n-\n-\tif (depth > 5) {\n-\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n-\t\t\t\trelative_base);\n-\t\treturn;\n-\t}\n-\n-\tparse_alternates(alt, sep, relative_base, &alternates);\n-\n-\tfor (size_t i = 0; i < alternates.nr; i++)\n-\t\todb_add_alternate_recursively(odb, alternates.v[i], depth);\n-\n-\tstrvec_clear(&alternates);\n-}\n-\n-static void read_info_alternates(struct object_database *odb,\n-\t\t\t\t const char *relative_base,\n-\t\t\t\t int depth)\n-{\n-\tchar *path;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tchar *path;\n \n \tpath = xstrfmt(\"%s/info/alternates\", relative_base);\n \tif (strbuf_read_file(&buf, path, 1024) < 0) {\n@@ -294,8 +282,8 @@ static void read_info_alternates(struct object_database *odb,\n \t\tfree(path);\n \t\treturn;\n \t}\n+\tparse_alternates(buf.buf, '\\n', relative_base, out);\n \n-\tlink_alt_odb_entries(odb, buf.buf, '\\n', relative_base, depth);\n \tstrbuf_release(&buf);\n \tfree(path);\n }\n@@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,\n \n void odb_prepare_alternates(struct object_database *odb)\n {\n+\tstruct strvec sources = STRVEC_INIT;\n+\n \tif (odb->loaded_alternates)\n \t\treturn;\n \n-\tlink_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);\n+\tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n+\tread_info_alternates(odb->sources->path, &sources);\n+\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n-\tread_info_alternates(odb, odb->sources->path, 0);\n \todb->loaded_alternates = 1;\n+\n+\tstrvec_clear(&sources);\n }\n \n int odb_has_alternates(struct object_database *odb)\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532035","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-6-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 6/8] odb: drop forward declaration of `read_info_alternates()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:15Z","receivedAt":"2025-12-11T09:30:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Now that we have removed the mutual recursion in the preceding commit\nit is not necessary anymore to have a forward declaration of the\n`read_info_alternates()` function. Move the function and its\ndependencies further up so that we can remove it.\n\nNote that this commit also removes the function documentation of\n`read_info_alternates()`. It's unclear what it's documenting, but it for\nsure isn't documenting the modern behaviour of the function anymore.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 125 +++++++++++++++++++++++++++++-------------------------------------\n 1 file changed, 54 insertions(+), 71 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex 59944d4649..dcf4a62cd2 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -132,77 +132,6 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)\n \treturn usable;\n }\n \n-/*\n- * Prepare alternate object database registry.\n- *\n- * The variable alt_odb_list points at the list of struct\n- * odb_source.  The elements on this list come from\n- * non-empty elements from colon separated ALTERNATE_DB_ENVIRONMENT\n- * environment variable, and $GIT_OBJECT_DIRECTORY/info/alternates,\n- * whose contents is similar to that environment variable but can be\n- * LF separated.  Its base points at a statically allocated buffer that\n- * contains \"/the/directory/corresponding/to/.git/objects/...\", while\n- * its name points just after the slash at the end of \".git/objects/\"\n- * in the example above, and has enough space to hold all hex characters\n- * of the object ID, an extra slash for the first level indirection, and\n- * the terminating NUL.\n- */\n-static void read_info_alternates(const char *relative_base,\n-\t\t\t\t struct strvec *out);\n-\n-static struct odb_source *odb_source_new(struct object_database *odb,\n-\t\t\t\t\t const char *path,\n-\t\t\t\t\t bool local)\n-{\n-\tstruct odb_source *source;\n-\n-\tCALLOC_ARRAY(source, 1);\n-\tsource->odb = odb;\n-\tsource->local = local;\n-\tsource->path = xstrdup(path);\n-\tsource->loose = odb_source_loose_new(source);\n-\n-\treturn source;\n-}\n-\n-static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n-\t\t\t\t\t\t\tconst char *source,\n-\t\t\t\t\t\t\tint depth)\n-{\n-\tstruct odb_source *alternate = NULL;\n-\tstruct strvec sources = STRVEC_INIT;\n-\tkhiter_t pos;\n-\tint ret;\n-\n-\tif (!odb_is_source_usable(odb, source))\n-\t\tgoto error;\n-\n-\talternate = odb_source_new(odb, source, false);\n-\n-\t/* add the alternate entry */\n-\t*odb->sources_tail = alternate;\n-\todb->sources_tail = &(alternate->next);\n-\n-\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n-\tif (!ret)\n-\t\tBUG(\"source must not yet exist\");\n-\tkh_value(odb->source_by_path, pos) = alternate;\n-\n-\t/* recursively add alternates */\n-\tread_info_alternates(alternate->path, &sources);\n-\tif (sources.nr && depth + 1 > 5) {\n-\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n-\t\t      source);\n-\t} else {\n-\t\tfor (size_t i = 0; i < sources.nr; i++)\n-\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n-\t}\n-\n- error:\n-\tstrvec_clear(&sources);\n-\treturn alternate;\n-}\n-\n static void parse_alternates(const char *string,\n \t\t\t     int sep,\n \t\t\t     const char *relative_base,\n@@ -288,6 +217,60 @@ static void read_info_alternates(const char *relative_base,\n \tfree(path);\n }\n \n+\n+static struct odb_source *odb_source_new(struct object_database *odb,\n+\t\t\t\t\t const char *path,\n+\t\t\t\t\t bool local)\n+{\n+\tstruct odb_source *source;\n+\n+\tCALLOC_ARRAY(source, 1);\n+\tsource->odb = odb;\n+\tsource->local = local;\n+\tsource->path = xstrdup(path);\n+\tsource->loose = odb_source_loose_new(source);\n+\n+\treturn source;\n+}\n+\n+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,\n+\t\t\t\t\t\t\tconst char *source,\n+\t\t\t\t\t\t\tint depth)\n+{\n+\tstruct odb_source *alternate = NULL;\n+\tstruct strvec sources = STRVEC_INIT;\n+\tkhiter_t pos;\n+\tint ret;\n+\n+\tif (!odb_is_source_usable(odb, source))\n+\t\tgoto error;\n+\n+\talternate = odb_source_new(odb, source, false);\n+\n+\t/* add the alternate entry */\n+\t*odb->sources_tail = alternate;\n+\todb->sources_tail = &(alternate->next);\n+\n+\tpos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);\n+\tif (!ret)\n+\t\tBUG(\"source must not yet exist\");\n+\tkh_value(odb->source_by_path, pos) = alternate;\n+\n+\t/* recursively add alternates */\n+\tread_info_alternates(alternate->path, &sources);\n+\tif (sources.nr && depth + 1 > 5) {\n+\t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n+\t\t      source);\n+\t} else {\n+\t\tfor (size_t i = 0; i < sources.nr; i++)\n+\t\t\todb_add_alternate_recursively(odb, sources.v[i], depth + 1);\n+\t}\n+\n+ error:\n+\tstrvec_clear(&sources);\n+\treturn alternate;\n+}\n+\n void odb_add_to_alternates_file(struct object_database *odb,\n \t\t\t\tconst char *dir)\n {\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532036","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-7-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 7/8] odb: read alternates via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:16Z","receivedAt":"2025-12-11T09:30:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Adapt how we read alternates so that the interface is structured around\nthe object database source we're reading from. This will eventually\nallow us to abstract away this behaviour with pluggable object databases\nso that every format can have its own mechanism for listing alternates.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex dcf4a62cd2..c5ba26b85f 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -199,19 +199,19 @@ static void parse_alternates(const char *string,\n \tstrbuf_release(&buf);\n }\n \n-static void read_info_alternates(const char *relative_base,\n-\t\t\t\t struct strvec *out)\n+static void odb_source_read_alternates(struct odb_source *source,\n+\t\t\t\t       struct strvec *out)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tchar *path;\n \n-\tpath = xstrfmt(\"%s/info/alternates\", relative_base);\n+\tpath = xstrfmt(\"%s/info/alternates\", source->path);\n \tif (strbuf_read_file(&buf, path, 1024) < 0) {\n \t\twarn_on_fopen_errors(path);\n \t\tfree(path);\n \t\treturn;\n \t}\n-\tparse_alternates(buf.buf, '\\n', relative_base, out);\n+\tparse_alternates(buf.buf, '\\n', source->path, out);\n \n \tstrbuf_release(&buf);\n \tfree(path);\n@@ -257,7 +257,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \tkh_value(odb->source_by_path, pos) = alternate;\n \n \t/* recursively add alternates */\n-\tread_info_alternates(alternate->path, &sources);\n+\todb_source_read_alternates(alternate, &sources);\n \tif (sources.nr && depth + 1 > 5) {\n \t\terror(_(\"%s: ignoring alternate object stores, nesting too deep\"),\n \t\t      source);\n@@ -599,7 +599,7 @@ void odb_prepare_alternates(struct object_database *odb)\n \t\treturn;\n \n \tparse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);\n-\tread_info_alternates(odb->sources->path, &sources);\n+\todb_source_read_alternates(odb->sources, &sources);\n \tfor (size_t i = 0; i < sources.nr; i++)\n \t\todb_add_alternate_recursively(odb, sources.v[i], 0);\n \n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"},{"id":"532037","messageId":"20251211-b4-pks-odb-alternates-via-source-v3-8-00e3f54d07ba@pks.im","threadId":"64595","inReplyTo":"20251211-b4-pks-odb-alternates-via-source-v3-0-00e3f54d07ba@pks.im","subject":"[PATCH v3 8/8] odb: write alternates via sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-11T09:30:17Z","receivedAt":"2025-12-11T09:30:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Refactor writing of alternates so that the actual business logic is\nstructured around the object database source we want to write the\nalternate to. Same as with the preceding commit, this will eventually\nallow us to have different logic for writing alternates depending on the\nbackend used.\n\nNote that after the refactoring we start to call\n`odb_add_alternate_recursively()` unconditionally. This is fine though\nas we know to skip adding sources that are tracked already.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n odb.c | 51 +++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 35 insertions(+), 16 deletions(-)\n\ndiff --git a/odb.c b/odb.c\nindex c5ba26b85f..cc7f832465 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -271,25 +271,28 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *\n \treturn alternate;\n }\n \n-void odb_add_to_alternates_file(struct object_database *odb,\n-\t\t\t\tconst char *dir)\n+static int odb_source_write_alternate(struct odb_source *source,\n+\t\t\t\t      const char *alternate)\n {\n \tstruct lock_file lock = LOCK_INIT;\n-\tchar *alts = repo_git_path(odb->repo, \"objects/info/alternates\");\n+\tchar *path = xstrfmt(\"%s/%s\", source->path, \"info/alternates\");\n \tFILE *in, *out;\n \tint found = 0;\n+\tint ret;\n \n-\thold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);\n+\thold_lock_file_for_update(&lock, path, LOCK_DIE_ON_ERROR);\n \tout = fdopen_lock_file(&lock, \"w\");\n-\tif (!out)\n-\t\tdie_errno(_(\"unable to fdopen alternates lockfile\"));\n+\tif (!out) {\n+\t\tret = error_errno(_(\"unable to fdopen alternates lockfile\"));\n+\t\tgoto out;\n+\t}\n \n-\tin = fopen(alts, \"r\");\n+\tin = fopen(path, \"r\");\n \tif (in) {\n \t\tstruct strbuf line = STRBUF_INIT;\n \n \t\twhile (strbuf_getline(&line, in) != EOF) {\n-\t\t\tif (!strcmp(dir, line.buf)) {\n+\t\t\tif (!strcmp(alternate, line.buf)) {\n \t\t\t\tfound = 1;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -298,20 +301,36 @@ void odb_add_to_alternates_file(struct object_database *odb,\n \n \t\tstrbuf_release(&line);\n \t\tfclose(in);\n+\t} else if (errno != ENOENT) {\n+\t\tret = error_errno(_(\"unable to read alternates file\"));\n+\t\tgoto out;\n \t}\n-\telse if (errno != ENOENT)\n-\t\tdie_errno(_(\"unable to read alternates file\"));\n \n \tif (found) {\n \t\trollback_lock_file(&lock);\n \t} else {\n-\t\tfprintf_or_die(out, \"%s\\n\", dir);\n-\t\tif (commit_lock_file(&lock))\n-\t\t\tdie_errno(_(\"unable to move new alternates file into place\"));\n-\t\tif (odb->loaded_alternates)\n-\t\t\todb_add_alternate_recursively(odb, dir, 0);\n+\t\tfprintf_or_die(out, \"%s\\n\", alternate);\n+\t\tif (commit_lock_file(&lock)) {\n+\t\t\tret = error_errno(_(\"unable to move new alternates file into place\"));\n+\t\t\tgoto out;\n+\t\t}\n \t}\n-\tfree(alts);\n+\n+\tret = 0;\n+\n+out:\n+\tfree(path);\n+\treturn ret;\n+}\n+\n+void odb_add_to_alternates_file(struct object_database *odb,\n+\t\t\t\tconst char *dir)\n+{\n+\tint ret = odb_source_write_alternate(odb->sources, dir);\n+\tif (ret < 0)\n+\t\tdie(NULL);\n+\tif (odb->loaded_alternates)\n+\t\todb_add_alternate_recursively(odb, dir, 0);\n }\n \n struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,\n\n-- \n2.52.0.270.g3f4935d65f.dirty\n\n"}]}