From 595bef7180b57889a4dec4b675a7fc6084c863ac Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 11 Aug 2025 15:46:41 +0200 Subject: odb: store locality in object database sources Object database sources are classified either as: - Local, which means that the source is the repository's primary source. This is typically ".git/objects". - Non-local, which is everything else. Most importantly this includes alternates and quarantine directories. This locality is often computed ad-hoc by checking whether a given object source is the first one. This works, but it is quite roundabout. Refactor the code so that we store locality when creating the sources in the first place. This makes it both more accessible and robust. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 1 + 1 file changed, 1 insertion(+) (limited to 'odb.c') diff --git a/odb.c b/odb.c index 1f48a0448e..1761a50840 100644 --- a/odb.c +++ b/odb.c @@ -176,6 +176,7 @@ static int link_alt_odb_entry(struct object_database *odb, CALLOC_ARRAY(alternate, 1); alternate->odb = odb; + alternate->local = false; /* pathbuf.buf is already in r->objects->source_by_path */ alternate->path = strbuf_detach(&pathbuf, NULL); -- cgit v1.3 From 0d61933b8f9a0392310196578e1374283496843c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 11 Aug 2025 15:46:42 +0200 Subject: odb: allow `odb_find_source()` to fail When trying to locate a source for an unknown object directory we will die right away. In subsequent patches we will add new callsites though that want to handle this situation gracefully instead. Refactor the function to return a `NULL` pointer if the source could not be found and adapt the callsites to die instead. Introduce a new wrapper `odb_find_source_or_die()` that continues to die in case the source could not be found. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/commit-graph.c | 4 ++-- midx-write.c | 2 +- odb.c | 6 ++++++ odb.h | 7 +++++-- 4 files changed, 14 insertions(+), 5 deletions(-) (limited to 'odb.c') diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 25018a0b9d..33fb7a5145 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -101,7 +101,7 @@ static int graph_verify(int argc, const char **argv, const char *prefix, if (opts.progress) flags |= COMMIT_GRAPH_WRITE_PROGRESS; - source = odb_find_source(the_repository->objects, opts.obj_dir); + source = odb_find_source_or_die(the_repository->objects, opts.obj_dir); graph_name = get_commit_graph_filename(source); chain_name = get_commit_graph_chain_filename(source); if (open_commit_graph(graph_name, &fd, &st)) @@ -289,7 +289,7 @@ static int graph_write(int argc, const char **argv, const char *prefix, git_env_bool(GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS, 0)) flags |= COMMIT_GRAPH_WRITE_BLOOM_FILTERS; - source = odb_find_source(the_repository->objects, opts.obj_dir); + source = odb_find_source_or_die(the_repository->objects, opts.obj_dir); if (opts.reachable) { if (write_commit_graph_reachable(source, flags, &write_opts)) diff --git a/midx-write.c b/midx-write.c index c1ae62d354..d38caceadb 100644 --- a/midx-write.c +++ b/midx-write.c @@ -916,7 +916,7 @@ cleanup: static struct multi_pack_index *lookup_multi_pack_index(struct repository *r, const char *object_dir) { - struct odb_source *source = odb_find_source(r->objects, object_dir); + struct odb_source *source = odb_find_source_or_die(r->objects, object_dir); return get_multi_pack_index(source); } diff --git a/odb.c b/odb.c index 1761a50840..4e7f14be4a 100644 --- a/odb.c +++ b/odb.c @@ -464,6 +464,12 @@ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_ free(obj_dir_real); strbuf_release(&odb_path_real); + return source; +} + +struct odb_source *odb_find_source_or_die(struct object_database *odb, const char *obj_dir) +{ + struct odb_source *source = odb_find_source(odb, obj_dir); if (!source) die(_("could not find object directory matching %s"), obj_dir); return source; diff --git a/odb.h b/odb.h index f9300439ba..312921077b 100644 --- a/odb.h +++ b/odb.h @@ -186,11 +186,14 @@ struct object_database *odb_new(struct repository *repo); void odb_clear(struct object_database *o); /* - * Find source by its object directory path. Dies in case the source couldn't - * be found. + * Find source by its object directory path. Returns a `NULL` pointer in case + * the source could not be found. */ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_dir); +/* Same as `odb_find_source()`, but dies in case the source doesn't exist. */ +struct odb_source *odb_find_source_or_die(struct object_database *odb, const char *obj_dir); + /* * Replace the current writable object directory with the specified temporary * object directory; returns the former primary source. -- cgit v1.3 From 25c532f6e0797ef501ce43835fb4af4bd9c33de5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 11 Aug 2025 15:46:43 +0200 Subject: odb: consistently use "dir" to refer to alternate's directory The functions that add an alternate object directory to the object database are somewhat inconsistent in how they call the paramater that refers to the directory path: in our headers we refer to it as "dir", whereas in the implementation we often call it "reference" or "entry". Unify this and consistently call the parameter "dir". While at it, refactor `link_alt_odb_entry()` to accept a C string instead of a `struct strbuf` as parameter to clarify that we really only need the path and nothing else. Suggested-by: Taylor Blau Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'odb.c') diff --git a/odb.c b/odb.c index 4e7f14be4a..e41e3952ea 100644 --- a/odb.c +++ b/odb.c @@ -140,7 +140,7 @@ static void read_info_alternates(struct object_database *odb, int depth); static int link_alt_odb_entry(struct object_database *odb, - const struct strbuf *entry, + const char *dir, const char *relative_base, int depth, const char *normalized_objdir) @@ -151,11 +151,11 @@ static int link_alt_odb_entry(struct object_database *odb, khiter_t pos; int ret = -1; - if (!is_absolute_path(entry->buf) && relative_base) { + if (!is_absolute_path(dir) && relative_base) { strbuf_realpath(&pathbuf, relative_base, 1); strbuf_addch(&pathbuf, '/'); } - strbuf_addbuf(&pathbuf, entry); + strbuf_addstr(&pathbuf, dir); if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) { error(_("unable to normalize alternate object path: %s"), @@ -229,7 +229,7 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt, int sep, const char *relative_base, int depth) { struct strbuf objdirbuf = STRBUF_INIT; - struct strbuf entry = STRBUF_INIT; + struct strbuf dir = STRBUF_INIT; if (!alt || !*alt) return; @@ -243,13 +243,13 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt, strbuf_realpath(&objdirbuf, odb->sources->path, 1); while (*alt) { - alt = parse_alt_odb_entry(alt, sep, &entry); - if (!entry.len) + alt = parse_alt_odb_entry(alt, sep, &dir); + if (!dir.len) continue; - link_alt_odb_entry(odb, &entry, + link_alt_odb_entry(odb, dir.buf, relative_base, depth, objdirbuf.buf); } - strbuf_release(&entry); + strbuf_release(&dir); strbuf_release(&objdirbuf); } @@ -273,7 +273,7 @@ static void read_info_alternates(struct object_database *odb, } void odb_add_to_alternates_file(struct object_database *odb, - const char *reference) + const char *dir) { struct lock_file lock = LOCK_INIT; char *alts = repo_git_path(odb->repo, "objects/info/alternates"); @@ -290,7 +290,7 @@ void odb_add_to_alternates_file(struct object_database *odb, struct strbuf line = STRBUF_INIT; while (strbuf_getline(&line, in) != EOF) { - if (!strcmp(reference, line.buf)) { + if (!strcmp(dir, line.buf)) { found = 1; break; } @@ -306,18 +306,17 @@ void odb_add_to_alternates_file(struct object_database *odb, if (found) { rollback_lock_file(&lock); } else { - fprintf_or_die(out, "%s\n", reference); + 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) - link_alt_odb_entries(odb, reference, - '\n', NULL, 0); + link_alt_odb_entries(odb, dir, '\n', NULL, 0); } free(alts); } void odb_add_to_alternates_memory(struct object_database *odb, - const char *reference) + const char *dir) { /* * Make sure alternates are initialized, or else our entry may be @@ -325,8 +324,7 @@ void odb_add_to_alternates_memory(struct object_database *odb, */ odb_prepare_alternates(odb); - link_alt_odb_entries(odb, reference, - '\n', NULL, 0); + link_alt_odb_entries(odb, dir, '\n', NULL, 0); } struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, -- cgit v1.3 From a59d44ff3f0f308f9577b05c858c063d2466b061 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 11 Aug 2025 15:46:44 +0200 Subject: odb: return newly created in-memory sources Callers have no trivial way to obtain the newly created object database source when adding it to the in-memory list of alternates. While not yet needed anywhere, a subsequent commit will want to obtain that pointer. Refactor the function to return the source to make it easily accessible. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 30 ++++++++++++++++++------------ odb.h | 4 ++-- 2 files changed, 20 insertions(+), 14 deletions(-) (limited to 'odb.c') diff --git a/odb.c b/odb.c index e41e3952ea..0c808bb288 100644 --- a/odb.c +++ b/odb.c @@ -139,17 +139,16 @@ static void read_info_alternates(struct object_database *odb, const char *relative_base, int depth); -static int link_alt_odb_entry(struct object_database *odb, - const char *dir, - const char *relative_base, - int depth, - const char *normalized_objdir) +static struct odb_source *link_alt_odb_entry(struct object_database *odb, + const char *dir, + const char *relative_base, + int depth, + const char *normalized_objdir) { - struct odb_source *alternate; + struct odb_source *alternate = NULL; struct strbuf pathbuf = STRBUF_INIT; struct strbuf tmp = STRBUF_INIT; khiter_t pos; - int ret = -1; if (!is_absolute_path(dir) && relative_base) { strbuf_realpath(&pathbuf, relative_base, 1); @@ -189,11 +188,11 @@ static int link_alt_odb_entry(struct object_database *odb, /* recursively add alternates */ read_info_alternates(odb, alternate->path, depth + 1); - ret = 0; + error: strbuf_release(&tmp); strbuf_release(&pathbuf); - return ret; + return alternate; } static const char *parse_alt_odb_entry(const char *string, @@ -315,16 +314,23 @@ void odb_add_to_alternates_file(struct object_database *odb, free(alts); } -void odb_add_to_alternates_memory(struct object_database *odb, - const char *dir) +struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, + const char *dir) { + struct odb_source *alternate; + char *objdir; + /* * Make sure alternates are initialized, or else our entry may be * overwritten when they are. */ odb_prepare_alternates(odb); - link_alt_odb_entries(odb, dir, '\n', NULL, 0); + objdir = real_pathdup(odb->sources->path, 1); + alternate = link_alt_odb_entry(odb, dir, NULL, 0, objdir); + + free(objdir); + return alternate; } struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, diff --git a/odb.h b/odb.h index 312921077b..d769132699 100644 --- a/odb.h +++ b/odb.h @@ -268,8 +268,8 @@ void odb_add_to_alternates_file(struct object_database *odb, * recursive alternates it points to), but do not modify the on-disk alternates * file. */ -void odb_add_to_alternates_memory(struct object_database *odb, - const char *dir); +struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, + const char *dir); /* * Read an object from the database. Returns the object data and assigns object -- cgit v1.3 From 57363dfa0dce05aac735d5cfd626e6aac8cb706c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 11 Aug 2025 15:46:45 +0200 Subject: odb: simplify calling `link_alt_odb_entry()` Callers of `link_alt_odb_entry()` are expected to pass in three different paths: - The (potentially relative) path of the object directory that we're about to add. - The base that should be used to resolve a relative object directory path. - The resolved path to the object database's objects directory. Juggling those three paths makes the calling convention somewhat hard to grok at first. As it turns out, the third parameter is redundant: we always pass in the resolved path of the object database's primary source, and we already pass in the database itself. So instead, we can resolve that path in the function itself. One downside of this is that one caller of `link_alt_odb_entry()` calls this function in a loop, so we were able to resolve the directory a single time, only. But ultimately, we only ever end up with a rather limited number of alternates anyway, so the extra couple of cycles we save feels more like a micro optimization. Refactor the code accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) (limited to 'odb.c') diff --git a/odb.c b/odb.c index 0c808bb288..4f884e3b50 100644 --- a/odb.c +++ b/odb.c @@ -142,8 +142,7 @@ static void read_info_alternates(struct object_database *odb, static struct odb_source *link_alt_odb_entry(struct object_database *odb, const char *dir, const char *relative_base, - int depth, - const char *normalized_objdir) + int depth) { struct odb_source *alternate = NULL; struct strbuf pathbuf = STRBUF_INIT; @@ -170,7 +169,10 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb, while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/') strbuf_setlen(&pathbuf, pathbuf.len - 1); - if (!alt_odb_usable(odb, &pathbuf, normalized_objdir, &pos)) + strbuf_reset(&tmp); + strbuf_realpath(&tmp, odb->sources->path, 1); + + if (!alt_odb_usable(odb, &pathbuf, tmp.buf, &pos)) goto error; CALLOC_ARRAY(alternate, 1); @@ -227,7 +229,6 @@ static const char *parse_alt_odb_entry(const char *string, static void link_alt_odb_entries(struct object_database *odb, const char *alt, int sep, const char *relative_base, int depth) { - struct strbuf objdirbuf = STRBUF_INIT; struct strbuf dir = STRBUF_INIT; if (!alt || !*alt) @@ -239,17 +240,13 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt, return; } - strbuf_realpath(&objdirbuf, odb->sources->path, 1); - while (*alt) { alt = parse_alt_odb_entry(alt, sep, &dir); if (!dir.len) continue; - link_alt_odb_entry(odb, dir.buf, - relative_base, depth, objdirbuf.buf); + link_alt_odb_entry(odb, dir.buf, relative_base, depth); } strbuf_release(&dir); - strbuf_release(&objdirbuf); } static void read_info_alternates(struct object_database *odb, @@ -317,20 +314,12 @@ void odb_add_to_alternates_file(struct object_database *odb, struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, const char *dir) { - struct odb_source *alternate; - char *objdir; - /* * Make sure alternates are initialized, or else our entry may be * overwritten when they are. */ odb_prepare_alternates(odb); - - objdir = real_pathdup(odb->sources->path, 1); - alternate = link_alt_odb_entry(odb, dir, NULL, 0, objdir); - - free(objdir); - return alternate; + return link_alt_odb_entry(odb, dir, NULL, 0); } struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, -- cgit v1.3