From f570bd91b3b2c6c5ef2035e3ce3ed76e613e74a7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Oct 2025 08:26:08 +0200 Subject: [PATCH 01/14] refs/files: deprecate writing symrefs as symbolic links The "files" backend has the ability to store symbolic refs as symbolic links, which can be configured via "core.preferSymlinkRefs". This feature stems back from the early days: the initial implementation of symbolic refs used symlinks exclusively. The symref format was only introduced in 9b143c6e15 (Teach update-ref about a symbolic ref stored in a textfile., 2005-09-25) and made the default in 9f0bb90d16 (core.prefersymlinkrefs: use symlinks for .git/HEAD, 2006-05-02). This is all about 20 years ago, and there are no known reasons nowadays why one would want to use symlinks instead of symrefs. Mark the feature for deprecation in Git 3.0. Note that this only deprecates _writing_ symrefs as symbolic links. Reading such symrefs is still supported for now. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/BreakingChanges.adoc | 20 ++++++++++++++++++++ Documentation/config/core.adoc | 3 +++ refs/files-backend.c | 19 +++++++++++++++++-- t/t0600-reffiles-backend.sh | 26 +++++++++++++++++++++++--- 4 files changed, 63 insertions(+), 5 deletions(-) diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc index 90b53abcea28f9..f814450d2f65ac 100644 --- a/Documentation/BreakingChanges.adoc +++ b/Documentation/BreakingChanges.adoc @@ -295,6 +295,26 @@ The command will be removed. + cf. +* Support for `core.preferSymlinkRefs=true` has been deprecated and will be + removed in Git 3.0. Writing symbolic refs as symbolic links will be phased + out in favor of using plain files using the textual representation of + symbolic refs. ++ +Symbolic references were initially always stored as a symbolic link. This was +changed in 9b143c6e15 (Teach update-ref about a symbolic ref stored in a +textfile., 2005-09-25), where a new textual symref format was introduced to +store those symbolic refs in a plain file. In 9f0bb90d16 +(core.prefersymlinkrefs: use symlinks for .git/HEAD, 2006-05-02), the Git +project switched the default to use the textual symrefs in favor of symbolic +links. ++ +The migration away from symbolic links has happened almost 20 years ago by now, +and there is no known reason why one should prefer them nowadays. Furthermore, +symbolic links are not supported on some platforms. ++ +Note that only the writing side for such symbolic links is deprecated. Reading +such symbolic links is still supported for now. + == Superseded features that will not be deprecated Some features have gained newer replacements that aim to improve the design in diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc index 08739bb9d428b8..406d7029d9dc9a 100644 --- a/Documentation/config/core.adoc +++ b/Documentation/config/core.adoc @@ -290,6 +290,9 @@ core.preferSymlinkRefs:: and other symbolic reference files, use symbolic links. This is sometimes needed to work with old scripts that expect HEAD to be a symbolic link. ++ +This configuration is deprecated and will be removed in Git 3.0. Symbolic refs +will always be written as textual symrefs. core.alternateRefsCommand:: When advertising tips of available history from an alternate, use the shell to diff --git a/refs/files-backend.c b/refs/files-backend.c index 5ddf418b181a7e..2c48526ef27ed6 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2113,20 +2113,35 @@ static int commit_ref_update(struct files_ref_store *refs, return 0; } -#ifdef NO_SYMLINK_HEAD +#if defined(NO_SYMLINK_HEAD) || defined(WITH_BREAKING_CHANGES) #define create_ref_symlink(a, b) (-1) #else static int create_ref_symlink(struct ref_lock *lock, const char *target) { + static int warn_once = 1; + char *ref_path; int ret = -1; - char *ref_path = get_locked_file_path(&lock->lk); + ref_path = get_locked_file_path(&lock->lk); unlink(ref_path); ret = symlink(target, ref_path); free(ref_path); if (ret) fprintf(stderr, "no symlink - falling back to symbolic ref\n"); + + if (warn_once) + warning(_("'core.preferSymlinkRefs=true' is nominated for removal.\n" + "hint: The use of symbolic links for symbolic refs is deprecated\n" + "hint: and will be removed in Git 3.0. The configuration that\n" + "hint: tells Git to use them is thus going away. You can unset\n" + "hint: it with:\n" + "hint:\n" + "hint:\tgit config unset core.preferSymlinkRefs\n" + "hint:\n" + "hint: Git will then use the textual symref format instead.")); + warn_once = 0; + return ret; } #endif diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh index 1e62c791d97250..b11126ed478129 100755 --- a/t/t0600-reffiles-backend.sh +++ b/t/t0600-reffiles-backend.sh @@ -477,9 +477,29 @@ test_expect_success SYMLINKS 'symref transaction supports symlinks' ' prepare commit EOF - git update-ref --no-deref --stdin err && + if test_have_prereq WITH_BREAKING_CHANGES + then + test_path_is_file .git/TEST_SYMREF_HEAD && + echo "ref: refs/heads/new" >expect && + test_cmp expect .git/TEST_SYMREF_HEAD && + test_must_be_empty err + else + test_path_is_symlink .git/TEST_SYMREF_HEAD && + test "$(test_readlink .git/TEST_SYMREF_HEAD)" = refs/heads/new && + cat >expect <<-EOF && + warning: ${SQ}core.preferSymlinkRefs=true${SQ} is nominated for removal. + hint: The use of symbolic links for symbolic refs is deprecated + hint: and will be removed in Git 3.0. The configuration that + hint: tells Git to use them is thus going away. You can unset + hint: it with: + hint: + hint: git config unset core.preferSymlinkRefs + hint: + hint: Git will then use the textual symref format instead. + EOF + test_cmp expect err + fi ' test_expect_success 'symref transaction supports false symlink config' ' From 181acc5f7f5f3d569ab0ab0d69b211371e6d2c48 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 10:01:35 +0200 Subject: [PATCH 02/14] object-name: convert to use `packfile_store_get_all_packs()` When searching for abbreviated or when trying to disambiguate object IDs we do this in two steps: 1. We search through the multi-pack index. 2. We search through all packfiles not part of any multi-pack index. The second step uses `packfile_store_get_packs()`, which knows to skip loading any packfiles that are indexed by an MIDX; this is exactly what we want. But that function is somewhat problematic, as its behaviour is stateful and is influenced by `packfile_store_get_all_packs()`. This function basically does the same as `packfile_store_get_packs()`, but in addition it also loads all packfiles indexed by an MIDX. The problem here is that both of these functions act on the same linked list of packfiles, and thus depending on whether or not `get_all_packs()` was called the result returned by `get_packs()` will be different. Consequently, all callers of `get_packs()` need to be prepared to see MIDX'd packs even though these should in theory be excluded. This interface is confusing and thus potentially dangerous, which is why we're converting all callers of `get_packs()` to use `get_all_packs()` instead. Do so for the above functions in "object-name.c". As explained, we already know to skip any MIDX'd packs in both `find_abbrev_len_packed()` and `find_short_packed_object()`, so it's fine to start loading MIDX'd packfiles. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-name.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/object-name.c b/object-name.c index f6902e140dd43e..4e62bfa330e5ab 100644 --- a/object-name.c +++ b/object-name.c @@ -213,7 +213,7 @@ static void find_short_packed_object(struct disambiguate_state *ds) unique_in_midx(m, ds); } - for (p = packfile_store_get_packs(ds->repo->objects->packfiles); p && !ds->ambiguous; + for (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous; p = p->next) unique_in_pack(p, ds); } @@ -805,7 +805,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad) find_abbrev_len_for_midx(m, mad); } - for (p = packfile_store_get_packs(mad->repo->objects->packfiles); p; p = p->next) + for (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next) find_abbrev_len_for_pack(p, mad); } From 07fbf2be2fdaa3629c06b2d6021a24c903e7890c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 10:01:36 +0200 Subject: [PATCH 03/14] builtin/gc: convert to use `packfile_store_get_all_packs()` When running maintenance tasks via git-maintenance(1) we have a couple of auto-conditions that check whether or not a specific task should be running. One such check is for incremental repacks, which essentially use `git multi-pack-index repack` to repack a set of smaller packfiles into one larger packfile. The auto-condition for this task checks how many packfiles there are that aren't indexed by any multi-pack index. If there is a sufficient number then we execute the above command to combine those into a single pack and add that pack to the MIDX. As we don't care about MIDX'd packs we use `packfile_store_get_packs()`, which knows to not load any packs that are indexed by a MIDX. But as explained in the preceding commit, we want to get rid of that function. We already handle packfiles that have a MIDX by the very nature of this function, as we explicitly count non-MIDX'd packs. As such, we can trivially switch over to use `packfile_store_get_all_packs()` instead. Do so. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/gc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/gc.c b/builtin/gc.c index e19e13d9788076..ab6d6d3bd1b445 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1425,7 +1425,7 @@ static int incremental_repack_auto_condition(struct gc_config *cfg UNUSED) if (incremental_repack_auto_limit < 0) return 1; - for (p = packfile_store_get_packs(the_repository->objects->packfiles); + for (p = packfile_store_get_all_packs(the_repository->objects->packfiles); count < incremental_repack_auto_limit && p; p = p->next) { if (!p->multi_pack_index) From fdebc5d4da055c281f27d2fe9b2022ebdd4171d4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 10:01:37 +0200 Subject: [PATCH 04/14] builtin/grep: simplify how we preload packs When using multiple threads in git-grep(1) we eagerly preload both the gitmodules file as well as the packfiles so that the threads won't race with one another to initialize these data structures. For packfiles, this is done by calling `packfile_store_get_packs()`, which first loads our packfiles and then returns a pointer to the first such packfile. This pointer is ignored though, as all we really care about is that `packfile_store_prepare()` was called. Historically, that function was file-local to "packfile.c", but that changed with 4188332569 (packfile: move `get_multi_pack_index()` into "midx.c", 2025-09-02). We can thus simplify the code by calling that function directly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/grep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/grep.c b/builtin/grep.c index 13841fbf00fed1..53cccf2d25068c 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1214,7 +1214,7 @@ int cmd_grep(int argc, if (recurse_submodules) repo_read_gitmodules(the_repository, 1); if (startup_info->have_repository) - (void)packfile_store_get_packs(the_repository->objects->packfiles); + packfile_store_prepare(the_repository->objects->packfiles); start_threads(&opt); } else { From 5b410c82768c025814af17e23cea3b7f253f111d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 10:01:38 +0200 Subject: [PATCH 05/14] packfile: drop `packfile_store_get_packs()` In the preceding commits we have removed all remaining callers of `packfile_store_get_packs()`, the function is thus unused now. Remove it. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- packfile.c | 6 ------ packfile.h | 6 ------ 2 files changed, 12 deletions(-) diff --git a/packfile.c b/packfile.c index 5a7caec2925977..db748b0bd48c27 100644 --- a/packfile.c +++ b/packfile.c @@ -1027,12 +1027,6 @@ void packfile_store_reprepare(struct packfile_store *store) packfile_store_prepare(store); } -struct packed_git *packfile_store_get_packs(struct packfile_store *store) -{ - packfile_store_prepare(store); - return store->packs; -} - struct packed_git *packfile_store_get_all_packs(struct packfile_store *store) { packfile_store_prepare(store); diff --git a/packfile.h b/packfile.h index e7a5792b6cf691..3f38c63476dcc1 100644 --- a/packfile.h +++ b/packfile.h @@ -136,12 +136,6 @@ void packfile_store_reprepare(struct packfile_store *store); void packfile_store_add_pack(struct packfile_store *store, struct packed_git *pack); -/* - * Get packs managed by the given store. Does not load the MIDX or any packs - * referenced by it. - */ -struct packed_git *packfile_store_get_packs(struct packfile_store *store); - /* * Get all packs managed by the given store, including packfiles that are * referenced by multi-pack indices. From 86d8c62f48a1b193299de19c4dbc664650a853f1 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 10:01:39 +0200 Subject: [PATCH 06/14] packfile: introduce macro to iterate through packs We have a bunch of different sites that want to iterate through all packs of a given `struct packfile_store`. This pattern is somewhat verbose and repetitive, which makes it somewhat cumbersome. Introduce a new macro `repo_for_each_pack()` that removes some of the boilerplate. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 3 +-- builtin/count-objects.c | 3 +-- builtin/fsck.c | 15 ++++++--------- builtin/gc.c | 16 +++++++--------- builtin/pack-objects.c | 22 +++++++--------------- builtin/pack-redundant.c | 14 ++++---------- connected.c | 3 +-- http-backend.c | 5 ++--- http.c | 3 +-- object-name.c | 8 +++++--- pack-bitmap.c | 6 +++--- pack-objects.c | 5 ++--- packfile.c | 4 ++-- packfile.h | 8 ++++++++ repack-cruft.c | 3 +-- repack-geometry.c | 3 +-- repack.c | 3 +-- server-info.c | 3 +-- t/helper/test-find-pack.c | 3 ++- t/helper/test-pack-mtimes.c | 2 +- 20 files changed, 57 insertions(+), 75 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index ee6715fa523ce6..0ab076aeb30ad9 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -852,10 +852,9 @@ static void batch_each_object(struct batch_options *opt, if (bitmap && !for_each_bitmapped_object(bitmap, &opt->objects_filter, batch_one_object_bitmapped, &payload)) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *pack; - for (pack = packfile_store_get_all_packs(packs); pack; pack = pack->next) { + repo_for_each_pack(the_repository, pack) { if (bitmap_index_contains_pack(bitmap, pack) || open_pack_index(pack)) continue; diff --git a/builtin/count-objects.c b/builtin/count-objects.c index f2f407c2a78183..18f6e33b6f913f 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -122,7 +122,6 @@ int cmd_count_objects(int argc, count_loose, count_cruft, NULL, NULL); if (verbose) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; unsigned long num_pack = 0; off_t size_pack = 0; @@ -130,7 +129,7 @@ int cmd_count_objects(int argc, struct strbuf pack_buf = STRBUF_INIT; struct strbuf garbage_buf = STRBUF_INIT; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (!p->pack_local) continue; if (open_pack_index(p)) diff --git a/builtin/fsck.c b/builtin/fsck.c index 8ee95e0d67cf37..b1a650c6731d32 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -867,20 +867,20 @@ static int mark_packed_for_connectivity(const struct object_id *oid, static int check_pack_rev_indexes(struct repository *r, int show_progress) { - struct packfile_store *packs = r->objects->packfiles; struct progress *progress = NULL; + struct packed_git *p; uint32_t pack_count = 0; int res = 0; if (show_progress) { - for (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next) + repo_for_each_pack(r, p) pack_count++; progress = start_delayed_progress(the_repository, "Verifying reverse pack-indexes", pack_count); pack_count = 0; } - for (struct packed_git *p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(r, p) { int load_error = load_pack_revindex_from_disk(p); if (load_error < 0) { @@ -1000,8 +1000,6 @@ int cmd_fsck(int argc, for_each_packed_object(the_repository, mark_packed_for_connectivity, NULL, 0); } else { - struct packfile_store *packs = the_repository->objects->packfiles; - odb_prepare_alternates(the_repository->objects); for (source = the_repository->objects->sources; source; source = source->next) fsck_source(source); @@ -1012,8 +1010,7 @@ int cmd_fsck(int argc, struct progress *progress = NULL; if (show_progress) { - for (p = packfile_store_get_all_packs(packs); p; - p = p->next) { + repo_for_each_pack(the_repository, p) { if (open_pack_index(p)) continue; total += p->num_objects; @@ -1022,8 +1019,8 @@ int cmd_fsck(int argc, progress = start_progress(the_repository, _("Checking objects"), total); } - for (p = packfile_store_get_all_packs(packs); p; - p = p->next) { + + repo_for_each_pack(the_repository, p) { /* verify gives error messages itself */ if (verify_pack(the_repository, p, fsck_obj_buffer, diff --git a/builtin/gc.c b/builtin/gc.c index ab6d6d3bd1b445..541d7471f19072 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -487,10 +487,9 @@ static int too_many_loose_objects(struct gc_config *cfg) static struct packed_git *find_base_packs(struct string_list *packs, unsigned long limit) { - struct packfile_store *packfiles = the_repository->objects->packfiles; struct packed_git *p, *base = NULL; - for (p = packfile_store_get_all_packs(packfiles); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (!p->pack_local || p->is_cruft) continue; if (limit) { @@ -509,14 +508,13 @@ static struct packed_git *find_base_packs(struct string_list *packs, static int too_many_packs(struct gc_config *cfg) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; - int cnt; + int cnt = 0; if (cfg->gc_auto_pack_limit <= 0) return 0; - for (cnt = 0, p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (!p->pack_local) continue; if (p->pack_keep) @@ -1425,9 +1423,9 @@ static int incremental_repack_auto_condition(struct gc_config *cfg UNUSED) if (incremental_repack_auto_limit < 0) return 1; - for (p = packfile_store_get_all_packs(the_repository->objects->packfiles); - count < incremental_repack_auto_limit && p; - p = p->next) { + repo_for_each_pack(the_repository, p) { + if (count >= incremental_repack_auto_limit) + break; if (!p->multi_pack_index) count++; } @@ -1494,7 +1492,7 @@ static off_t get_auto_pack_size(void) struct repository *r = the_repository; odb_reprepare(r->objects); - for (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) { + repo_for_each_pack(r, p) { if (p->pack_size > max_size) { second_largest_size = max_size; max_size = p->pack_size; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index fe987fbb8b6375..50618e1073ed99 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -3831,12 +3831,10 @@ static int pack_mtime_cmp(const void *_a, const void *_b) static void read_packs_list_from_stdin(struct rev_info *revs) { - struct packfile_store *packs = the_repository->objects->packfiles; struct strbuf buf = STRBUF_INIT; struct string_list include_packs = STRING_LIST_INIT_DUP; struct string_list exclude_packs = STRING_LIST_INIT_DUP; struct string_list_item *item = NULL; - struct packed_git *p; while (strbuf_getline(&buf, stdin) != EOF) { @@ -3856,7 +3854,7 @@ static void read_packs_list_from_stdin(struct rev_info *revs) string_list_sort(&exclude_packs); string_list_remove_duplicates(&exclude_packs, 0); - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { const char *pack_name = pack_basename(p); if ((item = string_list_lookup(&include_packs, pack_name))) @@ -4077,7 +4075,6 @@ static void enumerate_cruft_objects(void) static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; struct rev_info revs; int ret; @@ -4107,7 +4104,7 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs * Re-mark only the fresh packs as kept so that objects in * unknown packs do not halt the reachability traversal early. */ - for (p = packfile_store_get_all_packs(packs); p; p = p->next) + repo_for_each_pack(the_repository, p) p->pack_keep_in_core = 0; mark_pack_kept_in_core(fresh_packs, 1); @@ -4124,7 +4121,6 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs static void read_cruft_objects(void) { - struct packfile_store *packs = the_repository->objects->packfiles; struct strbuf buf = STRBUF_INIT; struct string_list discard_packs = STRING_LIST_INIT_DUP; struct string_list fresh_packs = STRING_LIST_INIT_DUP; @@ -4145,7 +4141,7 @@ static void read_cruft_objects(void) string_list_sort(&discard_packs); string_list_sort(&fresh_packs); - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { const char *pack_name = pack_basename(p); struct string_list_item *item; @@ -4440,13 +4436,12 @@ static int loosened_object_can_be_discarded(const struct object_id *oid, static void loosen_unused_packed_objects(void) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; uint32_t i; uint32_t loosened_objects_nr = 0; struct object_id oid; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (!p->pack_local || p->pack_keep || p->pack_keep_in_core) continue; @@ -4747,13 +4742,12 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv) static void add_extra_kept_packs(const struct string_list *names) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; if (!names->nr) return; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { const char *name = basename(p->pack_name); int i; @@ -5191,10 +5185,9 @@ int cmd_pack_objects(int argc, add_extra_kept_packs(&keep_pack_list); if (ignore_packed_keep_on_disk) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) + repo_for_each_pack(the_repository, p) if (p->pack_local && p->pack_keep) break; if (!p) /* no keep-able packs found */ @@ -5206,10 +5199,9 @@ int cmd_pack_objects(int argc, * want to unset "local" based on looking at packs, as * it also covers non-local objects */ - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (!p->pack_local) { have_non_local_packs = 1; break; diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index dd28171f0a179a..fca7f195d6d4e0 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@ -566,29 +566,23 @@ static struct pack_list * add_pack(struct packed_git *p) static struct pack_list * add_pack_file(const char *filename) { - struct packfile_store *packs = the_repository->objects->packfiles; - struct packed_git *p = packfile_store_get_all_packs(packs); + struct packed_git *p; if (strlen(filename) < 40) die("Bad pack filename: %s", filename); - while (p) { + repo_for_each_pack(the_repository, p) if (strstr(p->pack_name, filename)) return add_pack(p); - p = p->next; - } die("Filename %s not found in packed_git", filename); } static void load_all(void) { - struct packfile_store *packs = the_repository->objects->packfiles; - struct packed_git *p = packfile_store_get_all_packs(packs); + struct packed_git *p; - while (p) { + repo_for_each_pack(the_repository, p) add_pack(p); - p = p->next; - } } int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, struct repository *repo UNUSED) { diff --git a/connected.c b/connected.c index b288a18b17c33f..79403108dd8f57 100644 --- a/connected.c +++ b/connected.c @@ -74,10 +74,9 @@ int check_connected(oid_iterate_fn fn, void *cb_data, */ odb_reprepare(the_repository->objects); do { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *p; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (!p->pack_promisor) continue; if (find_pack_entry_one(oid, p)) diff --git a/http-backend.c b/http-backend.c index 9084058f1e9f13..52f0483dd309d7 100644 --- a/http-backend.c +++ b/http-backend.c @@ -603,19 +603,18 @@ static void get_head(struct strbuf *hdr, char *arg UNUSED) static void get_info_packs(struct strbuf *hdr, char *arg UNUSED) { size_t objdirlen = strlen(repo_get_object_directory(the_repository)); - struct packfile_store *packs = the_repository->objects->packfiles; struct strbuf buf = STRBUF_INIT; struct packed_git *p; size_t cnt = 0; select_getanyfile(hdr); - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (p->pack_local) cnt++; } strbuf_grow(&buf, cnt * 53 + 2); - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (p->pack_local) strbuf_addf(&buf, "P %s\n", p->pack_name + objdirlen + 6); } diff --git a/http.c b/http.c index 7e3af1e72f7401..17130823f006f2 100644 --- a/http.c +++ b/http.c @@ -2416,7 +2416,6 @@ static char *fetch_pack_index(unsigned char *hash, const char *base_url) static int fetch_and_setup_pack_index(struct packed_git **packs_head, unsigned char *sha1, const char *base_url) { - struct packfile_store *packs = the_repository->objects->packfiles; struct packed_git *new_pack, *p; char *tmp_idx = NULL; int ret; @@ -2425,7 +2424,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head, * If we already have the pack locally, no need to fetch its index or * even add it to list; we already have all of its objects. */ - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(the_repository, p) { if (hasheq(p->hash, sha1, the_repository->hash_algo)) return 0; } diff --git a/object-name.c b/object-name.c index 4e62bfa330e5ab..766c757042a389 100644 --- a/object-name.c +++ b/object-name.c @@ -213,9 +213,11 @@ static void find_short_packed_object(struct disambiguate_state *ds) unique_in_midx(m, ds); } - for (p = packfile_store_get_all_packs(ds->repo->objects->packfiles); p && !ds->ambiguous; - p = p->next) + repo_for_each_pack(ds->repo, p) { + if (ds->ambiguous) + break; unique_in_pack(p, ds); + } } static int finish_object_disambiguation(struct disambiguate_state *ds, @@ -805,7 +807,7 @@ static void find_abbrev_len_packed(struct min_abbrev_data *mad) find_abbrev_len_for_midx(m, mad); } - for (p = packfile_store_get_all_packs(mad->repo->objects->packfiles); p; p = p->next) + repo_for_each_pack(mad->repo, p) find_abbrev_len_for_pack(p, mad); } diff --git a/pack-bitmap.c b/pack-bitmap.c index ac71035d7715f7..291e1a9cf47158 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -664,7 +664,7 @@ static int open_pack_bitmap(struct repository *r, struct packed_git *p; int ret = -1; - for (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) { + repo_for_each_pack(r, p) { if (open_pack_bitmap_1(bitmap_git, p) == 0) { ret = 0; /* @@ -3347,6 +3347,7 @@ static int verify_bitmap_file(const struct git_hash_algo *algop, int verify_bitmap_files(struct repository *r) { struct odb_source *source; + struct packed_git *p; int res = 0; odb_prepare_alternates(r->objects); @@ -3362,8 +3363,7 @@ int verify_bitmap_files(struct repository *r) free(midx_bitmap_name); } - for (struct packed_git *p = packfile_store_get_all_packs(r->objects->packfiles); - p; p = p->next) { + repo_for_each_pack(r, p) { char *pack_bitmap_name = pack_bitmap_filename(p); res |= verify_bitmap_file(r->hash_algo, pack_bitmap_name); free(pack_bitmap_name); diff --git a/pack-objects.c b/pack-objects.c index d8eb679735484a..d6adf0759ccd77 100644 --- a/pack-objects.c +++ b/pack-objects.c @@ -86,7 +86,6 @@ struct object_entry *packlist_find(struct packing_data *pdata, static void prepare_in_pack_by_idx(struct packing_data *pdata) { - struct packfile_store *packs = pdata->repo->objects->packfiles; struct packed_git **mapping, *p; int cnt = 0, nr = 1U << OE_IN_PACK_BITS; @@ -96,13 +95,13 @@ static void prepare_in_pack_by_idx(struct packing_data *pdata) * (i.e. in_pack_idx also zero) should return NULL. */ mapping[cnt++] = NULL; - for (p = packfile_store_get_all_packs(packs); p; p = p->next, cnt++) { + repo_for_each_pack(pdata->repo, p) { if (cnt == nr) { free(mapping); return; } p->index = cnt; - mapping[cnt] = p; + mapping[cnt++] = p; } pdata->in_pack_by_idx = mapping; } diff --git a/packfile.c b/packfile.c index db748b0bd48c27..ab5859518d45ea 100644 --- a/packfile.c +++ b/packfile.c @@ -2099,7 +2099,7 @@ struct packed_git **kept_pack_cache(struct repository *r, unsigned flags) * covers, one kept and one not kept, but the midx returns only * the non-kept version. */ - for (p = packfile_store_get_all_packs(r->objects->packfiles); p; p = p->next) { + repo_for_each_pack(r, p) { if ((p->pack_keep && (flags & ON_DISK_KEEP_PACKS)) || (p->pack_keep_in_core && (flags & IN_CORE_KEEP_PACKS))) { ALLOC_GROW(packs, nr + 1, alloc); @@ -2196,7 +2196,7 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb, int r = 0; int pack_errors = 0; - for (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next) { + repo_for_each_pack(repo, p) { if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local) continue; if ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) && diff --git a/packfile.h b/packfile.h index 3f38c63476dcc1..49484a9b09b581 100644 --- a/packfile.h +++ b/packfile.h @@ -136,6 +136,14 @@ void packfile_store_reprepare(struct packfile_store *store); void packfile_store_add_pack(struct packfile_store *store, struct packed_git *pack); +/* + * Load and iterate through all packs of the given repository. This helper + * function will yield packfiles from all object sources connected to the + * repository. + */ +#define repo_for_each_pack(repo, p) \ + for (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next) + /* * Get all packs managed by the given store, including packfiles that are * referenced by multi-pack indices. diff --git a/repack-cruft.c b/repack-cruft.c index c51df367226d63..0653e88792332e 100644 --- a/repack-cruft.c +++ b/repack-cruft.c @@ -7,12 +7,11 @@ static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size, struct existing_packs *existing) { - struct packfile_store *packs = existing->repo->objects->packfiles; struct packed_git *p; struct strbuf buf = STRBUF_INIT; size_t i; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(existing->repo, p) { if (!(p->is_cruft && p->pack_local)) continue; diff --git a/repack-geometry.c b/repack-geometry.c index e2f9794d7debce..b3e32cd07ec119 100644 --- a/repack-geometry.c +++ b/repack-geometry.c @@ -29,11 +29,10 @@ void pack_geometry_init(struct pack_geometry *geometry, struct existing_packs *existing, const struct pack_objects_args *args) { - struct packfile_store *packs = existing->repo->objects->packfiles; struct packed_git *p; struct strbuf buf = STRBUF_INIT; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(existing->repo, p) { if (args->local && !p->pack_local) /* * When asked to only repack local packfiles we skip diff --git a/repack.c b/repack.c index 2ab33c665aeec3..596841027af93f 100644 --- a/repack.c +++ b/repack.c @@ -123,11 +123,10 @@ int finish_pack_objects_cmd(const struct git_hash_algo *algop, void existing_packs_collect(struct existing_packs *existing, const struct string_list *extra_keep) { - struct packfile_store *packs = existing->repo->objects->packfiles; struct packed_git *p; struct strbuf buf = STRBUF_INIT; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(existing->repo, p) { size_t i; const char *base; diff --git a/server-info.c b/server-info.c index 1d33de821e9f5e..b9a710544ab285 100644 --- a/server-info.c +++ b/server-info.c @@ -287,13 +287,12 @@ static int compare_info(const void *a_, const void *b_) static void init_pack_info(struct repository *r, const char *infofile, int force) { - struct packfile_store *packs = r->objects->packfiles; struct packed_git *p; int stale; int i; size_t alloc = 0; - for (p = packfile_store_get_all_packs(packs); p; p = p->next) { + repo_for_each_pack(r, p) { /* we ignore things on alternate path since they are * not available to the pullers in general. */ diff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c index e001dc3066db70..fc4b8a77b3007a 100644 --- a/t/helper/test-find-pack.c +++ b/t/helper/test-find-pack.c @@ -39,11 +39,12 @@ int cmd__find_pack(int argc, const char **argv) if (repo_get_oid(the_repository, argv[0], &oid)) die("cannot parse %s as an object name", argv[0]); - for (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next) + repo_for_each_pack(the_repository, p) { if (find_pack_entry_one(&oid, p)) { printf("%s\n", p->pack_name); actual_count++; } + } if (count > -1 && count != actual_count) die("bad packfile count %d instead of %d", actual_count, count); diff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c index 7c428c16011a23..7a8ee1de24ba83 100644 --- a/t/helper/test-pack-mtimes.c +++ b/t/helper/test-pack-mtimes.c @@ -37,7 +37,7 @@ int cmd__pack_mtimes(int argc, const char **argv) if (argc != 2) usage(pack_mtimes_usage); - for (p = packfile_store_get_all_packs(the_repository->objects->packfiles); p; p = p->next) { + repo_for_each_pack(the_repository, p) { strbuf_addstr(&buf, basename(p->pack_name)); strbuf_strip_suffix(&buf, ".pack"); strbuf_addstr(&buf, ".mtimes"); From ecad863c127cd167647e5929d94627c799587134 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 9 Oct 2025 10:01:40 +0200 Subject: [PATCH 07/14] packfile: rename `packfile_store_get_all_packs()` In a preceding commit we have removed `packfile_store_get_packs()`. With this function removed it's somewhat useless to still have the "all" infix in `packfile_store_get_all_packs()`. Rename the latter to drop that infix. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 4 ++-- builtin/pack-objects.c | 4 ++-- packfile.c | 2 +- packfile.h | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index b1d5549815ac66..fea914cf9eb7f2 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -976,7 +976,7 @@ static int store_object( if (e->idx.offset) { duplicate_count_by_type[type]++; return 1; - } else if (find_oid_pack(&oid, packfile_store_get_all_packs(packs))) { + } else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) { e->type = type; e->pack_id = MAX_PACK_ID; e->idx.offset = 1; /* just not zero! */ @@ -1177,7 +1177,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark) duplicate_count_by_type[OBJ_BLOB]++; truncate_pack(&checkpoint); - } else if (find_oid_pack(&oid, packfile_store_get_all_packs(packs))) { + } else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) { e->type = OBJ_BLOB; e->pack_id = MAX_PACK_ID; e->idx.offset = 1; /* just not zero! */ diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 50618e1073ed99..3a19bddd574ef2 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4394,7 +4394,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid) struct packed_git *p; p = (last_found != (void *)1) ? last_found : - packfile_store_get_all_packs(packs); + packfile_store_get_packs(packs); while (p) { if ((!p->pack_local || p->pack_keep || @@ -4404,7 +4404,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid) return 1; } if (p == last_found) - p = packfile_store_get_all_packs(packs); + p = packfile_store_get_packs(packs); else p = p->next; if (p == last_found) diff --git a/packfile.c b/packfile.c index ab5859518d45ea..1ae2b2fe1eda77 100644 --- a/packfile.c +++ b/packfile.c @@ -1027,7 +1027,7 @@ void packfile_store_reprepare(struct packfile_store *store) packfile_store_prepare(store); } -struct packed_git *packfile_store_get_all_packs(struct packfile_store *store) +struct packed_git *packfile_store_get_packs(struct packfile_store *store) { packfile_store_prepare(store); diff --git a/packfile.h b/packfile.h index 49484a9b09b581..c9d0b93446b5f5 100644 --- a/packfile.h +++ b/packfile.h @@ -142,13 +142,13 @@ void packfile_store_add_pack(struct packfile_store *store, * repository. */ #define repo_for_each_pack(repo, p) \ - for (p = packfile_store_get_all_packs(repo->objects->packfiles); p; p = p->next) + for (p = packfile_store_get_packs(repo->objects->packfiles); p; p = p->next) /* * Get all packs managed by the given store, including packfiles that are * referenced by multi-pack indices. */ -struct packed_git *packfile_store_get_all_packs(struct packfile_store *store); +struct packed_git *packfile_store_get_packs(struct packfile_store *store); /* * Get all packs in most-recently-used order. From c568fa8e1c740c19f8b1cc7efeeef2c6c52961dd Mon Sep 17 00:00:00 2001 From: KIYOTA Fumiya Date: Mon, 20 Oct 2025 17:32:57 +0000 Subject: [PATCH 08/14] completion: complete some 'git log' options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. '--exclude=' option to 'git log' and 'git shortlog' are missing. Add the option to __git_log_shortlog_options. 2. The `--committer` option in `git log` requires a pattern, such as `--committer=ba`, but in `git shortlog`, specifying a pattern results in an error: “error: option `committer' takes no value.” Handle them as separate options for completion rather than a shared one. Signed-off-by: KIYOTA Fumiya Signed-off-by: Junio C Hamano --- contrib/completion/git-completion.bash | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e3d88b06721b39..73abea31b428f3 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2218,7 +2218,7 @@ __git_log_gitk_options=" " # Options that go well for log and shortlog (not gitk) __git_log_shortlog_options=" - --author= --committer= --grep= + --author= --grep= --exclude= --all-match --invert-grep " # Options accepted by log and show @@ -2296,6 +2296,7 @@ __git_complete_log_opts () $__git_log_shortlog_options $__git_log_gitk_options $__git_log_show_options + --committer= --root --topo-order --date-order --reverse --follow --full-diff --abbrev-commit --no-abbrev-commit --abbrev= @@ -3229,7 +3230,7 @@ _git_shortlog () __gitcomp " $__git_log_common_options $__git_log_shortlog_options - --numbered --summary --email + --committer --numbered --summary --email " return ;; From fafdf23b2f57bdf6a74513b3cc03902f0a8e954d Mon Sep 17 00:00:00 2001 From: Emily Yang Date: Fri, 17 Oct 2025 20:58:59 +0000 Subject: [PATCH 09/14] commit-graph: add new config for changed-paths & recommend it in scalar The changed-path Bloom filters feature has proven stable and reliable over several years of use, delivering significant performance improvement for file history computation in large monorepos. Currently a user can opt-in to writing the changed-path Bloom filters using the "--changed-paths" option to "git commit-graph write". The filters will be persisted until the user drops the filters using the "--no-changed-paths" option. For this functionality, refer to 0087a87ba8 (commit-graph: persist existence of changed-paths, 2020-07-01). Large monorepos using Git's background maintenance to build and update commit-graph files could use an easy switch to enable this feature without a foreground computation. In this commit, we're proposing a new config option "commitGraph.changedPaths": * If "true", "git commit-graph write" will write Bloom filters, equivalent to passing "--changed-paths"; * If "false" or "unset", Bloom filters will be written during "git commit-graph write" only if the filters already exist in the current commit-graph file. This matches the default behaviour of "git commit-graph write" without any "--[no-]changed-paths" option. Note "false" can disable a previous "true" config value but doesn't imply "--no-changed-paths". This config will always respect the precedence of command line option "--[no-]changed-paths". We also set this new config as optional recommended config in scalar to turn on this feature for large repos. Helped-by: Derrick Stolee Signed-off-by: Emily Yang Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Documentation/config/commitgraph.adoc | 11 +++++++ Documentation/git-commit-graph.adoc | 2 +- builtin/commit-graph.c | 2 ++ scalar.c | 1 + t/t5318-commit-graph.sh | 44 +++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 1 deletion(-) diff --git a/Documentation/config/commitgraph.adoc b/Documentation/config/commitgraph.adoc index 7f8c9d6638f1a1..70a56c53d2a109 100644 --- a/Documentation/config/commitgraph.adoc +++ b/Documentation/config/commitgraph.adoc @@ -8,6 +8,17 @@ commitGraph.maxNewFilters:: Specifies the default value for the `--max-new-filters` option of `git commit-graph write` (c.f., linkgit:git-commit-graph[1]). +commitGraph.changedPaths:: + If true, then `git commit-graph write` will compute and write + changed-path Bloom filters by default, equivalent to passing + `--changed-paths`. If false or unset, changed-paths Bloom filters will + be written during `git commit-graph write` only if the filters already + exist in the current commit-graph file. This matches the default + behavior of `git commit-graph write` without any `--[no-]changed-paths` + option. To rewrite a commit-graph file without any filters, use the + `--no-changed-paths` option. Command-line option `--[no-]changed-paths` + always takes precedence over this configuration. Defaults to unset. + commitGraph.readChangedPaths:: Deprecated. Equivalent to commitGraph.changedPathsVersion=-1 if true, and commitGraph.changedPathsVersion=0 if false. (If commitGraph.changedPathVersion diff --git a/Documentation/git-commit-graph.adoc b/Documentation/git-commit-graph.adoc index e9558173c001f1..6d19026035f96a 100644 --- a/Documentation/git-commit-graph.adoc +++ b/Documentation/git-commit-graph.adoc @@ -71,7 +71,7 @@ take a while on large repositories. It provides significant performance gains for getting history of a directory or a file with `git log -- `. If this option is given, future commit-graph writes will automatically assume that this option was intended. Use `--no-changed-paths` to stop storing this -data. +data. `--changed-paths` is implied by config `commitGraph.changedPaths=true`. + With the `--max-new-filters=` option, generate at most `n` new Bloom filters (if `--changed-paths` is specified). If `n` is `-1`, no limit is diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index fe3ebaadadadb6..d62005edc0469c 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -210,6 +210,8 @@ static int git_commit_graph_write_config(const char *var, const char *value, { if (!strcmp(var, "commitgraph.maxnewfilters")) write_opts.max_new_filters = git_config_int(var, value, ctx->kvi); + else if (!strcmp(var, "commitgraph.changedpaths")) + opts.enable_changed_paths = git_config_bool(var, value) ? 1 : -1; /* * No need to fall-back to 'git_default_config', since this was already * called in 'cmd_commit_graph()'. diff --git a/scalar.c b/scalar.c index 4a373c133d8562..f7543116272b77 100644 --- a/scalar.c +++ b/scalar.c @@ -166,6 +166,7 @@ static int set_recommended_config(int reconfigure) #endif /* Optional */ { "status.aheadBehind", "false" }, + { "commitGraph.changedPaths", "true" }, { "commitGraph.generationVersion", "1" }, { "core.autoCRLF", "false" }, { "core.safeCRLF", "false" }, diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 0b3404f58fe5f9..98c69109632c2d 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -946,4 +946,48 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' ' ) ' +test_expect_success 'config commitGraph.changedPaths acts like --changed-paths' ' + git init config-changed-paths && + ( + cd config-changed-paths && + + # commitGraph.changedPaths is not set and it should not write Bloom filters + test_commit first && + GIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error && + test_grep ! "Bloom filters" error && + + # Set commitGraph.changedPaths to true and it should write Bloom filters + test_commit second && + git config commitGraph.changedPaths true && + GIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error && + test_grep "Bloom filters" error && + + # Add one more config commitGraph.changedPaths as false to disable the previous true config value + # It should still write Bloom filters due to existing filters + test_commit third && + git config --add commitGraph.changedPaths false && + GIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error && + test_grep "Bloom filters" error && + + # commitGraph.changedPaths is still false and command line options should take precedence + test_commit fourth && + GIT_PROGRESS_DELAY=0 git commit-graph write --no-changed-paths --reachable --progress 2>error && + test_grep ! "Bloom filters" error && + GIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error && + test_grep ! "Bloom filters" error && + + # commitGraph.changedPaths is all cleared and then set to false again, command line options should take precedence + test_commit fifth && + git config --unset-all commitGraph.changedPaths && + git config commitGraph.changedPaths false && + GIT_PROGRESS_DELAY=0 git commit-graph write --changed-paths --reachable --progress 2>error && + test_grep "Bloom filters" error && + + # commitGraph.changedPaths is still false and it should write Bloom filters due to existing filters + test_commit sixth && + GIT_PROGRESS_DELAY=0 git commit-graph write --reachable --progress 2>error && + test_grep "Bloom filters" error + ) +' + test_done From 2bb3a012f3d756ad7101c359f38285a018f9e517 Mon Sep 17 00:00:00 2001 From: Ruoyu Zhong Date: Wed, 22 Oct 2025 08:36:13 +0000 Subject: [PATCH 10/14] bisect: fix handling of `help` and invalid subcommands As documented in git-bisect(1), `git bisect help` should display usage information. However, since the migration of `git bisect` to a full builtin command in 73fce29427 (Turn `git bisect` into a full built-in, 2022-11-10), this behavior was broken. Running `git bisect help` would, instead of showing usage, either fail silently if already in a bisect session, or otherwise trigger an interactive autostart prompt asking "Do you want me to do it for you [Y/n]?". Similarly, since df63421be9 (bisect--helper: handle states directly, 2022-11-10), running invalid subcommands like `git bisect foobar` also led to the same behavior. This occurred because `help` and other unrecognized subcommands were being unconditionally passed to `bisect_state`, which then called `bisect_autostart`, triggering the interactive prompt. Fix this by: 1. Adding explicit handling for the `help` subcommand to show usage; 2. Validating that unrecognized commands are actually valid state commands before calling `bisect_state`; 3. Showing an error with usage for truly invalid commands. This ensures that `git bisect help` displays the usage as documented, and invalid commands fail cleanly without entering interactive mode. Alternate terms are still handled correctly through `check_and_set_terms`. Signed-off-by: Ruoyu Zhong Signed-off-by: Junio C Hamano --- builtin/bisect.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builtin/bisect.c b/builtin/bisect.c index 8b8d870cd1ef08..993caf545dbd2c 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -1453,9 +1453,13 @@ int cmd_bisect(int argc, if (!argc) usage_msg_opt(_("need a command"), git_bisect_usage, options); + if (!strcmp(argv[0], "help")) + usage_with_options(git_bisect_usage, options); + set_terms(&terms, "bad", "good"); get_terms(&terms); - if (check_and_set_terms(&terms, argv[0])) + if (check_and_set_terms(&terms, argv[0]) || + !one_of(argv[0], terms.term_good, terms.term_bad, NULL)) usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage, options, argv[0]); res = bisect_state(&terms, argc, argv); From 3ed5d8bd7366076fd049e735e363e5a77656743c Mon Sep 17 00:00:00 2001 From: Lidong Yan Date: Mon, 20 Oct 2025 00:30:24 +0800 Subject: [PATCH 11/14] diff: stop output garbled message in dry run mode Earlier, b55e6d36 (diff: ensure consistent diff behavior with ignore options, 2025-08-08) introduced "dry-run" mode to the diff machinery so that content-based diff filtering (like ignoring space changes or those that match -I) can first try to produce a patch without emitting any output to see if under the given diff filtering condition we would get any output lines, and a new helper function diff_flush_patch_quietly() was introduced to use the mode to see an individual filepair needs to be shown. However, the solution was not complete. When files are deleted, file modes change, or there are unmerged entries in the index, dry-run mode still produces output because we overlooked these conditions, and as a result, dry-run mode was not quiet. To fix this, return early in emit_diff_symbol_from_struct() if we are in dry-run mode. This function will be called by all the emit functions to output the results. Returning early can avoid diff output when files are deleted or file modes are changed. Stop print message in dry-run mode if we have unmerged entries in index. Discard output of external diff tool in dry-run mode. Signed-off-by: Lidong Yan Signed-off-by: Junio C Hamano --- diff.c | 8 ++++++-- t/t4013-diff-various.sh | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/diff.c b/diff.c index a74e701806be52..22415aeceec6aa 100644 --- a/diff.c +++ b/diff.c @@ -1351,6 +1351,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o, int len = eds->len; unsigned flags = eds->flags; + if (o->dry_run) + return; + switch (s) { case DIFF_SYMBOL_NO_LF_EOF: context = diff_get_color_opt(o, DIFF_CONTEXT); @@ -4420,7 +4423,7 @@ static void run_external_diff(const struct external_diff *pgm, { struct child_process cmd = CHILD_PROCESS_INIT; struct diff_queue_struct *q = &diff_queued_diff; - int quiet = !(o->output_format & DIFF_FORMAT_PATCH); + int quiet = !(o->output_format & DIFF_FORMAT_PATCH) || o->dry_run; int rc; /* @@ -4615,7 +4618,8 @@ static void run_diff_cmd(const struct external_diff *pgm, p->status == DIFF_STATUS_RENAMED) o->found_changes = 1; } else { - fprintf(o->file, "* Unmerged path %s\n", name); + if (!o->dry_run) + fprintf(o->file, "* Unmerged path %s\n", name); o->found_changes = 1; } } diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 55a06eadb3175e..d35695f5b0bcf2 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -661,6 +661,43 @@ test_expect_success 'diff -I: ignore matching file' ' test_grep ! "file1" actual ' +test_expect_success 'diff -I: ignore all content changes' ' + test_when_finished "git rm -f file1 file2 file3" && + : >file1 && + git add file1 && + : >file2 && + git add file2 && + : >file3 && + git add file3 && + + rm -f file1 file2 && + mkdir file2 && + echo "A" >file3 && + A_hash=$(git hash-object -w file3) && + echo "B" >file3 && + B_hash=$(git hash-object -w file3) && + cat <<-EOF | git update-index --index-info && + 100644 $A_hash 1 file3 + 100644 $B_hash 2 file3 + EOF + + test_diff_no_content_changes () { + git diff $1 --ignore-blank-lines -I".*" >actual && + test_line_count = 3 actual && + test_grep "file1" actual && + test_grep "file2" actual && + test_grep "file3" actual && + test_grep ! "diff --git" actual + } && + test_diff_no_content_changes "--raw" && + test_diff_no_content_changes "--name-only" && + test_diff_no_content_changes "--name-status" && + + : >actual && + test_must_fail git diff --quiet -I".*" >actual && + test_must_be_empty actual +' + # check_prefix # check only lines with paths to avoid dependency on exact oid/contents check_prefix () { From bee1bdd5888aafd1a8d51df000170f18b6a299ac Mon Sep 17 00:00:00 2001 From: Olamide Caleb Bello Date: Thu, 23 Oct 2025 11:13:46 +0000 Subject: [PATCH 12/14] gpg-interface: do not use misdesigned strbuf_split*() In get_ssh_finger_print(), the output of the `ssh-keygen` command is put into `fingerprint_stdout` strbuf. The string in `fingerprint_stdout` is then split into up to 3 strbufs using strbuf_split_max(). However they are not modified after the split thereby not making use of the strbuf API as the fingerprint token is merely returned as a char * and not a strbuf. Hence they do not need to be strbufs. Simplify the process of retrieving and returning the desired token by using strchr() to isolate the token and xmemdupz() to return a copy of the token. This removes the roundabout way of splitting the string into strbufs just to return the token. Reported-by: Junio Hamano Helped-by: Christian Couder Helped-by: Kristoffer Haugsbakk Signed-off-by: Olamide Caleb Bello Signed-off-by: Junio C Hamano --- gpg-interface.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/gpg-interface.c b/gpg-interface.c index 06e7fb50603d22..68cb584732f8a0 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -821,8 +821,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key) struct child_process ssh_keygen = CHILD_PROCESS_INIT; int ret = -1; struct strbuf fingerprint_stdout = STRBUF_INIT; - struct strbuf **fingerprint; - char *fingerprint_ret; + char *fingerprint_ret, *begin, *delim; const char *literal_key = NULL; /* @@ -845,13 +844,17 @@ static char *get_ssh_key_fingerprint(const char *signing_key) die_errno(_("failed to get the ssh fingerprint for key '%s'"), signing_key); - fingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3); - if (!fingerprint[1]) - die_errno(_("failed to get the ssh fingerprint for key '%s'"), + begin = fingerprint_stdout.buf; + delim = strchr(begin, ' '); + if (!delim) + die(_("failed to get the ssh fingerprint for key %s"), signing_key); - - fingerprint_ret = strbuf_detach(fingerprint[1], NULL); - strbuf_list_free(fingerprint); + begin = delim + 1; + delim = strchr(begin, ' '); + if (!delim) + die(_("failed to get the ssh fingerprint for key %s"), + signing_key); + fingerprint_ret = xmemdupz(begin, delim - begin); strbuf_release(&fingerprint_stdout); return fingerprint_ret; } From 2ab72a16d9e3f1aca223f5da5aaf8b533d8fa35a Mon Sep 17 00:00:00 2001 From: Olamide Caleb Bello Date: Thu, 23 Oct 2025 11:13:47 +0000 Subject: [PATCH 13/14] gpg-interface: do not use misdesigned strbuf_split*() In get_default_ssh_signing_key(), the default ssh signing key is retrieved in `key_stdout` buf, which is then split using strbuf_split_max() into up to two strbufs at a new line and the first strbuf is returned as a `char *`and not a strbuf. This makes the function lack the use of strbuf API as no edits are performed on the split tokens. Simplify the process of retrieving and returning the desired line by using strchr() to isolate the line and xmemdupz() to return a copy of the line. This removes the roundabout way of splitting the string into strbufs, just to return the line. Reported-by: Junio Hamano Helped-by: Christian Couder Helped-by: Kristoffer Haugsbakk Signed-off-by: Olamide Caleb Bello Signed-off-by: Junio C Hamano --- gpg-interface.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/gpg-interface.c b/gpg-interface.c index 68cb584732f8a0..48f6e0d55f5c4b 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -865,12 +865,12 @@ static char *get_default_ssh_signing_key(void) struct child_process ssh_default_key = CHILD_PROCESS_INIT; int ret = -1; struct strbuf key_stdout = STRBUF_INIT, key_stderr = STRBUF_INIT; - struct strbuf **keys; char *key_command = NULL; const char **argv; int n; char *default_key = NULL; const char *literal_key = NULL; + char *begin, *new_line, *first_line; if (!ssh_default_key_command) die(_("either user.signingkey or gpg.ssh.defaultKeyCommand needs to be configured")); @@ -887,19 +887,24 @@ static char *get_default_ssh_signing_key(void) &key_stderr, 0); if (!ret) { - keys = strbuf_split_max(&key_stdout, '\n', 2); - if (keys[0] && is_literal_ssh_key(keys[0]->buf, &literal_key)) { + begin = key_stdout.buf; + new_line = strchr(begin, '\n'); + if (new_line) + first_line = xmemdupz(begin, new_line - begin); + else + first_line = xstrdup(begin); + if (is_literal_ssh_key(first_line, &literal_key)) { /* * We only use `is_literal_ssh_key` here to check validity * The prefix will be stripped when the key is used. */ - default_key = strbuf_detach(keys[0], NULL); + default_key = first_line; } else { + free(first_line); warning(_("gpg.ssh.defaultKeyCommand succeeded but returned no keys: %s %s"), key_stderr.buf, key_stdout.buf); } - strbuf_list_free(keys); } else { warning(_("gpg.ssh.defaultKeyCommand failed: %s %s"), key_stderr.buf, key_stdout.buf); From a99f379adf116d53eb11957af5bab5214915f91d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 30 Oct 2025 07:34:16 -0700 Subject: [PATCH 14/14] The 27th batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.52.0.adoc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Documentation/RelNotes/2.52.0.adoc b/Documentation/RelNotes/2.52.0.adoc index a7f9afc5a18f16..a86e2c09e06969 100644 --- a/Documentation/RelNotes/2.52.0.adoc +++ b/Documentation/RelNotes/2.52.0.adoc @@ -64,6 +64,12 @@ UI, Workflows & Features * "git fast-import" is taught to handle signed tags, just like it recently learned to handle signed commits, in different ways. + * A new configuration variable commitGraph.changedPaths allows to + turn "--changed-paths" on by default for "git commit-graph". + + * "Symlink symref" has been added to the list of things that will + disappear at Git 3.0 boundary. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -149,6 +155,9 @@ Performance, Internal Implementation, Development Support etc. * The code in "git repack" machinery has been cleaned up to prepare for incremental update of midx files. + * Two slightly different ways to get at "all the packfiles" in API + has been cleaned up. + Fixes since v2.51 ----------------- @@ -359,6 +368,20 @@ including security updates, are included in this release. fail doe to overly long pathname in our test environment, which has been worked around by using "ssh-agent -T". + * strbuf_split*() to split a string into multiple strbufs is often a + wrong API to use. A few uses of it have been removed by + simplifying the code. + (merge 2ab72a16d9 ob/gpg-interface-cleanup later to maint). + + * "git shortlog" knows "--committer" and "--author" options, which + the command line completion (in contrib/) did not handle well, + which has been corrected. + (merge c568fa8e1c kf/log-shortlog-completion-fix later to maint). + + * "git bisect" command did not react correctly to "git bisect help" + and "git bisect unknown", which has been corrected. + (merge 2bb3a012f3 rz/bisect-help-unknown later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 529a60a885 ua/t1517-short-help-tests later to maint). (merge 22d421fed9 ac/deglobal-fmt-merge-log-config later to maint).