From b4dedbc0615bddbad6a5bd18345d9da766d95dfa Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:44 +0200 Subject: [PATCH 01/27] cache-tree: drop `the_repository` in `cache_tree_fully_valid()` The function `cache_tree_fully_valid()` verifies whether the cache tree owned by the index is valid or not. As part of that, the function checks whether the objects referenced by the cache all exist. But because the function has no repository available, it is using the object database of `the_repository` instead. We could of course adapt callers to pass in a repository as parameter explicitly to get rid of this implicit dependency on global state. But all of them pass the cache tree owned by a `struct index_state`, and that structure already has a reference to its owning repository. So instead, adapt the function to accept a `struct index_state`, which ensures that callers will implicitly always pass the correct repository. Adapt callers accordingly. Suggested-by: Junio C Hamano Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/checkout.c | 2 +- builtin/commit.c | 2 +- cache-tree.c | 17 ++++++++++++----- cache-tree.h | 2 +- sequencer.c | 2 +- sparse-index.c | 2 +- unpack-trees.c | 2 +- 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 55e3a89a852712..505d3f7bf33cdf 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -921,7 +921,7 @@ static int merge_working_tree(const struct checkout_opts *opts, } } - if (!cache_tree_fully_valid(the_repository->index->cache_tree)) + if (!cache_tree_fully_valid(the_repository->index)) cache_tree_update(the_repository->index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR); if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK)) diff --git a/builtin/commit.c b/builtin/commit.c index 28f61745034506..840b6b4083b945 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -484,7 +484,7 @@ static const char *prepare_index(const char **argv, const char *prefix, LOCK_DIE_ON_ERROR); refresh_cache_or_die(refresh_flags); if (the_repository->index->cache_changed - || !cache_tree_fully_valid(the_repository->index->cache_tree)) + || !cache_tree_fully_valid(the_repository->index)) cache_tree_update(the_repository->index, WRITE_TREE_SILENT); if (write_locked_index(the_repository->index, &index_lock, COMMIT_LOCK | SKIP_IF_UNCHANGED)) diff --git a/cache-tree.c b/cache-tree.c index a220372a420197..6103b3fcb30000 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -275,22 +275,29 @@ static void discard_unused_subtrees(struct cache_tree *it) } } -int cache_tree_fully_valid(struct cache_tree *it) +static int cache_tree_fully_valid_recursive(struct object_database *odb, + struct cache_tree *it) { int i; if (!it) return 0; if (it->entry_count < 0 || - !odb_has_object(the_repository->objects, &it->oid, + !odb_has_object(odb, &it->oid, ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return 0; for (i = 0; i < it->subtree_nr; i++) { - if (!cache_tree_fully_valid(it->down[i]->cache_tree)) + if (!cache_tree_fully_valid_recursive(odb, it->down[i]->cache_tree)) return 0; } return 1; } +int cache_tree_fully_valid(struct index_state *istate) +{ + return cache_tree_fully_valid_recursive(istate->repo->objects, + istate->cache_tree); +} + static int must_check_existence(const struct cache_entry *ce) { return !(repo_has_promisor_remote(the_repository) && ce_skip_worktree(ce)); @@ -775,7 +782,7 @@ struct tree *write_in_core_index_as_tree(struct repository *repo, int was_valid, ret; was_valid = index_state->cache_tree && - cache_tree_fully_valid(index_state->cache_tree); + cache_tree_fully_valid(index_state); ret = write_index_as_tree_internal(&o, index_state, was_valid, 0, NULL); if (ret == WRITE_TREE_UNMERGED_INDEX) { @@ -811,7 +818,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state, was_valid = !(flags & WRITE_TREE_IGNORE_CACHE_TREE) && index_state->cache_tree && - cache_tree_fully_valid(index_state->cache_tree); + cache_tree_fully_valid(index_state); ret = write_index_as_tree_internal(oid, index_state, was_valid, flags, prefix); diff --git a/cache-tree.h b/cache-tree.h index f8bddae5235f54..4b3f60d6db481e 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -31,7 +31,7 @@ int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen) void cache_tree_write(struct strbuf *, struct cache_tree *root); struct cache_tree *cache_tree_read(const char *buffer, unsigned long size); -int cache_tree_fully_valid(struct cache_tree *); +int cache_tree_fully_valid(struct index_state *); int cache_tree_update(struct index_state *, int); int cache_tree_verify(struct repository *, struct index_state *); diff --git a/sequencer.c b/sequencer.c index 65afd100d98e61..11a95c031b3556 100644 --- a/sequencer.c +++ b/sequencer.c @@ -814,7 +814,7 @@ static int do_recursive_merge(struct repository *r, static struct object_id *get_cache_tree_oid(struct index_state *istate) { - if (!cache_tree_fully_valid(istate->cache_tree)) + if (!cache_tree_fully_valid(istate)) if (cache_tree_update(istate, 0)) { error(_("unable to update cache tree")); return NULL; diff --git a/sparse-index.c b/sparse-index.c index c1fa231a89fc07..3d77dadae56d08 100644 --- a/sparse-index.c +++ b/sparse-index.c @@ -228,7 +228,7 @@ int convert_to_sparse(struct index_state *istate, int flags) if (index_has_unmerged_entries(istate)) return 0; - if (!cache_tree_fully_valid(istate->cache_tree)) { + if (!cache_tree_fully_valid(istate)) { /* Clear and recompute the cache-tree */ cache_tree_free(&istate->cache_tree); diff --git a/unpack-trees.c b/unpack-trees.c index 154d6d40a15934..f6bb1e6d2bf541 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -2086,7 +2086,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options } if (!o->skip_cache_tree_update && - !cache_tree_fully_valid(o->internal.result.cache_tree)) + !cache_tree_fully_valid(&o->internal.result)) cache_tree_update(&o->internal.result, WRITE_TREE_SILENT | WRITE_TREE_REPAIR); From f9de4b7ff70a70d8e5c4abca418756e959aaea85 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:45 +0200 Subject: [PATCH 02/27] cache-tree: remove dependency on `the_repository` The "cache-tree" subsystem still depends on `the_repository`. Adapt it to instead use repositories provided via the context, either as a new parameter or the one passed in via `struct index_state`. Besides getting rid of `the_repository`, this also removes the last dependency on registering submodule sources with the main object database. When reading gitmodules from a submodule's index we implicitly read that object via `the_repository`'s object database, which is of course wrong. This works though because we would then register the submodule's object database with the main object database, but a later patch is going to get rid of that mechanism. You can verify that we indeed no longer depend on this mechanism by running tests with `GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=true`. Without this patch we fail in t1092, with this patch we never register submodule object databases anymore. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- cache-tree.c | 78 ++++++++++++++++++++++++++----------------------- cache-tree.h | 5 ++-- read-cache-ll.h | 5 ++-- read-cache.c | 9 +++--- unpack-trees.c | 7 +++-- 5 files changed, 57 insertions(+), 47 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 6103b3fcb30000..b8cbb5da221020 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -1,4 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" @@ -298,12 +297,14 @@ int cache_tree_fully_valid(struct index_state *istate) istate->cache_tree); } -static int must_check_existence(const struct cache_entry *ce) +static int must_check_existence(const struct cache_entry *ce, void *cb_data) { - return !(repo_has_promisor_remote(the_repository) && ce_skip_worktree(ce)); + struct repository *repo = cb_data; + return !(repo_has_promisor_remote(repo) && ce_skip_worktree(ce)); } -static int update_one(struct cache_tree *it, +static int update_one(struct repository *repo, + struct cache_tree *it, struct cache_entry **cache, int entries, const char *base, @@ -341,7 +342,7 @@ static int update_one(struct cache_tree *it, } if (0 <= it->entry_count && - odb_has_object(the_repository->objects, &it->oid, + odb_has_object(repo->objects, &it->oid, ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return it->entry_count; @@ -382,7 +383,8 @@ static int update_one(struct cache_tree *it, sub = find_subtree(it, path + baselen, sublen, 1); if (!sub->cache_tree) sub->cache_tree = cache_tree(); - subcnt = update_one(sub->cache_tree, + subcnt = update_one(repo, + sub->cache_tree, cache + i, entries - i, path, baselen + sublen + 1, @@ -446,10 +448,10 @@ static int update_one(struct cache_tree *it, } ce_missing_ok = mode == S_IFGITLINK || missing_ok || - !must_check_existence(ce); + !must_check_existence(ce, repo); if (is_null_oid(oid) || (!ce_missing_ok && - !odb_has_object(the_repository->objects, oid, + !odb_has_object(repo->objects, oid, ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))) { strbuf_release(&buffer); if (expected_missing) @@ -481,12 +483,12 @@ static int update_one(struct cache_tree *it, /* * "sub" can be an empty tree if all subentries are i-t-a. */ - if (contains_ita && is_empty_tree_oid(oid, the_repository->hash_algo)) + if (contains_ita && is_empty_tree_oid(oid, repo->hash_algo)) continue; strbuf_grow(&buffer, entlen + 100); strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0'); - strbuf_add(&buffer, oid->hash, the_hash_algo->rawsz); + strbuf_add(&buffer, oid->hash, repo->hash_algo->rawsz); #if DEBUG_CACHE_TREE fprintf(stderr, "cache-tree update-one %o %.*s\n", @@ -496,16 +498,16 @@ static int update_one(struct cache_tree *it, if (repair) { struct object_id oid; - hash_object_file(the_hash_algo, buffer.buf, buffer.len, + hash_object_file(repo->hash_algo, buffer.buf, buffer.len, OBJ_TREE, &oid); - if (odb_has_object(the_repository->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED)) + if (odb_has_object(repo->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED)) oidcpy(&it->oid, &oid); else to_invalidate = 1; } else if (dryrun) { - hash_object_file(the_hash_algo, buffer.buf, buffer.len, + hash_object_file(repo->hash_algo, buffer.buf, buffer.len, OBJ_TREE, &it->oid); - } else if (odb_write_object_ext(the_repository->objects, buffer.buf, buffer.len, OBJ_TREE, + } else if (odb_write_object_ext(repo->objects, buffer.buf, buffer.len, OBJ_TREE, &it->oid, NULL, flags & WRITE_TREE_SILENT ? ODB_WRITE_OBJECT_SILENT : 0)) { strbuf_release(&buffer); return -1; @@ -523,7 +525,7 @@ static int update_one(struct cache_tree *it, int cache_tree_update(struct index_state *istate, int flags) { - int inflight = !!the_repository->objects->transaction; + int inflight = !!istate->repo->objects->transaction; struct odb_transaction *transaction; int skip, i; @@ -535,14 +537,14 @@ int cache_tree_update(struct index_state *istate, int flags) if (!istate->cache_tree) istate->cache_tree = cache_tree(); - if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(the_repository)) - prefetch_cache_entries(istate, must_check_existence); + if (!(flags & WRITE_TREE_MISSING_OK) && repo_has_promisor_remote(istate->repo)) + prefetch_cache_entries(istate, must_check_existence, istate->repo); trace_performance_enter(); trace2_region_enter("cache_tree", "update", istate->repo); if (!inflight) - odb_transaction_begin_or_die(the_repository->objects, &transaction, 0); - i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, + odb_transaction_begin_or_die(istate->repo->objects, &transaction, 0); + i = update_one(istate->repo, istate->cache_tree, istate->cache, istate->cache_nr, "", 0, &skip, flags); if (!inflight) odb_transaction_commit_and_finalize_or_die(transaction); @@ -554,7 +556,8 @@ int cache_tree_update(struct index_state *istate, int flags) return 0; } -static void write_one(struct strbuf *buffer, struct cache_tree *it, +static void write_one(struct repository *repo, + struct strbuf *buffer, struct cache_tree *it, const char *path, int pathlen) { int i; @@ -580,7 +583,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it, #endif if (0 <= it->entry_count) { - strbuf_add(buffer, it->oid.hash, the_hash_algo->rawsz); + strbuf_add(buffer, it->oid.hash, repo->hash_algo->rawsz); } for (i = 0; i < it->subtree_nr; i++) { struct cache_tree_sub *down = it->down[i]; @@ -590,15 +593,16 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it, prev->name, prev->namelen) <= 0) die("fatal - unsorted cache subtree"); } - write_one(buffer, down->cache_tree, down->name, down->namelen); + write_one(repo, buffer, down->cache_tree, down->name, down->namelen); } } -void cache_tree_write(struct strbuf *sb, struct cache_tree *root) +void cache_tree_write(struct repository *repo, + struct strbuf *sb, struct cache_tree *root) { - trace2_region_enter("cache_tree", "write", the_repository); - write_one(sb, root, "", 0); - trace2_region_leave("cache_tree", "write", the_repository); + trace2_region_enter("cache_tree", "write", repo); + write_one(repo, sb, root, "", 0); + trace2_region_leave("cache_tree", "write", repo); } static int parse_int(const char **ptr, unsigned long *len_p, int *out) @@ -632,13 +636,14 @@ static int parse_int(const char **ptr, unsigned long *len_p, int *out) return 0; } -static struct cache_tree *read_one(const char **buffer, unsigned long *size_p) +static struct cache_tree *read_one(struct repository *repo, + const char **buffer, unsigned long *size_p) { const char *buf = *buffer; unsigned long size = *size_p; struct cache_tree *it; int i, subtree_nr; - const unsigned rawsz = the_hash_algo->rawsz; + const unsigned rawsz = repo->hash_algo->rawsz; it = NULL; /* skip name, but make sure name exists */ @@ -665,7 +670,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p) if (size < rawsz) goto free_return; oidread(&it->oid, (const unsigned char *)buf, - the_repository->hash_algo); + repo->hash_algo); buf += rawsz; size -= rawsz; } @@ -693,7 +698,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p) struct cache_tree_sub *subtree; const char *name = buf; - sub = read_one(&buf, &size); + sub = read_one(repo, &buf, &size); if (!sub) goto free_return; subtree = cache_tree_sub(it, name); @@ -710,16 +715,17 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p) return NULL; } -struct cache_tree *cache_tree_read(const char *buffer, unsigned long size) +struct cache_tree *cache_tree_read(struct repository *repo, + const char *buffer, unsigned long size) { struct cache_tree *result; if (buffer[0]) return NULL; /* not the whole tree */ - trace2_region_enter("cache_tree", "read", the_repository); - result = read_one(&buffer, &size); - trace2_region_leave("cache_tree", "read", the_repository); + trace2_region_enter("cache_tree", "read", repo); + result = read_one(repo, &buffer, &size); + trace2_region_leave("cache_tree", "read", repo); return result; } @@ -810,7 +816,7 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state, hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR); entries = read_index_from(index_state, index_path, - repo_get_git_dir(the_repository)); + repo_get_git_dir(index_state->repo)); if (entries < 0) { ret = WRITE_TREE_UNREADABLE_INDEX; goto out; @@ -866,7 +872,7 @@ static void prime_cache_tree_rec(struct repository *r, struct cache_tree_sub *sub; struct tree *subtree = lookup_tree(r, &entry.oid); - if (repo_parse_tree(the_repository, subtree) < 0) + if (repo_parse_tree(r, subtree) < 0) exit(128); sub = cache_tree_sub(it, entry.path); sub->cache_tree = cache_tree(); diff --git a/cache-tree.h b/cache-tree.h index 4b3f60d6db481e..7a2177de83eef9 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -28,8 +28,9 @@ struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *); int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen); -void cache_tree_write(struct strbuf *, struct cache_tree *root); -struct cache_tree *cache_tree_read(const char *buffer, unsigned long size); +void cache_tree_write(struct repository *repo, struct strbuf *, struct cache_tree *root); +struct cache_tree *cache_tree_read(struct repository *repo, + const char *buffer, unsigned long size); int cache_tree_fully_valid(struct index_state *); int cache_tree_update(struct index_state *, int); diff --git a/read-cache-ll.h b/read-cache-ll.h index 8eb266cfd13308..066dd8bc3b8397 100644 --- a/read-cache-ll.h +++ b/read-cache-ll.h @@ -269,9 +269,10 @@ void validate_cache_entries(const struct index_state *istate); * the given predicate. This function should only be called if * repo_has_promisor_remote() returns true. */ -typedef int (*must_prefetch_predicate)(const struct cache_entry *); +typedef int (*must_prefetch_predicate)(const struct cache_entry *, void *cb_data); void prefetch_cache_entries(const struct index_state *istate, - must_prefetch_predicate must_prefetch); + must_prefetch_predicate must_prefetch, + void *cb_data); /* Initialize and use the cache information */ struct lock_file; diff --git a/read-cache.c b/read-cache.c index 8044ff820b7d72..e40f290bb385e9 100644 --- a/read-cache.c +++ b/read-cache.c @@ -1748,7 +1748,7 @@ static int read_index_extension(struct index_state *istate, { switch (CACHE_EXT(ext)) { case CACHE_EXT_TREE: - istate->cache_tree = cache_tree_read(data, sz); + istate->cache_tree = cache_tree_read(istate->repo, data, sz); break; case CACHE_EXT_RESOLVE_UNDO: istate->resolve_undo = resolve_undo_read(data, sz, the_hash_algo); @@ -3012,7 +3012,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, !drop_cache_tree && istate->cache_tree) { strbuf_reset(&sb); - cache_tree_write(&sb, istate->cache_tree); + cache_tree_write(istate->repo, &sb, istate->cache_tree); err = write_index_ext_header(f, eoie_c, CACHE_EXT_TREE, sb.len) < 0; hashwrite(f, sb.buf, sb.len); if (err) { @@ -3733,7 +3733,8 @@ static void write_ieot_extension(struct strbuf *sb, struct index_entry_offset_ta } void prefetch_cache_entries(const struct index_state *istate, - must_prefetch_predicate must_prefetch) + must_prefetch_predicate must_prefetch, + void *cb_data) { int i; struct oid_array to_fetch = OID_ARRAY_INIT; @@ -3741,7 +3742,7 @@ void prefetch_cache_entries(const struct index_state *istate, for (i = 0; i < istate->cache_nr; i++) { struct cache_entry *ce = istate->cache[i]; - if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce)) + if (S_ISGITLINK(ce->ce_mode) || !must_prefetch(ce, cb_data)) continue; if (!odb_read_object_info_extended(the_repository->objects, &ce->oid, NULL, diff --git a/unpack-trees.c b/unpack-trees.c index f6bb1e6d2bf541..1802809ad3c50a 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -416,7 +416,8 @@ static void report_collided_checkout(struct index_state *index) string_list_clear(&list, 0); } -static int must_checkout(const struct cache_entry *ce) +static int must_checkout(const struct cache_entry *ce, + void *cb_data UNUSED) { return ce->ce_flags & CE_UPDATE; } @@ -477,7 +478,7 @@ static int check_updates(struct unpack_trees_options *o, * Prefetch the objects that are to be checked out in the loop * below. */ - prefetch_cache_entries(index, must_checkout); + prefetch_cache_entries(index, must_checkout, NULL); get_parallel_checkout_configs(&pc_workers, &pc_threshold); @@ -487,7 +488,7 @@ static int check_updates(struct unpack_trees_options *o, for (i = 0; i < index->cache_nr; i++) { struct cache_entry *ce = index->cache[i]; - if (must_checkout(ce)) { + if (must_checkout(ce, NULL)) { size_t last_pc_queue_size = pc_queue_size(); if (ce->ce_flags & CE_WT_REMOVE) From 8751a0ffc30dc91233f968d064fe913b3eeba885 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:46 +0200 Subject: [PATCH 03/27] submodule-config: remove uses of `the_repository` Several functions in the submodule-config subsystem implicitly depend on `the_repository`. Refactor these to take a `struct repository` as parameter and adapt callers accordingly. Note that as usual with these refactorings, callers simply pass `the_repository` even if they already have a different repository available in the calling context. This simplifies the migration and ensures that we don't have a change in behaviour. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fetch.c | 2 +- builtin/grep.c | 2 +- builtin/submodule--helper.c | 8 +++--- submodule-config.c | 49 +++++++++++++++++++++---------------- submodule-config.h | 12 ++++++--- submodule.c | 2 +- t/helper/test-submodule.c | 4 +-- 7 files changed, 45 insertions(+), 34 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ab7db2be06d14c..533fdfe7d80e9d 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2681,7 +2681,7 @@ int cmd_fetch(int argc, int *rs = config.recurse_submodules == RECURSE_SUBMODULES_DEFAULT ? &config.recurse_submodules : NULL; - fetch_config_from_gitmodules(sfjc, rs); + fetch_config_from_gitmodules(the_repository, sfjc, rs); } diff --git a/builtin/grep.c b/builtin/grep.c index d3d86abe01e032..073dfaaf451c7b 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -897,7 +897,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec, if (recurse_submodules) { submodule_free(opt->repo); obj_read_lock(); - gitmodules_config_oid(&real_obj->oid); + gitmodules_config_oid(the_repository, &real_obj->oid); obj_read_unlock(); } if (grep_object(opt, pathspec, real_obj, list->objects[i].name, diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index e7cd3225fa84c4..aaaa963fd88d5a 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -3041,7 +3041,7 @@ static int module_update(int argc, const char **argv, const char *prefix, NULL }; - update_clone_config_from_gitmodules(&opt.max_jobs); + update_clone_config_from_gitmodules(the_repository, &opt.max_jobs); repo_config(the_repository, git_update_clone_config, &opt.max_jobs); argc = parse_options(argc, argv, prefix, module_update_options, @@ -3255,7 +3255,7 @@ static int module_set_url(int argc, const char **argv, const char *prefix, path); config_name = xstrfmt("submodule.%s.url", sub->name); - ret = config_set_in_gitmodules_file_gently(config_name, newurl); + ret = config_set_in_gitmodules_file_gently(the_repository, config_name, newurl); if (!ret) { repo_read_gitmodules(the_repository, 0); @@ -3311,7 +3311,7 @@ static int module_set_branch(int argc, const char **argv, const char *prefix, path); config_name = xstrfmt("submodule.%s.branch", sub->name); - ret = config_set_in_gitmodules_file_gently(config_name, opt_branch); + ret = config_set_in_gitmodules_file_gently(the_repository, config_name, opt_branch); free(config_name); return !!ret; @@ -3510,7 +3510,7 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con die(_("please make sure that the .gitmodules file is in the working tree")); key = xstrfmt("submodule.%s.%s", name, var); - ret = config_set_in_gitmodules_file_gently(key, value); + ret = config_set_in_gitmodules_file_gently(the_repository, key, value); free(key); return ret; diff --git a/submodule-config.c b/submodule-config.c index f75997402a189b..f8c2cf7a9369d3 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -667,19 +667,20 @@ static int parse_config(const char *var, const char *value, return ret; } -static int gitmodule_oid_from_commit(const struct object_id *treeish_name, +static int gitmodule_oid_from_commit(struct repository *repo, + const struct object_id *treeish_name, struct object_id *gitmodules_oid, struct strbuf *rev) { int ret = 0; if (is_null_oid(treeish_name)) { - oidclr(gitmodules_oid, the_repository->hash_algo); + oidclr(gitmodules_oid, repo->hash_algo); return 1; } strbuf_addf(rev, "%s:.gitmodules", oid_to_hex(treeish_name)); - if (repo_get_oid(the_repository, rev->buf, gitmodules_oid) >= 0) + if (repo_get_oid(repo, rev->buf, gitmodules_oid) >= 0) ret = 1; return ret; @@ -689,9 +690,11 @@ static int gitmodule_oid_from_commit(const struct object_id *treeish_name, * (key) with on-demand reading of the appropriate .gitmodules from * revisions. */ -static const struct submodule *config_from(struct submodule_cache *cache, - const struct object_id *treeish_name, const char *key, - enum lookup_type lookup_type) +static const struct submodule *config_from(struct repository *repo, + struct submodule_cache *cache, + const struct object_id *treeish_name, + const char *key, + enum lookup_type lookup_type) { struct strbuf rev = STRBUF_INIT; size_t config_size; @@ -718,7 +721,7 @@ static const struct submodule *config_from(struct submodule_cache *cache, return entry->config; } - if (!gitmodule_oid_from_commit(treeish_name, &oid, &rev)) + if (!gitmodule_oid_from_commit(repo, treeish_name, &oid, &rev)) goto out; switch (lookup_type) { @@ -732,7 +735,7 @@ static const struct submodule *config_from(struct submodule_cache *cache, if (submodule) goto out; - config = odb_read_object(the_repository->objects, &oid, + config = odb_read_object(repo->objects, &oid, &type, &config_size); if (!config || type != OBJ_BLOB) goto out; @@ -843,21 +846,22 @@ void repo_read_gitmodules(struct repository *repo, int skip_if_read) repo->submodule_cache->gitmodules_read = 1; } -void gitmodules_config_oid(const struct object_id *commit_oid) +void gitmodules_config_oid(struct repository *repo, + const struct object_id *commit_oid) { struct strbuf rev = STRBUF_INIT; struct object_id oid; - submodule_cache_check_init(the_repository); + submodule_cache_check_init(repo); - if (gitmodule_oid_from_commit(commit_oid, &oid, &rev)) { + if (gitmodule_oid_from_commit(repo, commit_oid, &oid, &rev)) { git_config_from_blob_oid(gitmodules_cb, rev.buf, - the_repository, &oid, the_repository, + repo, &oid, repo, CONFIG_SCOPE_UNKNOWN); } strbuf_release(&rev); - the_repository->submodule_cache->gitmodules_read = 1; + repo->submodule_cache->gitmodules_read = 1; } const struct submodule *submodule_from_name(struct repository *r, @@ -865,7 +869,7 @@ const struct submodule *submodule_from_name(struct repository *r, const char *name) { repo_read_gitmodules(r, 1); - return config_from(r->submodule_cache, treeish_name, name, lookup_name); + return config_from(r, r->submodule_cache, treeish_name, name, lookup_name); } const struct submodule *submodule_from_path(struct repository *r, @@ -873,7 +877,7 @@ const struct submodule *submodule_from_path(struct repository *r, const char *path) { repo_read_gitmodules(r, 1); - return config_from(r->submodule_cache, treeish_name, path, lookup_path); + return config_from(r, r->submodule_cache, treeish_name, path, lookup_path); } /** @@ -980,11 +984,12 @@ int print_config_from_gitmodules(struct repository *repo, const char *key) return 0; } -int config_set_in_gitmodules_file_gently(const char *key, const char *value) +int config_set_in_gitmodules_file_gently(struct repository *repo, + const char *key, const char *value) { int ret; - ret = repo_config_set_in_file_gently(the_repository, GITMODULES_FILE, key, NULL, value); + ret = repo_config_set_in_file_gently(repo, GITMODULES_FILE, key, NULL, value); if (ret < 0) /* Maybe the user already did that, don't error out here */ warning(_("Could not update .gitmodules entry %s"), key); @@ -1017,13 +1022,15 @@ static int gitmodules_fetch_config(const char *var, const char *value, return 0; } -void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules) +void fetch_config_from_gitmodules(struct repository *repo, + int *max_children, + int *recurse_submodules) { struct fetch_config config = { .max_children = max_children, .recurse_submodules = recurse_submodules }; - config_from_gitmodules(gitmodules_fetch_config, the_repository, &config); + config_from_gitmodules(gitmodules_fetch_config, repo, &config); } static int gitmodules_update_clone_config(const char *var, const char *value, @@ -1036,7 +1043,7 @@ static int gitmodules_update_clone_config(const char *var, const char *value, return 0; } -void update_clone_config_from_gitmodules(int *max_jobs) +void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs) { - config_from_gitmodules(gitmodules_update_clone_config, the_repository, max_jobs); + config_from_gitmodules(gitmodules_update_clone_config, repo, max_jobs); } diff --git a/submodule-config.h b/submodule-config.h index f55d4e3b61a59b..755570d5d123c4 100644 --- a/submodule-config.h +++ b/submodule-config.h @@ -57,7 +57,8 @@ int option_fetch_parse_recurse_submodules(const struct option *opt, int parse_update_recurse_submodules_arg(const char *opt, const char *arg); int parse_push_recurse_submodules_arg(const char *opt, const char *arg); void repo_read_gitmodules(struct repository *repo, int skip_if_read); -void gitmodules_config_oid(const struct object_id *commit_oid); +void gitmodules_config_oid(struct repository *repo, + const struct object_id *commit_oid); /** * Same as submodule_from_path but lookup by name. @@ -80,7 +81,8 @@ const struct submodule *submodule_from_path(struct repository *r, void submodule_free(struct repository *r); int print_config_from_gitmodules(struct repository *repo, const char *key); -int config_set_in_gitmodules_file_gently(const char *key, const char *value); +int config_set_in_gitmodules_file_gently(struct repository *repo, + const char *key, const char *value); /* * Returns 0 if the name is syntactically acceptable as a submodule "name" @@ -100,8 +102,10 @@ int check_submodule_url(const char *url); * New helpers to retrieve arbitrary configuration from the '.gitmodules' file * should NOT be added. */ -void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules); -void update_clone_config_from_gitmodules(int *max_jobs); +void fetch_config_from_gitmodules(struct repository *repo, + int *max_children, + int *recurse_submodules); +void update_clone_config_from_gitmodules(struct repository *repo, int *max_jobs); /* * Submodule entry that contains relevant information about a diff --git a/submodule.c b/submodule.c index 5c9257588856a9..6fcb606f7ee837 100644 --- a/submodule.c +++ b/submodule.c @@ -133,7 +133,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath) strbuf_addstr(&entry, "submodule."); strbuf_addstr(&entry, submodule->name); strbuf_addstr(&entry, ".path"); - ret = config_set_in_gitmodules_file_gently(entry.buf, newpath); + ret = config_set_in_gitmodules_file_gently(the_repository, entry.buf, newpath); strbuf_release(&entry); return ret; } diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c index 3c5c4c4a090e98..ea9bef0904ba61 100644 --- a/t/helper/test-submodule.c +++ b/t/helper/test-submodule.c @@ -168,7 +168,7 @@ static int cmd__submodule_config_set(int argc, const char **argv) if (!is_writing_gitmodules_ok()) die("please make sure that the .gitmodules file is in the working tree"); - return config_set_in_gitmodules_file_gently(argv[1], argv[2]); + return config_set_in_gitmodules_file_gently(the_repository, argv[1], argv[2]); } usage_with_options(usage, options); } @@ -188,7 +188,7 @@ static int cmd__submodule_config_unset(int argc, const char **argv) if (argc == 2) { if (!is_writing_gitmodules_ok()) die("please make sure that the .gitmodules file is in the working tree"); - return config_set_in_gitmodules_file_gently(argv[1], NULL); + return config_set_in_gitmodules_file_gently(the_repository, argv[1], NULL); } usage_with_options(usage, options); } From e4ad79fd9d8984827b2dc304f1ed99fa69e1ad11 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:47 +0200 Subject: [PATCH 04/27] submodule-config: stop using `the_hash_algo` We have two uses of `the_hash_algo` in "submodule-config.c": - One trivial use in `gitmodules_cb`, which we can convert to use the hash algorithm of the repository that's already available in the caller's context. - One use where we compute the hashmap key of an object ID. We should only ever get valid, populated object IDs here, and consequently we can easily adapt that function to use the hash algorithm of the passed-in object ID. Adapt both sites accordingly. Safeguard us against the case where the passed-in object ID is _not_ properly initialized. While this case shouldn't ever happen, it doesn't hurt to be defensive. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- submodule-config.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/submodule-config.c b/submodule-config.c index f8c2cf7a9369d3..7c73fa108b3b5b 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -133,7 +133,9 @@ void submodule_cache_free(struct submodule_cache *cache) static unsigned int hash_oid_string(const struct object_id *oid, const char *string) { - return memhash(oid->hash, the_hash_algo->rawsz) + strhash(string); + if (oid->algo == GIT_HASH_UNKNOWN) + BUG("hashing an object ID with unknown algorithm"); + return memhash(oid->hash, hash_algos[oid->algo].rawsz) + strhash(string); } static void cache_put_path(struct submodule_cache *cache, @@ -824,7 +826,7 @@ static int gitmodules_cb(const char *var, const char *value, parameter.cache = repo->submodule_cache; parameter.treeish_name = NULL; - parameter.gitmodules_oid = null_oid(the_hash_algo); + parameter.gitmodules_oid = null_oid(repo->hash_algo); parameter.overwrite = 1; return parse_config(var, value, ctx, ¶meter); From a532feba892a3034d04b379e61c3cb18756007d1 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:48 +0200 Subject: [PATCH 05/27] submodule-config: stop registering submodule sources When reading the ".gitmodules" file from a blob in a repository other than `the_repository`, we register that repository's object database as an in-memory source of `the_repository`'s object database. This call has its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for reading .gitmodules, 2019-04-16): back then, `config_with_options()` was not able to read a blob from an arbitrary repository, but would always read it via `the_repository`. So even though the blob could be resolved in the submodule repository via `repo_get_oid()`, the submodule's object database had to be registered as an in-memory source of `the_repository` so that the subsequent object read was able to find the blob at all. That need went away with e3e8bf046e (submodule-config: pass repo upon blob config read, 2021-08-16), which taught the config machinery to read the blob from the repository we pass to it. The same series converted the eager submodule source registration into a lazy mechanism that only registers submodule sources with the object database when an object lookup failed. The intent though was that we don't ever have to fall back to this mechanism in the first place, and to verify that this is the case we introduced GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB. If set, then any such lazy registration would cause us to BUG. At the beginning of this series, we still triggered this bug in t1092. But now that we have converted the "cache-tree" subsystem to not depend on `the_repository` anymore it also knows to properly access objects via the submodule. With that change, GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB does not cause any failures anymore. Remove the call to `odb_add_submodule_source_by_path()`. This removes the last user of `the_repository`, so at the same time we can also get rid of `USE_THE_REPOSITORY_VARIABLE`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- submodule-config.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/submodule-config.c b/submodule-config.c index 7c73fa108b3b5b..37c3be377bcc18 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -1,4 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" @@ -803,9 +802,6 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void } else if (repo_get_oid(repo, GITMODULES_INDEX, &oid) >= 0 || repo_get_oid(repo, GITMODULES_HEAD, &oid) >= 0) { config_source.blob = oidstr = xstrdup(oid_to_hex(&oid)); - if (repo != the_repository) - odb_add_submodule_source_by_path(the_repository->objects, - repo->objects->sources->path); } else { goto out; } From 93cd344e34f6f0baab4525fbd5074d53fee4d3b9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:49 +0200 Subject: [PATCH 06/27] builtin/grep: stop registering submodule ODB as source Same as with the preceding commit, git-grep(1) registers each submodule's object database as an in-memory source of the main object database before grepping it. This was introduced as an eager alternate registration and converted into the lazy mechanism via 8d33c3af0b (grep: use submodule-ODB-as-alternate lazy-addition, 2021-08-16). Starting with 0693806bf8 (grep: add repository to OID grep sources, 2021-08-16), the command instead knows to pass submodule repositories to our workers, which means that those now use that repository to look up objects, too. As a consequence, registering submodule sources as alternates is not required anymore. Remove the logic to register submodule sources. Unfortunately, this does not allow us to get rid of the object read lock as initializing the subrepository is still racy. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/grep.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 073dfaaf451c7b..b045f8a488da04 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -463,16 +463,6 @@ static int grep_submodule(struct grep_opt *opt, ALLOC_GROW(repos_to_free, repos_to_free_nr + 1, repos_to_free_alloc); repos_to_free[repos_to_free_nr++] = subrepo; - /* - * NEEDSWORK: repo_read_gitmodules() might call - * odb_add_to_alternates_memory() via config_from_gitmodules(). This - * operation causes a race condition with concurrent object readings - * performed by the worker threads. That's why we need obj_read_lock() - * here. It should be removed once it's no longer necessary to add the - * subrepo's odbs to the in-memory alternates list. - */ - obj_read_lock(); - /* * NEEDSWORK: when reading a submodule, the sparsity settings in the * superproject are incorrectly forgotten or misused. For example: @@ -498,18 +488,14 @@ static int grep_submodule(struct grep_opt *opt, * ditto. * * Note that this list is not exhaustive. + * + * NEEDSWORK: initializing the subrepository is not thread-safe, + * either, as it may cause us to race around `get_main_ref_store()`. We + * thus need to hold the object-read lock to serialize all readers with + * one another. */ + obj_read_lock(); repo_read_gitmodules(subrepo, 0); - - /* - * All code paths tested by test code no longer need submodule ODBs to - * be added as alternates, but add it to the list just in case. - * Submodule ODBs added through add_submodule_odb_by_path() will be - * lazily registered as alternates when needed (and except in an - * unexpected code interaction, it won't be needed). - */ - odb_add_submodule_source_by_path(the_repository->objects, - subrepo->objects->sources->path); obj_read_unlock(); memcpy(&subopt, opt, sizeof(subopt)); From 5c5dfbeef6ab58debad8daaa43dfdd0370c48301 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:50 +0200 Subject: [PATCH 07/27] odb: remove infrastructure to register submodule sources The preceding commits have removed the last two users of `odb_add_submodule_source_by_path()`. The mechanism was only ever meant as a transitional crutch while migrating submodule object access away from "add the submodule ODB as an alternate of the_repository" towards explicitly passing the submodule repository, see a35e03dee0 (submodule: lazily add submodule ODBs as alternates, 2021-08-16). Remove it. As GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is now a no-op, remove its documentation and the exports from the test suite, as well. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 36 -------------------------- odb.h | 14 ---------- t/README | 7 ----- t/t5526-fetch-submodules.sh | 3 --- t/t5531-deep-submodule-push.sh | 3 --- t/t5545-push-options.sh | 3 --- t/t5572-pull-submodule.sh | 3 --- t/t6437-submodule-merge.sh | 3 --- t/t7418-submodule-sparse-gitmodules.sh | 3 --- t/t7814-grep-recurse-submodules.sh | 3 --- 10 files changed, 78 deletions(-) diff --git a/odb.c b/odb.c index 6d5943e5ea4f1a..2f8a70a90cf6df 100644 --- a/odb.c +++ b/odb.c @@ -388,12 +388,6 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha return source; } -void odb_add_submodule_source_by_path(struct object_database *odb, - const char *path) -{ - string_list_insert(&odb->submodule_source_paths, path); -} - static void fill_alternate_refs_command(struct repository *repo, struct child_process *cmd, const char *repo_path) @@ -549,23 +543,6 @@ void disable_obj_read_lock(void) pthread_mutex_destroy(&obj_read_mutex); } -static int register_all_submodule_sources(struct object_database *odb) -{ - int ret = odb->submodule_source_paths.nr; - - for (size_t i = 0; i < odb->submodule_source_paths.nr; i++) - odb_add_to_alternates_memory(odb, - odb->submodule_source_paths.items[i].string); - if (ret) { - string_list_clear(&odb->submodule_source_paths, 0); - trace2_data_intmax("submodule", odb->repo, - "register_all_submodule_sources/registered", ret); - if (git_env_bool("GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB", 0)) - BUG("register_all_submodule_sources() called"); - } - return ret; -} - static enum odb_read_status do_oid_object_info_extended(struct object_database *odb, const struct object_id *oid, struct object_info *oi, unsigned flags) @@ -614,16 +591,6 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database * } } - /* - * This might be an attempt at accessing a submodule object as - * if it were in main object store (having called - * `odb_add_submodule_source_by_path()` on that submodule's - * ODB). If any such ODBs exist, register them and try again. - */ - if (register_all_submodule_sources(odb)) - /* We added some alternates; retry */ - continue; - /* Check if it is a missing object */ if (odb->repo->fetch_if_missing && repo_has_promisor_remote(odb->repo) && !already_retried && @@ -1109,7 +1076,6 @@ struct object_database *odb_new(struct repository *repo, CALLOC_ARRAY(o, 1); o->repo = repo; pthread_mutex_init(&o->replace_mutex, NULL); - string_list_init_dup(&o->submodule_source_paths); hashmap_init(&o->source_by_path, odb_source_by_path_cmp, o, 0); o->source_paths_icase = -1; @@ -1166,8 +1132,6 @@ void odb_free(struct object_database *o) odb_close(o); odb_free_sources(o); - string_list_clear(&o->submodule_source_paths, 0); - free(o); } diff --git a/odb.h b/odb.h index 248ee9cdfaa99c..54548efc551ac1 100644 --- a/odb.h +++ b/odb.h @@ -89,12 +89,6 @@ struct object_database { unsigned long object_count; unsigned object_count_flags; unsigned object_count_valid : 1; - - /* - * Submodule source paths that will be added as additional sources to - * allow lookup of submodule objects via the main object database. - */ - struct string_list submodule_source_paths; }; enum odb_new_flags { @@ -224,14 +218,6 @@ void odb_restore_primary_source(struct object_database *odb, struct odb_source *restore_source, const char *old_path); -/* - * Call odb_add_submodule_source_by_path() to add the submodule at the given - * path to a list. The object stores of all submodules in that list will be - * added as additional sources in the object store when looking up objects. - */ -void odb_add_submodule_source_by_path(struct object_database *odb, - const char *path); - /* * Iterate through all alternates of the database and execute the provided * callback function for each of them. Stop iterating once the callback diff --git a/t/README b/t/README index 9a9daaf2afe5e2..f831c5355b4530 100644 --- a/t/README +++ b/t/README @@ -462,13 +462,6 @@ GIT_TEST_CHECKOUT_WORKERS= overrides the 'checkout.workers' setting to and 'checkout.thresholdForParallelism' to 0, forcing the execution of the parallel-checkout code. -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=, when true, makes -registering submodule ODBs as alternates a fatal action. Support for -this environment variable can be removed once the migration to -explicitly providing repositories when accessing submodule objects is -complete or needs to be abandoned for whatever reason (in which case the -migrated codepaths still retain their performance benefits). - GIT_TEST_REQUIRE_PREREQ= allows specifying a space separated list of prereqs that are required to succeed. If a prereq in this list is triggered by a test and then fails then the whole test run will abort. This can help to make diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 7b3b7359da0108..37d7373b3619d2 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -3,9 +3,6 @@ test_description='Recursive "git fetch" for submodules' -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 -export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB - . ./test-lib.sh pwd=$(pwd) diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh index 7d239dd31f5ef6..73429ec6e3ba2e 100755 --- a/t/t5531-deep-submodule-push.sh +++ b/t/t5531-deep-submodule-push.sh @@ -5,9 +5,6 @@ test_description='test push with submodules' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 -export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB - . ./test-lib.sh test_expect_success setup ' diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh index fb13549da7f305..239edd7d62f303 100755 --- a/t/t5545-push-options.sh +++ b/t/t5545-push-options.sh @@ -5,9 +5,6 @@ test_description='pushing to a repository using push options' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 -export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB - . ./test-lib.sh mk_repo_pair () { diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh index 42d14328b6b42a..9969a3294ee3bf 100755 --- a/t/t5572-pull-submodule.sh +++ b/t/t5572-pull-submodule.sh @@ -2,9 +2,6 @@ test_description='pull can handle submodules' -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 -export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB - . ./test-lib.sh . "$TEST_DIRECTORY"/lib-submodule-update.sh diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh index 107e13afbcd6c9..1546d5f773396a 100755 --- a/t/t6437-submodule-merge.sh +++ b/t/t6437-submodule-merge.sh @@ -5,9 +5,6 @@ test_description='merging with submodules' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 -export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB - . ./test-lib.sh # diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh index dde11ecce806c4..cf94e30e780b47 100755 --- a/t/t7418-submodule-sparse-gitmodules.sh +++ b/t/t7418-submodule-sparse-gitmodules.sh @@ -12,9 +12,6 @@ The test setup uses a sparse checkout, however the same scenario can be set up also by committing .gitmodules and then just removing it from the filesystem. ' -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 -export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB - . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh index e1cf53dc9eb1a5..3d149d34c15dd9 100755 --- a/t/t7814-grep-recurse-submodules.sh +++ b/t/t7814-grep-recurse-submodules.sh @@ -9,9 +9,6 @@ submodules. TEST_CREATE_REPO_NO_TEMPLATE=1 . ./test-lib.sh -GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 -export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB - test_expect_success 'setup directory structure and submodule' ' echo "(1|2)d(3|4)" >a && mkdir b && From 60942e28b3c49f462c820ec95f30624464a11764 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:51 +0200 Subject: [PATCH 08/27] tmp-objdir: drop unused function to register alternate The last caller of `tmp_objdir_add_as_alternate()` went away in bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB transactions, 2026-07-10) and is unused now. Remove the function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- tmp-objdir.c | 5 ----- tmp-objdir.h | 6 ------ 2 files changed, 11 deletions(-) diff --git a/tmp-objdir.c b/tmp-objdir.c index 0eaa79ffd7cd44..deaaf6ba2ea315 100644 --- a/tmp-objdir.c +++ b/tmp-objdir.c @@ -321,11 +321,6 @@ const char **tmp_objdir_env(const struct tmp_objdir *t) return t->env.v; } -void tmp_objdir_add_as_alternate(const struct tmp_objdir *t) -{ - odb_add_to_alternates_memory(t->repo->objects, t->path.buf); -} - struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy) { diff --git a/tmp-objdir.h b/tmp-objdir.h index 81eb9274136e69..05f0d08d1065c4 100644 --- a/tmp-objdir.h +++ b/tmp-objdir.h @@ -55,12 +55,6 @@ int tmp_objdir_destroy(struct tmp_objdir *); */ void tmp_objdir_discard_objects(struct tmp_objdir *); -/* - * Add the temporary object directory as an alternate object store in the - * current process. - */ -void tmp_objdir_add_as_alternate(const struct tmp_objdir *); - /* * Replaces the writable object store in the current process with the temporary * object directory and makes the former main object store an alternate. From a5a7e86df19a720a611d4a149fd77435414ac62f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:52 +0200 Subject: [PATCH 09/27] odb/packed: fix memory leaks when freeing source When freeing a "packed" source we don't close either its packs nor its multi-pack indices. This can cause memory leaks in case we create an ad-hoc packed source. As we used to always link packed sources to the main object database we never noticed this issue until now, but it's going to surface in subsequent commits where we stop linking them. Plug the memory leaks by closing the source first. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb/source-packed.c | 1 + 1 file changed, 1 insertion(+) diff --git a/odb/source-packed.c b/odb/source-packed.c index 1d90e714e62ed2..166e76e2d6c78d 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -844,6 +844,7 @@ static void odb_source_packed_free(struct odb_source *source) chdir_notify_unregister(odb_source_packed_reparent, packed); + odb_source_close(source); for (struct packfile_list_entry *e = packed->packs.head; e; e = e->next) free(e->pack); packfile_list_clear(&packed->packs); From 314b468c68de5a61f5d3b35ada15ee9cd6ba875d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:53 +0200 Subject: [PATCH 10/27] builtin/multi-pack-index: refuse unknown sources with "--object-dir=" Users can tell git-multi-pack-index(1) to access multi-pack indices that are stored in a different object directory via the "--object-dir=" option. This allows them to for example write or verify a multi-pack index other than the one located in the main object directory in case a repository has alternates with multiple multi-pack indices. But while the documentation explicitly points out that the specified object directory must be an alternate of the current repository, we never verify that property. Instead, starting with 017db7bb14 (midx: load multi-pack indices via their source, 2025-08-11), we now construct an ad-hoc source and link it to the main object directory. Besides contradicting the documentation, it's dubious that this really ought to work in the first place: creating a multi-pack index (and potentially a bitmap) for a completely foreign object directory is of questionable value, as bitmap commit selection operates on the invoking repository's refs. Furthermore, this is the only remaining caller outside of our test helpers that constructs an ad-hoc source and links it to the database, and we want to get rid of this mechanism as part of this series. Stop constructing the ad-hoc source and instead refuse the operation. While this results in a change in behaviour, this restriction has been documented as such ever since f57a739691 (midx: avoid opening multiple MIDXs when writing, 2021-09-01). Note that this change requires us to adapt one test chain in t5319, as it creates an object directory that is not connected to any repository and then uses it via "--object-dir=". The setup itself already documents this and does the necessary gymnastics to link the object directory to a temporary repository, but subsequent tests don't. Adapt those tests to retain and reuse the temporary repository. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/multi-pack-index.c | 3 ++- t/t5319-multi-pack-index.sh | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c index 6e73c85cde324f..753bd53a70fccc 100644 --- a/builtin/multi-pack-index.c +++ b/builtin/multi-pack-index.c @@ -90,7 +90,8 @@ static struct odb_source_files *handle_object_dir_option(struct repository *repo { struct odb_source *source = odb_find_source(repo->objects, opts.object_dir); if (!source) - source = odb_add_to_alternates_memory(repo->objects, opts.object_dir); + die(_("object directory is not an alternate of the current repository: '%s'"), + opts.object_dir); return odb_source_files_downcast(source); } diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 68143cb5b76952..00e90f163fc845 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -698,10 +698,9 @@ test_expect_success 'force some 64-bit offsets with pack-objects' ' corrupt_data $idx64 $(test_oid idxoff) "\02" && # objects64 is not a real repository, but can serve as an alternate # anyway so we can write a MIDX into it - git init repo && - test_when_finished "rm -fr repo" && + git init repo64 && ( - cd repo && + cd repo64 && ( cd ../objects64 && pwd ) >.git/objects/info/alternates && midx64=$(git multi-pack-index --object-dir=../objects64 write) ) && @@ -709,7 +708,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' ' ' test_expect_success 'verify multi-pack-index with 64-bit offsets' ' - git multi-pack-index verify --object-dir=objects64 + git -C repo64 multi-pack-index verify --object-dir=../objects64 ' NUM_OBJECTS=63 @@ -721,7 +720,7 @@ MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3)) test_expect_success 'verify incorrect 64-bit offset' ' corrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET "\07" objects64 \ - "incorrect object offset" + "incorrect object offset" "git -C repo64 multi-pack-index verify --object-dir=../objects64" ' test_expect_success 'setup expire tests' ' From f1d88b9698c4d9965fa72f1ce5739836ab97940f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:54 +0200 Subject: [PATCH 11/27] t/helper: adapt read-midx to not link ad-hoc source anymore Same as in the preceding commit, refactor the setup of ad-hoc object database sources when accessing a multi-pack index in an arbitrary location to not link the newly created source into the main object database anymore. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/helper/test-read-midx.c | 43 ++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c index 27a05da957afc2..1f7a1927e4c429 100644 --- a/t/helper/test-read-midx.c +++ b/t/helper/test-read-midx.c @@ -5,34 +5,42 @@ #include "midx.h" #include "repository.h" #include "odb.h" +#include "odb/source-packed.h" #include "pack-bitmap.h" #include "packfile.h" #include "setup.h" #include "gettext.h" #include "pack-revindex.h" -static struct multi_pack_index *setup_midx(const char *object_dir) +static struct multi_pack_index *setup_midx(const char *object_dir, + struct odb_source_packed **out) { - struct odb_source_files *files; + struct odb_source_packed *packed; struct odb_source *source; + setup_git_directory(the_repository); + source = odb_find_source(the_repository->objects, object_dir); - if (!source) - source = odb_add_to_alternates_memory(the_repository->objects, - object_dir); - files = odb_source_files_downcast(source); + if (source) { + packed = odb_source_files_downcast(source)->packed; + } else { + packed = odb_source_packed_new(the_repository->objects, + object_dir, false); + *out = packed; + } - return load_multi_pack_index(files->packed); + return load_multi_pack_index(packed); } static int read_midx_file(const char *object_dir, const char *checksum, int show_objects) { + struct odb_source_packed *packed = NULL; uint32_t i; struct multi_pack_index *m, *tip; int ret = 0; - m = tip = setup_midx(object_dir); + m = tip = setup_midx(object_dir, &packed); if (!m) return 1; @@ -91,29 +99,35 @@ static int read_midx_file(const char *object_dir, const char *checksum, out: close_midx(tip); + if (packed) + odb_source_free(&packed->base); return ret; } static int read_midx_checksum(const char *object_dir) { + struct odb_source_packed *packed = NULL; struct multi_pack_index *m; - m = setup_midx(object_dir); + m = setup_midx(object_dir, &packed); if (!m) return 1; printf("%s\n", midx_get_checksum_hex(m)); close_midx(m); + if (packed) + odb_source_free(&packed->base); return 0; } static int read_midx_preferred_pack(const char *object_dir) { + struct odb_source_packed *packed = NULL; struct multi_pack_index *midx = NULL; uint32_t preferred_pack; - midx = setup_midx(object_dir); + midx = setup_midx(object_dir, &packed); if (!midx) return 1; @@ -124,17 +138,21 @@ static int read_midx_preferred_pack(const char *object_dir) } printf("%s\n", midx->pack_names[preferred_pack]); + close_midx(midx); + if (packed) + odb_source_free(&packed->base); return 0; } static int read_midx_bitmapped_packs(const char *object_dir) { + struct odb_source_packed *packed = NULL; struct multi_pack_index *midx = NULL; struct bitmapped_pack pack; uint32_t i; - midx = setup_midx(object_dir); + midx = setup_midx(object_dir, &packed); if (!midx) return 1; @@ -150,7 +168,8 @@ static int read_midx_bitmapped_packs(const char *object_dir) } close_midx(midx); - + if (packed) + odb_source_free(&packed->base); return 0; } From 46bbc8651579a572040c41bf7ff5d698e62758f7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:55 +0200 Subject: [PATCH 12/27] t/helper: stop registering alternates in "ref-store" command When using the "ref-store" command we support access to multiple different reference stores. As part of that we allow the caller to explicitly exercise stores of a submodule. This allows us to verify low-level behaviour of submodule stores, which is exercised in t1406. When doing so we also link the submodule's object database into the main object database. The intent of this is that it allows us to access objects of the submodule, too. But that functionality is not even needed anymore: when creating a submodule reference store, we will first initialize the submodule repository and then initialize the store with that repository. And as the reference subsystem doesn't depend on `the_repository` anymore all subsequent object lookups performed by the reference store will be routed to the submodule repository. It is thus not needed anymore to register the submodule object store with the main object database. Remove the call. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- t/helper/test-ref-store.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 5a9a3053d9d81a..db58f00589f80b 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -74,14 +74,6 @@ static const char **get_store(const char **argv, struct ref_store **refs) } else if (!strcmp(argv[0], "main")) { *refs = get_main_ref_store(the_repository); } else if (skip_prefix(argv[0], "submodule:", &gitdir)) { - struct strbuf sb = STRBUF_INIT; - - if (!repo_submodule_path_append(the_repository, - &sb, gitdir, "objects/")) - die("computing submodule path failed"); - odb_add_to_alternates_memory(the_repository->objects, sb.buf); - strbuf_release(&sb); - *refs = repo_get_submodule_ref_store(the_repository, gitdir); } else if (skip_prefix(argv[0], "worktree:", &gitdir)) { struct worktree **p, **worktrees = get_worktrees(the_repository); From f0eb23a8f57d06cd461e72b35caf500145980ce8 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 07:51:56 +0200 Subject: [PATCH 13/27] odb: remove the ability to link sources ad-hoc Over the course of this patch series we have adapted all callers of `odb_add_to_alternates_memory()` to not do so anymore. Remove the function. This series of refactorings doesn't only simplify our code base. More importantly, with those changes in place we can now unconditionally assume that the list of sources linked to the object database only consists of the primary source and its alternates. This serves as the foundation to eventually move handling of alternates into the "files" backend itself. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 6 ------ odb.h | 8 -------- 2 files changed, 14 deletions(-) diff --git a/odb.c b/odb.c index 2f8a70a90cf6df..5fe081496fe5bf 100644 --- a/odb.c +++ b/odb.c @@ -247,12 +247,6 @@ void odb_add_to_alternates_file(struct object_database *odb, odb_add_alternate_recursively(odb, dir, 0); } -struct odb_source *odb_add_to_alternates_memory(struct object_database *odb, - const char *dir) -{ - return odb_add_alternate_recursively(odb, dir, 0); -} - struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, const char *dir, int will_destroy, struct odb_source **prev_source) diff --git a/odb.h b/odb.h index 54548efc551ac1..9025239df5d01a 100644 --- a/odb.h +++ b/odb.h @@ -258,14 +258,6 @@ int odb_has_alternates(struct object_database *odb); void odb_add_to_alternates_file(struct object_database *odb, const char *dir); -/* - * Add the directory to the in-memory list of alternate sources (along with any - * recursive alternates it points to), but do not modify the on-disk alternates - * file. - */ -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 * type and size to the `type` and `size` pointers, if these pointers are From 301a1ce92adc3128a3705a210c104c21bf862d41 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:25 +0200 Subject: [PATCH 14/27] builtin/fsck: use `fsck_obj_buffer()` when checking loose objects When checking loose objects we manually parse the object buffer we have read from the on-disk file, mark the object and then call `fsck_obj()`. The exact same steps are also performed by `fsck_obj_buffer()`. Stop open-coding this logic and call `fsck_obj_buffer()` instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 892c5661d93668..3c4127f4d84a9b 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -722,7 +722,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *cb_data) { struct for_each_loose_cb *data = cb_data; - struct object *obj; enum object_type type = OBJ_NONE; size_t size; void *contents = NULL; @@ -751,21 +750,7 @@ static int fsck_loose(const struct object_id *oid, const char *path, if (!contents && type != OBJ_BLOB) BUG("read_loose_object streamed a non-blob"); - obj = parse_object_buffer(data->repo, oid, type, size, - contents, &eaten); - - if (!obj) { - errors_found |= ERROR_OBJECT; - error(_("%s: object could not be parsed: %s"), - oid_to_hex(oid), path); - if (!eaten) - free(contents); - return 0; /* keep checking other objects */ - } - - obj->flags &= ~(REACHABLE | SEEN); - obj->flags |= HAS_OBJ; - if (fsck_obj(data->repo, obj, contents, size)) + if (fsck_obj_buffer(oid, type, size, contents, &eaten, data->repo)) errors_found |= ERROR_OBJECT; if (!eaten) From dbdea7a91889cf4bfc2e64f59e9889f71f3a258e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:26 +0200 Subject: [PATCH 15/27] builtin/fsck: merge `fsck_obj_buffer()` and `fsck_obj()` The interfaces of the functions `fsck_obj()` and `fsck_obj_buffer()` are somewhat similar to one another. The only difference between those two is that `fsck_obj()` takes an already-parsed object as input, whereas `fsck_obj_buffer()` parses the buffer and then calls `fsck_obj()`. Furthermore, `fsck_obj()` has no callers other than `fsck_obj_buffer()`. Refactor the code by merging those two functions. This makes it obvious which function does what, and it allows us to get rid of the early return in `fsck_obj()` in case `SEEN` is set as the only caller unconditionally clears that bit before calling it anyway. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 47 ++++++++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 3c4127f4d84a9b..bed84818930148 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -401,14 +401,27 @@ static void check_connectivity(struct repository *repo) } } -static int fsck_obj(struct repository *repo, - struct object *obj, void *buffer, unsigned long size) +static int fsck_obj_buffer(const struct object_id *oid, enum object_type type, + unsigned long size, void *buffer, int *eaten, void *cb_data) { + struct repository *repo = cb_data; + struct object *obj; int err; - if (obj->flags & SEEN) - return 0; - obj->flags |= SEEN; + /* + * Note, buffer may be NULL if type is OBJ_BLOB. See + * verify_packfile(), data_valid variable for details. + */ + obj = parse_object_buffer(repo, oid, type, size, buffer, eaten); + if (!obj) { + errors_found |= ERROR_OBJECT; + err = error(_("%s: object corrupt or missing"), + oid_to_hex(oid)); + goto out; + } + + obj->flags &= ~REACHABLE; + obj->flags |= HAS_OBJ | SEEN; if (verbose) fprintf_ln(stderr, _("Checking %s %s"), @@ -417,6 +430,7 @@ static int fsck_obj(struct repository *repo, if (fsck_walk(obj, NULL, &fsck_obj_options)) objerror(repo, obj, _("broken links")); + err = fsck_object(obj, buffer, size, &fsck_obj_options); if (err) goto out; @@ -442,32 +456,11 @@ static int fsck_obj(struct repository *repo, } out: - if (obj->type == OBJ_TREE) + if (obj && obj->type == OBJ_TREE) free_tree_buffer((struct tree *)obj); return err; } -static int fsck_obj_buffer(const struct object_id *oid, enum object_type type, - unsigned long size, void *buffer, int *eaten, void *cb_data) -{ - struct repository *repo = cb_data; - struct object *obj; - - /* - * Note, buffer may be NULL if type is OBJ_BLOB. See - * verify_packfile(), data_valid variable for details. - */ - obj = parse_object_buffer(repo, oid, type, size, buffer, eaten); - if (!obj) { - errors_found |= ERROR_OBJECT; - return error(_("%s: object corrupt or missing"), - oid_to_hex(oid)); - } - obj->flags &= ~(REACHABLE | SEEN); - obj->flags |= HAS_OBJ; - return fsck_obj(repo, obj, buffer, size); -} - static int default_refs; static void fsck_handle_reflog_oid(struct repository *repo, From e25440bad1c9158703daeb22de07d4663872b71d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:27 +0200 Subject: [PATCH 16/27] builtin/fsck: de-globalize option handling In subsequent commits we're about to rework some of the option handling in git-fsck(1) a bit. It is currently a bit of a mess though due to lots of global state that makes it hard to see which flags are used where exactly. Refactor the code by moving the fsck options into `cmd_fsck()`. This allows us to convert some of the options into function-local variables. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index bed84818930148..5132ff0f15a3e9 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -37,10 +37,8 @@ static int show_root; static int show_tags; static int show_unreachable; static int include_reflogs = 1; -static int check_full = 1; static int connectivity_only; static int check_strict; -static int keep_cache_objects; static struct fsck_options fsck_walk_options; static struct fsck_options fsck_obj_options; static int errors_found; @@ -48,8 +46,6 @@ static int write_lost_and_found; static int verbose; static int show_progress = -1; static int show_dangling = 1; -static int name_objects; -static int check_references = 1; static timestamp_t now; #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02 @@ -964,30 +960,33 @@ static char const * const fsck_usage[] = { NULL }; -static struct option fsck_opts[] = { - OPT__VERBOSE(&verbose, N_("be verbose")), - OPT_BOOL(0, "unreachable", &show_unreachable, N_("show unreachable objects")), - OPT_BOOL(0, "dangling", &show_dangling, N_("show dangling objects")), - OPT_BOOL(0, "tags", &show_tags, N_("report tags")), - OPT_BOOL(0, "root", &show_root, N_("report root nodes")), - OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head nodes")), - OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")), - OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate objects")), - OPT_BOOL(0, "connectivity-only", &connectivity_only, N_("check only connectivity")), - OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")), - OPT_BOOL(0, "lost-found", &write_lost_and_found, - N_("write dangling objects in .git/lost-found")), - OPT_BOOL(0, "progress", &show_progress, N_("show progress")), - OPT_BOOL(0, "name-objects", &name_objects, N_("show verbose names for reachable objects")), - OPT_BOOL(0, "references", &check_references, N_("check reference database consistency")), - OPT_END(), -}; - int cmd_fsck(int argc, const char **argv, const char *prefix, struct repository *repo) { + int check_full = 1; + int keep_cache_objects = 0; + int name_objects = 0; + int check_references = 1; + struct option fsck_opts[] = { + OPT__VERBOSE(&verbose, N_("be verbose")), + OPT_BOOL(0, "unreachable", &show_unreachable, N_("show unreachable objects")), + OPT_BOOL(0, "dangling", &show_dangling, N_("show dangling objects")), + OPT_BOOL(0, "tags", &show_tags, N_("report tags")), + OPT_BOOL(0, "root", &show_root, N_("report root nodes")), + OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head nodes")), + OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")), + OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate objects")), + OPT_BOOL(0, "connectivity-only", &connectivity_only, N_("check only connectivity")), + OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")), + OPT_BOOL(0, "lost-found", &write_lost_and_found, + N_("write dangling objects in .git/lost-found")), + OPT_BOOL(0, "progress", &show_progress, N_("show progress")), + OPT_BOOL(0, "name-objects", &name_objects, N_("show verbose names for reachable objects")), + OPT_BOOL(0, "references", &check_references, N_("check reference database consistency")), + OPT_END(), + }; struct odb_source *source; struct snapshot snap = { .nr = 0, From 485f5aeb94caa2f33a465f5460d2e18594080d8e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:28 +0200 Subject: [PATCH 17/27] builtin/fsck: don't check alternates with "--no-full" According to git-fsck(1), the "--full" option behaves in the following way: Check not just objects in GIT_OBJECT_DIRECTORY ($GIT_DIR/objects), but also the ones found in alternate object pools listed in GIT_ALTERNATE_OBJECT_DIRECTORIES or $GIT_DIR/objects/info/alternates, and in packed Git archives found in $GIT_DIR/objects/pack and corresponding pack subdirectories in alternate object pools. So ultimately, it is supposed to control two things: (1) whether we only check the main object directory, and (2) whether we check packfiles. In its current state though, the flag only controls whether we check packfiles or not, and if so we verify packfiles of all attached sources. But we also have checks for loose objects in git-fsck(1), and here we unconditionally check them in all sources. The flag is arguably conflating two unrelated concerns with one another, and it really should be split up into two flags: one that controls how thorough we want to check individual sources, and one that controls which sources we want to check in the first place. So ideally, we would have: - "--include-alternates": check all sources, not only the local one. - "--include-optimized-objects": check not only loose objects, but also those that have been packed. Note that we explicitly don't say "--include-packed-objects" here to be more backend-agnostic. - "--full": implies both of the above flags. This feels out of scope for this series though. So for now, simply fix the code by honoring locality of the sources for loose objects. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 3 ++- t/t1450-fsck.sh | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 5132ff0f15a3e9..3f6056535fdd4f 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -1047,7 +1047,8 @@ int cmd_fsck(int argc, mark_object_for_connectivity, repo, 0); } else { for (source = repo->objects->sources; source; source = source->next) - fsck_source(repo, source); + if (check_full || source->local) + fsck_source(repo, source); if (check_full) { struct packed_git *p; diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 77cd96de78eced..1b4074304cbdb6 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -844,6 +844,11 @@ test_expect_success 'alternate objects are correctly blamed' ' echo "../../alt.git/objects" >.git/objects/info/alternates && mkdir alt.git/objects/$(dirname $path) && >alt.git/objects/$(dirname $path)/$(basename $path) && + + # Without "--full", only the local object source is checked. + git fsck --no-full >out 2>&1 && + test_must_be_empty out && + test_must_fail git fsck >out 2>&1 && test_grep alt.git out ' From a51b77aa1d8ed033a018849465a65fd4e1dde02b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:29 +0200 Subject: [PATCH 18/27] odb: provide infrastructure for pluggable fsck checks The on-disk consistency checks in git-fsck(1) are conceptually backend-specific: while connectivity checks and object-level parsing checks are generic, verifying the physical integrity of packfiles and loose objects is meaningful only to backends that use these formats: Having these checks live in "builtin/fsck.c" violates that layering, because it forces the command to reach directly into format-specific internals. Provide new infrastructure to make these format-specific checks pluggable and implement stubs for the different source types we already have. In subsequent commits we'll move functionality over piece by piece. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 16 +++++++++++----- odb.c | 8 ++++++++ odb.h | 23 +++++++++++++++++++++++ odb/source-files.c | 16 ++++++++++++++++ odb/source-inmemory.c | 8 ++++++++ odb/source-loose.c | 7 +++++++ odb/source-packed.c | 8 ++++++++ odb/source.h | 21 +++++++++++++++++++++ 8 files changed, 102 insertions(+), 5 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 3f6056535fdd4f..adbe192e563869 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -965,7 +965,9 @@ int cmd_fsck(int argc, const char *prefix, struct repository *repo) { - int check_full = 1; + struct odb_fsck_options odb_fsck_opts = { + .flags = ODB_FSCK_FULL, + }; int keep_cache_objects = 0; int name_objects = 0; int check_references = 1; @@ -977,7 +979,8 @@ int cmd_fsck(int argc, OPT_BOOL(0, "root", &show_root, N_("report root nodes")), OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head nodes")), OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")), - OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate objects")), + OPT_BIT(0, "full", &odb_fsck_opts.flags, + N_("also consider packs and alternate objects"), ODB_FSCK_FULL), OPT_BOOL(0, "connectivity-only", &connectivity_only, N_("check only connectivity")), OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")), OPT_BOOL(0, "lost-found", &write_lost_and_found, @@ -1018,7 +1021,7 @@ int cmd_fsck(int argc, show_progress = 0; if (write_lost_and_found) { - check_full = 1; + odb_fsck_opts.flags |= ODB_FSCK_FULL; include_reflogs = 0; } @@ -1047,10 +1050,13 @@ int cmd_fsck(int argc, mark_object_for_connectivity, repo, 0); } else { for (source = repo->objects->sources; source; source = source->next) - if (check_full || source->local) + if ((odb_fsck_opts.flags & ODB_FSCK_FULL) || source->local) fsck_source(repo, source); - if (check_full) { + if (odb_fsck(repo->objects, &odb_fsck_opts) < 0) + errors_found |= ERROR_OBJECT; + + if (odb_fsck_opts.flags & ODB_FSCK_FULL) { struct packed_git *p; uint32_t total = 0, count = 0; struct progress *progress = NULL; diff --git a/odb.c b/odb.c index 1fe20808eb52a6..1c40da4cad9865 100644 --- a/odb.c +++ b/odb.c @@ -1177,3 +1177,11 @@ void odb_reprepare(struct object_database *o) { odb_prepare(o, ODB_PREPARE_FLUSH_CACHES); } + +int odb_fsck(struct object_database *odb, struct odb_fsck_options *options) +{ + int ret = 0; + for (struct odb_source *source = odb->sources; source; source = source->next) + ret |= odb_source_fsck(source, options); + return ret; +} diff --git a/odb.h b/odb.h index e60174070fd1c4..76c15e48f5fda8 100644 --- a/odb.h +++ b/odb.h @@ -206,6 +206,29 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags); /* Equivalent to `odb_prepare(o, ODB_PREPARE_FLUSH_CACHES)`. */ void odb_reprepare(struct object_database *o); +enum odb_fsck_flags { + /* + * If set, perform a full consistency check for the full object + * database, including all of its sources and the contents of their + * optimized formats. Otherwise, only check the local source, and + * restrict checks of its optimized formats to cheap structural + * verification of their metadata. + */ + ODB_FSCK_FULL = (1 << 0), +}; + +/* Options that shall be passed to `odb_fsck()`. */ +struct odb_fsck_options { + enum odb_fsck_flags flags; +}; + +/* + * Run backend-specific integrity checks on all object sources. Each source + * performs the checks appropriate to its type. Returns 0 on success, a + * negative error code otherwise. + */ +int odb_fsck(struct object_database *odb, struct odb_fsck_options *opts); + /* * Find source by its object directory path. Returns a `NULL` pointer in case * the source could not be found. diff --git a/odb/source-files.c b/odb/source-files.c index bd4fdf3a6c27ed..66a95e2b48ff73 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -893,6 +893,21 @@ static int odb_source_files_generate_pack(struct odb_source *source UNUSED, return 0; } +static int odb_source_files_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + struct odb_source_files *files = odb_source_files_downcast(source); + int ret = 0; + + if (!(opts->flags & ODB_FSCK_FULL) && !source->local) + return 0; + + ret |= odb_source_fsck(&files->loose->base, opts); + ret |= odb_source_fsck(&files->packed->base, opts); + + return ret; +} + struct odb_source_files *odb_source_files_new(struct object_database *odb, const char *path, bool local) @@ -908,6 +923,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb, files->base.close = odb_source_files_close; files->base.create_on_disk = odb_source_files_create_on_disk; files->base.prepare = odb_source_files_prepare; + files->base.fsck = odb_source_files_fsck; files->base.read_object_info = odb_source_files_read_object_info; files->base.read_object_stream = odb_source_files_read_object_stream; files->base.for_each_object = odb_source_files_for_each_object; diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 795672adf255c6..ba0f86da26c421 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -1,6 +1,7 @@ #include "git-compat-util.h" #include "object-file.h" #include "odb.h" +#include "fsck.h" #include "odb/source-inmemory.h" #include "odb/streaming.h" #include "oidtree.h" @@ -368,6 +369,12 @@ static void odb_source_inmemory_free(struct odb_source *source) free(inmemory); } +static int odb_source_inmemory_fsck(struct odb_source *source UNUSED, + struct odb_fsck_options *opts UNUSED) +{ + return 0; +} + struct odb_source_inmemory *odb_source_inmemory_new(struct object_database *odb) { struct odb_source_inmemory *source; @@ -378,6 +385,7 @@ struct odb_source_inmemory *odb_source_inmemory_new(struct object_database *odb) source->base.free = odb_source_inmemory_free; source->base.close = odb_source_inmemory_close; source->base.prepare = odb_source_inmemory_prepare; + source->base.fsck = odb_source_inmemory_fsck; source->base.read_object_info = odb_source_inmemory_read_object_info; source->base.read_object_stream = odb_source_inmemory_read_object_stream; source->base.for_each_object = odb_source_inmemory_for_each_object; diff --git a/odb/source-loose.c b/odb/source-loose.c index bb3455dfbd3334..f68d3c4d6cdc1b 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -1031,6 +1031,12 @@ static void odb_source_loose_free(struct odb_source *source) free(loose); } +static int odb_source_loose_fsck(struct odb_source *source UNUSED, + struct odb_fsck_options *opts UNUSED) +{ + return 0; +} + struct odb_source_loose *odb_source_loose_new(struct object_database *odb, const char *path, bool local) @@ -1043,6 +1049,7 @@ struct odb_source_loose *odb_source_loose_new(struct object_database *odb, loose->base.free = odb_source_loose_free; loose->base.close = odb_source_loose_close; loose->base.prepare = odb_source_loose_prepare; + loose->base.fsck = odb_source_loose_fsck; loose->base.read_object_info = odb_source_loose_read_object_info; loose->base.read_object_stream = odb_source_loose_read_object_stream; loose->base.for_each_object = odb_source_loose_for_each_object; diff --git a/odb/source-packed.c b/odb/source-packed.c index 630d9555856d7c..7aacf4bc452635 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -2,6 +2,7 @@ #include "abspath.h" #include "chdir-notify.h" #include "dir.h" +#include "fsck.h" #include "git-zlib.h" #include "list-objects-filter-options.h" #include "mergesort.h" @@ -826,6 +827,12 @@ static void odb_source_packed_free(struct odb_source *source) free(packed); } +static int odb_source_packed_fsck(struct odb_source *source UNUSED, + struct odb_fsck_options *opts UNUSED) +{ + return 0; +} + struct odb_source_packed *odb_source_packed_new(struct object_database *odb, const char *path, bool local) @@ -839,6 +846,7 @@ struct odb_source_packed *odb_source_packed_new(struct object_database *odb, packed->base.free = odb_source_packed_free; packed->base.close = odb_source_packed_close; packed->base.prepare = odb_source_packed_prepare; + packed->base.fsck = odb_source_packed_fsck; packed->base.read_object_info = odb_source_packed_read_object_info; packed->base.read_object_stream = odb_source_packed_read_object_stream; packed->base.for_each_object = odb_source_packed_for_each_object; diff --git a/odb/source.h b/odb/source.h index 559e2ea2e9ae8f..10a5dd5194a246 100644 --- a/odb/source.h +++ b/odb/source.h @@ -320,6 +320,17 @@ struct odb_source { int (*generate_pack)(struct odb_source *source, struct odb_pack_generator **out, const struct odb_generate_pack_options *opts); + + /* + * This callback is expected to check the integrity of the object source + * and report any errors found via the fsck options. The checks performed + * are backend-specific. + * + * The callback is expected to return 0 on success, a negative error + * code otherwise. + */ + int (*fsck)(struct odb_source *source, + struct odb_fsck_options *options); }; /* @@ -588,4 +599,14 @@ static inline int odb_source_generate_pack(struct odb_source *source, return source->generate_pack(source, out, opts); } +/* + * Check the integrity of the object database source. The checks performed + * are backend-specific. Returns 0 on success, a negative error code otherwise. + */ +static inline int odb_source_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + return source->fsck(source, opts); +} + #endif From 6bc77804047f3aba512b9f559b6c713fc7775954 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:30 +0200 Subject: [PATCH 19/27] builtin/fsck: move packfile verification into the packed source Move the packfile verification out of `cmd_fsck()` and into the "packed" source. While doing so, thread the progress meter and object callback through the newly introduced `struct odb_fsck_options` so that the caller's preferences are honoured without exposing those details at the "builtin/fsck.c" level. Note that the old code reported failures when verifying packfiles with the `ERROR_PACK` bit, which gets returned to the caller via the exit code. This bit is neither exercised in our test suite nor is it documented anywhere in our codebase. Furthermore, this bit is highly specific to the object storage backend, which makes it a bad fit for the new pluggable infrastructure. So instead of retaining these semantics, we drop them and return the generic `ERROR_OBJECT` bit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 33 ++++---------------------------- odb.h | 7 +++++++ odb/source-packed.c | 46 ++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index adbe192e563869..e504dae904423e 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -7,7 +7,6 @@ #include "blob.h" #include "tag.h" #include "refs.h" -#include "pack.h" #include "cache-tree.h" #include "fsck.h" #include "parse-options.h" @@ -49,7 +48,6 @@ static int show_dangling = 1; static timestamp_t now; #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02 -#define ERROR_PACK 04 #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 #define ERROR_MULTI_PACK_INDEX 040 @@ -967,6 +965,8 @@ int cmd_fsck(int argc, { struct odb_fsck_options odb_fsck_opts = { .flags = ODB_FSCK_FULL, + .object_cb = fsck_obj_buffer, + .object_payload = repo, }; int keep_cache_objects = 0; int name_objects = 0; @@ -1019,6 +1019,8 @@ int cmd_fsck(int argc, show_progress = isatty(2); if (verbose) show_progress = 0; + if (show_progress) + odb_fsck_opts.flags |= ODB_FSCK_PROGRESS; if (write_lost_and_found) { odb_fsck_opts.flags |= ODB_FSCK_FULL; @@ -1056,33 +1058,6 @@ int cmd_fsck(int argc, if (odb_fsck(repo->objects, &odb_fsck_opts) < 0) errors_found |= ERROR_OBJECT; - if (odb_fsck_opts.flags & ODB_FSCK_FULL) { - struct packed_git *p; - uint32_t total = 0, count = 0; - struct progress *progress = NULL; - - if (show_progress) { - repo_for_each_pack(repo, p) { - if (open_pack_index(p)) - continue; - total += p->num_objects; - } - - progress = start_progress(repo, - _("Checking objects"), total); - } - - repo_for_each_pack(repo, p) { - /* verify gives error messages itself */ - if (verify_pack(repo, - p, fsck_obj_buffer, repo, - progress, count)) - errors_found |= ERROR_PACK; - count += p->num_objects; - } - stop_progress(&progress); - } - if (fsck_finish(&fsck_obj_options)) errors_found |= ERROR_OBJECT; } diff --git a/odb.h b/odb.h index 76c15e48f5fda8..0bf6c8d7d28581 100644 --- a/odb.h +++ b/odb.h @@ -215,11 +215,18 @@ enum odb_fsck_flags { * verification of their metadata. */ ODB_FSCK_FULL = (1 << 0), + + /* Display a progress meter, if sensible. */ + ODB_FSCK_PROGRESS = (1 << 1), }; /* Options that shall be passed to `odb_fsck()`. */ struct odb_fsck_options { enum odb_fsck_flags flags; + + int (*object_cb)(const struct object_id *oid, enum object_type type, + unsigned long size, void *buffer, int *eaten, void *cb_data); + void *object_payload; }; /* diff --git a/odb/source-packed.c b/odb/source-packed.c index 7aacf4bc452635..0d3599f8fe0028 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -9,8 +9,10 @@ #include "midx.h" #include "odb/source-packed.h" #include "odb/streaming.h" +#include "pack.h" #include "packfile.h" #include "pack-bitmap.h" +#include "progress.h" static int find_pack_entry(struct odb_source_packed *store, const struct object_id *oid, @@ -827,10 +829,48 @@ static void odb_source_packed_free(struct odb_source *source) free(packed); } -static int odb_source_packed_fsck(struct odb_source *source UNUSED, - struct odb_fsck_options *opts UNUSED) +static int verify_packs(struct odb_source_packed *source, + struct odb_fsck_options *opts) { - return 0; + struct progress *progress = NULL; + struct packfile_list_entry *e; + uint32_t total = 0, count = 0; + int ret = 0; + + if (opts->flags & ODB_FSCK_PROGRESS) { + for (e = packfile_store_get_packs(source); e; e = e->next) { + if (open_pack_index(e->pack)) + continue; + total += e->pack->num_objects; + } + + progress = start_progress(source->base.odb->repo, + _("Checking objects"), total); + } + + for (e = packfile_store_get_packs(source); e; e = e->next) { + /* verify gives error messages itself */ + if (verify_pack(source->base.odb->repo, e->pack, + opts->object_cb, opts->object_payload, + progress, count)) + ret = -1; + count += e->pack->num_objects; + } + stop_progress(&progress); + + return ret; +} + +static int odb_source_packed_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + struct odb_source_packed *packed = odb_source_packed_downcast(source); + int ret = 0; + + if ((opts->flags & ODB_FSCK_FULL) && verify_packs(packed, opts) < 0) + ret = -1; + + return ret; } struct odb_source_packed *odb_source_packed_new(struct object_database *odb, From 426d291b6095a51ee249b70a088f99cd67d4bb27 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:31 +0200 Subject: [PATCH 20/27] builtin/fsck: move reverse index verification into the packed source The checks for reverse indexes live in `check_pack_rev_indexes()`, which is hosted in "builtin/fsck.c". These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in the preceding commit, drop the dedicated `ERROR_PACK_REV_INDEX` bit and instead use the generic `ERROR_OBJECT` bit. Note that this changes behaviour in two ways: - The checks are now skipped when "--connectivity-only" was passed. This is because we don't even run `odb_fsck()` at all when that flag has been passed by the user, and not verifying data structures of the object database matches the documented intent of that flag, which is to only check the connectivity of reachable objects. - The checks are now skipped for non-local sources when "--no-full" was passed. This is, again, in line with the documented intent of that flag. Add a test to cast these semantics into stone. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 37 ------------------------------------- odb/source-packed.c | 39 +++++++++++++++++++++++++++++++++++++++ t/t5325-reverse-index.sh | 8 ++++++++ 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index e504dae904423e..06e72877f3362f 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -23,7 +23,6 @@ #include "run-command.h" #include "sparse-index.h" #include "worktree.h" -#include "pack-revindex.h" #include "pack-bitmap.h" #define REACHABLE 0x0001 @@ -51,7 +50,6 @@ static timestamp_t now; #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 #define ERROR_MULTI_PACK_INDEX 040 -#define ERROR_PACK_REV_INDEX 0100 #define ERROR_BITMAP 0200 static const char *describe_object(const struct object_id *oid) @@ -890,40 +888,6 @@ static int mark_object_for_connectivity(const struct object_id *oid, return 0; } -static int check_pack_rev_indexes(struct repository *r, int show_progress) -{ - struct progress *progress = NULL; - struct packed_git *p; - uint32_t pack_count = 0; - int res = 0; - - if (show_progress) { - repo_for_each_pack(r, p) - pack_count++; - progress = start_delayed_progress(r, - "Verifying reverse pack-indexes", pack_count); - pack_count = 0; - } - - repo_for_each_pack(r, p) { - int load_error = load_pack_revindex_from_disk(p); - - if (load_error < 0) { - error(_("unable to load rev-index for pack '%s'"), p->pack_name); - res = ERROR_PACK_REV_INDEX; - } else if (!load_error && - !load_pack_revindex(r, p) && - verify_pack_revindex(p)) { - error(_("invalid rev-index for pack '%s'"), p->pack_name); - res = ERROR_PACK_REV_INDEX; - } - display_progress(progress, ++pack_count); - } - stop_progress(&progress); - - return res; -} - static void fsck_refs(struct repository *r) { struct child_process refs_verify = CHILD_PROCESS_INIT; @@ -1104,7 +1068,6 @@ int cmd_fsck(int argc, free_worktrees(worktrees); } - errors_found |= check_pack_rev_indexes(repo, show_progress); if (verify_bitmap_files(repo)) errors_found |= ERROR_BITMAP; diff --git a/odb/source-packed.c b/odb/source-packed.c index 0d3599f8fe0028..e5e69636dd782f 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -10,6 +10,7 @@ #include "odb/source-packed.h" #include "odb/streaming.h" #include "pack.h" +#include "pack-revindex.h" #include "packfile.h" #include "pack-bitmap.h" #include "progress.h" @@ -861,6 +862,41 @@ static int verify_packs(struct odb_source_packed *source, return ret; } +static int verify_reverse_indices(struct odb_source_packed *source, + struct odb_fsck_options *opts) +{ + struct progress *progress = NULL; + struct packfile_list_entry *e; + uint32_t pack_count = 0; + int res = 0; + + if (opts->flags & ODB_FSCK_PROGRESS) { + for (e = packfile_store_get_packs(source); e; e = e->next) + pack_count++; + progress = start_delayed_progress(source->base.odb->repo, + "Verifying reverse pack-indexes", pack_count); + pack_count = 0; + } + + for (e = packfile_store_get_packs(source); e; e = e->next) { + int load_error = load_pack_revindex_from_disk(e->pack); + + if (load_error < 0) { + error(_("unable to load rev-index for pack '%s'"), e->pack->pack_name); + res = -1; + } else if (!load_error && + !load_pack_revindex(source->base.odb->repo, e->pack) && + verify_pack_revindex(e->pack)) { + error(_("invalid rev-index for pack '%s'"), e->pack->pack_name); + res = -1; + } + display_progress(progress, ++pack_count); + } + stop_progress(&progress); + + return res; +} + static int odb_source_packed_fsck(struct odb_source *source, struct odb_fsck_options *opts) { @@ -870,6 +906,9 @@ static int odb_source_packed_fsck(struct odb_source *source, if ((opts->flags & ODB_FSCK_FULL) && verify_packs(packed, opts) < 0) ret = -1; + if (verify_reverse_indices(packed, opts) < 0) + ret = -1; + return ret; } diff --git a/t/t5325-reverse-index.sh b/t/t5325-reverse-index.sh index 54937919381707..6b81abf66378b7 100755 --- a/t/t5325-reverse-index.sh +++ b/t/t5325-reverse-index.sh @@ -204,4 +204,12 @@ test_expect_success 'fsck catches invalid header: hash function' ' "reverse-index file .* has unsupported hash id" ' +test_expect_success 'fsck --no-full checks rev-index, --connectivity-only does not' ' + test_must_fail git -C corrupt fsck --no-full 2>err && + test_grep "has unsupported hash id" err && + + git -C corrupt fsck --connectivity-only 2>err && + test_grep ! "has unsupported hash id" err +' + test_done From 1bbb92d54019be5317e39182c6b2696f4abd0755 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:32 +0200 Subject: [PATCH 21/27] builtin/fsck: move bitmap verification into the packed source The checks for bitmaps live in `verify_bitmap_files()`, which is called by "builtin/fsck.c". These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in preceding commits, this means that we now properly honor both "--connectivity-only" and "--no-full". Furthermore, we drop the dedicated `ERROR_BITMAP` bit and instead use the generic `ERROR_OBJECT` bit. Note that this change also adapts `verify_bitmap_files()` to be focused on a single "packed" source instead of verifying bitmaps from all sources. This change is required as we already know to loop around the sources in `odb_fsck()` itself. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 5 ----- odb/source-packed.c | 3 +++ pack-bitmap.c | 26 ++++++++++---------------- pack-bitmap.h | 2 +- t/t5326-multi-pack-bitmaps.sh | 10 +++++++++- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 06e72877f3362f..2f7d29aa56209b 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -23,7 +23,6 @@ #include "run-command.h" #include "sparse-index.h" #include "worktree.h" -#include "pack-bitmap.h" #define REACHABLE 0x0001 #define SEEN 0x0002 @@ -50,7 +49,6 @@ static timestamp_t now; #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 #define ERROR_MULTI_PACK_INDEX 040 -#define ERROR_BITMAP 0200 static const char *describe_object(const struct object_id *oid) { @@ -1068,9 +1066,6 @@ int cmd_fsck(int argc, free_worktrees(worktrees); } - if (verify_bitmap_files(repo)) - errors_found |= ERROR_BITMAP; - check_connectivity(repo); if (repo->settings.core_commit_graph) { diff --git a/odb/source-packed.c b/odb/source-packed.c index e5e69636dd782f..2b5dc502f53b16 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -909,6 +909,9 @@ static int odb_source_packed_fsck(struct odb_source *source, if (verify_reverse_indices(packed, opts) < 0) ret = -1; + if (verify_bitmap_files(packed)) + ret = -1; + return ret; } diff --git a/pack-bitmap.c b/pack-bitmap.c index e0fb57d3321889..3de8e9590cff4f 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -3410,28 +3410,22 @@ static int verify_bitmap_file(const struct git_hash_algo *algop, return res; } -int verify_bitmap_files(struct repository *r) +int verify_bitmap_files(struct odb_source_packed *source) { - struct odb_source *source; - struct packed_git *p; + struct packfile_list_entry *e; + struct multi_pack_index *m; int res = 0; - for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - struct multi_pack_index *m = get_multi_pack_index(files->packed); - char *midx_bitmap_name; - - if (!m) - continue; - - midx_bitmap_name = midx_bitmap_filename(m); - res |= verify_bitmap_file(r->hash_algo, midx_bitmap_name); + m = get_multi_pack_index(source); + if (m) { + char *midx_bitmap_name = midx_bitmap_filename(m); + res |= verify_bitmap_file(source->base.odb->repo->hash_algo, midx_bitmap_name); free(midx_bitmap_name); } - repo_for_each_pack(r, p) { - char *pack_bitmap_name = pack_bitmap_filename(p); - res |= verify_bitmap_file(r->hash_algo, pack_bitmap_name); + for (e = packfile_store_get_packs(source); e; e = e->next) { + char *pack_bitmap_name = pack_bitmap_filename(e->pack); + res |= verify_bitmap_file(source->base.odb->repo->hash_algo, pack_bitmap_name); free(pack_bitmap_name); } diff --git a/pack-bitmap.h b/pack-bitmap.h index 1385027c1ff5fa..847ad4762d4bfe 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -205,7 +205,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git); int bitmap_is_preferred_refname(struct repository *r, const char *refname); -int verify_bitmap_files(struct repository *r); +int verify_bitmap_files(struct odb_source_packed *source); struct ewah_bitmap *read_bitmap(const unsigned char *map, size_t map_size, size_t *map_pos); diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 86beab1dae491e..8047459b00eef7 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -498,7 +498,15 @@ test_expect_success 'git fsck correctly identifies good and bad bitmaps' ' corrupt_file "$packbitmap" && test_must_fail git fsck 2>err && test_grep "bitmap file '\''$midxbitmap'\'' has invalid checksum" err && - test_grep "bitmap file '\''$packbitmap'\'' has invalid checksum" err + test_grep "bitmap file '\''$packbitmap'\'' has invalid checksum" err && + + # The bitmap checks are performed with "--no-full", but not with + # "--connectivity-only". + test_must_fail git fsck --no-full 2>err && + test_grep "bitmap file '\''$midxbitmap'\'' has invalid checksum" err && + test_grep "bitmap file '\''$packbitmap'\'' has invalid checksum" err && + git fsck --connectivity-only 2>err && + test_grep ! "invalid checksum" err ' test_expect_success 'corrupt MIDX with bitmap causes fallback' ' From ae7a0ffc25df0103fe28c8504f8120e85edb9181 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:33 +0200 Subject: [PATCH 22/27] builtin/fsck: move multi-pack index verification into the packed source The checks for multi-pack indexes are hosted in `cmd_fsck()` directly. These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in preceding commits, this means that we now properly honor both "--connectivity-only" and "--no-full". Furthermore, we drop the dedicated `ERROR_MULTI_PACK_INDEX` bit and instead use the generic `ERROR_OBJECT` bit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 18 ------------------ odb/source-packed.c | 27 +++++++++++++++++++++++++++ t/t5319-multi-pack-index.sh | 13 +++++++++++++ 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 2f7d29aa56209b..7eaea340b0235e 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -48,7 +48,6 @@ static timestamp_t now; #define ERROR_REACHABLE 02 #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 -#define ERROR_MULTI_PACK_INDEX 040 static const char *describe_object(const struct object_id *oid) { @@ -1085,23 +1084,6 @@ int cmd_fsck(int argc, } } - if (repo->settings.core_multi_pack_index) { - struct child_process midx_verify = CHILD_PROCESS_INIT; - - for (source = repo->objects->sources; source; source = source->next) { - child_process_init(&midx_verify); - midx_verify.git_cmd = 1; - strvec_pushl(&midx_verify.args, "multi-pack-index", - "verify", "--object-dir", source->path, NULL); - if (show_progress) - strvec_push(&midx_verify.args, "--progress"); - else - strvec_push(&midx_verify.args, "--no-progress"); - if (run_command(&midx_verify)) - errors_found |= ERROR_MULTI_PACK_INDEX; - } - } - free_snapshot_refs(&snap); return errors_found; } diff --git a/odb/source-packed.c b/odb/source-packed.c index 2b5dc502f53b16..9f54a5e83ad53a 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -14,6 +14,7 @@ #include "packfile.h" #include "pack-bitmap.h" #include "progress.h" +#include "run-command.h" static int find_pack_entry(struct odb_source_packed *store, const struct object_id *oid, @@ -897,6 +898,29 @@ static int verify_reverse_indices(struct odb_source_packed *source, return res; } +static int verify_midx(struct odb_source_packed *source, + struct odb_fsck_options *opts) +{ + struct child_process midx_verify = CHILD_PROCESS_INIT; + + prepare_repo_settings(source->base.odb->repo); + if (!source->base.odb->repo->settings.core_multi_pack_index) + return 0; + + child_process_init(&midx_verify); + midx_verify.git_cmd = 1; + strvec_pushl(&midx_verify.args, "multi-pack-index", + "verify", "--object-dir", source->base.path, NULL); + if (opts->flags & ODB_FSCK_PROGRESS) + strvec_push(&midx_verify.args, "--progress"); + else + strvec_push(&midx_verify.args, "--no-progress"); + if (run_command(&midx_verify)) + return -1; + + return 0; +} + static int odb_source_packed_fsck(struct odb_source *source, struct odb_fsck_options *opts) { @@ -912,6 +936,9 @@ static int odb_source_packed_fsck(struct odb_source *source, if (verify_bitmap_files(packed)) ret = -1; + if (verify_midx(packed, opts) < 0) + ret = -1; + return ret; } diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 68143cb5b76952..20b010c33b7790 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -573,6 +573,19 @@ test_expect_success 'verify incorrect checksum' ' $objdir "incorrect checksum" ' +test_expect_success 'git fsck --no-full checks multi-pack-index, --connectivity-only does not' ' + pos=$(($(wc -c <$objdir/pack/multi-pack-index) - 10)) && + corrupt_midx_and_verify $pos \ + "\377\377\377\377\377\377\377\377\377\377" \ + $objdir "incorrect checksum" && + + test_must_fail git fsck --no-full 2>err && + test_grep "incorrect checksum" err && + + git fsck --connectivity-only 2>err && + test_grep ! "incorrect checksum" err +' + test_expect_success 'setup for v1-specific fsck tests' ' git -c midx.version=1 multi-pack-index write ' From 0d5ebb323b19c92ea6d5e31f932c040267760466 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Sep 2026 15:27:34 +0200 Subject: [PATCH 23/27] builtin/fsck: move loose object verification into the loose source The consistency checks for loose objects are hosted by "builtin/fsck.c". These checks are obviously specific to the "loose" backend. Move the logic into `odb_source_loose_fsck()`. Introduce a new "verbose" flag so that we can properly retain semantics around whether or not we want to print some status messages. Note that this fixes a bug as a side effect: the progress meter was captured in the callback data before `start_progress()` was even called, so the per-subdirectory progress updates always operated on a NULL pointer and the meter jumped straight from 0 to 256 upon completion. The new code only sets up the callback data's progress meter after it has been created, so the progress display now advances incrementally again. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 91 ++-------------------------------------------- odb.h | 3 ++ odb/source-loose.c | 89 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 90 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 7eaea340b0235e..4af1d874cc70db 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -12,7 +12,6 @@ #include "parse-options.h" #include "progress.h" #include "packfile.h" -#include "object-file.h" #include "object-name.h" #include "odb.h" #include "odb/streaming.h" @@ -695,88 +694,6 @@ static void process_refs(struct repository *repo, struct snapshot *snap) } } -struct for_each_loose_cb { - struct repository *repo; - struct progress *progress; -}; - -static int fsck_loose(const struct object_id *oid, const char *path, - void *cb_data) -{ - struct for_each_loose_cb *data = cb_data; - enum object_type type = OBJ_NONE; - size_t size; - void *contents = NULL; - int eaten; - struct object_info oi = OBJECT_INFO_INIT; - struct object_id real_oid = *null_oid(data->repo->hash_algo); - int err = 0; - - oi.sizep = &size; - oi.typep = &type; - - if (read_loose_object(data->repo, path, oid, &real_oid, &contents, &oi) < 0) { - if (contents && !oideq(&real_oid, oid)) - err = error(_("%s: hash-path mismatch, found at: %s"), - oid_to_hex(&real_oid), path); - else - err = error(_("%s: object corrupt or missing: %s"), - oid_to_hex(oid), path); - } - if (err < 0) { - errors_found |= ERROR_OBJECT; - free(contents); - return 0; /* keep checking other objects */ - } - - if (!contents && type != OBJ_BLOB) - BUG("read_loose_object streamed a non-blob"); - - if (fsck_obj_buffer(oid, type, size, contents, &eaten, data->repo)) - errors_found |= ERROR_OBJECT; - - if (!eaten) - free(contents); - return 0; /* keep checking other objects, even if we saw an error */ -} - -static int fsck_cruft(const char *basename, const char *path, - void *data UNUSED) -{ - if (!starts_with(basename, "tmp_obj_")) - fprintf_ln(stderr, _("bad sha1 file: %s"), path); - return 0; -} - -static int fsck_subdir(unsigned int nr, const char *path UNUSED, void *data) -{ - struct for_each_loose_cb *cb_data = data; - struct progress *progress = cb_data->progress; - display_progress(progress, nr + 1); - return 0; -} - -static void fsck_source(struct repository *repo, struct odb_source *source) -{ - struct progress *progress = NULL; - struct for_each_loose_cb cb_data = { - .repo = source->odb->repo, - .progress = progress, - }; - - if (verbose) - fprintf_ln(stderr, _("Checking object directory")); - - if (show_progress) - progress = start_progress(repo, - _("Checking object directories"), 256); - - for_each_loose_file_in_source(source, fsck_loose, - fsck_cruft, fsck_subdir, &cb_data); - display_progress(progress, 256); - stop_progress(&progress); -} - static int fsck_cache_tree(struct repository *repo, struct cache_tree *it, const char *index_path) { int i; @@ -978,8 +895,10 @@ int cmd_fsck(int argc, if (show_progress == -1) show_progress = isatty(2); - if (verbose) + if (verbose) { show_progress = 0; + odb_fsck_opts.flags |= ODB_FSCK_VERBOSE; + } if (show_progress) odb_fsck_opts.flags |= ODB_FSCK_PROGRESS; @@ -1012,10 +931,6 @@ int cmd_fsck(int argc, odb_for_each_object(repo->objects, NULL, mark_object_for_connectivity, repo, 0); } else { - for (source = repo->objects->sources; source; source = source->next) - if ((odb_fsck_opts.flags & ODB_FSCK_FULL) || source->local) - fsck_source(repo, source); - if (odb_fsck(repo->objects, &odb_fsck_opts) < 0) errors_found |= ERROR_OBJECT; diff --git a/odb.h b/odb.h index 0bf6c8d7d28581..b87f281cbde4ef 100644 --- a/odb.h +++ b/odb.h @@ -218,6 +218,9 @@ enum odb_fsck_flags { /* Display a progress meter, if sensible. */ ODB_FSCK_PROGRESS = (1 << 1), + + /* Be extra verbose when checking the database. */ + ODB_FSCK_VERBOSE = (1 << 2), }; /* Options that shall be passed to `odb_fsck()`. */ diff --git a/odb/source-loose.c b/odb/source-loose.c index f68d3c4d6cdc1b..efef9ca61f59e0 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -12,6 +12,7 @@ #include "odb/streaming.h" #include "oidtree.h" #include "path.h" +#include "progress.h" #include "repository.h" #include "strbuf.h" #include "tempfile.h" @@ -1031,12 +1032,96 @@ static void odb_source_loose_free(struct odb_source *source) free(loose); } -static int odb_source_loose_fsck(struct odb_source *source UNUSED, - struct odb_fsck_options *opts UNUSED) +struct fsck_loose_data { + struct odb_source_loose *source; + struct odb_fsck_options *opts; + struct progress *progress; + bool error_found; +}; + +static int fsck_loose(const struct object_id *oid, const char *path, + void *cb_data) { + struct fsck_loose_data *data = cb_data; + enum object_type type = OBJ_NONE; + size_t size; + void *contents = NULL; + int eaten = 0; + struct object_info oi = OBJECT_INFO_INIT; + struct object_id real_oid = *null_oid(data->source->base.odb->repo->hash_algo); + int err = 0; + + oi.sizep = &size; + oi.typep = &type; + + if (read_loose_object(data->source->base.odb->repo, + path, oid, &real_oid, &contents, &oi) < 0) { + if (contents && !oideq(&real_oid, oid)) + err = error(_("%s: hash-path mismatch, found at: %s"), + oid_to_hex(&real_oid), path); + else + err = error(_("%s: object corrupt or missing: %s"), + oid_to_hex(oid), path); + } + if (err < 0) + goto out; + + if (!contents && type != OBJ_BLOB) + BUG("read_loose_object streamed a non-blob"); + + if (data->opts->object_cb(oid, type, size, contents, &eaten, + data->opts->object_payload)) { + err = -1; + goto out; + } + +out: + if (err) + data->error_found = true; + if (!eaten) + free(contents); + return 0; /* keep checking other objects, even if we saw an error */ +} + +static int fsck_cruft(const char *basename, const char *path, + void *data UNUSED) +{ + if (!starts_with(basename, "tmp_obj_")) + fprintf_ln(stderr, _("bad sha1 file: %s"), path); + return 0; +} + +static int fsck_subdir(unsigned int nr, const char *path UNUSED, void *cb_data) +{ + struct fsck_loose_data *data = cb_data; + display_progress(data->progress, nr + 1); return 0; } +static int odb_source_loose_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + struct odb_source_loose *loose = odb_source_loose_downcast(source); + struct fsck_loose_data data = { + .source = loose, + .opts = opts, + }; + + if (opts->flags & ODB_FSCK_VERBOSE) + fprintf_ln(stderr, _("Checking object directory")); + + if (opts->flags & ODB_FSCK_PROGRESS) + data.progress = start_progress(source->odb->repo, + _("Checking object directories"), 256); + + for_each_loose_file_in_source(source, fsck_loose, + fsck_cruft, fsck_subdir, &data); + display_progress(data.progress, 256); + stop_progress(&data.progress); + + return data.error_found ? -1 : 0; +} + struct odb_source_loose *odb_source_loose_new(struct object_database *odb, const char *path, bool local) From 8fff84c417c59ce94a7d4cb230fdc17a9cf6100f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 13 Sep 2026 19:11:06 +0000 Subject: [PATCH 24/27] t9700: accommodate for MSYS2 Perl reporting as `cygwin` As of a year or two ago, there is this push to align MSYS2 more closely with Cygwin, so as to benefit from a closer collaboration. Part of that is that the triplet `x86_64-pc-cygwin` is used nowadays, whereas it had been `x86_64-pc-msys` previously. Likewise, Perl now reports `$^O` as `cygwin` instead of `msys`. The Perl module test used `msys` as tell-tale when to accommodate for a native Windows version of `git.exe` which would report absolute _Windows_ paths rather than those pseudo-Unix paths. We cannot use that tell-tale anymore, and we also cannot adjust it to `cygwin` because that would break in Cygwin (where `git.exe` reports absolute pseudo-Unix paths). Let's use the environment variable `MSYSTEM` instead (being mindful that the `MSYSTEM=MSYS` variant would _also_ reflect a setup where `git.exe` won't report absolute _Windows_ paths). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t9700/test.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9700/test.pl b/t/t9700/test.pl index f83e6169e2c100..43a6dc266ee836 100755 --- a/t/t9700/test.pl +++ b/t/t9700/test.pl @@ -118,7 +118,7 @@ sub adjust_dirsep { # paths my $abs_git_dir = $abs_repo_dir . "/.git"; -if ($^O eq 'msys') { +if (defined $ENV{MSYSTEM} && $ENV{MSYSTEM} ne 'MSYS') { $abs_git_dir = `cygpath -am "$abs_repo_dir/.git"`; $abs_git_dir =~ s/\r?\n?$//; } From aa40ca02170b805a55c8c2460f4bde6d86490306 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 13 Sep 2026 19:11:07 +0000 Subject: [PATCH 25/27] t9129: skip UTF-8 tests on Windows The assumption of this test is that Perl and Git have the same idea how to perform encoding conversions. However, in Git for Windows, Git is a native Win32 program, and such programs have a very different concept of encodings (called "Code Pages", and they are not controlled via environment variables at all), whereas the Perl interpreter used in Git for Windows is a pseudo-Unix one that uses the MSYS2 runtime (which _does_ try very much to abide by Unix' `LC_ALL` and friends, and totally ignores Windows' current or active code pages). As such, these test cases _cannot_ work with Git for Windows. So let's just skip them on that platform. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t9129-git-svn-i18n-commitencoding.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh index 01e1e8a8f76765..f72a425dc1ebc9 100755 --- a/t/t9129-git-svn-i18n-commitencoding.sh +++ b/t/t9129-git-svn-i18n-commitencoding.sh @@ -71,7 +71,7 @@ do ' done -test_expect_success UTF8 'ISO-8859-1 should match UTF-8 in svn' ' +test_expect_success UTF8,!MINGW 'ISO-8859-1 should match UTF-8 in svn' ' ( cd ISO8859-1 && compare_svn_head_with "$TEST_DIRECTORY"/t3900/1-UTF-8.txt @@ -80,7 +80,7 @@ test_expect_success UTF8 'ISO-8859-1 should match UTF-8 in svn' ' for H in eucJP ISO-2022-JP do - test_expect_success UTF8 "$H should match UTF-8 in svn" ' + test_expect_success UTF8,!MINGW "$H should match UTF-8 in svn" ' ( cd $H && compare_svn_head_with "$TEST_DIRECTORY"/t3900/2-UTF-8.txt From 1cd6b7de52f9f28c769749a1c4d4daac6a7d89b9 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 13 Sep 2026 20:41:12 +0000 Subject: [PATCH 26/27] cmake(windows): accommodate for Git for Windows' migration to UCRT64 Git for Windows needs to ship with a lot of Unix tools that Git takes for granted, such as `sed`, `awk`, a C compiler and a Unix shell, just to name a few. In Git for Windows, these are provided by the MSYS2 project. Part of these tools (such as `bash.exe`) use a POSIX emulation layer ("MSYS2 runtime", a friendly fork of the Cygwin runtime), but others target a native Win32 toolchain, e.g. `git.exe`. There are multiple flavors of that toolchain, and historically Git for Windows used MINGW64 on x64 Windows. This toolchain uses the old MSVC runtime, and therefore the MSYS2 project deprecated it. As a consequence, Git for Windows switches to UCRT64 with v2.56.0. That flavor still uses GCC to compile native Win32 binaries, but targets the Universal C Runtime ("UCRT"). Internally, this means that the new `git.exe` is installed into a new prefix, `/ucrt64/`, whereas the old `git.exe` was installed into `/mingw64/`. A recently-upstreamed commit hard-codes this expectation even into the CMake-based build, so that the built `git.exe` "knows where it lives" and can ensure that the tools it expects on the `PATH` are found. Naturally, this hard-coded MINGW64 needs to change to UCRT64 now, too. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- contrib/buildsystems/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index e76bd19b65642b..7874e5a32601d5 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -258,7 +258,7 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Windows") OVERRIDE_STRDUP MMAP_PREVENTS_DELETE USE_WIN32_MMAP HAVE_WPGMPTR HAVE_RTLGENRANDOM) if(CMAKE_GENERATOR_PLATFORM STREQUAL "x64") - add_compile_definitions(ENSURE_MSYSTEM_IS_SET="MINGW64" MINGW_PREFIX="mingw64") + add_compile_definitions(ENSURE_MSYSTEM_IS_SET="UCRT64" MINGW_PREFIX="ucrt64") elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "arm64") add_compile_definitions(ENSURE_MSYSTEM_IS_SET="CLANGARM64" MINGW_PREFIX="clangarm64") elseif(CMAKE_GENERATOR_PLATFORM STREQUAL "x86") From f0ef1b96a076d08dc972a8d2cb0d1cfd60931eb6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 15 Sep 2026 11:07:26 -0700 Subject: [PATCH 27/27] 4th batch for -rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.56.0.adoc | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Documentation/RelNotes/2.56.0.adoc b/Documentation/RelNotes/2.56.0.adoc index 214ab1e87b0371..796a51e5a2998d 100644 --- a/Documentation/RelNotes/2.56.0.adoc +++ b/Documentation/RelNotes/2.56.0.adoc @@ -546,6 +546,20 @@ Performance, Internal Implementation, Development Support etc. interfered with "git rebase" etc. too much. The conditions "rerere gc" gets triggered have been tweaked. + * Windows build switches from MINGW64 to URCR64 runtime starting Git + 2.56.0; switch the cmake based build at the same time. + + * The mechanism to register in-memory alternate object sources has + been removed, as submodule object databases are now accessed + natively via their own repository structures. This simplifies + object database management and prepares the codebase for migrating + alternate tracking into the files backend. + + * The consistency checks for the object database (fsck) have been + decoupled from the generic builtin implementation and moved into the + backend-specific object source layers, making them pluggable for + different object storage formats. + Fixes since v2.55 -----------------