Re: [PATCH v4 7/9] odb/source: support writing alternates when creating the database
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Sep 10, 2026, 11:10 UTC
- Message-ID
- <CAOLa=ZQaPstiQmXm9=TyWPUxL6X2=Lcqeg6y2XeXzSJDpq-GBA@mail.gmail.com>
- In-Reply-To
- <20260909-pks-odb-write-alternates-at-creation-time-v4-7-d8a78ffc32e4@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 46 quoted lines
> Add the ability to write alternates when creating the object database.
> This change allows us to remove the `write_alternates()` callback in a
> subsequent patch.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> odb/source-files.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> odb/source.h | 17 +++++++++---
> setup.c | 4 ++-
> 3 files changed, 91 insertions(+), 6 deletions(-)
>
> 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
> @@ -18,6 +18,7 @@
> #include "run-command.h"
> #include "strbuf.h"
> #include "string-list.h"
> +#include "strmap.h"
> #include "strvec.h"
> #include "tree.h"
> #include "write-or-die.h"
> @@ -51,9 +52,14 @@ static void odb_source_files_close(struct odb_source *source)
> odb_source_close(&files->packed->base);
> }
>
> -static int odb_source_files_create_on_disk(struct odb_source *source)
> +static int odb_source_files_create_on_disk(struct odb_source *source,
> + const struct odb_create_on_disk_options *opts)
> {
> + struct lock_file alternates_lock = LOCK_INIT;
> struct strbuf path = STRBUF_INIT;
> + struct strset seen = STRSET_INIT;
> + struct strbuf line = STRBUF_INIT;
> + int ret;
>
> safe_create_dir(source->odb->repo, source->path, 1);
>
> @@ -64,8 +70,74 @@ static int odb_source_files_create_on_disk(struct odb_source *source)
> strbuf_addf(&path, "%s/info", source->path);
> safe_create_dir(source->odb->repo, path.buf, 1);
>
> + if (opts->alternates && opts->alternates->nr) {
> + FILE *alternates, *orig;
> +So this is similar to what we already do in `odb_source_files_write_alternate()`.
Show 29 quoted lines
> + strbuf_reset(&path);
> + strbuf_addf(&path, "%s/info/alternates", source->path);
> +
> + repo_hold_lock_file_for_update(source->odb->repo, &alternates_lock,
> + path.buf, LOCK_DIE_ON_ERROR);
> +
> + alternates = fdopen_lock_file(&alternates_lock, "w");
> + if (!alternates) {
> + ret = error_errno(_("unable to fdopen alternates lockfile"));
> + goto out;
> + }
> +
> + /*
> + * 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?
Show 19 quoted lines
> +
> + fclose(orig);
> + } else if (errno != ENOENT) {
> + ret = error_errno(_("unable to read alternates file"));
> + goto out;
> + }
> +
> + for (size_t i = 0; i < opts->alternates->nr; i++) {
> + const char *alternate = opts->alternates->v[i];
> + if (!strset_add(&seen, alternate))
> + continue;
> + fprintf(alternates, "%s\n", alternate);
> + }
> +
> + if (ferror(alternates)) {
> + ret = error_errno(_("unable to write alternates file"));
> + goto out;
> + }
> +same here.
Show 35 quoted lines
> + if (commit_lock_file(&alternates_lock)) {
> + ret = error_errno(_("unable to commit alternates file"));
> + goto out;
> + }
> + }
> +
> + /* Reprepare the object database to activate alternates. */
> + odb_reprepare(source->odb);
> +
> + ret = 0;
> +
> +out:
> + rollback_lock_file(&alternates_lock);
> + strbuf_release(&line);
> strbuf_release(&path);
> - return 0;
> + strset_clear(&seen);
> + return ret;
> }
>
> static void odb_source_files_prepare(struct odb_source *source,
> 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?
> + const struct strvec *alternates; > +}; > +
[snip]