builtin/refs: add list subcommand by inosmeet · Pull Request #2 · inosmeet/git

inosmeet · 2025-06-09T06:05:26Z

Currently, Git's reference management is distributed across multiple commands and as part of an ongoing effort to streamline and modernize reference handling, we are beginning to consolidate these operations into a cohesive git refs command.

Add a list subcommand to git refs as a modern replacement for git show-ref, consolidating ref listing functionality under the unified git refs interface.

Test the implemented functionality of git refs list and verify backward compatibility with git show-ref for the supported flags and patterns.

shejialuo

Currently, Git's reference management is distributed across multiple
commands and as part of an ongoing effort to streamline and modernize
reference handling, we are beginning to consolidate these operations
into a cohesive git refs command.

In Git, we do not use Currently. Because when you are writing the commit message, it is the current. And Let's drop that, simply say "Git's reference..."

Add a list subcommand to git refs as a modern replacement for
git show-ref, consolidating ref listing functionality under the
unified git refs interface.

There are some other options that you do not consider in this commit. So, I think you should explicitlty mention which options you add and why you chooese these options and why you not touch other options.

Documentation/git-refs.adoc

builtin/refs.c

inosmeet · 2025-06-11T06:43:03Z

builtin/refs.c

+static int show_ref(const char *refname, const char *referent UNUSED,
+		    const struct object_id *oid, int flag UNUSED, void *cbdata)


I wonder if we should rename it to list_ref or something similar?

I think that list_ref would be OK. Each way is OK.

KarthikNayak

Reviewed from my side. One big question I have is why did we decide to side with git-show-ref(1) and ignore git-for-each-ref(1).

KarthikNayak · 2025-06-11T14:39:46Z

Documentation/git-refs.adoc

Add a list subcommand to git refs as a modern replacement for git show-ref, consolidating ref listing functionality under the unified git refs command.

In the prev para, we mention both git-for-each-ref(1) and git-show-ref(1) but here, we talk about the latter only. Why? What about the features in former. How are we consolidating them together? It would be nice to talk more about the decisions taken here.

All existing git show-ref options are within the eventual scope of git refs list subcommand.

Is it though? Will git show-ref --verify be ported to git-refs-list? So perhaps some more digging into the different options of both git-for-each-ref(1) and git-show-ref(1) and stating what will and will not be ported would help understand the goal. This can probably go into the cover letter.

KarthikNayak · 2025-06-11T14:44:37Z

Documentation/git-refs.adoc


 verify::
 	Verify reference database consistency.
+list::


missing newline above this.

KarthikNayak · 2025-06-11T14:46:14Z

Documentation/git-refs.adoc

+list::
+	Displays references available in a local repository along with the associated
+	commit IDs. This subcommand serves as a functional replacement for
+	`git show-ref`, offering a more consistent interface within `git refs`.


I would expect the inverse, i.e. once this project is at a state which can replace git-show-ref(1) we add something like this to git show-ref urging users to migrate to using git refs list instead.

Why mention it here, when you already have users using it.

Make sense, in the first version, I suggested that we could mention that. By seeing your comment, I'd agree we should use the inverse way to notify the user.

KarthikNayak · 2025-06-11T14:49:27Z

Documentation/git-refs.adoc

+<pattern>...::
+	Show references matching one or more patterns. Patterns are matched from
+	the end of the full name, and only complete parts are matched, e.g.
+	'master' matches 'refs/heads/master', 'refs/remotes/origin/master',
+	'refs/tags/jedi/master' but not 'refs/heads/mymaster' or
+	'refs/remotes/master/jedi'.


From man git-for-each-ref we have

<pattern>...
If one or more patterns are given, only refs are shown that match against at least one pattern, either using fnmatch(3) or literally, in the
latter case matching completely or from the beginning up to a slash.

Why did we pick the one from git-show-ref(1)? I would assume most would prefer the pattern match listed above.

Initially, I hadn't implemented pattern matching at all, but during code review, Patrick suggested adding it. I assumed the expectation was to mirror the behavior of git-show-ref, given the context of replacing that command specifically, so I based my implementation on its pattern-matching logic.

At the time, I hadn't considered the behavior of git-for-each-ref. My plan was to go command-by-command, ensuring compatibility with each legacy command individually. If aligning with git-for-each-ref's approach to pattern matching is preferred (or considered more consistent across Git tools), I'd be happy to revisit and adapt the implementation accordingly.

is preferred

I wouldn't say it is preferred, but more on the lines of that this needs to be something you investigate and note down in the commit message around which pattern matching was picked and why.

KarthikNayak · 2025-06-11T14:52:44Z

builtin/refs.c

+	int filter_branches;
+	int filter_tags;
+	int found_match;
+	const char **patterns;


Shouldn't this be const char **patterns?

This probably means multiple patterns don't work correctly, also there is no test for multiple patterns.

I think this might be based on an earlier version of the code, it's already const char **patterns now. That said, you're right that we don't have a test for multiple patterns yet. I'll go ahead and add one.

builtin/refs.c

KarthikNayak · 2025-06-11T15:02:20Z

builtin/refs.c

+	if (!has_object(the_repository, oid,
+			HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))
+		die("git refs list: bad ref %s (%s)", refname,
+		    oid_to_hex(oid));


This was unexpected, seems like git-show-ref(1) also does object verification. This can really slow down the command, do we want this? git-for-each-ref(1) doesn't do this AFAIK.

Due to git-show-ref(1) does object verification, I suggested Meet to do above in the first version. I think we need to think about this design question: whether we need to do check.

How about we default to the faster for-each-refstyle enumeration and introduce a --strict flag? Without --strict, git refs list would simply list all refs (even dangling ones). With --strict, it would perform the object-existence check and error out on any dangling refs, exactly as git show-ref does. This gives us full performance by default, plus an opt-in safety mode. Let me know if that sounds like the right balance.

builtin/refs.c

t/t1461-refs-list.sh

inosmeet · 2025-06-12T10:39:31Z

Since the pattern matching used by for-each-ref is more flexible, and supports both complete and prefix matches, it makes sense for git refs list to adopt this.

At the same time, we want to preserve backward compatibility with git show-ref. Would it make sense to use for-each-ref-style matching as the default, and provide show-ref-style matching via a dedicated flag?

I’m a little unsure about which direction is best here, would love your thoughts.
@KarthikNayak @shejialuo

KarthikNayak · 2025-06-12T13:41:24Z

Since the pattern matching used by for-each-ref is more flexible, and supports both complete and prefix matches, it makes sense for git refs list to adopt this.

At the same time, we want to preserve backward compatibility with git show-ref. Would it make sense to use for-each-ref-style matching as the default, and provide show-ref-style matching via a dedicated flag?

I’m a little unsure about which direction is best here, would love your thoughts. @KarthikNayak @shejialuo

My hunch is to also use the pattern matching provided by git-for-each-ref and eventually provide an option for the one present in git-show-ref. But we should probably pick one and send an RFC to the list and get more opinions

inosmeet · 2025-06-13T07:23:41Z

Since I’ve already implemented git show-ref-style pattern matching, I was thinking of sending the RFC based on that, as a way to demonstrate the current behavior and open up discussion on whether we should switch to the for-each-ref matching style by default.

If the consensus leans toward for-each-ref-style matching, I can follow up with a patch to switch the default and add a flag for legacy matching.

Does this approach make sense?

KarthikNayak · 2025-06-13T07:35:48Z

Since I’ve already implemented git show-ref-style pattern matching, I was thinking of sending the RFC based on that, as a way to demonstrate the current behavior and open up discussion on whether we should switch to the for-each-ref matching style by default.

If the consensus leans toward for-each-ref-style matching, I can follow up with a patch to switch the default and add a flag for legacy matching.

Does this approach make sense?

That's okay from my side, I do request that you highlight all of this in the cover letter.

shejialuo · 2025-06-13T12:04:02Z

Since I’ve already implemented git show-ref-style pattern matching, I was thinking of sending the RFC based on that, as a way to demonstrate the current behavior and open up discussion on whether we should switch to the for-each-ref matching style by default.

If the consensus leans toward for-each-ref-style matching, I can follow up with a patch to switch the default and add a flag for legacy matching.

Does this approach make sense?

Yeah, make sense. Let's discuss with the mailing list further.

shejialuo

I think we indeed introduce repeated code path. However, we should explain why we have to do that in the commit message.

So, the motivation is super important. Do we want to git for-each-ref always be the same as git refs list? I think this is not our goal, and that's the main reason why we need to make a copy as we want to add more options to the git refs list.

shejialuo · 2025-06-24T13:56:11Z

builtin/refs.c

 	return ret;
 }

+static int cmd_refs_list(int argc, const char **argv, const char *prefix,


The cmd_refs_list is the same as cmd_for_each_ref, I think we should mention this part in the commit message, mostly about our motivation.

Should we mention the following things?

Most of logic is implemented in the shared module.

We would add more functionality into "git refs list" later but at now we don't want to break back compatibility and we just make a copy of cmd_for_each_ref.

Should we mention the following things?

1. Most of logic is implemented in the shared module.

We already mention that, but I'll update it to be more direct.

2. We would add more functionality into "git refs list" later but at now we don't want to break back compatibility and we just make a copy of `cmd_for_each_ref`.

Yeah, makes sense.

shejialuo · 2025-06-24T14:06:36Z

t/lib-for-each-ref.sh

I don't mind that you create a helper shell script to extract the logic. But do we really want to share the same logic? At current, we would assume that git for-each-ref would always be the same as git refs list. We indeed want to keep back compatibility but this does not mean that everything would be the same.

I think somehow we would add more options or functionalities into git refs list later. So, we may just make a copy from t6300-for-each-ref.sh and mention why we do this in the commit message.

That's what I was curious about; for internal reviews I chose extracting since the test file of for-each-ref was quite big. Let's wait for @KarthikNayak's opinion before sending the updated RFC.

Well, this isn't really a lib, is it?

A lib is something that provides functionality. Here, there are tests which are added.

My suggestion would perhaps be to keep as t/t6300-for-each-ref.sh as is, instead to replace the calls to git for-each-ref in the test with GIT_REFS_LIST_CMD and then t/t1461-refs-list.sh would simply be

GIT_REFS_LIST_CMD="git refs list" ./t6300-for-each-ref.sh

wdyt?

This one sounds good too.

@shejialuo raised an interesting question here, following on this approach, what do we do when for-each-ref and refs list diverges?

Well the new tests will then be added to t/t1461-refs-list.sh directly

This patch adds a basic support of SHA256 Git repository to Gitk, so that Gitk can show and operate on both SHA1 and SHA256 repos gracefully. Since SHA256 has a longer ID length (64 char) than SHA1 (40 char), many field widths are adjusted to fit with it. A caveat is that the configuration of auto selection length is shared between SHA1 and SHA256 repos. That is, once when this value is saved and read, it's applied to both repo types, which may result in shorter selection than the full SHA256 ID. We may introduce another individual config for sha256 (actually I did write in the first version), but for simplicity, the common config is used as of writing this. Many lines still refer "sha1" although they may point to both SHA1 and SHA256. They are left untouched for making the changes simpler. This patch is based on the early work by Rostislav Krasny: https://patchwork.kernel.org/project/git/patch/pull.979.git.1623687519832.gitgitgadget@gmail.com I refreshed, revised and extended to the latest state. Signed-off-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

In bloom.h, murmur3_seeded_v2() is exported for the use of test murmur3 hash. To clarify that murmur3_seeded_v2() is exported solely for testing purposes, a new helper function test_murmur3_seeded() was added instead of exporting murmur3_seeded_v2() directly. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>

git code style requires that functions operating on a struct S should be named in the form S_verb. However, the functions operating on struct bloom_key do not follow this convention. Therefore, fill_bloom_key() and clear_bloom_key() are renamed to bloom_key_fill() and bloom_key_clear(), respectively. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Previously, we stored bloom keys in a flat array and marked a commit as NOT TREESAME if any key reported "definitely not changed". To support multiple pathspec items, we now require that for each pathspec item, there exists a bloom key reporting "definitely not changed". This "for every" condition makes a flat array insufficient, so we introduce a new structure to group keys by a single pathspec item. `struct bloom_keyvec` is introduced to replace `struct bloom_key *` and `bloom_key_nr`. And because we want to support multiple pathspec items, we added a bloom_keyvec * and a bloom_keyvec_nr field to `struct rev_info` to represent an array of bloom_keyvecs. This commit still optimize only one pathspec item, thus bloom_keyvec_nr can only be 0 or 1. New bloom_keyvec_* functions are added to create and destroy a keyvec. bloom_filter_contains_vec() is added to check if all key in keyvec is contained in a bloom filter. Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>

When preparing to use bloom filters in a revision walk, Git populates a boom_keyvec with an array of bloom keys for the components of a path. Before we create the ability to map multiple pathspecs to multiple bloom_keyvecs, extract the conversion from a pathspec to a bloom_keyvec into its own helper method. This simplifies the state that persists in prepare_to_use_bloom_filter() as well as makes the future change much simpler. Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>

This allows using a custom gpg program under the user's home directory by specifying a path starting with '~' [gpg] program = "~/.local/bin/mygpg" Signed-off-by: Jonas Brandstötter <jonas.brandstoetter@gmx.at> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Docfix. * bs/remote-helpers-doc-markup-fix: gitremote-helpers.adoc: fix formatting

Use of sysctl() system call to learn the total RAM size used on BSDs has been corrected. * cb/total-ram-bsd-fix: builtin/gc: correct total_ram calculation with HAVE_BSD_SYSCTL

Drop FreeBSD 4 support and assume we are at least at FreeBSD 6 with memmem() supported. * bs/config-mak-freebsd: build: retire NO_UINTMAX_T config.mak.uname: set NO_MEMMEM only for functional version

Some code paths in the "git prune" used to ignore passed in repository object and used the_repository singleton instance instead, which has been corrected. * ac/prune-wo-the-repository: builtin/prune: stop depending on 'the_repository' repository: move 'repository_format_precious_objects' to repo scope

A diff-filter with negative-only specification like "git log --diff-filter=d" did not trigger correctly, which has been fixed. * jk/all-negative-diff-filter-fix: setup_revisions(): turn on diffs for all-negative diff filter

The reftable ref backend has matured enough; Git 3.0 will make it the default format in a newly created repositories by default. * ps/use-reftable-as-default-in-3.0: setup: use "reftable" format when experimental features are enabled BreakingChanges: announce switch to "reftable" format

"make coccicheck" succeeds even when spatch made suggestions, which has been updated to fail in such a case. * jc/coccicheck-fails-make-when-it-fails: coccicheck: fail "make" when it fails

"netrc" credential helper has been improved to understand textual service names (like smtp) in addition to the numeric port numbers (like 25). * mc/netrc-service-names: contrib: better support symbolic port names in git-credential-netrc contrib: warn for invalid netrc file ports in git-credential-netrc contrib: use a more portable shebang for git-credential-netrc

"git for-each-ref" learns "--start-after" option to help applications that want to page its output. * kn/for-each-ref-skip: ref-cache: set prefix_state when seeking for-each-ref: introduce a '--start-after' option ref-filter: remove unnecessary else clause refs: selectively set prefix in the seek functions ref-cache: remove unused function 'find_ref_entry()' refs: expose `ref_iterator` via 'refs.h'

Redefine where the multi-pack-index sits in the object subsystem, which recently was restructured to allow multiple backends that support a single object source that belongs to one repository. A midx does span mulitple "object sources". * ps/object-store-midx: midx: remove now-unused linked list of multi-pack indices packfile: stop using linked MIDX list in `get_all_packs()` packfile: stop using linked MIDX list in `find_pack_entry()` packfile: refactor `get_multi_pack_index()` to work on sources midx: stop using linked list when closing MIDX packfile: refactor `prepare_packed_git_one()` to work on sources midx: start tracking per object database source

"git rebase -i" with bogus rebase.instructionFormat configuration failed to produce the todo file after recording the state files, leading to confused "git status"; this has been corrected. * ow/rebase-verify-insn-fmt-before-initializing-state: rebase: write script before initializing state

A few file descriptors left unclosed upon program completion in a few test helper programs are now closed. * hl/test-helper-fd-close: test-delta: close output descriptor after use test-delta: use strbufs to hold input files test-delta: handle errors with die() t/helper/test-truncate: close file descriptor after truncation

Signed-off-by: Junio C Hamano <gitster@pobox.com>

Code simplification. * hy/blame-simplify-get-commit-info: blame: remove parameter detailed in get_commit_info()

Test clean-up. * cc/t9350-cleanup: t9350: redirect input to only fast-import

A new test to ensure that a recent change will keep working. * jb/t7510-gpg-program-path: t7510: use $PWD instead of $(pwd) inside PATH t7510: add test cases for non-absolute gpg program

"git switch" and "git restore" are declared to be no longer experimental. * jt/switch-restore-no-longer-experimental: builtin: unmark git-switch and git-restore as experimental

Code clean-up. * kn/for-each-ref-skip-updates: ref-filter: use REF_ITERATOR_SEEK_SET_PREFIX instead of '1' t6302: add test combining '--start-after' with '--exclude' for-each-ref: reword the documentation for '--start-after' for-each-ref: fix documentation argument ordering ref-cache: use 'size_t' instead of int for length

The config API had a set of convenience wrapper functions that implicitly use the_repository instance; they have been removed and inlined at the calling sites. * ps/config-wo-the-repository: (21 commits) config: fix sign comparison warnings config: move Git config parsing into "environment.c" config: remove unused `the_repository` wrappers config: drop `git_config_set_multivar()` wrapper config: drop `git_config_get_multivar_gently()` wrapper config: drop `git_config_set_multivar_in_file_gently()` wrapper config: drop `git_config_set_in_file_gently()` wrapper config: drop `git_config_set()` wrapper config: drop `git_config_set_gently()` wrapper config: drop `git_config_set_in_file()` wrapper config: drop `git_config_get_bool()` wrapper config: drop `git_config_get_ulong()` wrapper config: drop `git_config_get_int()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string()` wrapper config: drop `git_config_get_string_multi()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get_value()` wrapper config: drop `git_config_get()` wrapper config: drop `git_config_clear()` wrapper ...

"git add/etc -p" now honor the diff.context configuration variable, and also they learn to honor the -U<n> command-line option. * lm/add-p-context: add-patch: add diff.context command line overrides add-patch: respect diff.context configuration t: use test_config in t4055 t: use test_grep in t3701 and t4055

Windows fixes. * js/mingw-fixes: mingw: support Windows Server 2016 again mingw_rename: support ReFS on Windows 2022 mingw: drop Windows 7-specific work-around mingw_open_existing: handle directories better

Build fix. * ps/meson-clar-decls-fix: meson: ensure correct "clar-decls.h" header is used

Interactive prompt code did not correctly strip CRLF from the end of line on Windows. * js/prompt-crlf-fix: interactive: do strip trailing CRLF from input

Test fix. * ch/t7450-recursive-clone-test-fix: t7450: inspect the correct path a broken code would write to

Doc update. * jc/doc-release-vs-clear: CodingGuidelines: clarify that S_release() does not reinitialize

Build fix. * ms/meson-with-ancient-git-wo-ls-files-dedup: meson: tolerate errors from git ls-files --deduplicate

Doc update. * kh/doc-fast-import-historical: doc: fast-import: contextualize the hardware cost

Comment fix. * jc/test-hashmap-is-still-here: test-hashmap: document why it is no longer used but still there

Signed-off-by: Junio C Hamano <gitster@pobox.com>

In preparation for adding documentation for `git refs list`, factor out the common options from the `git-for-each-ref` man page into a shareable file `for-each-ref-options.adoc` and update `git-for-each-ref.adoc` to use an `include::` macro. This change is a pure refactoring and results in no change to the final rendered documentation for `for-each-ref`. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: shejialuo <shejialuo@gmail.com> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>

Usage string for `git for-each-ref` was out of sync with its official documentation. The test `t0450-txt-doc-vs-help.sh` was marked as broken due to this. Update the usage string to match the documentation. This allows the test to pass, so remove the corresponding 'known breakage' marker from the test file. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: shejialuo <shejialuo@gmail.com> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>

The implementation of `git for-each-ref` is monolithic within `cmd_for_each_ref()`, making it impossible to share its logic with other commands. To enable code reuse for the upcoming `git refs list` subcommand, refactor the core logic into a shared helper function. Introduce a new `for-each-ref.h` header to define the public interface for this shared logic. It contains the declaration for a new helper function, `for_each_ref_core()`, and a macro for the common usage options. Move the option parsing, filtering, and formatting logic from `cmd_for_each_ref()` into a new helper function named `for_each_ref_core()`. This helper is made generic by accepting the command's usage string as a parameter. The original `cmd_for_each_ref()` is simplified to a thin wrapper that is only responsible for defining its specific usage array and calling the shared helper. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: shejialuo <shejialuo@gmail.com> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>

Git's reference management is distributed across multiple commands. As part of an ongoing effort to consolidate and modernize reference handling, introduce a `list` subcommand under the `git refs` umbrella as a replacement for `git for-each-ref`. Implement `cmd_refs_list` by having it call the `for_each_ref_core()` helper function. This helper was factored out of the original `cmd_for_each_ref` in a preceding commit, allowing both commands to share the same core logic as independent peers. Add documentation for the new command. The man page leverages the shared options file, created in a previous commit, by using the AsciiDoc `include::` macro to ensure consistency with git-for-each-ref(1). Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: shejialuo <shejialuo@gmail.com> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>

In preparation for adding tests for the new `git refs list` command, refactor the existing t6300 test suite to make its logic shareable. Move the core test logic from `t6300-for-each-ref.sh` into a new `for-each-ref-tests.sh` file. Inside this new script, replace hardcoded calls to "git for-each-ref" with the `$git_for_each_ref` variable. The original `t6300-for-each-ref.sh` script now becomes a simple "driver". It is responsible for setting the default value of the variable and then sourcing the test library. This new structure follows the established pattern used for sharing tests between `git-blame` and `git-annotate` and prepares the test suite for the `refs list` tests to be added in a subsequent commit. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: shejialuo <shejialuo@gmail.com> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>

Add a test script, `t/t1461-refs-list.sh`, for the new `git refs list` command. This script acts as a simple driver, leveraging the shared test library created in the preceding commit. It works by overriding the `$git_for_each_ref` variable to "git refs list" and then sourcing the shared library (`t/for-each-ref-tests.sh`). This approach ensures that `git refs list` is tested against the entire comprehensive test suite of `git for-each-ref`, verifying that it acts as a compatible drop-in replacement. Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: shejialuo <shejialuo@gmail.com> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Meet Soni <meetsoni3017@gmail.com>

The fill_packs_from_midx() method was refactored in fcb2205 (midx: implement support for writing incremental MIDX chains, 2024-08-06) to allow for preferred packfiles and incremental multi-pack-indexes. However, this led to some conditions that can cause improperly initialized memory in the context's list of packfiles. The conditions caring about the preferred pack name or the incremental flag are currently necessary to load a packfile. But the context is still being populated with pack_info structs based on the packfile array for the existing multi-pack-index even if prepare_midx_pack() isn't called. Add a new test that breaks under --stress when compiled with SANITIZE=address. The chosen number of 100 packfiles was selected to get the --stress output to fail about 50% of the time, while 50 packfiles could not get a failure in most --stress runs. The test case is marked as EXPENSIVE not only because of the number of packfiles it creates, but because some CI environments were reporting errors during the test that I could not reproduce, specifically around being unable to open the packfiles or their pack-indexes. When it fails under SANITIZE=address, it provides the following error: AddressSanitizer:DEADLYSIGNAL ================================================================= ==3263517==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000027 ==3263517==The signal is caused by a READ memory access. ==3263517==Hint: address points to the zero page. #0 0x562d5d82d1fb in close_pack_windows packfile.c:299 #1 0x562d5d82d3ab in close_pack packfile.c:354 #2 0x562d5d7bfdb4 in write_midx_internal midx-write.c:1490 #3 0x562d5d7c7aec in midx_repack midx-write.c:1795 #4 0x562d5d46fff6 in cmd_multi_pack_index builtin/multi-pack-index.c:305 ... This failure stack trace is disconnected from the real fix because the bad pointers are accessed later when closing the packfiles from the context. There are a few different aspects to this fix that are worth noting: 1. We return to the previous behavior of fill_packs_from_midx to not rely on the incremental flag or existence of a preferred pack. 2. The behavior to scan all layers of an incremental midx is kept, so this is not a full revert of the change. 3. We skip allocating more room in the pack_info array if the pack fails prepare_midx_pack(). 4. The method has always returned 0 for success and 1 for failure, but the condition checking for error added a check for a negative result for failure, so that is now updated. 5. The call to open_pack_index() is removed, but this is needed later in the case of a preferred pack. That call is moved to immediately before its result is needed (checking for the object count). Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

shejialuo reviewed Jun 9, 2025

View reviewed changes

Documentation/git-refs.adoc Outdated Show resolved Hide resolved

builtin/refs.c Outdated Show resolved Hide resolved

builtin/refs.c Outdated Show resolved Hide resolved

builtin/refs.c Outdated Show resolved Hide resolved

builtin/refs.c Outdated Show resolved Hide resolved

inosmeet force-pushed the internal-refs-list branch from 1527a6a to 462de7c Compare June 11, 2025 06:34

inosmeet requested a review from shejialuo June 11, 2025 06:34

inosmeet commented Jun 11, 2025

View reviewed changes

KarthikNayak reviewed Jun 11, 2025

View reviewed changes

inosmeet force-pushed the internal-refs-list branch 2 times, most recently from 65ce9e3 to bbc95d1 Compare June 14, 2025 05:57

inosmeet force-pushed the internal-refs-list branch from bbc95d1 to 3b1cebf Compare June 23, 2025 07:33

shejialuo reviewed Jun 24, 2025

View reviewed changes

inosmeet force-pushed the internal-refs-list branch 2 times, most recently from 81d0073 to 80388c8 Compare June 27, 2025 06:34

tiwai and others added 14 commits July 14, 2025 18:53

Merge branch 'bs/remote-helpers-doc-markup-fix'

cc876f2

Docfix. * bs/remote-helpers-doc-markup-fix: gitremote-helpers.adoc: fix formatting

Merge branch 'cb/total-ram-bsd-fix'

e02d718

Use of sysctl() system call to learn the total RAM size used on BSDs has been corrected. * cb/total-ram-bsd-fix: builtin/gc: correct total_ram calculation with HAVE_BSD_SYSCTL

Merge branch 'bs/config-mak-freebsd'

45c50a1

Drop FreeBSD 4 support and assume we are at least at FreeBSD 6 with memmem() supported. * bs/config-mak-freebsd: build: retire NO_UINTMAX_T config.mak.uname: set NO_MEMMEM only for functional version

Merge branch 'jk/all-negative-diff-filter-fix'

50d9c34

A diff-filter with negative-only specification like "git log --diff-filter=d" did not trigger correctly, which has been fixed. * jk/all-negative-diff-filter-fix: setup_revisions(): turn on diffs for all-negative diff filter

Merge branch 'jc/coccicheck-fails-make-when-it-fails'

0d046cb

"make coccicheck" succeeds even when spatch made suggestions, which has been updated to fail in such a case. * jc/coccicheck-fails-make-when-it-fails: coccicheck: fail "make" when it fails

gitster added 5 commits August 3, 2025 18:44

The seventeenth batch, just before -rc0

e075325

Signed-off-by: Junio C Hamano <gitster@pobox.com>

inosmeet force-pushed the internal-refs-list branch from 0fd1f71 to a037a47 Compare August 4, 2025 08:20

gitster and others added 22 commits August 4, 2025 08:10

Merge branch 'hy/blame-simplify-get-commit-info'

41ca6a9

Code simplification. * hy/blame-simplify-get-commit-info: blame: remove parameter detailed in get_commit_info()

Merge branch 'cc/t9350-cleanup'

8e75941

Test clean-up. * cc/t9350-cleanup: t9350: redirect input to only fast-import

Merge branch 'jb/t7510-gpg-program-path'

fab5968

A new test to ensure that a recent change will keep working. * jb/t7510-gpg-program-path: t7510: use $PWD instead of $(pwd) inside PATH t7510: add test cases for non-absolute gpg program

Merge branch 'jt/switch-restore-no-longer-experimental'

0dc39a6

"git switch" and "git restore" are declared to be no longer experimental. * jt/switch-restore-no-longer-experimental: builtin: unmark git-switch and git-restore as experimental

Merge branch 'js/mingw-fixes'

817d661

Windows fixes. * js/mingw-fixes: mingw: support Windows Server 2016 again mingw_rename: support ReFS on Windows 2022 mingw: drop Windows 7-specific work-around mingw_open_existing: handle directories better

Merge branch 'ps/meson-clar-decls-fix'

c4c628f

Build fix. * ps/meson-clar-decls-fix: meson: ensure correct "clar-decls.h" header is used

Merge branch 'js/prompt-crlf-fix'

ea58ada

Interactive prompt code did not correctly strip CRLF from the end of line on Windows. * js/prompt-crlf-fix: interactive: do strip trailing CRLF from input

Merge branch 'ch/t7450-recursive-clone-test-fix'

d8f795e

Test fix. * ch/t7450-recursive-clone-test-fix: t7450: inspect the correct path a broken code would write to

Merge branch 'jc/doc-release-vs-clear'

67424f5

Doc update. * jc/doc-release-vs-clear: CodingGuidelines: clarify that S_release() does not reinitialize

Merge branch 'ms/meson-with-ancient-git-wo-ls-files-dedup'

dc87d07

Build fix. * ms/meson-with-ancient-git-wo-ls-files-dedup: meson: tolerate errors from git ls-files --deduplicate

Merge branch 'kh/doc-fast-import-historical'

67fd255

Doc update. * kh/doc-fast-import-historical: doc: fast-import: contextualize the hardware cost

Merge branch 'jc/test-hashmap-is-still-here'

f674965

Comment fix. * jc/test-hashmap-is-still-here: test-hashmap: document why it is no longer used but still there

Git 2.51-rc0

721c9e5

Signed-off-by: Junio C Hamano <gitster@pobox.com>

inosmeet force-pushed the internal-refs-list branch from a037a47 to df2c3fc Compare August 5, 2025 07:38

		static int show_ref(const char refname, const char referent UNUSED,
		const struct object_id oid, int flag UNUSED, void cbdata)

Conversation

inosmeet commented Jun 9, 2025

Uh oh!

shejialuo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KarthikNayak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

inosmeet commented Jun 12, 2025

Uh oh!

KarthikNayak commented Jun 12, 2025

Uh oh!

inosmeet commented Jun 13, 2025

Uh oh!

KarthikNayak commented Jun 13, 2025

Uh oh!

shejialuo commented Jun 13, 2025

Uh oh!

shejialuo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone