[PATCH 0/8] Refactor handling of alternates to work via sources

[PATCH 0/8] Refactor handling of alternates to work via sources

[Patrick Steinhardt]

Hi,

this patch series refactors how we handle alternate object directories
so that the interface is structured around the object database source.

Next to being simpler to reason about, it also allows us to eventually
abstract handling of alternates to use different mechanisms based on the
specific backend used. In a world of pluggable object databases not
every backend may use a physical directory, so it may not be possible to
read alternates via "objects/info/alternates". Consequently, formats may
need a different mechanism entirely to make this list available.

Thanks!

Patrick

---
Patrick Steinhardt (8):
      odb: refactor parsing of alternates to be self-contained
      odb: resolve relative alternative paths when parsing
      odb: move computation of normalized objdir into `alt_odb_usable()`
      odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`
      odb: remove mutual recursion when parsing alternates
      odb: drop forward declaration of `read_info_alternates()`
      odb: read alternates via sources
      odb: write alternates via sources

 odb.c | 307 ++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 158 insertions(+), 149 deletions(-)


---
base-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06
change-id: 20251206-b4-pks-odb-alternates-via-source-802d87cbbda5

[PATCH 1/8] odb: refactor parsing of alternates to be self-contained

[Patrick Steinhardt]

Parsing of the alternates file and environment variable is currently
split up across multiple different functions and is entangled with
`link_alt_odb_entries()`, which is responsible for linking the parsed
object database sources. This results in two downsides:

  - We have mutual recursion between parsing alternates and linking them
    into the object database. This is because we also parse alternates
    that the newly added sources may have.

  - We mix up the actual logic to parse the data and to link them into
    place.

Refactor the logic so that parsing of the alternates file is entirely
self-contained. Note that this doesn't yet fix the above two issues, but
it is a necessary step to get there.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/odb.c b/odb.c
index dc8f292f3d..9785f62cb6 100644
--- a/odb.c
+++ b/odb.c
@@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
 	return alternate;
 }
 
-static const char *parse_alt_odb_entry(const char *string,
-				       int sep,
-				       struct strbuf *out)
+static void parse_alternates(const char *string,
+			     int sep,
+			     struct strvec *out)
 {
-	const char *end;
+	struct strbuf buf = STRBUF_INIT;
 
-	strbuf_reset(out);
+	while (*string) {
+		const char *end;
+
+		strbuf_reset(&buf);
+
+		if (*string == '#') {
+			/* comment; consume up to next separator */
+			end = strchrnul(string, sep);
+		} else if (*string == '"' && !unquote_c_style(&buf, string, &end)) {
+			/*
+			 * quoted path; unquote_c_style has copied the
+			 * data for us and set "end". Broken quoting (e.g.,
+			 * an entry that doesn't end with a quote) falls
+			 * back to the unquoted case below.
+			 */
+		} else {
+			/* normal, unquoted path */
+			end = strchrnul(string, sep);
+			strbuf_add(&buf, string, end - string);
+		}
 
-	if (*string == '#') {
-		/* comment; consume up to next separator */
-		end = strchrnul(string, sep);
-	} else if (*string == '"' && !unquote_c_style(out, string, &end)) {
-		/*
-		 * quoted path; unquote_c_style has copied the
-		 * data for us and set "end". Broken quoting (e.g.,
-		 * an entry that doesn't end with a quote) falls
-		 * back to the unquoted case below.
-		 */
-	} else {
-		/* normal, unquoted path */
-		end = strchrnul(string, sep);
-		strbuf_add(out, string, end - string);
+		if (*end)
+			end++;
+		string = end;
+
+		if (!buf.len)
+			continue;
+
+		strvec_push(out, buf.buf);
 	}
 
-	if (*end)
-		end++;
-	return end;
+	strbuf_release(&buf);
 }
 
 static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 				 int sep, const char *relative_base, int depth)
 {
-	struct strbuf dir = STRBUF_INIT;
+	struct strvec alternates = STRVEC_INIT;
 
 	if (!alt || !*alt)
 		return;
@@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 		return;
 	}
 
-	while (*alt) {
-		alt = parse_alt_odb_entry(alt, sep, &dir);
-		if (!dir.len)
-			continue;
-		link_alt_odb_entry(odb, dir.buf, relative_base, depth);
-	}
-	strbuf_release(&dir);
+	parse_alternates(alt, sep, &alternates);
+
+	for (size_t i = 0; i < alternates.nr; i++)
+		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
+
+	strvec_clear(&alternates);
 }
 
 static void read_info_alternates(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH 2/8] odb: resolve relative alternative paths when parsing

[Patrick Steinhardt]

Parsing alternates and resolving potential relative paths is currently
handled in two separate steps. This has the effect that the logic to
retrieve alternates is not entirely self-contained. We want it to be
just that though so that we can eventually move the logic to list
alternates into the `struct odb_source`.

Move the logic to resolve relative alternative paths into
`parse_alternates()`. Besides bringing us a step closer towards the
above goal, it also neatly separates concerns of generating the list of
alternatives and linking them into the object database.

Note that we ignore any errors when the relative path cannot be
resolved. This isn't really a change in behaviour though: if the path
cannot be resolved to a directory then `alt_odb_usable()` still knows to
bail out.

While at it, rename the function to `odb_add_source()` to more clearly
indicate what its intent is and to align it with modern terminology.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/odb.c b/odb.c
index 9785f62cb6..3ffeece567 100644
--- a/odb.c
+++ b/odb.c
@@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,
 	return source;
 }
 
-static struct odb_source *link_alt_odb_entry(struct object_database *odb,
-					     const char *dir,
-					     const char *relative_base,
-					     int depth)
+static struct odb_source *odb_add_source(struct object_database *odb,
+					 const char *source,
+					 int depth)
 {
 	struct odb_source *alternate = NULL;
-	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf tmp = STRBUF_INIT;
 	khiter_t pos;
 	int ret;
 
-	if (!is_absolute_path(dir) && relative_base) {
-		strbuf_realpath(&pathbuf, relative_base, 1);
-		strbuf_addch(&pathbuf, '/');
-	}
-	strbuf_addstr(&pathbuf, dir);
-
-	if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {
-		error(_("unable to normalize alternate object path: %s"),
-		      pathbuf.buf);
-		goto error;
-	}
-	strbuf_swap(&pathbuf, &tmp);
-
-	/*
-	 * The trailing slash after the directory name is given by
-	 * this function at the end. Remove duplicates.
-	 */
-	while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
-		strbuf_setlen(&pathbuf, pathbuf.len - 1);
-
-	strbuf_reset(&tmp);
 	strbuf_realpath(&tmp, odb->sources->path, 1);
 
-	if (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))
+	if (!alt_odb_usable(odb, source, tmp.buf))
 		goto error;
 
-	alternate = odb_source_new(odb, pathbuf.buf, false);
+	alternate = odb_source_new(odb, source, false);
 
 	/* add the alternate entry */
 	*odb->sources_tail = alternate;
@@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
 
  error:
 	strbuf_release(&tmp);
-	strbuf_release(&pathbuf);
 	return alternate;
 }
 
 static void parse_alternates(const char *string,
 			     int sep,
+			     const char *relative_base,
 			     struct strvec *out)
 {
+	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 
 	while (*string) {
 		const char *end;
 
 		strbuf_reset(&buf);
+		strbuf_reset(&pathbuf);
 
 		if (*string == '#') {
 			/* comment; consume up to next separator */
@@ -250,9 +229,30 @@ static void parse_alternates(const char *string,
 		if (!buf.len)
 			continue;
 
+		if (!is_absolute_path(buf.buf) && relative_base) {
+			strbuf_realpath(&pathbuf, relative_base, 1);
+			strbuf_addch(&pathbuf, '/');
+		}
+		strbuf_addbuf(&pathbuf, &buf);
+
+		strbuf_reset(&buf);
+		if (!strbuf_realpath(&buf, pathbuf.buf, 0)) {
+			error(_("unable to normalize alternate object path: %s"),
+			      pathbuf.buf);
+			continue;
+		}
+
+		/*
+		 * The trailing slash after the directory name is given by
+		 * this function at the end. Remove duplicates.
+		 */
+		while (buf.len && buf.buf[buf.len - 1] == '/')
+			strbuf_setlen(&buf, buf.len - 1);
+
 		strvec_push(out, buf.buf);
 	}
 
+	strbuf_release(&pathbuf);
 	strbuf_release(&buf);
 }
 
@@ -270,10 +270,10 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 		return;
 	}
 
-	parse_alternates(alt, sep, &alternates);
+	parse_alternates(alt, sep, relative_base, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
-		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
+		odb_add_source(odb, alternates.v[i], depth);
 
 	strvec_clear(&alternates);
 }
@@ -348,7 +348,7 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
 	 * overwritten when they are.
 	 */
 	odb_prepare_alternates(odb);
-	return link_alt_odb_entry(odb, dir, NULL, 0);
+	return odb_add_source(odb, dir, 0);
 }
 
 struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`

[Patrick Steinhardt]

The function `alt_odb_usable()` receives as input the object database,
the path it's supposed to determine usability for as well as the
normalized path of the main object directory of the repository. The last
part is derived by the function's caller from the object database. As we
already pass the object database to `alt_odb_usable()` it is redundant
information.

Drop the extra parameter and compute the normalized object directory in
the function itself.

While at it, rename the function to `odb_is_source_usable()` to align it
with modern terminology.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/odb.c b/odb.c
index 3ffeece567..2513457a31 100644
--- a/odb.c
+++ b/odb.c
@@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
-static int alt_odb_usable(struct object_database *o, const char *path,
-			  const char *normalized_objdir)
+static bool odb_is_source_usable(struct object_database *o, const char *path)
 {
 	int r;
+	struct strbuf normalized_objdir = STRBUF_INIT;
+	bool usable = false;
+
+	strbuf_realpath(&normalized_objdir, o->sources->path, 1);
 
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path)) {
 		error(_("object directory %s does not exist; "
 			"check .git/objects/info/alternates"),
 		      path);
-		return 0;
+		goto out;
 	}
 
 	/*
@@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,
 		kh_value(o->source_by_path, p) = o->sources;
 	}
 
-	if (fspatheq(path, normalized_objdir))
-		return 0;
+	if (fspatheq(path, normalized_objdir.buf))
+		goto out;
 
 	if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))
-		return 0;
+		goto out;
+
+	usable = true;
 
-	return 1;
+out:
+	strbuf_release(&normalized_objdir);
+	return usable;
 }
 
 /*
@@ -164,13 +171,10 @@ static struct odb_source *odb_add_source(struct object_database *odb,
 					 int depth)
 {
 	struct odb_source *alternate = NULL;
-	struct strbuf tmp = STRBUF_INIT;
 	khiter_t pos;
 	int ret;
 
-	strbuf_realpath(&tmp, odb->sources->path, 1);
-
-	if (!alt_odb_usable(odb, source, tmp.buf))
+	if (!odb_is_source_usable(odb, source))
 		goto error;
 
 	alternate = odb_source_new(odb, source, false);
@@ -188,7 +192,6 @@ static struct odb_source *odb_add_source(struct object_database *odb,
 	read_info_alternates(odb, alternate->path, depth + 1);
 
  error:
-	strbuf_release(&tmp);
 	return alternate;
 }
 

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`

[Patrick Steinhardt]

When calling `odb_add_to_alternates_file()` we know to add the newly
added source to the object database in case we have already loaded
alternates. This is done so that we can make its objects accessible
immediately without having to fully reload all alternates.

The way we do this though is to call `link_alt_odb_entries()`, which
adds _multiple_ sources to the object database source in case we have
newline-separated entries. This behaviour is not documented in the
function documentation of `odb_add_to_alternates_file()`, and all
callers only ever pass a single directory to it. It's thus entirely
surprising and a conceptual mismatch.

Fix this issue by directly calling `odb_add_source()` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/odb.c b/odb.c
index 2513457a31..94cff19221 100644
--- a/odb.c
+++ b/odb.c
@@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,
 		if (commit_lock_file(&lock))
 			die_errno(_("unable to move new alternates file into place"));
 		if (odb->loaded_alternates)
-			link_alt_odb_entries(odb, dir, '\n', NULL, 0);
+			odb_add_source(odb, dir, 0);
 	}
 	free(alts);
 }

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH 5/8] odb: remove mutual recursion when parsing alternates

[Patrick Steinhardt]

When adding an alternative object database source we not only have to
consider the added source itself, but we also have to add _its_ sources
to our database. We implement this via mutual recursion:

  1. We first call `link_alt_odb_entries()`.

  2. `link_alt_odb_entries()` calls `parse_alternates()`.

  3. We then add each parsed alternate via `odb_add_source()`.

  4. `odb_add_source()` calls `link_alt_odb_entries()` again.

This flow is somewhat hard to follow, but more importantly it means that
parsing of alternates is somewhat tied to the recursive behaviour.

Refactor the function to remove the mutual recursion between adding
sources and parsing alternates. The parsing step thus becomes completely
oblivious to the fact that there is recursive behaviour going on at all.
Instead, the recursion is handled exclusively by `odb_add_source()`,
which now recurses with itself.

This refactoring allows us to move parsing of alternates into object
database sources in a subsequent step.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 60 +++++++++++++++++++++++++++---------------------------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/odb.c b/odb.c
index 94cff19221..27f3c8e263 100644
--- a/odb.c
+++ b/odb.c
@@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)
  * of the object ID, an extra slash for the first level indirection, and
  * the terminating NUL.
  */
-static void read_info_alternates(struct object_database *odb,
-				 const char *relative_base,
-				 int depth);
+static void read_info_alternates(const char *relative_base,
+				 struct strvec *out);
 
 static struct odb_source *odb_source_new(struct object_database *odb,
 					 const char *path,
@@ -171,6 +170,7 @@ static struct odb_source *odb_add_source(struct object_database *odb,
 					 int depth)
 {
 	struct odb_source *alternate = NULL;
+	struct strvec sources = STRVEC_INIT;
 	khiter_t pos;
 	int ret;
 
@@ -189,9 +189,17 @@ static struct odb_source *odb_add_source(struct object_database *odb,
 	kh_value(odb->source_by_path, pos) = alternate;
 
 	/* recursively add alternates */
-	read_info_alternates(odb, alternate->path, depth + 1);
+	read_info_alternates(alternate->path, &sources);
+	if (sources.nr && depth + 1 > 5) {
+		error(_("%s: ignoring alternate object stores, nesting too deep"),
+		      source);
+	} else {
+		for (size_t i = 0; i < sources.nr; i++)
+			odb_add_source(odb, sources.v[i], depth + 1);
+	}
 
  error:
+	strvec_clear(&sources);
 	return alternate;
 }
 
@@ -203,6 +211,9 @@ static void parse_alternates(const char *string,
 	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 
+	if (!string || !*string)
+		return;
+
 	while (*string) {
 		const char *end;
 
@@ -259,34 +270,11 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static void link_alt_odb_entries(struct object_database *odb, const char *alt,
-				 int sep, const char *relative_base, int depth)
+static void read_info_alternates(const char *relative_base,
+				 struct strvec *out)
 {
-	struct strvec alternates = STRVEC_INIT;
-
-	if (!alt || !*alt)
-		return;
-
-	if (depth > 5) {
-		error(_("%s: ignoring alternate object stores, nesting too deep"),
-				relative_base);
-		return;
-	}
-
-	parse_alternates(alt, sep, relative_base, &alternates);
-
-	for (size_t i = 0; i < alternates.nr; i++)
-		odb_add_source(odb, alternates.v[i], depth);
-
-	strvec_clear(&alternates);
-}
-
-static void read_info_alternates(struct object_database *odb,
-				 const char *relative_base,
-				 int depth)
-{
-	char *path;
 	struct strbuf buf = STRBUF_INIT;
+	char *path;
 
 	path = xstrfmt("%s/info/alternates", relative_base);
 	if (strbuf_read_file(&buf, path, 1024) < 0) {
@@ -294,8 +282,8 @@ static void read_info_alternates(struct object_database *odb,
 		free(path);
 		return;
 	}
+	parse_alternates(buf.buf, '\n', relative_base, out);
 
-	link_alt_odb_entries(odb, buf.buf, '\n', relative_base, depth);
 	strbuf_release(&buf);
 	free(path);
 }
@@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,
 
 void odb_prepare_alternates(struct object_database *odb)
 {
+	struct strvec sources = STRVEC_INIT;
+
 	if (odb->loaded_alternates)
 		return;
 
-	link_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);
+	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
+	read_info_alternates(odb->sources->path, &sources);
+	for (size_t i = 0; i < sources.nr; i++)
+		odb_add_source(odb, sources.v[i], 0);
 
-	read_info_alternates(odb, odb->sources->path, 0);
 	odb->loaded_alternates = 1;
+
+	strvec_clear(&sources);
 }
 
 int odb_has_alternates(struct object_database *odb)

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH 6/8] odb: drop forward declaration of `read_info_alternates()`

[Patrick Steinhardt]

Now that we have removed the mutual recursion in the preceding commit
it is not necessary anymore to have a forward declaration of the
`read_info_alternates()` function. Move the function and its
dependencies further up so that we can remove it.

Note that this commit also removes the function documentation of
`read_info_alternates()`. It's unclear what it's documenting, but it for
sure isn't documenting the modern behaviour of the function anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 125 +++++++++++++++++++++++++++++-------------------------------------
 1 file changed, 54 insertions(+), 71 deletions(-)

diff --git a/odb.c b/odb.c
index 27f3c8e263..1d83a915e3 100644
--- a/odb.c
+++ b/odb.c
@@ -132,77 +132,6 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)
 	return usable;
 }
 
-/*
- * Prepare alternate object database registry.
- *
- * The variable alt_odb_list points at the list of struct
- * odb_source.  The elements on this list come from
- * non-empty elements from colon separated ALTERNATE_DB_ENVIRONMENT
- * environment variable, and $GIT_OBJECT_DIRECTORY/info/alternates,
- * whose contents is similar to that environment variable but can be
- * LF separated.  Its base points at a statically allocated buffer that
- * contains "/the/directory/corresponding/to/.git/objects/...", while
- * its name points just after the slash at the end of ".git/objects/"
- * in the example above, and has enough space to hold all hex characters
- * of the object ID, an extra slash for the first level indirection, and
- * the terminating NUL.
- */
-static void read_info_alternates(const char *relative_base,
-				 struct strvec *out);
-
-static struct odb_source *odb_source_new(struct object_database *odb,
-					 const char *path,
-					 bool local)
-{
-	struct odb_source *source;
-
-	CALLOC_ARRAY(source, 1);
-	source->odb = odb;
-	source->local = local;
-	source->path = xstrdup(path);
-	source->loose = odb_source_loose_new(source);
-
-	return source;
-}
-
-static struct odb_source *odb_add_source(struct object_database *odb,
-					 const char *source,
-					 int depth)
-{
-	struct odb_source *alternate = NULL;
-	struct strvec sources = STRVEC_INIT;
-	khiter_t pos;
-	int ret;
-
-	if (!odb_is_source_usable(odb, source))
-		goto error;
-
-	alternate = odb_source_new(odb, source, false);
-
-	/* add the alternate entry */
-	*odb->sources_tail = alternate;
-	odb->sources_tail = &(alternate->next);
-
-	pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);
-	if (!ret)
-		BUG("source must not yet exist");
-	kh_value(odb->source_by_path, pos) = alternate;
-
-	/* recursively add alternates */
-	read_info_alternates(alternate->path, &sources);
-	if (sources.nr && depth + 1 > 5) {
-		error(_("%s: ignoring alternate object stores, nesting too deep"),
-		      source);
-	} else {
-		for (size_t i = 0; i < sources.nr; i++)
-			odb_add_source(odb, sources.v[i], depth + 1);
-	}
-
- error:
-	strvec_clear(&sources);
-	return alternate;
-}
-
 static void parse_alternates(const char *string,
 			     int sep,
 			     const char *relative_base,
@@ -288,6 +217,60 @@ static void read_info_alternates(const char *relative_base,
 	free(path);
 }
 
+
+static struct odb_source *odb_source_new(struct object_database *odb,
+					 const char *path,
+					 bool local)
+{
+	struct odb_source *source;
+
+	CALLOC_ARRAY(source, 1);
+	source->odb = odb;
+	source->local = local;
+	source->path = xstrdup(path);
+	source->loose = odb_source_loose_new(source);
+
+	return source;
+}
+
+static struct odb_source *odb_add_source(struct object_database *odb,
+					 const char *source,
+					 int depth)
+{
+	struct odb_source *alternate = NULL;
+	struct strvec sources = STRVEC_INIT;
+	khiter_t pos;
+	int ret;
+
+	if (!odb_is_source_usable(odb, source))
+		goto error;
+
+	alternate = odb_source_new(odb, source, false);
+
+	/* add the alternate entry */
+	*odb->sources_tail = alternate;
+	odb->sources_tail = &(alternate->next);
+
+	pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);
+	if (!ret)
+		BUG("source must not yet exist");
+	kh_value(odb->source_by_path, pos) = alternate;
+
+	/* recursively add alternates */
+	read_info_alternates(alternate->path, &sources);
+	if (sources.nr && depth + 1 > 5) {
+		error(_("%s: ignoring alternate object stores, nesting too deep"),
+		      source);
+	} else {
+		for (size_t i = 0; i < sources.nr; i++)
+			odb_add_source(odb, sources.v[i], depth + 1);
+	}
+
+ error:
+	strvec_clear(&sources);
+	return alternate;
+}
+
 void odb_add_to_alternates_file(struct object_database *odb,
 				const char *dir)
 {

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH 7/8] odb: read alternates via sources

[Patrick Steinhardt]

Adapt how we read alternates so that the interface is structured around
the object database source we're reading from. This will eventually
allow us to abstract away this behaviour with pluggable object databases
so that every format can have its own mechanism for listing alternates.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/odb.c b/odb.c
index 1d83a915e3..bf364fe3dd 100644
--- a/odb.c
+++ b/odb.c
@@ -199,19 +199,19 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static void read_info_alternates(const char *relative_base,
-				 struct strvec *out)
+static void odb_source_read_alternates(struct odb_source *source,
+				       struct strvec *out)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *path;
 
-	path = xstrfmt("%s/info/alternates", relative_base);
+	path = xstrfmt("%s/info/alternates", source->path);
 	if (strbuf_read_file(&buf, path, 1024) < 0) {
 		warn_on_fopen_errors(path);
 		free(path);
 		return;
 	}
-	parse_alternates(buf.buf, '\n', relative_base, out);
+	parse_alternates(buf.buf, '\n', source->path, out);
 
 	strbuf_release(&buf);
 	free(path);
@@ -257,7 +257,7 @@ static struct odb_source *odb_add_source(struct object_database *odb,
 	kh_value(odb->source_by_path, pos) = alternate;
 
 	/* recursively add alternates */
-	read_info_alternates(alternate->path, &sources);
+	odb_source_read_alternates(alternate, &sources);
 	if (sources.nr && depth + 1 > 5) {
 		error(_("%s: ignoring alternate object stores, nesting too deep"),
 		      source);
@@ -599,7 +599,7 @@ void odb_prepare_alternates(struct object_database *odb)
 		return;
 
 	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
-	read_info_alternates(odb->sources->path, &sources);
+	odb_source_read_alternates(odb->sources, &sources);
 	for (size_t i = 0; i < sources.nr; i++)
 		odb_add_source(odb, sources.v[i], 0);
 

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH 8/8] odb: write alternates via sources

[Patrick Steinhardt]

Refactor writing of alternates so that the actual business logic is
structured around the object database source we want to write the
alternate to. Same as with the preceding commit, this will eventually
allow us to have different logic for writing alternates depending on the
backend used.

Note that after the refactoring we start to call `odb_add_source()`
unconditionally. This is fine though as we know to skip adding sources
that are tracked already.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 51 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/odb.c b/odb.c
index bf364fe3dd..9e7d078a46 100644
--- a/odb.c
+++ b/odb.c
@@ -271,25 +271,28 @@ static struct odb_source *odb_add_source(struct object_database *odb,
 	return alternate;
 }
 
-void odb_add_to_alternates_file(struct object_database *odb,
-				const char *dir)
+static int odb_source_write_alternate(struct odb_source *source,
+				      const char *alternate)
 {
 	struct lock_file lock = LOCK_INIT;
-	char *alts = repo_git_path(odb->repo, "objects/info/alternates");
+	char *path = xstrfmt("%s/%s", source->path, "info/alternates");
 	FILE *in, *out;
 	int found = 0;
+	int ret;
 
-	hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);
+	hold_lock_file_for_update(&lock, path, LOCK_DIE_ON_ERROR);
 	out = fdopen_lock_file(&lock, "w");
-	if (!out)
-		die_errno(_("unable to fdopen alternates lockfile"));
+	if (!out) {
+		ret = error_errno(_("unable to fdopen alternates lockfile"));
+		goto out;
+	}
 
-	in = fopen(alts, "r");
+	in = fopen(path, "r");
 	if (in) {
 		struct strbuf line = STRBUF_INIT;
 
 		while (strbuf_getline(&line, in) != EOF) {
-			if (!strcmp(dir, line.buf)) {
+			if (!strcmp(alternate, line.buf)) {
 				found = 1;
 				break;
 			}
@@ -298,20 +301,36 @@ void odb_add_to_alternates_file(struct object_database *odb,
 
 		strbuf_release(&line);
 		fclose(in);
+	} else if (errno != ENOENT) {
+		ret = error_errno(_("unable to read alternates file"));
+		goto out;
 	}
-	else if (errno != ENOENT)
-		die_errno(_("unable to read alternates file"));
 
 	if (found) {
 		rollback_lock_file(&lock);
 	} else {
-		fprintf_or_die(out, "%s\n", dir);
-		if (commit_lock_file(&lock))
-			die_errno(_("unable to move new alternates file into place"));
-		if (odb->loaded_alternates)
-			odb_add_source(odb, dir, 0);
+		fprintf_or_die(out, "%s\n", alternate);
+		if (commit_lock_file(&lock)) {
+			ret = error_errno(_("unable to move new alternates file into place"));
+			goto out;
+		}
 	}
-	free(alts);
+
+	ret = 0;
+
+out:
+	free(path);
+	return ret;
+}
+
+void odb_add_to_alternates_file(struct object_database *odb,
+				const char *dir)
+{
+	int ret = odb_source_write_alternate(odb->sources, dir);
+	if (ret < 0)
+		die(NULL);
+	if (odb->loaded_alternates)
+		odb_add_source(odb, dir, 0);
 }
 
 struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

Re: [PATCH 1/8] odb: refactor parsing of alternates to be self-contained

[Justin Tobler]

On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> Parsing of the alternates file and environment variable is currently
> split up across multiple different functions and is entangled with
> `link_alt_odb_entries()`, which is responsible for linking the parsed
> object database sources. This results in two downsides:
> 
>   - We have mutual recursion between parsing alternates and linking them
>     into the object database. This is because we also parse alternates
>     that the newly added sources may have.
> 
>   - We mix up the actual logic to parse the data and to link them into
>     place.
> 
> Refactor the logic so that parsing of the alternates file is entirely
> self-contained. Note that this doesn't yet fix the above two issues, but
> it is a necessary step to get there.

Looking at the existing code, parse_alt_odb_entry() only reads a single
entry at a time and relies on link_alt_odb_entries() to call it in a
look to get all alternate entries. I agree that handling alternates
parsing on a single file in one place is a bit nicer.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 40 insertions(+), 30 deletions(-)
> 
> diff --git a/odb.c b/odb.c
> index dc8f292f3d..9785f62cb6 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
>  	return alternate;
>  }
>  
> -static const char *parse_alt_odb_entry(const char *string,
> -				       int sep,
> -				       struct strbuf *out)
> +static void parse_alternates(const char *string,
> +			     int sep,
> +			     struct strvec *out)
>  {
> -	const char *end;
> +	struct strbuf buf = STRBUF_INIT;
>  
> -	strbuf_reset(out);
> +	while (*string) {
> +		const char *end;
> +
> +		strbuf_reset(&buf);
> +
> +		if (*string == '#') {
> +			/* comment; consume up to next separator */
> +			end = strchrnul(string, sep);
> +		} else if (*string == '"' && !unquote_c_style(&buf, string, &end)) {
> +			/*
> +			 * quoted path; unquote_c_style has copied the
> +			 * data for us and set "end". Broken quoting (e.g.,
> +			 * an entry that doesn't end with a quote) falls
> +			 * back to the unquoted case below.
> +			 */
> +		} else {
> +			/* normal, unquoted path */
> +			end = strchrnul(string, sep);
> +			strbuf_add(&buf, string, end - string);
> +		}
>  
> -	if (*string == '#') {
> -		/* comment; consume up to next separator */
> -		end = strchrnul(string, sep);
> -	} else if (*string == '"' && !unquote_c_style(out, string, &end)) {
> -		/*
> -		 * quoted path; unquote_c_style has copied the
> -		 * data for us and set "end". Broken quoting (e.g.,
> -		 * an entry that doesn't end with a quote) falls
> -		 * back to the unquoted case below.
> -		 */
> -	} else {
> -		/* normal, unquoted path */
> -		end = strchrnul(string, sep);
> -		strbuf_add(out, string, end - string);
> +		if (*end)
> +			end++;
> +		string = end;
> +
> +		if (!buf.len)
> +			continue;
> +
> +		strvec_push(out, buf.buf);

We parse entries in the exact same way as before, but now we read all
entries into a strvec up front. Nice.

>  	}
>  
> -	if (*end)
> -		end++;
> -	return end;
> +	strbuf_release(&buf);
>  }
>  
>  static void link_alt_odb_entries(struct object_database *odb, const char *alt,
>  				 int sep, const char *relative_base, int depth)
>  {
> -	struct strbuf dir = STRBUF_INIT;
> +	struct strvec alternates = STRVEC_INIT;
>  
>  	if (!alt || !*alt)
>  		return;
> @@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
>  		return;
>  	}
>  
> -	while (*alt) {
> -		alt = parse_alt_odb_entry(alt, sep, &dir);
> -		if (!dir.len)
> -			continue;
> -		link_alt_odb_entry(odb, dir.buf, relative_base, depth);
> -	}
> -	strbuf_release(&dir);
> +	parse_alternates(alt, sep, &alternates);
> +
> +	for (size_t i = 0; i < alternates.nr; i++)
> +		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);

Now with this impletation we parse alternate entries up front and then
iterate through each of them to link. Linking may still result in
recursive alternate parsing if further alternates file are defined.

Looks good so far.

-Justin

Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing

[Justin Tobler]

On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> Parsing alternates and resolving potential relative paths is currently
> handled in two separate steps. This has the effect that the logic to
> retrieve alternates is not entirely self-contained. We want it to be
> just that though so that we can eventually move the logic to list
> alternates into the `struct odb_source`.

Naive question: is the intent here to eventually move alternate ODB
sources under the primary ODB source? Or just to record the alternate
dir info in the ODB source?

> Move the logic to resolve relative alternative paths into
> `parse_alternates()`. Besides bringing us a step closer towards the
> above goal, it also neatly separates concerns of generating the list of
> alternatives and linking them into the object database.
> 
> Note that we ignore any errors when the relative path cannot be
> resolved. This isn't really a change in behaviour though: if the path
> cannot be resolved to a directory then `alt_odb_usable()` still knows to
> bail out.
> 
> While at it, rename the function to `odb_add_source()` to more clearly
> indicate what its intent is and to align it with modern terminology.

Alternates are indeed just additional ODB sources appended to the
sources list. IIUC though, doesn't this function only add alternate
sources? If so, maybe it would be better to use
`odb_add_alternate_source()`?

> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/odb.c b/odb.c
> index 9785f62cb6..3ffeece567 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,
>  	return source;
>  }
>  
> -static struct odb_source *link_alt_odb_entry(struct object_database *odb,
> -					     const char *dir,
> -					     const char *relative_base,
> -					     int depth)
> +static struct odb_source *odb_add_source(struct object_database *odb,
> +					 const char *source,
> +					 int depth)
>  {
>  	struct odb_source *alternate = NULL;
> -	struct strbuf pathbuf = STRBUF_INIT;
>  	struct strbuf tmp = STRBUF_INIT;
>  	khiter_t pos;
>  	int ret;
>  
> -	if (!is_absolute_path(dir) && relative_base) {
> -		strbuf_realpath(&pathbuf, relative_base, 1);
> -		strbuf_addch(&pathbuf, '/');
> -	}
> -	strbuf_addstr(&pathbuf, dir);
> -
> -	if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {
> -		error(_("unable to normalize alternate object path: %s"),
> -		      pathbuf.buf);
> -		goto error;
> -	}
> -	strbuf_swap(&pathbuf, &tmp);
> -
> -	/*
> -	 * The trailing slash after the directory name is given by
> -	 * this function at the end. Remove duplicates.
> -	 */
> -	while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
> -		strbuf_setlen(&pathbuf, pathbuf.len - 1);
> -
> -	strbuf_reset(&tmp);
>  	strbuf_realpath(&tmp, odb->sources->path, 1);
>  
> -	if (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))
> +	if (!alt_odb_usable(odb, source, tmp.buf))
>  		goto error;
>  
> -	alternate = odb_source_new(odb, pathbuf.buf, false);
> +	alternate = odb_source_new(odb, source, false);
>  
>  	/* add the alternate entry */
>  	*odb->sources_tail = alternate;
> @@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
>  
>   error:
>  	strbuf_release(&tmp);
> -	strbuf_release(&pathbuf);
>  	return alternate;
>  }
>  
>  static void parse_alternates(const char *string,
>  			     int sep,
> +			     const char *relative_base,
>  			     struct strvec *out)
>  {
> +	struct strbuf pathbuf = STRBUF_INIT;
>  	struct strbuf buf = STRBUF_INIT;
>  
>  	while (*string) {
>  		const char *end;
>  
>  		strbuf_reset(&buf);
> +		strbuf_reset(&pathbuf);
>  
>  		if (*string == '#') {
>  			/* comment; consume up to next separator */
> @@ -250,9 +229,30 @@ static void parse_alternates(const char *string,
>  		if (!buf.len)
>  			continue;
>  
> +		if (!is_absolute_path(buf.buf) && relative_base) {
> +			strbuf_realpath(&pathbuf, relative_base, 1);
> +			strbuf_addch(&pathbuf, '/');
> +		}
> +		strbuf_addbuf(&pathbuf, &buf);
> +
> +		strbuf_reset(&buf);
> +		if (!strbuf_realpath(&buf, pathbuf.buf, 0)) {
> +			error(_("unable to normalize alternate object path: %s"),
> +			      pathbuf.buf);
> +			continue;
> +		}
> +
> +		/*
> +		 * The trailing slash after the directory name is given by
> +		 * this function at the end. Remove duplicates.
> +		 */
> +		while (buf.len && buf.buf[buf.len - 1] == '/')
> +			strbuf_setlen(&buf, buf.len - 1);
> +

Here we move the logic to resolve relative paths into
parse_alternates(). This seems reasonable to me.

-Justin

Re: [PATCH 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`

[Justin Tobler]

On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> The function `alt_odb_usable()` receives as input the object database,
> the path it's supposed to determine usability for as well as the
> normalized path of the main object directory of the repository. The last
> part is derived by the function's caller from the object database. As we
> already pass the object database to `alt_odb_usable()` it is redundant
> information.
> 
> Drop the extra parameter and compute the normalized object directory in
> the function itself.
> 
> While at it, rename the function to `odb_is_source_usable()` to align it
> with modern terminology.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/odb.c b/odb.c
> index 3ffeece567..2513457a31 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,
>  /*
>   * Return non-zero iff the path is usable as an alternate object database.

While we are here we could fix this typo: s/iff/if/

>   */
> -static int alt_odb_usable(struct object_database *o, const char *path,
> -			  const char *normalized_objdir)
> +static bool odb_is_source_usable(struct object_database *o, const char *path)
>  {
>  	int r;
> +	struct strbuf normalized_objdir = STRBUF_INIT;
> +	bool usable = false;
> +
> +	strbuf_realpath(&normalized_objdir, o->sources->path, 1);
>  
>  	/* Detect cases where alternate disappeared */
>  	if (!is_directory(path)) {
>  		error(_("object directory %s does not exist; "
>  			"check .git/objects/info/alternates"),
>  		      path);
> -		return 0;
> +		goto out;
>  	}
>  
>  	/*
> @@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,
>  		kh_value(o->source_by_path, p) = o->sources;
>  	}
>  
> -	if (fspatheq(path, normalized_objdir))
> -		return 0;
> +	if (fspatheq(path, normalized_objdir.buf))
> +		goto out;
>  
>  	if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))
> -		return 0;
> +		goto out;
> +
> +	usable = true;
>  
> -	return 1;
> +out:
> +	strbuf_release(&normalized_objdir);
> +	return usable;
>  }
>  
>  /*
> @@ -164,13 +171,10 @@ static struct odb_source *odb_add_source(struct object_database *odb,
>  					 int depth)
>  {
>  	struct odb_source *alternate = NULL;
> -	struct strbuf tmp = STRBUF_INIT;
>  	khiter_t pos;
>  	int ret;
>  
> -	strbuf_realpath(&tmp, odb->sources->path, 1);
> -
> -	if (!alt_odb_usable(odb, source, tmp.buf))
> +	if (!odb_is_source_usable(odb, source))

The normalized ODB path is only being used in alt_odb_usable() so
relocating it inside that function make sense. Looks good.

-Justin

Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing

[Patrick Steinhardt]

On Mon, Dec 08, 2025 at 08:09:30PM -0600, Justin Tobler wrote:
> On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> > Parsing alternates and resolving potential relative paths is currently
> > handled in two separate steps. This has the effect that the logic to
> > retrieve alternates is not entirely self-contained. We want it to be
> > just that though so that we can eventually move the logic to list
> > alternates into the `struct odb_source`.
> 
> Naive question: is the intent here to eventually move alternate ODB
> sources under the primary ODB source? Or just to record the alternate
> dir info in the ODB source?

Not only the primary ODB source, but into ODB sources in general as
alternates are recursive by nature.

The problem I am trying to solve is that ODB sources may not even have a
filesystem-local directory, but the way we use alternates recursively
very much assumes they do. I don't want to treat "files" sources
specially though and only recursively add their alternates. Instead, I
want to move the logic of enumerating alternates into the source so that
every source can have a different way of enumerating them that may or
may not use the filesystem.

> > Move the logic to resolve relative alternative paths into
> > `parse_alternates()`. Besides bringing us a step closer towards the
> > above goal, it also neatly separates concerns of generating the list of
> > alternatives and linking them into the object database.
> > 
> > Note that we ignore any errors when the relative path cannot be
> > resolved. This isn't really a change in behaviour though: if the path
> > cannot be resolved to a directory then `alt_odb_usable()` still knows to
> > bail out.
> > 
> > While at it, rename the function to `odb_add_source()` to more clearly
> > indicate what its intent is and to align it with modern terminology.
> 
> Alternates are indeed just additional ODB sources appended to the
> sources list. IIUC though, doesn't this function only add alternate
> sources? If so, maybe it would be better to use
> `odb_add_alternate_source()`?

Hm, yeah, I think you're right. We still have the recursive nature at
the end of this series, so let's call it accordingly.

Patrick

Re: [PATCH 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`

[Patrick Steinhardt]

On Mon, Dec 08, 2025 at 08:34:25PM -0600, Justin Tobler wrote:
> On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> > diff --git a/odb.c b/odb.c
> > index 3ffeece567..2513457a31 100644
> > --- a/odb.c
> > +++ b/odb.c
> > @@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,
> >  /*
> >   * Return non-zero iff the path is usable as an alternate object database.
> 
> While we are here we could fix this typo: s/iff/if/

This is not a typo: "iff" generally means "if and only if".

Patrick

Re: [PATCH 5/8] odb: remove mutual recursion when parsing alternates

[Justin Tobler]

On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> When adding an alternative object database source we not only have to
> consider the added source itself, but we also have to add _its_ sources
> to our database. We implement this via mutual recursion:
> 
>   1. We first call `link_alt_odb_entries()`.
> 
>   2. `link_alt_odb_entries()` calls `parse_alternates()`.
> 
>   3. We then add each parsed alternate via `odb_add_source()`.
> 
>   4. `odb_add_source()` calls `link_alt_odb_entries()` again.
> 
> This flow is somewhat hard to follow, but more importantly it means that
> parsing of alternates is somewhat tied to the recursive behaviour.
> 
> Refactor the function to remove the mutual recursion between adding
> sources and parsing alternates. The parsing step thus becomes completely
> oblivious to the fact that there is recursive behaviour going on at all.
> Instead, the recursion is handled exclusively by `odb_add_source()`,
> which now recurses with itself.
> 
> This refactoring allows us to move parsing of alternates into object
> database sources in a subsequent step.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c | 60 +++++++++++++++++++++++++++---------------------------------
>  1 file changed, 27 insertions(+), 33 deletions(-)
> 
> diff --git a/odb.c b/odb.c
> index 94cff19221..27f3c8e263 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)
>   * of the object ID, an extra slash for the first level indirection, and
>   * the terminating NUL.
>   */
> -static void read_info_alternates(struct object_database *odb,
> -				 const char *relative_base,
> -				 int depth);
> +static void read_info_alternates(const char *relative_base,
> +				 struct strvec *out);
>  
>  static struct odb_source *odb_source_new(struct object_database *odb,
>  					 const char *path,
> @@ -171,6 +170,7 @@ static struct odb_source *odb_add_source(struct object_database *odb,
>  					 int depth)
>  {
>  	struct odb_source *alternate = NULL;
> +	struct strvec sources = STRVEC_INIT;
>  	khiter_t pos;
>  	int ret;
>  
> @@ -189,9 +189,17 @@ static struct odb_source *odb_add_source(struct object_database *odb,
>  	kh_value(odb->source_by_path, pos) = alternate;
>  
>  	/* recursively add alternates */
> -	read_info_alternates(odb, alternate->path, depth + 1);
> +	read_info_alternates(alternate->path, &sources);
> +	if (sources.nr && depth + 1 > 5) {
> +		error(_("%s: ignoring alternate object stores, nesting too deep"),
> +		      source);
> +	} else {
> +		for (size_t i = 0; i < sources.nr; i++)
> +			odb_add_source(odb, sources.v[i], depth + 1);
> +	}

Ok, prior to this, read_info_alternates() would not only parse the
alternates file for the ODB source at hand, but also recursively parse
and add alternates of alternates. Now, read_info_alternates() is only
responsible for parsing a single alternates file at a time.

Recursing into child alternates is now handled by odb_add_source(). IMO
this is much easier to reason about and ultimately matches the previous
behavior.

>  
>   error:
> +	strvec_clear(&sources);
>  	return alternate;
>  }
>  
[snip]
> @@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,
>  
>  void odb_prepare_alternates(struct object_database *odb)
>  {
> +	struct strvec sources = STRVEC_INIT;
> +
>  	if (odb->loaded_alternates)
>  		return;
>  
> -	link_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);
> +	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
> +	read_info_alternates(odb->sources->path, &sources);
> +	for (size_t i = 0; i < sources.nr; i++)
> +		odb_add_source(odb, sources.v[i], 0);

When preparing alternates, sources from the environment and alternates
file are parsed first and then added. Adding sources is now handled
explicitly and is responsible for add child alternates. Looks good.

-Justin

Re: [PATCH 7/8] odb: read alternates via sources

[Justin Tobler]

On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> Adapt how we read alternates so that the interface is structured around
> the object database source we're reading from. This will eventually
> allow us to abstract away this behaviour with pluggable object databases
> so that every format can have its own mechanism for listing alternates.

Ok so IIUC, the idea here is that eventually we want each source to be
able to define it's own way to parse its alternates. This is needed
because with pluggable ODBs a given ODB source maynot even use the
filesystem and thus any alternates it may define would need to be parsed
in a different manner. I suppose some future ODB source types may not
even want to support child alternates.

Question: the interface of odb_source_read_alternates() still expects
parsed alternates to be written to the output strvec. The sources don't
get added to the ODB source list until odb_add_source() is invoked on
the source. Does this mean odb_add_source() will have to be able to
handle various different types of ODB sources? If so, will these be
differentiated by some sort of URI?

The patch itself here looks good.

-Justin

Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing

[Justin Tobler]

On 25/12/09 09:04AM, Patrick Steinhardt wrote:
> On Mon, Dec 08, 2025 at 08:09:30PM -0600, Justin Tobler wrote:
> > On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> > > Parsing alternates and resolving potential relative paths is currently
> > > handled in two separate steps. This has the effect that the logic to
> > > retrieve alternates is not entirely self-contained. We want it to be
> > > just that though so that we can eventually move the logic to list
> > > alternates into the `struct odb_source`.
> > 
> > Naive question: is the intent here to eventually move alternate ODB
> > sources under the primary ODB source? Or just to record the alternate
> > dir info in the ODB source?
> 
> Not only the primary ODB source, but into ODB sources in general as
> alternates are recursive by nature.
> 
> The problem I am trying to solve is that ODB sources may not even have a
> filesystem-local directory, but the way we use alternates recursively
> very much assumes they do. I don't want to treat "files" sources
> specially though and only recursively add their alternates. Instead, I
> want to move the logic of enumerating alternates into the source so that
> every source can have a different way of enumerating them that may or
> may not use the filesystem.

Ah, that makes more sense now. Thanks for the explaination. :)

> > > Move the logic to resolve relative alternative paths into
> > > `parse_alternates()`. Besides bringing us a step closer towards the
> > > above goal, it also neatly separates concerns of generating the list of
> > > alternatives and linking them into the object database.
> > > 
> > > Note that we ignore any errors when the relative path cannot be
> > > resolved. This isn't really a change in behaviour though: if the path
> > > cannot be resolved to a directory then `alt_odb_usable()` still knows to
> > > bail out.
> > > 
> > > While at it, rename the function to `odb_add_source()` to more clearly
> > > indicate what its intent is and to align it with modern terminology.
> > 
> > Alternates are indeed just additional ODB sources appended to the
> > sources list. IIUC though, doesn't this function only add alternate
> > sources? If so, maybe it would be better to use
> > `odb_add_alternate_source()`?
> 
> Hm, yeah, I think you're right. We still have the recursive nature at
> the end of this series, so let's call it accordingly.

On a semi-related note, part of me thinks it would be nice if alternate
sources were a bit more first class in `struct object_database`. IOW,
explicitly defining the primary and list of alternate sources
separately. From the perspective of reading objects, having a single
list of sources is nice, but when writing objects only the first source
is used. This isn't too big of a deal, but certain operations like ODB
trasactions will reorder the source list to change where objects get
written to which feels a bit fragile to me. I guess another way to
resolve this concern could be to change ODB transactions to use a
separate mechanism though.

-Justin

Re: [PATCH 2/8] odb: resolve relative alternative paths when parsing

[Patrick Steinhardt]

On Tue, Dec 09, 2025 at 12:06:09PM -0600, Justin Tobler wrote:
> On a semi-related note, part of me thinks it would be nice if alternate
> sources were a bit more first class in `struct object_database`. IOW,
> explicitly defining the primary and list of alternate sources
> separately. From the perspective of reading objects, having a single
> list of sources is nice, but when writing objects only the first source
> is used. This isn't too big of a deal, but certain operations like ODB
> trasactions will reorder the source list to change where objects get
> written to which feels a bit fragile to me. I guess another way to
> resolve this concern could be to change ODB transactions to use a
> separate mechanism though.

Agreed, especially the writing side is a bit weird, and reordering
sources when we create transactions is one of the weirdest parts. I
think this is out of scope for this patch series, but I certainly think
that we should address this by polishing the ODB transactions a bit
going forward.

Thanks!

Patrick

Re: [PATCH 7/8] odb: read alternates via sources

[Patrick Steinhardt]

On Tue, Dec 09, 2025 at 11:49:55AM -0600, Justin Tobler wrote:
> Question: the interface of odb_source_read_alternates() still expects
> parsed alternates to be written to the output strvec. The sources don't
> get added to the ODB source list until odb_add_source() is invoked on
> the source. Does this mean odb_add_source() will have to be able to
> handle various different types of ODB sources? If so, will these be
> differentiated by some sort of URI?

Yes, exactly. The plan is to use syntax like "files://path" or
"postgres://127.0.0.1:5432?database=myrepo". See also the discussion in
[1].

Patrick

[1]: <aS2V4TKeS4V_oxAb@pks.im>

[PATCH v2 0/8] Refactor handling of alternates to work via sources

[Patrick Steinhardt]

Hi,

this patch series refactors how we handle alternate object directories
so that the interface is structured around the object database source.

Next to being simpler to reason about, it also allows us to eventually
abstract handling of alternates to use different mechanisms based on the
specific backend used. In a world of pluggable object databases not
every backend may use a physical directory, so it may not be possible to
read alternates via "objects/info/alternates". Consequently, formats may
need a different mechanism entirely to make this list available.

Changes in v2:
  - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to
    highlight that this function is recursive.
  - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (8):
      odb: refactor parsing of alternates to be self-contained
      odb: resolve relative alternative paths when parsing
      odb: move computation of normalized objdir into `alt_odb_usable()`
      odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`
      odb: remove mutual recursion when parsing alternates
      odb: drop forward declaration of `read_info_alternates()`
      odb: read alternates via sources
      odb: write alternates via sources

 odb.c | 307 ++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 158 insertions(+), 149 deletions(-)

Range-diff versus v1:

1:  18b0d15865 = 1:  392036039f odb: refactor parsing of alternates to be self-contained
2:  aaf9d4e162 ! 2:  0107d40816 odb: resolve relative alternative paths when parsing
    @@ Commit message
         cannot be resolved to a directory then `alt_odb_usable()` still knows to
         bail out.
     
    -    While at it, rename the function to `odb_add_source()` to more clearly
    -    indicate what its intent is and to align it with modern terminology.
    +    While at it, rename the function to `odb_add_alternate_recursively()` to
    +    more clearly indicate what its intent is and to align it with modern
    +    terminology.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ odb.c: static struct odb_source *odb_source_new(struct object_database *odb,
     -					     const char *dir,
     -					     const char *relative_base,
     -					     int depth)
    -+static struct odb_source *odb_add_source(struct object_database *odb,
    -+					 const char *source,
    -+					 int depth)
    ++static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
    ++							const char *source,
    ++							int depth)
      {
      	struct odb_source *alternate = NULL;
     -	struct strbuf pathbuf = STRBUF_INIT;
    @@ odb.c: static void link_alt_odb_entries(struct object_database *odb, const char
      
      	for (size_t i = 0; i < alternates.nr; i++)
     -		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
    -+		odb_add_source(odb, alternates.v[i], depth);
    ++		odb_add_alternate_recursively(odb, alternates.v[i], depth);
      
      	strvec_clear(&alternates);
      }
    @@ odb.c: struct odb_source *odb_add_to_alternates_memory(struct object_database *o
      	 */
      	odb_prepare_alternates(odb);
     -	return link_alt_odb_entry(odb, dir, NULL, 0);
    -+	return odb_add_source(odb, dir, 0);
    ++	return odb_add_alternate_recursively(odb, dir, 0);
      }
      
      struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
3:  077480d200 ! 3:  8b918fec33 odb: move computation of normalized objdir into `alt_odb_usable()`
    @@ odb.c: static int alt_odb_usable(struct object_database *o, const char *path,
      }
      
      /*
    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
    - 					 int depth)
    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *
    + 							int depth)
      {
      	struct odb_source *alternate = NULL;
     -	struct strbuf tmp = STRBUF_INIT;
    @@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
      		goto error;
      
      	alternate = odb_source_new(odb, source, false);
    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *
      	read_info_alternates(odb, alternate->path, depth + 1);
      
       error:
4:  f536d0afc3 = 4:  618bfedf22 odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`
5:  0930371378 ! 5:  50e93145e4 odb: remove mutual recursion when parsing alternates
    @@ Commit message
         Refactor the function to remove the mutual recursion between adding
         sources and parsing alternates. The parsing step thus becomes completely
         oblivious to the fact that there is recursive behaviour going on at all.
    -    Instead, the recursion is handled exclusively by `odb_add_source()`,
    +    The recursion is handled by `odb_add_alternate_recursively()` instead,
         which now recurses with itself.
     
         This refactoring allows us to move parsing of alternates into object
    @@ odb.c: static bool odb_is_source_usable(struct object_database *o, const char *p
      
      static struct odb_source *odb_source_new(struct object_database *odb,
      					 const char *path,
    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
    - 					 int depth)
    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *
    + 							int depth)
      {
      	struct odb_source *alternate = NULL;
     +	struct strvec sources = STRVEC_INIT;
      	khiter_t pos;
      	int ret;
      
    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *
      	kh_value(odb->source_by_path, pos) = alternate;
      
      	/* recursively add alternates */
    @@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
     +		      source);
     +	} else {
     +		for (size_t i = 0; i < sources.nr; i++)
    -+			odb_add_source(odb, sources.v[i], depth + 1);
    ++			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
     +	}
      
       error:
    @@ odb.c: static void parse_alternates(const char *string,
     -	parse_alternates(alt, sep, relative_base, &alternates);
     -
     -	for (size_t i = 0; i < alternates.nr; i++)
    --		odb_add_source(odb, alternates.v[i], depth);
    +-		odb_add_alternate_recursively(odb, alternates.v[i], depth);
     -
     -	strvec_clear(&alternates);
     -}
    @@ odb.c: static void read_info_alternates(struct object_database *odb,
      	strbuf_release(&buf);
      	free(path);
      }
    +@@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,
    + 		if (commit_lock_file(&lock))
    + 			die_errno(_("unable to move new alternates file into place"));
    + 		if (odb->loaded_alternates)
    +-			odb_add_source(odb, dir, 0);
    ++			odb_add_alternate_recursively(odb, dir, 0);
    + 	}
    + 	free(alts);
    + }
     @@ odb.c: int odb_for_each_alternate(struct object_database *odb,
      
      void odb_prepare_alternates(struct object_database *odb)
    @@ odb.c: int odb_for_each_alternate(struct object_database *odb,
     +	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
     +	read_info_alternates(odb->sources->path, &sources);
     +	for (size_t i = 0; i < sources.nr; i++)
    -+		odb_add_source(odb, sources.v[i], 0);
    ++		odb_add_alternate_recursively(odb, sources.v[i], 0);
      
     -	read_info_alternates(odb, odb->sources->path, 0);
      	odb->loaded_alternates = 1;
6:  be857d1b09 ! 6:  d397255cdb odb: drop forward declaration of `read_info_alternates()`
    @@ odb.c: static bool odb_is_source_usable(struct object_database *o, const char *p
     -	return source;
     -}
     -
    --static struct odb_source *odb_add_source(struct object_database *odb,
    --					 const char *source,
    --					 int depth)
    +-static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
    +-							const char *source,
    +-							int depth)
     -{
     -	struct odb_source *alternate = NULL;
     -	struct strvec sources = STRVEC_INIT;
    @@ odb.c: static bool odb_is_source_usable(struct object_database *o, const char *p
     -		      source);
     -	} else {
     -		for (size_t i = 0; i < sources.nr; i++)
    --			odb_add_source(odb, sources.v[i], depth + 1);
    +-			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
     -	}
     -
     - error:
    @@ odb.c: static void read_info_alternates(const char *relative_base,
     +	return source;
     +}
     +
    -+static struct odb_source *odb_add_source(struct object_database *odb,
    -+					 const char *source,
    -+					 int depth)
    ++static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
    ++							const char *source,
    ++							int depth)
     +{
     +	struct odb_source *alternate = NULL;
     +	struct strvec sources = STRVEC_INIT;
    @@ odb.c: static void read_info_alternates(const char *relative_base,
     +		      source);
     +	} else {
     +		for (size_t i = 0; i < sources.nr; i++)
    -+			odb_add_source(odb, sources.v[i], depth + 1);
    ++			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
     +	}
     +
     + error:
7:  a811f6abd6 ! 7:  a39997318c odb: read alternates via sources
    @@ odb.c: static void parse_alternates(const char *string,
      
      	strbuf_release(&buf);
      	free(path);
    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *
      	kh_value(odb->source_by_path, pos) = alternate;
      
      	/* recursively add alternates */
    @@ odb.c: void odb_prepare_alternates(struct object_database *odb)
     -	read_info_alternates(odb->sources->path, &sources);
     +	odb_source_read_alternates(odb->sources, &sources);
      	for (size_t i = 0; i < sources.nr; i++)
    - 		odb_add_source(odb, sources.v[i], 0);
    + 		odb_add_alternate_recursively(odb, sources.v[i], 0);
      
8:  be62ab52ab ! 8:  082eb43b82 odb: write alternates via sources
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## odb.c ##
    -@@ odb.c: static struct odb_source *odb_add_source(struct object_database *odb,
    +@@ odb.c: static struct odb_source *odb_add_alternate_recursively(struct object_database *
      	return alternate;
      }
      
    @@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,
     -		if (commit_lock_file(&lock))
     -			die_errno(_("unable to move new alternates file into place"));
     -		if (odb->loaded_alternates)
    --			odb_add_source(odb, dir, 0);
    +-			odb_add_alternate_recursively(odb, dir, 0);
     +		fprintf_or_die(out, "%s\n", alternate);
     +		if (commit_lock_file(&lock)) {
     +			ret = error_errno(_("unable to move new alternates file into place"));
    @@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,
     +	if (ret < 0)
     +		die(NULL);
     +	if (odb->loaded_alternates)
    -+		odb_add_source(odb, dir, 0);
    ++		odb_add_alternate_recursively(odb, dir, 0);
      }
      
      struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,

---
base-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06
change-id: 20251206-b4-pks-odb-alternates-via-source-802d87cbbda5

[PATCH v2 1/8] odb: refactor parsing of alternates to be self-contained

[Patrick Steinhardt]

Parsing of the alternates file and environment variable is currently
split up across multiple different functions and is entangled with
`link_alt_odb_entries()`, which is responsible for linking the parsed
object database sources. This results in two downsides:

  - We have mutual recursion between parsing alternates and linking them
    into the object database. This is because we also parse alternates
    that the newly added sources may have.

  - We mix up the actual logic to parse the data and to link them into
    place.

Refactor the logic so that parsing of the alternates file is entirely
self-contained. Note that this doesn't yet fix the above two issues, but
it is a necessary step to get there.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/odb.c b/odb.c
index dc8f292f3d..9785f62cb6 100644
--- a/odb.c
+++ b/odb.c
@@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
 	return alternate;
 }
 
-static const char *parse_alt_odb_entry(const char *string,
-				       int sep,
-				       struct strbuf *out)
+static void parse_alternates(const char *string,
+			     int sep,
+			     struct strvec *out)
 {
-	const char *end;
+	struct strbuf buf = STRBUF_INIT;
 
-	strbuf_reset(out);
+	while (*string) {
+		const char *end;
+
+		strbuf_reset(&buf);
+
+		if (*string == '#') {
+			/* comment; consume up to next separator */
+			end = strchrnul(string, sep);
+		} else if (*string == '"' && !unquote_c_style(&buf, string, &end)) {
+			/*
+			 * quoted path; unquote_c_style has copied the
+			 * data for us and set "end". Broken quoting (e.g.,
+			 * an entry that doesn't end with a quote) falls
+			 * back to the unquoted case below.
+			 */
+		} else {
+			/* normal, unquoted path */
+			end = strchrnul(string, sep);
+			strbuf_add(&buf, string, end - string);
+		}
 
-	if (*string == '#') {
-		/* comment; consume up to next separator */
-		end = strchrnul(string, sep);
-	} else if (*string == '"' && !unquote_c_style(out, string, &end)) {
-		/*
-		 * quoted path; unquote_c_style has copied the
-		 * data for us and set "end". Broken quoting (e.g.,
-		 * an entry that doesn't end with a quote) falls
-		 * back to the unquoted case below.
-		 */
-	} else {
-		/* normal, unquoted path */
-		end = strchrnul(string, sep);
-		strbuf_add(out, string, end - string);
+		if (*end)
+			end++;
+		string = end;
+
+		if (!buf.len)
+			continue;
+
+		strvec_push(out, buf.buf);
 	}
 
-	if (*end)
-		end++;
-	return end;
+	strbuf_release(&buf);
 }
 
 static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 				 int sep, const char *relative_base, int depth)
 {
-	struct strbuf dir = STRBUF_INIT;
+	struct strvec alternates = STRVEC_INIT;
 
 	if (!alt || !*alt)
 		return;
@@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 		return;
 	}
 
-	while (*alt) {
-		alt = parse_alt_odb_entry(alt, sep, &dir);
-		if (!dir.len)
-			continue;
-		link_alt_odb_entry(odb, dir.buf, relative_base, depth);
-	}
-	strbuf_release(&dir);
+	parse_alternates(alt, sep, &alternates);
+
+	for (size_t i = 0; i < alternates.nr; i++)
+		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
+
+	strvec_clear(&alternates);
 }
 
 static void read_info_alternates(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v2 2/8] odb: resolve relative alternative paths when parsing

[Patrick Steinhardt]

Parsing alternates and resolving potential relative paths is currently
handled in two separate steps. This has the effect that the logic to
retrieve alternates is not entirely self-contained. We want it to be
just that though so that we can eventually move the logic to list
alternates into the `struct odb_source`.

Move the logic to resolve relative alternative paths into
`parse_alternates()`. Besides bringing us a step closer towards the
above goal, it also neatly separates concerns of generating the list of
alternatives and linking them into the object database.

Note that we ignore any errors when the relative path cannot be
resolved. This isn't really a change in behaviour though: if the path
cannot be resolved to a directory then `alt_odb_usable()` still knows to
bail out.

While at it, rename the function to `odb_add_alternate_recursively()` to
more clearly indicate what its intent is and to align it with modern
terminology.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/odb.c b/odb.c
index 9785f62cb6..699bdbffd1 100644
--- a/odb.c
+++ b/odb.c
@@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,
 	return source;
 }
 
-static struct odb_source *link_alt_odb_entry(struct object_database *odb,
-					     const char *dir,
-					     const char *relative_base,
-					     int depth)
+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
+							const char *source,
+							int depth)
 {
 	struct odb_source *alternate = NULL;
-	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf tmp = STRBUF_INIT;
 	khiter_t pos;
 	int ret;
 
-	if (!is_absolute_path(dir) && relative_base) {
-		strbuf_realpath(&pathbuf, relative_base, 1);
-		strbuf_addch(&pathbuf, '/');
-	}
-	strbuf_addstr(&pathbuf, dir);
-
-	if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {
-		error(_("unable to normalize alternate object path: %s"),
-		      pathbuf.buf);
-		goto error;
-	}
-	strbuf_swap(&pathbuf, &tmp);
-
-	/*
-	 * The trailing slash after the directory name is given by
-	 * this function at the end. Remove duplicates.
-	 */
-	while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
-		strbuf_setlen(&pathbuf, pathbuf.len - 1);
-
-	strbuf_reset(&tmp);
 	strbuf_realpath(&tmp, odb->sources->path, 1);
 
-	if (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))
+	if (!alt_odb_usable(odb, source, tmp.buf))
 		goto error;
 
-	alternate = odb_source_new(odb, pathbuf.buf, false);
+	alternate = odb_source_new(odb, source, false);
 
 	/* add the alternate entry */
 	*odb->sources_tail = alternate;
@@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
 
  error:
 	strbuf_release(&tmp);
-	strbuf_release(&pathbuf);
 	return alternate;
 }
 
 static void parse_alternates(const char *string,
 			     int sep,
+			     const char *relative_base,
 			     struct strvec *out)
 {
+	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 
 	while (*string) {
 		const char *end;
 
 		strbuf_reset(&buf);
+		strbuf_reset(&pathbuf);
 
 		if (*string == '#') {
 			/* comment; consume up to next separator */
@@ -250,9 +229,30 @@ static void parse_alternates(const char *string,
 		if (!buf.len)
 			continue;
 
+		if (!is_absolute_path(buf.buf) && relative_base) {
+			strbuf_realpath(&pathbuf, relative_base, 1);
+			strbuf_addch(&pathbuf, '/');
+		}
+		strbuf_addbuf(&pathbuf, &buf);
+
+		strbuf_reset(&buf);
+		if (!strbuf_realpath(&buf, pathbuf.buf, 0)) {
+			error(_("unable to normalize alternate object path: %s"),
+			      pathbuf.buf);
+			continue;
+		}
+
+		/*
+		 * The trailing slash after the directory name is given by
+		 * this function at the end. Remove duplicates.
+		 */
+		while (buf.len && buf.buf[buf.len - 1] == '/')
+			strbuf_setlen(&buf, buf.len - 1);
+
 		strvec_push(out, buf.buf);
 	}
 
+	strbuf_release(&pathbuf);
 	strbuf_release(&buf);
 }
 
@@ -270,10 +270,10 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 		return;
 	}
 
-	parse_alternates(alt, sep, &alternates);
+	parse_alternates(alt, sep, relative_base, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
-		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
+		odb_add_alternate_recursively(odb, alternates.v[i], depth);
 
 	strvec_clear(&alternates);
 }
@@ -348,7 +348,7 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
 	 * overwritten when they are.
 	 */
 	odb_prepare_alternates(odb);
-	return link_alt_odb_entry(odb, dir, NULL, 0);
+	return odb_add_alternate_recursively(odb, dir, 0);
 }
 
 struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v2 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`

[Patrick Steinhardt]

The function `alt_odb_usable()` receives as input the object database,
the path it's supposed to determine usability for as well as the
normalized path of the main object directory of the repository. The last
part is derived by the function's caller from the object database. As we
already pass the object database to `alt_odb_usable()` it is redundant
information.

Drop the extra parameter and compute the normalized object directory in
the function itself.

While at it, rename the function to `odb_is_source_usable()` to align it
with modern terminology.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/odb.c b/odb.c
index 699bdbffd1..e314f86c3b 100644
--- a/odb.c
+++ b/odb.c
@@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
-static int alt_odb_usable(struct object_database *o, const char *path,
-			  const char *normalized_objdir)
+static bool odb_is_source_usable(struct object_database *o, const char *path)
 {
 	int r;
+	struct strbuf normalized_objdir = STRBUF_INIT;
+	bool usable = false;
+
+	strbuf_realpath(&normalized_objdir, o->sources->path, 1);
 
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path)) {
 		error(_("object directory %s does not exist; "
 			"check .git/objects/info/alternates"),
 		      path);
-		return 0;
+		goto out;
 	}
 
 	/*
@@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,
 		kh_value(o->source_by_path, p) = o->sources;
 	}
 
-	if (fspatheq(path, normalized_objdir))
-		return 0;
+	if (fspatheq(path, normalized_objdir.buf))
+		goto out;
 
 	if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))
-		return 0;
+		goto out;
+
+	usable = true;
 
-	return 1;
+out:
+	strbuf_release(&normalized_objdir);
+	return usable;
 }
 
 /*
@@ -164,13 +171,10 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 							int depth)
 {
 	struct odb_source *alternate = NULL;
-	struct strbuf tmp = STRBUF_INIT;
 	khiter_t pos;
 	int ret;
 
-	strbuf_realpath(&tmp, odb->sources->path, 1);
-
-	if (!alt_odb_usable(odb, source, tmp.buf))
+	if (!odb_is_source_usable(odb, source))
 		goto error;
 
 	alternate = odb_source_new(odb, source, false);
@@ -188,7 +192,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	read_info_alternates(odb, alternate->path, depth + 1);
 
  error:
-	strbuf_release(&tmp);
 	return alternate;
 }
 

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v2 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`

[Patrick Steinhardt]

When calling `odb_add_to_alternates_file()` we know to add the newly
added source to the object database in case we have already loaded
alternates. This is done so that we can make its objects accessible
immediately without having to fully reload all alternates.

The way we do this though is to call `link_alt_odb_entries()`, which
adds _multiple_ sources to the object database source in case we have
newline-separated entries. This behaviour is not documented in the
function documentation of `odb_add_to_alternates_file()`, and all
callers only ever pass a single directory to it. It's thus entirely
surprising and a conceptual mismatch.

Fix this issue by directly calling `odb_add_source()` instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/odb.c b/odb.c
index e314f86c3b..d97e50fb61 100644
--- a/odb.c
+++ b/odb.c
@@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,
 		if (commit_lock_file(&lock))
 			die_errno(_("unable to move new alternates file into place"));
 		if (odb->loaded_alternates)
-			link_alt_odb_entries(odb, dir, '\n', NULL, 0);
+			odb_add_source(odb, dir, 0);
 	}
 	free(alts);
 }

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v2 5/8] odb: remove mutual recursion when parsing alternates

[Patrick Steinhardt]

When adding an alternative object database source we not only have to
consider the added source itself, but we also have to add _its_ sources
to our database. We implement this via mutual recursion:

  1. We first call `link_alt_odb_entries()`.

  2. `link_alt_odb_entries()` calls `parse_alternates()`.

  3. We then add each parsed alternate via `odb_add_source()`.

  4. `odb_add_source()` calls `link_alt_odb_entries()` again.

This flow is somewhat hard to follow, but more importantly it means that
parsing of alternates is somewhat tied to the recursive behaviour.

Refactor the function to remove the mutual recursion between adding
sources and parsing alternates. The parsing step thus becomes completely
oblivious to the fact that there is recursive behaviour going on at all.
The recursion is handled by `odb_add_alternate_recursively()` instead,
which now recurses with itself.

This refactoring allows us to move parsing of alternates into object
database sources in a subsequent step.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 62 ++++++++++++++++++++++++++++----------------------------------
 1 file changed, 28 insertions(+), 34 deletions(-)

diff --git a/odb.c b/odb.c
index d97e50fb61..59944d4649 100644
--- a/odb.c
+++ b/odb.c
@@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)
  * of the object ID, an extra slash for the first level indirection, and
  * the terminating NUL.
  */
-static void read_info_alternates(struct object_database *odb,
-				 const char *relative_base,
-				 int depth);
+static void read_info_alternates(const char *relative_base,
+				 struct strvec *out);
 
 static struct odb_source *odb_source_new(struct object_database *odb,
 					 const char *path,
@@ -171,6 +170,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 							int depth)
 {
 	struct odb_source *alternate = NULL;
+	struct strvec sources = STRVEC_INIT;
 	khiter_t pos;
 	int ret;
 
@@ -189,9 +189,17 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	kh_value(odb->source_by_path, pos) = alternate;
 
 	/* recursively add alternates */
-	read_info_alternates(odb, alternate->path, depth + 1);
+	read_info_alternates(alternate->path, &sources);
+	if (sources.nr && depth + 1 > 5) {
+		error(_("%s: ignoring alternate object stores, nesting too deep"),
+		      source);
+	} else {
+		for (size_t i = 0; i < sources.nr; i++)
+			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
+	}
 
  error:
+	strvec_clear(&sources);
 	return alternate;
 }
 
@@ -203,6 +211,9 @@ static void parse_alternates(const char *string,
 	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 
+	if (!string || !*string)
+		return;
+
 	while (*string) {
 		const char *end;
 
@@ -259,34 +270,11 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static void link_alt_odb_entries(struct object_database *odb, const char *alt,
-				 int sep, const char *relative_base, int depth)
+static void read_info_alternates(const char *relative_base,
+				 struct strvec *out)
 {
-	struct strvec alternates = STRVEC_INIT;
-
-	if (!alt || !*alt)
-		return;
-
-	if (depth > 5) {
-		error(_("%s: ignoring alternate object stores, nesting too deep"),
-				relative_base);
-		return;
-	}
-
-	parse_alternates(alt, sep, relative_base, &alternates);
-
-	for (size_t i = 0; i < alternates.nr; i++)
-		odb_add_alternate_recursively(odb, alternates.v[i], depth);
-
-	strvec_clear(&alternates);
-}
-
-static void read_info_alternates(struct object_database *odb,
-				 const char *relative_base,
-				 int depth)
-{
-	char *path;
 	struct strbuf buf = STRBUF_INIT;
+	char *path;
 
 	path = xstrfmt("%s/info/alternates", relative_base);
 	if (strbuf_read_file(&buf, path, 1024) < 0) {
@@ -294,8 +282,8 @@ static void read_info_alternates(struct object_database *odb,
 		free(path);
 		return;
 	}
+	parse_alternates(buf.buf, '\n', relative_base, out);
 
-	link_alt_odb_entries(odb, buf.buf, '\n', relative_base, depth);
 	strbuf_release(&buf);
 	free(path);
 }
@@ -338,7 +326,7 @@ void odb_add_to_alternates_file(struct object_database *odb,
 		if (commit_lock_file(&lock))
 			die_errno(_("unable to move new alternates file into place"));
 		if (odb->loaded_alternates)
-			odb_add_source(odb, dir, 0);
+			odb_add_alternate_recursively(odb, dir, 0);
 	}
 	free(alts);
 }
@@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,
 
 void odb_prepare_alternates(struct object_database *odb)
 {
+	struct strvec sources = STRVEC_INIT;
+
 	if (odb->loaded_alternates)
 		return;
 
-	link_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);
+	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
+	read_info_alternates(odb->sources->path, &sources);
+	for (size_t i = 0; i < sources.nr; i++)
+		odb_add_alternate_recursively(odb, sources.v[i], 0);
 
-	read_info_alternates(odb, odb->sources->path, 0);
 	odb->loaded_alternates = 1;
+
+	strvec_clear(&sources);
 }
 
 int odb_has_alternates(struct object_database *odb)

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v2 6/8] odb: drop forward declaration of `read_info_alternates()`

[Patrick Steinhardt]

Now that we have removed the mutual recursion in the preceding commit
it is not necessary anymore to have a forward declaration of the
`read_info_alternates()` function. Move the function and its
dependencies further up so that we can remove it.

Note that this commit also removes the function documentation of
`read_info_alternates()`. It's unclear what it's documenting, but it for
sure isn't documenting the modern behaviour of the function anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 125 +++++++++++++++++++++++++++++-------------------------------------
 1 file changed, 54 insertions(+), 71 deletions(-)

diff --git a/odb.c b/odb.c
index 59944d4649..dcf4a62cd2 100644
--- a/odb.c
+++ b/odb.c
@@ -132,77 +132,6 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)
 	return usable;
 }
 
-/*
- * Prepare alternate object database registry.
- *
- * The variable alt_odb_list points at the list of struct
- * odb_source.  The elements on this list come from
- * non-empty elements from colon separated ALTERNATE_DB_ENVIRONMENT
- * environment variable, and $GIT_OBJECT_DIRECTORY/info/alternates,
- * whose contents is similar to that environment variable but can be
- * LF separated.  Its base points at a statically allocated buffer that
- * contains "/the/directory/corresponding/to/.git/objects/...", while
- * its name points just after the slash at the end of ".git/objects/"
- * in the example above, and has enough space to hold all hex characters
- * of the object ID, an extra slash for the first level indirection, and
- * the terminating NUL.
- */
-static void read_info_alternates(const char *relative_base,
-				 struct strvec *out);
-
-static struct odb_source *odb_source_new(struct object_database *odb,
-					 const char *path,
-					 bool local)
-{
-	struct odb_source *source;
-
-	CALLOC_ARRAY(source, 1);
-	source->odb = odb;
-	source->local = local;
-	source->path = xstrdup(path);
-	source->loose = odb_source_loose_new(source);
-
-	return source;
-}
-
-static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
-							const char *source,
-							int depth)
-{
-	struct odb_source *alternate = NULL;
-	struct strvec sources = STRVEC_INIT;
-	khiter_t pos;
-	int ret;
-
-	if (!odb_is_source_usable(odb, source))
-		goto error;
-
-	alternate = odb_source_new(odb, source, false);
-
-	/* add the alternate entry */
-	*odb->sources_tail = alternate;
-	odb->sources_tail = &(alternate->next);
-
-	pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);
-	if (!ret)
-		BUG("source must not yet exist");
-	kh_value(odb->source_by_path, pos) = alternate;
-
-	/* recursively add alternates */
-	read_info_alternates(alternate->path, &sources);
-	if (sources.nr && depth + 1 > 5) {
-		error(_("%s: ignoring alternate object stores, nesting too deep"),
-		      source);
-	} else {
-		for (size_t i = 0; i < sources.nr; i++)
-			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
-	}
-
- error:
-	strvec_clear(&sources);
-	return alternate;
-}
-
 static void parse_alternates(const char *string,
 			     int sep,
 			     const char *relative_base,
@@ -288,6 +217,60 @@ static void read_info_alternates(const char *relative_base,
 	free(path);
 }
 
+
+static struct odb_source *odb_source_new(struct object_database *odb,
+					 const char *path,
+					 bool local)
+{
+	struct odb_source *source;
+
+	CALLOC_ARRAY(source, 1);
+	source->odb = odb;
+	source->local = local;
+	source->path = xstrdup(path);
+	source->loose = odb_source_loose_new(source);
+
+	return source;
+}
+
+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
+							const char *source,
+							int depth)
+{
+	struct odb_source *alternate = NULL;
+	struct strvec sources = STRVEC_INIT;
+	khiter_t pos;
+	int ret;
+
+	if (!odb_is_source_usable(odb, source))
+		goto error;
+
+	alternate = odb_source_new(odb, source, false);
+
+	/* add the alternate entry */
+	*odb->sources_tail = alternate;
+	odb->sources_tail = &(alternate->next);
+
+	pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);
+	if (!ret)
+		BUG("source must not yet exist");
+	kh_value(odb->source_by_path, pos) = alternate;
+
+	/* recursively add alternates */
+	read_info_alternates(alternate->path, &sources);
+	if (sources.nr && depth + 1 > 5) {
+		error(_("%s: ignoring alternate object stores, nesting too deep"),
+		      source);
+	} else {
+		for (size_t i = 0; i < sources.nr; i++)
+			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
+	}
+
+ error:
+	strvec_clear(&sources);
+	return alternate;
+}
+
 void odb_add_to_alternates_file(struct object_database *odb,
 				const char *dir)
 {

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v2 7/8] odb: read alternates via sources

[Patrick Steinhardt]

Adapt how we read alternates so that the interface is structured around
the object database source we're reading from. This will eventually
allow us to abstract away this behaviour with pluggable object databases
so that every format can have its own mechanism for listing alternates.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/odb.c b/odb.c
index dcf4a62cd2..c5ba26b85f 100644
--- a/odb.c
+++ b/odb.c
@@ -199,19 +199,19 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static void read_info_alternates(const char *relative_base,
-				 struct strvec *out)
+static void odb_source_read_alternates(struct odb_source *source,
+				       struct strvec *out)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *path;
 
-	path = xstrfmt("%s/info/alternates", relative_base);
+	path = xstrfmt("%s/info/alternates", source->path);
 	if (strbuf_read_file(&buf, path, 1024) < 0) {
 		warn_on_fopen_errors(path);
 		free(path);
 		return;
 	}
-	parse_alternates(buf.buf, '\n', relative_base, out);
+	parse_alternates(buf.buf, '\n', source->path, out);
 
 	strbuf_release(&buf);
 	free(path);
@@ -257,7 +257,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	kh_value(odb->source_by_path, pos) = alternate;
 
 	/* recursively add alternates */
-	read_info_alternates(alternate->path, &sources);
+	odb_source_read_alternates(alternate, &sources);
 	if (sources.nr && depth + 1 > 5) {
 		error(_("%s: ignoring alternate object stores, nesting too deep"),
 		      source);
@@ -599,7 +599,7 @@ void odb_prepare_alternates(struct object_database *odb)
 		return;
 
 	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
-	read_info_alternates(odb->sources->path, &sources);
+	odb_source_read_alternates(odb->sources, &sources);
 	for (size_t i = 0; i < sources.nr; i++)
 		odb_add_alternate_recursively(odb, sources.v[i], 0);
 

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v2 8/8] odb: write alternates via sources

[Patrick Steinhardt]

Refactor writing of alternates so that the actual business logic is
structured around the object database source we want to write the
alternate to. Same as with the preceding commit, this will eventually
allow us to have different logic for writing alternates depending on the
backend used.

Note that after the refactoring we start to call `odb_add_source()`
unconditionally. This is fine though as we know to skip adding sources
that are tracked already.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 51 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/odb.c b/odb.c
index c5ba26b85f..cc7f832465 100644
--- a/odb.c
+++ b/odb.c
@@ -271,25 +271,28 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	return alternate;
 }
 
-void odb_add_to_alternates_file(struct object_database *odb,
-				const char *dir)
+static int odb_source_write_alternate(struct odb_source *source,
+				      const char *alternate)
 {
 	struct lock_file lock = LOCK_INIT;
-	char *alts = repo_git_path(odb->repo, "objects/info/alternates");
+	char *path = xstrfmt("%s/%s", source->path, "info/alternates");
 	FILE *in, *out;
 	int found = 0;
+	int ret;
 
-	hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);
+	hold_lock_file_for_update(&lock, path, LOCK_DIE_ON_ERROR);
 	out = fdopen_lock_file(&lock, "w");
-	if (!out)
-		die_errno(_("unable to fdopen alternates lockfile"));
+	if (!out) {
+		ret = error_errno(_("unable to fdopen alternates lockfile"));
+		goto out;
+	}
 
-	in = fopen(alts, "r");
+	in = fopen(path, "r");
 	if (in) {
 		struct strbuf line = STRBUF_INIT;
 
 		while (strbuf_getline(&line, in) != EOF) {
-			if (!strcmp(dir, line.buf)) {
+			if (!strcmp(alternate, line.buf)) {
 				found = 1;
 				break;
 			}
@@ -298,20 +301,36 @@ void odb_add_to_alternates_file(struct object_database *odb,
 
 		strbuf_release(&line);
 		fclose(in);
+	} else if (errno != ENOENT) {
+		ret = error_errno(_("unable to read alternates file"));
+		goto out;
 	}
-	else if (errno != ENOENT)
-		die_errno(_("unable to read alternates file"));
 
 	if (found) {
 		rollback_lock_file(&lock);
 	} else {
-		fprintf_or_die(out, "%s\n", dir);
-		if (commit_lock_file(&lock))
-			die_errno(_("unable to move new alternates file into place"));
-		if (odb->loaded_alternates)
-			odb_add_alternate_recursively(odb, dir, 0);
+		fprintf_or_die(out, "%s\n", alternate);
+		if (commit_lock_file(&lock)) {
+			ret = error_errno(_("unable to move new alternates file into place"));
+			goto out;
+		}
 	}
-	free(alts);
+
+	ret = 0;
+
+out:
+	free(path);
+	return ret;
+}
+
+void odb_add_to_alternates_file(struct object_database *odb,
+				const char *dir)
+{
+	int ret = odb_source_write_alternate(odb->sources, dir);
+	if (ret < 0)
+		die(NULL);
+	if (odb->loaded_alternates)
+		odb_add_alternate_recursively(odb, dir, 0);
 }
 
 struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

Re: [PATCH v2 0/8] Refactor handling of alternates to work via sources

[Justin Tobler]

On 25/12/10 04:32PM, Patrick Steinhardt wrote:
> Changes in v2:
>   - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to
>     highlight that this function is recursive.
>   - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im

Thanks the changes in the version look good to me.

-Justin

Re: [PATCH v2 0/8] Refactor handling of alternates to work via sources

[Patrick Steinhardt]

On Wed, Dec 10, 2025 at 02:00:39PM -0600, Justin Tobler wrote:
> On 25/12/10 04:32PM, Patrick Steinhardt wrote:
> > Changes in v2:
> >   - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to
> >     highlight that this function is recursive.
> >   - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im
> 
> Thanks the changes in the version look good to me.

Thanks for your review!

Patrick

Re: [PATCH v2 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`

[SZEDER Gábor]

On Wed, Dec 10, 2025 at 04:32:37PM +0100, Patrick Steinhardt wrote:
> When calling `odb_add_to_alternates_file()` we know to add the newly
> added source to the object database in case we have already loaded
> alternates. This is done so that we can make its objects accessible
> immediately without having to fully reload all alternates.
> 
> The way we do this though is to call `link_alt_odb_entries()`, which
> adds _multiple_ sources to the object database source in case we have
> newline-separated entries. This behaviour is not documented in the
> function documentation of `odb_add_to_alternates_file()`, and all
> callers only ever pass a single directory to it. It's thus entirely
> surprising and a conceptual mismatch.
> 
> Fix this issue by directly calling `odb_add_source()` instead.

OK, but:

> diff --git a/odb.c b/odb.c
> index e314f86c3b..d97e50fb61 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,
>  		if (commit_lock_file(&lock))
>  			die_errno(_("unable to move new alternates file into place"));
>  		if (odb->loaded_alternates)
> -			link_alt_odb_entries(odb, dir, '\n', NULL, 0);
> +			odb_add_source(odb, dir, 0);

      CC odb.o
  odb.c: In function ‘odb_add_to_alternates_file’:
  odb.c:341:25: error: implicit declaration of function ‘odb_add_source’; did you mean ‘odb_find_source’? [-Werror=implicit-function-declaration]
    341 |                         odb_add_source(odb, dir, 0);
        |                         ^~~~~~~~~~~~~~
        |                         odb_find_source
  cc1: all warnings being treated as errors
  make: *** [Makefile:2864: odb.o] Error 1

Note, that several commit messages also refer to this non-existing
function from the previous round.

Re: [PATCH v2 4/8] odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`

[Patrick Steinhardt]

On Thu, Dec 11, 2025 at 08:21:43AM +0100, SZEDER Gábor wrote:
> On Wed, Dec 10, 2025 at 04:32:37PM +0100, Patrick Steinhardt wrote:
> > When calling `odb_add_to_alternates_file()` we know to add the newly
> > added source to the object database in case we have already loaded
> > alternates. This is done so that we can make its objects accessible
> > immediately without having to fully reload all alternates.
> > 
> > The way we do this though is to call `link_alt_odb_entries()`, which
> > adds _multiple_ sources to the object database source in case we have
> > newline-separated entries. This behaviour is not documented in the
> > function documentation of `odb_add_to_alternates_file()`, and all
> > callers only ever pass a single directory to it. It's thus entirely
> > surprising and a conceptual mismatch.
> > 
> > Fix this issue by directly calling `odb_add_source()` instead.
> 
> OK, but:
> 
> > diff --git a/odb.c b/odb.c
> > index e314f86c3b..d97e50fb61 100644
> > --- a/odb.c
> > +++ b/odb.c
> > @@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,
> >  		if (commit_lock_file(&lock))
> >  			die_errno(_("unable to move new alternates file into place"));
> >  		if (odb->loaded_alternates)
> > -			link_alt_odb_entries(odb, dir, '\n', NULL, 0);
> > +			odb_add_source(odb, dir, 0);
> 
>       CC odb.o
>   odb.c: In function ‘odb_add_to_alternates_file’:
>   odb.c:341:25: error: implicit declaration of function ‘odb_add_source’; did you mean ‘odb_find_source’? [-Werror=implicit-function-declaration]
>     341 |                         odb_add_source(odb, dir, 0);
>         |                         ^~~~~~~~~~~~~~
>         |                         odb_find_source
>   cc1: all warnings being treated as errors
>   make: *** [Makefile:2864: odb.o] Error 1

Hrmpf, I only fixed this callsite in a later commit indeed.

> Note, that several commit messages also refer to this non-existing
> function from the previous round.

True. Will fix both of these issues, thanks!

Patrick

[PATCH v3 0/8] Refactor handling of alternates to work via sources

[Patrick Steinhardt]

Hi,

this patch series refactors how we handle alternate object directories
so that the interface is structured around the object database source.

Next to being simpler to reason about, it also allows us to eventually
abstract handling of alternates to use different mechanisms based on the
specific backend used. In a world of pluggable object databases not
every backend may use a physical directory, so it may not be possible to
read alternates via "objects/info/alternates". Consequently, formats may
need a different mechanism entirely to make this list available.

Changes in v3:
  - Fix commit messages that still refer to `odb_add_source()`.
  - Fix intermediate commit that still refers to `odb_add_source()`.
  - Link to v2: https://lore.kernel.org/r/20251210-b4-pks-odb-alternates-via-source-v2-0-eb336815f9ab@pks.im

Changes in v2:
  - Rename `odb_add_source()` to `odb_add_alternates_recursive()` to
    highlight that this function is recursive.
  - Link to v1: https://lore.kernel.org/r/20251208-b4-pks-odb-alternates-via-source-v1-0-e7ebb8b18c03@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (8):
      odb: refactor parsing of alternates to be self-contained
      odb: resolve relative alternative paths when parsing
      odb: move computation of normalized objdir into `alt_odb_usable()`
      odb: stop splitting alternate in `odb_add_to_alternates_file()`
      odb: remove mutual recursion when parsing alternates
      odb: drop forward declaration of `read_info_alternates()`
      odb: read alternates via sources
      odb: write alternates via sources

 odb.c | 307 ++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 158 insertions(+), 149 deletions(-)

Range-diff versus v2:

1:  74d2596ef6 = 1:  4a85139a75 odb: refactor parsing of alternates to be self-contained
2:  16d6e482d7 = 2:  1b16c0a164 odb: resolve relative alternative paths when parsing
3:  16cce7f52e = 3:  ceb6e8494c odb: move computation of normalized objdir into `alt_odb_usable()`
4:  b8a7138a51 ! 4:  99dbd11c48 odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    odb: adapt `odb_add_to_alternates_file()` to call `odb_add_source()`
    +    odb: stop splitting alternate in `odb_add_to_alternates_file()`
     
         When calling `odb_add_to_alternates_file()` we know to add the newly
         added source to the object database in case we have already loaded
    @@ Commit message
         callers only ever pass a single directory to it. It's thus entirely
         surprising and a conceptual mismatch.
     
    -    Fix this issue by directly calling `odb_add_source()` instead.
    +    Fix this issue by directly calling `odb_add_alternate_recursively()`
    +    instead.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    @@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,
      			die_errno(_("unable to move new alternates file into place"));
      		if (odb->loaded_alternates)
     -			link_alt_odb_entries(odb, dir, '\n', NULL, 0);
    -+			odb_add_source(odb, dir, 0);
    ++			odb_add_alternate_recursively(odb, dir, 0);
      	}
      	free(alts);
      }
5:  2b2d4788bf ! 5:  b9300667a6 odb: remove mutual recursion when parsing alternates
    @@ Commit message
     
           2. `link_alt_odb_entries()` calls `parse_alternates()`.
     
    -      3. We then add each parsed alternate via `odb_add_source()`.
    +      3. We then add each alternate via `odb_add_alternate_recursively()`.
     
    -      4. `odb_add_source()` calls `link_alt_odb_entries()` again.
    +      4. `odb_add_alternate_recursively()` calls `link_alt_odb_entries()`
    +         again.
     
         This flow is somewhat hard to follow, but more importantly it means that
         parsing of alternates is somewhat tied to the recursive behaviour.
    @@ odb.c: static void read_info_alternates(struct object_database *odb,
      	strbuf_release(&buf);
      	free(path);
      }
    -@@ odb.c: void odb_add_to_alternates_file(struct object_database *odb,
    - 		if (commit_lock_file(&lock))
    - 			die_errno(_("unable to move new alternates file into place"));
    - 		if (odb->loaded_alternates)
    --			odb_add_source(odb, dir, 0);
    -+			odb_add_alternate_recursively(odb, dir, 0);
    - 	}
    - 	free(alts);
    - }
     @@ odb.c: int odb_for_each_alternate(struct object_database *odb,
      
      void odb_prepare_alternates(struct object_database *odb)
6:  3294336d85 = 6:  1e3a1fb081 odb: drop forward declaration of `read_info_alternates()`
7:  55ba5815d4 = 7:  1d6a9b3c1b odb: read alternates via sources
8:  225bcc37de ! 8:  79a053fb2b odb: write alternates via sources
    @@ Commit message
         allow us to have different logic for writing alternates depending on the
         backend used.
     
    -    Note that after the refactoring we start to call `odb_add_source()`
    -    unconditionally. This is fine though as we know to skip adding sources
    -    that are tracked already.
    +    Note that after the refactoring we start to call
    +    `odb_add_alternate_recursively()` unconditionally. This is fine though
    +    as we know to skip adding sources that are tracked already.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     

---
base-commit: bdc5341ff65278a3cc80b2e8a02a2f02aa1fac06
change-id: 20251206-b4-pks-odb-alternates-via-source-802d87cbbda5

[PATCH v3 1/8] odb: refactor parsing of alternates to be self-contained

[Patrick Steinhardt]

Parsing of the alternates file and environment variable is currently
split up across multiple different functions and is entangled with
`link_alt_odb_entries()`, which is responsible for linking the parsed
object database sources. This results in two downsides:

  - We have mutual recursion between parsing alternates and linking them
    into the object database. This is because we also parse alternates
    that the newly added sources may have.

  - We mix up the actual logic to parse the data and to link them into
    place.

Refactor the logic so that parsing of the alternates file is entirely
self-contained. Note that this doesn't yet fix the above two issues, but
it is a necessary step to get there.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 70 ++++++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/odb.c b/odb.c
index dc8f292f3d..9785f62cb6 100644
--- a/odb.c
+++ b/odb.c
@@ -216,39 +216,50 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
 	return alternate;
 }
 
-static const char *parse_alt_odb_entry(const char *string,
-				       int sep,
-				       struct strbuf *out)
+static void parse_alternates(const char *string,
+			     int sep,
+			     struct strvec *out)
 {
-	const char *end;
+	struct strbuf buf = STRBUF_INIT;
 
-	strbuf_reset(out);
+	while (*string) {
+		const char *end;
+
+		strbuf_reset(&buf);
+
+		if (*string == '#') {
+			/* comment; consume up to next separator */
+			end = strchrnul(string, sep);
+		} else if (*string == '"' && !unquote_c_style(&buf, string, &end)) {
+			/*
+			 * quoted path; unquote_c_style has copied the
+			 * data for us and set "end". Broken quoting (e.g.,
+			 * an entry that doesn't end with a quote) falls
+			 * back to the unquoted case below.
+			 */
+		} else {
+			/* normal, unquoted path */
+			end = strchrnul(string, sep);
+			strbuf_add(&buf, string, end - string);
+		}
 
-	if (*string == '#') {
-		/* comment; consume up to next separator */
-		end = strchrnul(string, sep);
-	} else if (*string == '"' && !unquote_c_style(out, string, &end)) {
-		/*
-		 * quoted path; unquote_c_style has copied the
-		 * data for us and set "end". Broken quoting (e.g.,
-		 * an entry that doesn't end with a quote) falls
-		 * back to the unquoted case below.
-		 */
-	} else {
-		/* normal, unquoted path */
-		end = strchrnul(string, sep);
-		strbuf_add(out, string, end - string);
+		if (*end)
+			end++;
+		string = end;
+
+		if (!buf.len)
+			continue;
+
+		strvec_push(out, buf.buf);
 	}
 
-	if (*end)
-		end++;
-	return end;
+	strbuf_release(&buf);
 }
 
 static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 				 int sep, const char *relative_base, int depth)
 {
-	struct strbuf dir = STRBUF_INIT;
+	struct strvec alternates = STRVEC_INIT;
 
 	if (!alt || !*alt)
 		return;
@@ -259,13 +270,12 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 		return;
 	}
 
-	while (*alt) {
-		alt = parse_alt_odb_entry(alt, sep, &dir);
-		if (!dir.len)
-			continue;
-		link_alt_odb_entry(odb, dir.buf, relative_base, depth);
-	}
-	strbuf_release(&dir);
+	parse_alternates(alt, sep, &alternates);
+
+	for (size_t i = 0; i < alternates.nr; i++)
+		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
+
+	strvec_clear(&alternates);
 }
 
 static void read_info_alternates(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v3 2/8] odb: resolve relative alternative paths when parsing

[Patrick Steinhardt]

Parsing alternates and resolving potential relative paths is currently
handled in two separate steps. This has the effect that the logic to
retrieve alternates is not entirely self-contained. We want it to be
just that though so that we can eventually move the logic to list
alternates into the `struct odb_source`.

Move the logic to resolve relative alternative paths into
`parse_alternates()`. Besides bringing us a step closer towards the
above goal, it also neatly separates concerns of generating the list of
alternatives and linking them into the object database.

Note that we ignore any errors when the relative path cannot be
resolved. This isn't really a change in behaviour though: if the path
cannot be resolved to a directory then `alt_odb_usable()` still knows to
bail out.

While at it, rename the function to `odb_add_alternate_recursively()` to
more clearly indicate what its intent is and to align it with modern
terminology.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 64 ++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/odb.c b/odb.c
index 9785f62cb6..699bdbffd1 100644
--- a/odb.c
+++ b/odb.c
@@ -159,44 +159,21 @@ static struct odb_source *odb_source_new(struct object_database *odb,
 	return source;
 }
 
-static struct odb_source *link_alt_odb_entry(struct object_database *odb,
-					     const char *dir,
-					     const char *relative_base,
-					     int depth)
+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
+							const char *source,
+							int depth)
 {
 	struct odb_source *alternate = NULL;
-	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf tmp = STRBUF_INIT;
 	khiter_t pos;
 	int ret;
 
-	if (!is_absolute_path(dir) && relative_base) {
-		strbuf_realpath(&pathbuf, relative_base, 1);
-		strbuf_addch(&pathbuf, '/');
-	}
-	strbuf_addstr(&pathbuf, dir);
-
-	if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {
-		error(_("unable to normalize alternate object path: %s"),
-		      pathbuf.buf);
-		goto error;
-	}
-	strbuf_swap(&pathbuf, &tmp);
-
-	/*
-	 * The trailing slash after the directory name is given by
-	 * this function at the end. Remove duplicates.
-	 */
-	while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
-		strbuf_setlen(&pathbuf, pathbuf.len - 1);
-
-	strbuf_reset(&tmp);
 	strbuf_realpath(&tmp, odb->sources->path, 1);
 
-	if (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))
+	if (!alt_odb_usable(odb, source, tmp.buf))
 		goto error;
 
-	alternate = odb_source_new(odb, pathbuf.buf, false);
+	alternate = odb_source_new(odb, source, false);
 
 	/* add the alternate entry */
 	*odb->sources_tail = alternate;
@@ -212,20 +189,22 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
 
  error:
 	strbuf_release(&tmp);
-	strbuf_release(&pathbuf);
 	return alternate;
 }
 
 static void parse_alternates(const char *string,
 			     int sep,
+			     const char *relative_base,
 			     struct strvec *out)
 {
+	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 
 	while (*string) {
 		const char *end;
 
 		strbuf_reset(&buf);
+		strbuf_reset(&pathbuf);
 
 		if (*string == '#') {
 			/* comment; consume up to next separator */
@@ -250,9 +229,30 @@ static void parse_alternates(const char *string,
 		if (!buf.len)
 			continue;
 
+		if (!is_absolute_path(buf.buf) && relative_base) {
+			strbuf_realpath(&pathbuf, relative_base, 1);
+			strbuf_addch(&pathbuf, '/');
+		}
+		strbuf_addbuf(&pathbuf, &buf);
+
+		strbuf_reset(&buf);
+		if (!strbuf_realpath(&buf, pathbuf.buf, 0)) {
+			error(_("unable to normalize alternate object path: %s"),
+			      pathbuf.buf);
+			continue;
+		}
+
+		/*
+		 * The trailing slash after the directory name is given by
+		 * this function at the end. Remove duplicates.
+		 */
+		while (buf.len && buf.buf[buf.len - 1] == '/')
+			strbuf_setlen(&buf, buf.len - 1);
+
 		strvec_push(out, buf.buf);
 	}
 
+	strbuf_release(&pathbuf);
 	strbuf_release(&buf);
 }
 
@@ -270,10 +270,10 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
 		return;
 	}
 
-	parse_alternates(alt, sep, &alternates);
+	parse_alternates(alt, sep, relative_base, &alternates);
 
 	for (size_t i = 0; i < alternates.nr; i++)
-		link_alt_odb_entry(odb, alternates.v[i], relative_base, depth);
+		odb_add_alternate_recursively(odb, alternates.v[i], depth);
 
 	strvec_clear(&alternates);
 }
@@ -348,7 +348,7 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
 	 * overwritten when they are.
 	 */
 	odb_prepare_alternates(odb);
-	return link_alt_odb_entry(odb, dir, NULL, 0);
+	return odb_add_alternate_recursively(odb, dir, 0);
 }
 
 struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v3 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`

[Patrick Steinhardt]

The function `alt_odb_usable()` receives as input the object database,
the path it's supposed to determine usability for as well as the
normalized path of the main object directory of the repository. The last
part is derived by the function's caller from the object database. As we
already pass the object database to `alt_odb_usable()` it is redundant
information.

Drop the extra parameter and compute the normalized object directory in
the function itself.

While at it, rename the function to `odb_is_source_usable()` to align it
with modern terminology.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/odb.c b/odb.c
index 699bdbffd1..e314f86c3b 100644
--- a/odb.c
+++ b/odb.c
@@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,
 /*
  * Return non-zero iff the path is usable as an alternate object database.
  */
-static int alt_odb_usable(struct object_database *o, const char *path,
-			  const char *normalized_objdir)
+static bool odb_is_source_usable(struct object_database *o, const char *path)
 {
 	int r;
+	struct strbuf normalized_objdir = STRBUF_INIT;
+	bool usable = false;
+
+	strbuf_realpath(&normalized_objdir, o->sources->path, 1);
 
 	/* Detect cases where alternate disappeared */
 	if (!is_directory(path)) {
 		error(_("object directory %s does not exist; "
 			"check .git/objects/info/alternates"),
 		      path);
-		return 0;
+		goto out;
 	}
 
 	/*
@@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,
 		kh_value(o->source_by_path, p) = o->sources;
 	}
 
-	if (fspatheq(path, normalized_objdir))
-		return 0;
+	if (fspatheq(path, normalized_objdir.buf))
+		goto out;
 
 	if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))
-		return 0;
+		goto out;
+
+	usable = true;
 
-	return 1;
+out:
+	strbuf_release(&normalized_objdir);
+	return usable;
 }
 
 /*
@@ -164,13 +171,10 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 							int depth)
 {
 	struct odb_source *alternate = NULL;
-	struct strbuf tmp = STRBUF_INIT;
 	khiter_t pos;
 	int ret;
 
-	strbuf_realpath(&tmp, odb->sources->path, 1);
-
-	if (!alt_odb_usable(odb, source, tmp.buf))
+	if (!odb_is_source_usable(odb, source))
 		goto error;
 
 	alternate = odb_source_new(odb, source, false);
@@ -188,7 +192,6 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	read_info_alternates(odb, alternate->path, depth + 1);
 
  error:
-	strbuf_release(&tmp);
 	return alternate;
 }
 

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v3 4/8] odb: stop splitting alternate in `odb_add_to_alternates_file()`

[Patrick Steinhardt]

When calling `odb_add_to_alternates_file()` we know to add the newly
added source to the object database in case we have already loaded
alternates. This is done so that we can make its objects accessible
immediately without having to fully reload all alternates.

The way we do this though is to call `link_alt_odb_entries()`, which
adds _multiple_ sources to the object database source in case we have
newline-separated entries. This behaviour is not documented in the
function documentation of `odb_add_to_alternates_file()`, and all
callers only ever pass a single directory to it. It's thus entirely
surprising and a conceptual mismatch.

Fix this issue by directly calling `odb_add_alternate_recursively()`
instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/odb.c b/odb.c
index e314f86c3b..3112eab5d0 100644
--- a/odb.c
+++ b/odb.c
@@ -338,7 +338,7 @@ void odb_add_to_alternates_file(struct object_database *odb,
 		if (commit_lock_file(&lock))
 			die_errno(_("unable to move new alternates file into place"));
 		if (odb->loaded_alternates)
-			link_alt_odb_entries(odb, dir, '\n', NULL, 0);
+			odb_add_alternate_recursively(odb, dir, 0);
 	}
 	free(alts);
 }

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v3 5/8] odb: remove mutual recursion when parsing alternates

[Patrick Steinhardt]

When adding an alternative object database source we not only have to
consider the added source itself, but we also have to add _its_ sources
to our database. We implement this via mutual recursion:

  1. We first call `link_alt_odb_entries()`.

  2. `link_alt_odb_entries()` calls `parse_alternates()`.

  3. We then add each alternate via `odb_add_alternate_recursively()`.

  4. `odb_add_alternate_recursively()` calls `link_alt_odb_entries()`
     again.

This flow is somewhat hard to follow, but more importantly it means that
parsing of alternates is somewhat tied to the recursive behaviour.

Refactor the function to remove the mutual recursion between adding
sources and parsing alternates. The parsing step thus becomes completely
oblivious to the fact that there is recursive behaviour going on at all.
The recursion is handled by `odb_add_alternate_recursively()` instead,
which now recurses with itself.

This refactoring allows us to move parsing of alternates into object
database sources in a subsequent step.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 60 +++++++++++++++++++++++++++---------------------------------
 1 file changed, 27 insertions(+), 33 deletions(-)

diff --git a/odb.c b/odb.c
index 3112eab5d0..59944d4649 100644
--- a/odb.c
+++ b/odb.c
@@ -147,9 +147,8 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)
  * of the object ID, an extra slash for the first level indirection, and
  * the terminating NUL.
  */
-static void read_info_alternates(struct object_database *odb,
-				 const char *relative_base,
-				 int depth);
+static void read_info_alternates(const char *relative_base,
+				 struct strvec *out);
 
 static struct odb_source *odb_source_new(struct object_database *odb,
 					 const char *path,
@@ -171,6 +170,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 							int depth)
 {
 	struct odb_source *alternate = NULL;
+	struct strvec sources = STRVEC_INIT;
 	khiter_t pos;
 	int ret;
 
@@ -189,9 +189,17 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	kh_value(odb->source_by_path, pos) = alternate;
 
 	/* recursively add alternates */
-	read_info_alternates(odb, alternate->path, depth + 1);
+	read_info_alternates(alternate->path, &sources);
+	if (sources.nr && depth + 1 > 5) {
+		error(_("%s: ignoring alternate object stores, nesting too deep"),
+		      source);
+	} else {
+		for (size_t i = 0; i < sources.nr; i++)
+			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
+	}
 
  error:
+	strvec_clear(&sources);
 	return alternate;
 }
 
@@ -203,6 +211,9 @@ static void parse_alternates(const char *string,
 	struct strbuf pathbuf = STRBUF_INIT;
 	struct strbuf buf = STRBUF_INIT;
 
+	if (!string || !*string)
+		return;
+
 	while (*string) {
 		const char *end;
 
@@ -259,34 +270,11 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static void link_alt_odb_entries(struct object_database *odb, const char *alt,
-				 int sep, const char *relative_base, int depth)
+static void read_info_alternates(const char *relative_base,
+				 struct strvec *out)
 {
-	struct strvec alternates = STRVEC_INIT;
-
-	if (!alt || !*alt)
-		return;
-
-	if (depth > 5) {
-		error(_("%s: ignoring alternate object stores, nesting too deep"),
-				relative_base);
-		return;
-	}
-
-	parse_alternates(alt, sep, relative_base, &alternates);
-
-	for (size_t i = 0; i < alternates.nr; i++)
-		odb_add_alternate_recursively(odb, alternates.v[i], depth);
-
-	strvec_clear(&alternates);
-}
-
-static void read_info_alternates(struct object_database *odb,
-				 const char *relative_base,
-				 int depth)
-{
-	char *path;
 	struct strbuf buf = STRBUF_INIT;
+	char *path;
 
 	path = xstrfmt("%s/info/alternates", relative_base);
 	if (strbuf_read_file(&buf, path, 1024) < 0) {
@@ -294,8 +282,8 @@ static void read_info_alternates(struct object_database *odb,
 		free(path);
 		return;
 	}
+	parse_alternates(buf.buf, '\n', relative_base, out);
 
-	link_alt_odb_entries(odb, buf.buf, '\n', relative_base, depth);
 	strbuf_release(&buf);
 	free(path);
 }
@@ -622,13 +610,19 @@ int odb_for_each_alternate(struct object_database *odb,
 
 void odb_prepare_alternates(struct object_database *odb)
 {
+	struct strvec sources = STRVEC_INIT;
+
 	if (odb->loaded_alternates)
 		return;
 
-	link_alt_odb_entries(odb, odb->alternate_db, PATH_SEP, NULL, 0);
+	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
+	read_info_alternates(odb->sources->path, &sources);
+	for (size_t i = 0; i < sources.nr; i++)
+		odb_add_alternate_recursively(odb, sources.v[i], 0);
 
-	read_info_alternates(odb, odb->sources->path, 0);
 	odb->loaded_alternates = 1;
+
+	strvec_clear(&sources);
 }
 
 int odb_has_alternates(struct object_database *odb)

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v3 6/8] odb: drop forward declaration of `read_info_alternates()`

[Patrick Steinhardt]

Now that we have removed the mutual recursion in the preceding commit
it is not necessary anymore to have a forward declaration of the
`read_info_alternates()` function. Move the function and its
dependencies further up so that we can remove it.

Note that this commit also removes the function documentation of
`read_info_alternates()`. It's unclear what it's documenting, but it for
sure isn't documenting the modern behaviour of the function anymore.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 125 +++++++++++++++++++++++++++++-------------------------------------
 1 file changed, 54 insertions(+), 71 deletions(-)

diff --git a/odb.c b/odb.c
index 59944d4649..dcf4a62cd2 100644
--- a/odb.c
+++ b/odb.c
@@ -132,77 +132,6 @@ static bool odb_is_source_usable(struct object_database *o, const char *path)
 	return usable;
 }
 
-/*
- * Prepare alternate object database registry.
- *
- * The variable alt_odb_list points at the list of struct
- * odb_source.  The elements on this list come from
- * non-empty elements from colon separated ALTERNATE_DB_ENVIRONMENT
- * environment variable, and $GIT_OBJECT_DIRECTORY/info/alternates,
- * whose contents is similar to that environment variable but can be
- * LF separated.  Its base points at a statically allocated buffer that
- * contains "/the/directory/corresponding/to/.git/objects/...", while
- * its name points just after the slash at the end of ".git/objects/"
- * in the example above, and has enough space to hold all hex characters
- * of the object ID, an extra slash for the first level indirection, and
- * the terminating NUL.
- */
-static void read_info_alternates(const char *relative_base,
-				 struct strvec *out);
-
-static struct odb_source *odb_source_new(struct object_database *odb,
-					 const char *path,
-					 bool local)
-{
-	struct odb_source *source;
-
-	CALLOC_ARRAY(source, 1);
-	source->odb = odb;
-	source->local = local;
-	source->path = xstrdup(path);
-	source->loose = odb_source_loose_new(source);
-
-	return source;
-}
-
-static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
-							const char *source,
-							int depth)
-{
-	struct odb_source *alternate = NULL;
-	struct strvec sources = STRVEC_INIT;
-	khiter_t pos;
-	int ret;
-
-	if (!odb_is_source_usable(odb, source))
-		goto error;
-
-	alternate = odb_source_new(odb, source, false);
-
-	/* add the alternate entry */
-	*odb->sources_tail = alternate;
-	odb->sources_tail = &(alternate->next);
-
-	pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);
-	if (!ret)
-		BUG("source must not yet exist");
-	kh_value(odb->source_by_path, pos) = alternate;
-
-	/* recursively add alternates */
-	read_info_alternates(alternate->path, &sources);
-	if (sources.nr && depth + 1 > 5) {
-		error(_("%s: ignoring alternate object stores, nesting too deep"),
-		      source);
-	} else {
-		for (size_t i = 0; i < sources.nr; i++)
-			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
-	}
-
- error:
-	strvec_clear(&sources);
-	return alternate;
-}
-
 static void parse_alternates(const char *string,
 			     int sep,
 			     const char *relative_base,
@@ -288,6 +217,60 @@ static void read_info_alternates(const char *relative_base,
 	free(path);
 }
 
+
+static struct odb_source *odb_source_new(struct object_database *odb,
+					 const char *path,
+					 bool local)
+{
+	struct odb_source *source;
+
+	CALLOC_ARRAY(source, 1);
+	source->odb = odb;
+	source->local = local;
+	source->path = xstrdup(path);
+	source->loose = odb_source_loose_new(source);
+
+	return source;
+}
+
+static struct odb_source *odb_add_alternate_recursively(struct object_database *odb,
+							const char *source,
+							int depth)
+{
+	struct odb_source *alternate = NULL;
+	struct strvec sources = STRVEC_INIT;
+	khiter_t pos;
+	int ret;
+
+	if (!odb_is_source_usable(odb, source))
+		goto error;
+
+	alternate = odb_source_new(odb, source, false);
+
+	/* add the alternate entry */
+	*odb->sources_tail = alternate;
+	odb->sources_tail = &(alternate->next);
+
+	pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);
+	if (!ret)
+		BUG("source must not yet exist");
+	kh_value(odb->source_by_path, pos) = alternate;
+
+	/* recursively add alternates */
+	read_info_alternates(alternate->path, &sources);
+	if (sources.nr && depth + 1 > 5) {
+		error(_("%s: ignoring alternate object stores, nesting too deep"),
+		      source);
+	} else {
+		for (size_t i = 0; i < sources.nr; i++)
+			odb_add_alternate_recursively(odb, sources.v[i], depth + 1);
+	}
+
+ error:
+	strvec_clear(&sources);
+	return alternate;
+}
+
 void odb_add_to_alternates_file(struct object_database *odb,
 				const char *dir)
 {

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v3 7/8] odb: read alternates via sources

[Patrick Steinhardt]

Adapt how we read alternates so that the interface is structured around
the object database source we're reading from. This will eventually
allow us to abstract away this behaviour with pluggable object databases
so that every format can have its own mechanism for listing alternates.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/odb.c b/odb.c
index dcf4a62cd2..c5ba26b85f 100644
--- a/odb.c
+++ b/odb.c
@@ -199,19 +199,19 @@ static void parse_alternates(const char *string,
 	strbuf_release(&buf);
 }
 
-static void read_info_alternates(const char *relative_base,
-				 struct strvec *out)
+static void odb_source_read_alternates(struct odb_source *source,
+				       struct strvec *out)
 {
 	struct strbuf buf = STRBUF_INIT;
 	char *path;
 
-	path = xstrfmt("%s/info/alternates", relative_base);
+	path = xstrfmt("%s/info/alternates", source->path);
 	if (strbuf_read_file(&buf, path, 1024) < 0) {
 		warn_on_fopen_errors(path);
 		free(path);
 		return;
 	}
-	parse_alternates(buf.buf, '\n', relative_base, out);
+	parse_alternates(buf.buf, '\n', source->path, out);
 
 	strbuf_release(&buf);
 	free(path);
@@ -257,7 +257,7 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	kh_value(odb->source_by_path, pos) = alternate;
 
 	/* recursively add alternates */
-	read_info_alternates(alternate->path, &sources);
+	odb_source_read_alternates(alternate, &sources);
 	if (sources.nr && depth + 1 > 5) {
 		error(_("%s: ignoring alternate object stores, nesting too deep"),
 		      source);
@@ -599,7 +599,7 @@ void odb_prepare_alternates(struct object_database *odb)
 		return;
 
 	parse_alternates(odb->alternate_db, PATH_SEP, NULL, &sources);
-	read_info_alternates(odb->sources->path, &sources);
+	odb_source_read_alternates(odb->sources, &sources);
 	for (size_t i = 0; i < sources.nr; i++)
 		odb_add_alternate_recursively(odb, sources.v[i], 0);
 

-- 
2.52.0.270.g3f4935d65f.dirty

[PATCH v3 8/8] odb: write alternates via sources

[Patrick Steinhardt]

Refactor writing of alternates so that the actual business logic is
structured around the object database source we want to write the
alternate to. Same as with the preceding commit, this will eventually
allow us to have different logic for writing alternates depending on the
backend used.

Note that after the refactoring we start to call
`odb_add_alternate_recursively()` unconditionally. This is fine though
as we know to skip adding sources that are tracked already.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c | 51 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/odb.c b/odb.c
index c5ba26b85f..cc7f832465 100644
--- a/odb.c
+++ b/odb.c
@@ -271,25 +271,28 @@ static struct odb_source *odb_add_alternate_recursively(struct object_database *
 	return alternate;
 }
 
-void odb_add_to_alternates_file(struct object_database *odb,
-				const char *dir)
+static int odb_source_write_alternate(struct odb_source *source,
+				      const char *alternate)
 {
 	struct lock_file lock = LOCK_INIT;
-	char *alts = repo_git_path(odb->repo, "objects/info/alternates");
+	char *path = xstrfmt("%s/%s", source->path, "info/alternates");
 	FILE *in, *out;
 	int found = 0;
+	int ret;
 
-	hold_lock_file_for_update(&lock, alts, LOCK_DIE_ON_ERROR);
+	hold_lock_file_for_update(&lock, path, LOCK_DIE_ON_ERROR);
 	out = fdopen_lock_file(&lock, "w");
-	if (!out)
-		die_errno(_("unable to fdopen alternates lockfile"));
+	if (!out) {
+		ret = error_errno(_("unable to fdopen alternates lockfile"));
+		goto out;
+	}
 
-	in = fopen(alts, "r");
+	in = fopen(path, "r");
 	if (in) {
 		struct strbuf line = STRBUF_INIT;
 
 		while (strbuf_getline(&line, in) != EOF) {
-			if (!strcmp(dir, line.buf)) {
+			if (!strcmp(alternate, line.buf)) {
 				found = 1;
 				break;
 			}
@@ -298,20 +301,36 @@ void odb_add_to_alternates_file(struct object_database *odb,
 
 		strbuf_release(&line);
 		fclose(in);
+	} else if (errno != ENOENT) {
+		ret = error_errno(_("unable to read alternates file"));
+		goto out;
 	}
-	else if (errno != ENOENT)
-		die_errno(_("unable to read alternates file"));
 
 	if (found) {
 		rollback_lock_file(&lock);
 	} else {
-		fprintf_or_die(out, "%s\n", dir);
-		if (commit_lock_file(&lock))
-			die_errno(_("unable to move new alternates file into place"));
-		if (odb->loaded_alternates)
-			odb_add_alternate_recursively(odb, dir, 0);
+		fprintf_or_die(out, "%s\n", alternate);
+		if (commit_lock_file(&lock)) {
+			ret = error_errno(_("unable to move new alternates file into place"));
+			goto out;
+		}
 	}
-	free(alts);
+
+	ret = 0;
+
+out:
+	free(path);
+	return ret;
+}
+
+void odb_add_to_alternates_file(struct object_database *odb,
+				const char *dir)
+{
+	int ret = odb_source_write_alternate(odb->sources, dir);
+	if (ret < 0)
+		die(NULL);
+	if (odb->loaded_alternates)
+		odb_add_alternate_recursively(odb, dir, 0);
 }
 
 struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,

-- 
2.52.0.270.g3f4935d65f.dirty