Re: [PATCH v4 7/9] odb/source: support writing alternates when creating the database

On Thu, Sep 10, 2026 at 04:10:31AM -0700, Karthik Nayak wrote:

Patrick Steinhardt [off-list ref] writes:

quoted

diff --git a/odb/source-files.c b/odb/source-files.c
index b7b3a297bb..8fe65d91f8 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c

@@ -64,8 +70,74 @@ static int odb_source_files_create_on_disk(struct odb_source *source)

[snip]

quoted

+		/*
+		 * The alternates file may already exist, e.g. when it has been
+		 * seeded from a template directory. Read any preexisting
+		 * entries so that we don't end up writing duplicates.
+		 */
+		orig = fopen(path.buf, "r");
+		if (orig) {
+			while (strbuf_getline(&line, orig) != EOF) {
+				strset_add(&seen, line.buf);
+				fprintf(alternates, "%s\n", line.buf);
+			}
+
+			if (ferror(orig)) {
+				ret = error_errno(_("unable to read alternates file"));
+				fclose(orig);
+				goto out;
+			}

Shouldn't this be checked inside the for loop with every `fprintf` call?

There isn't really any need to, as the error indicator on file streams
is sticky. Sure, it would allow us to potentially abort earlier. But
it's unlikely that this really matters in practice.

quoted

diff --git a/odb/source.h b/odb/source.h
index ea8675247e..63f1c0c531 100644
--- a/odb/source.h
+++ b/odb/source.h

@@ -36,6 +36,15 @@ struct object_id;
 struct odb_stream;
 struct strvec;

+struct odb_create_on_disk_options {
+	/*
+	 * Alternates that shall be written into the newly created object
+	 * database. Whether or not this option can be handled is specific to
+	 * the backend.
+	 */

Would it make sense to formalize errors thrown by backends, so we know
when a backend specifically cannot handle alternates?

Maybe, but there's nothing that'd use it. So I'm a bit hesitant to
introduce that right now without us having a way to verify the logic at
all. We may want to eventually introduce such logic though.

Patrick