The function `for_each_bitmapped_object()` accepts an optional object
filter. This filter is never modified by the function, but is not
declared as `const`. Fix this.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
odb/source-packed: improve lookup when enumerating objects
When iterating through objects of a packed source that have a specific
prefix we do so via two different methods:
- When a multi-pack index is available we use that one to efficiently
loop through all objects.
- We then loop through all packfiles that aren't covered by a
multi-pack index.
Regardless of which mechanism we use, we then iterate through all the
objects indexed by the respective data structure. Curiously though,
while we use the indices for enumerating the objects, we completely
ignore it for the actual object lookup. Instead, we call into the
generic `odb_source_read_object_info()` function, which will itself
consult the indices to figure out where the object in question even
lives.
This has two consequences:
- It's inefficient, as we basically have to figure out the position of
the object a second time.
- It's subtly wrong, as it may now happen that a specific object will
be looked up via a different pack in case it exists multiple times.
This is unlikely to have any real-world consequences, but it's still
the wrong thing to do.
Fix the issue by using `packed_object_info()` directly. While at it,
rename the `store` variable to `source`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Tue, 14 Jul 2026 08:45:59 +0000 (10:45 +0200)]
strbuf: avoid redundant reset in strbuf_getwholeline()
The HAVE_GETDELIM variant of strbuf_getwholeline() calls strbuf_reset()
on the strbuf before handing it over to getdelim(3). This is
unnecessary:
- getdelim(3) doesn't care whether the old buffer contents is
NUL-terminated and has no access to ->len,
- on success getdelim(3) NUL-terminates the buffer and we set ->len,
- on error we either call strbuf_init() or strbuf_reset().
Remove the superfluous preparatory call.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pablo Sabater [Tue, 14 Jul 2026 12:09:37 +0000 (14:09 +0200)]
graph: move config reading into graph_read_config()
Move the repo_config_get_string() call out of graph_init() and into
graph_read_config(). This simplifies graph_init() and provides a
function for future graph-related config opt.
This commit is a preparatory commit for a subsequent one.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pablo Sabater [Tue, 14 Jul 2026 12:09:36 +0000 (14:09 +0200)]
graph: wrap cascading commits after 4 columns
Currently the visual root commits in a graph cascade indefinitely until
a commit which is not a visual root or the last commit appears.
On filters like --author where one author might contribute mostly on
single patches this can become a visual issue.
Make the cascading wrap after 4 columns.
There are two possible cases of the wrap:
1. No ambiguity:
* A
* B
* C
* D
* E
* F
2. Ambiguous conflict:
If F happens to not be a visual root and E gets wrapped back to the
initial column then E and F would be vertically adjacent. The solution
is to forcefully indent E one level:
* A
* B
* C
* D
* E
* F
* F
The magic number 4 comes as the minimum number of columns to wrap where
the output shows clearly the commits are unrelated and doesn't cause too
much "pyramid" effects
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pablo Sabater [Tue, 14 Jul 2026 12:09:35 +0000 (14:09 +0200)]
graph: indent visual root in graph
When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.
A fix for this issue was already attempted [1] a while ago.
This happens because the commits fill the space from left to right and
when a visual root ends, its column becomes free for the following
commit even if they are not related. Once this happens the unrelated
commit is rendered below the visual root. Because there is no special
character or way to identify when a visual root is rendered making the
graph confusing.
By indenting the visual roots when there are still commits to show the
vertical adjacency can be avoided.
Add is_visual_root flag to git_graph making it visible in all graph states,
give graph_update() a new function, graph_is_visual_root() to know if the
current commit is a visual root and set is_visual_root.
The different handled cases are:
- If a visual root has children: similar to GRAPH_PRE_COMMIT state when
octopus merges need space, an edge row needs to be printed to connect
the child with the indented visual root. A new state GRAPH_PRE_ROOT is
needed to connect the child with the visual root:
* child of the visual root
\ GRAPH_PRE_ROOT
* visual root indented
- If a visual root is child-less we can skip GRAPH_PRE_ROOT state and
render the indented commit directly.
* visual root indented
* unrelated commit
- If two or more visual roots are adjacent: by having a lookahead to the
next commit that will be rendered, if the next commit is also a visual
root and we are on a visual root, meaning two visual root adjacent in
the history, the top one can omit the indent, making the one below to
indent only once, if there are more adjacent visual commits, the
indentation will increase for each adjacent one, cascading.
Pablo Sabater [Tue, 14 Jul 2026 12:09:34 +0000 (14:09 +0200)]
graph: add a 2 commit buffer for lookahead
In a subsequent commit the graph renderer needs to know if the next
commit is a visual root or if it is the last commit to be shown. This
requires peeking 2 commits ahead.
Commits are pre-fetched in get_revision() through next_commit_to_show()
where they are also marked as SHOWN, regardless the source they come
from.
Update graph_is_interesting() so it considers commits inside the
lookahead buffer as interesting as well.
Helped-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pablo Sabater [Tue, 14 Jul 2026 12:09:33 +0000 (14:09 +0200)]
revision: add next_commit_to_show()
get_revision() gets its commits from two sources depending on the mode:
1. Normally it gets the commits from get_revision_internal().
2. --max-count-oldest which was introduced at bb4ce23284 (revision.c:
implement --max-count-oldest, 2026-05-19) gets the commits by popping
from a saved list at revs->commits marking SHOWN and CHILD_SHOWN on
each popped commit.
Extract the choice logic into a helper, next_commit_to_show(), which
returns the next commit regardless of the source it comes from.
This has no change in behavior. The helper is needed in a subsequent
commit that pre-fetches two commits into a buffer for lookahead purposes
and needs to pre-fetch from the same source.
The --reverse branch keeps its own pop loop. Using the helper for
--reverse would additionally set SHOWN and CHILD_SHOWN which is not
desired and a behavior change.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Pablo Sabater [Tue, 14 Jul 2026 12:09:32 +0000 (14:09 +0200)]
lib-log-graph: move check_graph function
check_graph is a function shared in the test files t4215 and t6016 used
to format the output graph, but instead of being in a file called by
both test, the function code is repeated in each file.
Move check_graph to lib-log-graph.sh file which both tests already
import graph functions from, renaming it to lib_test_check_graph.
This function is needed for the following commit which includes graph
tests in a new file and requires check_graph.
Mentored-by: Karthik Nayak <karthik.188@gmail.com> Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t1100: move creation of expected output into setup test
The "expected" file is created at the top-level of the script, outside
of any test. Code that runs outside of a test is not protected by the
test harness: a failure there is not reported as a test failure and is
easy to miss.
Move the here-doc that creates "expected" into the existing setup test
("test preparation: write empty tree"), using a "<<-" here-doc so its
body can be indented along with the rest of the test.
Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The tests in this script use the old style in which the test title and
body are passed as separate backslash-continued arguments, with bodies
indented using spaces:
test_expect_success \
'title' \
'body'
Convert them to the modern style in which the body is a single-quoted
block on its own lines, indented with a tab:
test_expect_success 'title' '
body
'
While at it, remove an extraneous blank line between two tests.
This is a style-only change; no test logic is modified.
Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Christian Couder [Mon, 13 Jul 2026 12:41:53 +0000 (14:41 +0200)]
fast-export: standardize usage string and SYNOPSIS
The output of `git fast-export -h` currently starts with:
usage: git fast-export [<rev-list-opts>]
while the SYNOPSIS section in this command's documentation shows:
'git fast-export' [<options>] | 'git fast-import'
Let's make both of these consistent with each other and with other Git
commands by describing the arguments with:
[<options>] [<revision-range>] [[--] <path>...]
This takes into account the following:
- `git fast-export` accepts both rev-list arguments and a number of
genuine options of its own (--[no-]progress, --[no-]signed-tags,
--[no-]signed-commits, etc).
- `git fast-export` was the only command using `[<rev-list-opts>]`
while many other commands describe their revision arguments as
`[<revision-range>] [[--] <path>...]`.
- In the DESCRIPTION section of the documentation, it's already
mentioned several times that the output should eventually be fed to
`git fast-import`.
This also enables us to remove fast-export from
"t/t0450/adoc-help-mismatches".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 13 Jul 2026 15:27:27 +0000 (08:27 -0700)]
Merge branch 'ps/odb-generalize-prepare'
The 'reprepare()' callback for object database sources has been
generalized into a 'prepare()' callback with an optional flush cache
flag, and a new 'odb_prepare()' wrapper has been introduced to allow
pre-opening object database sources.
Junio C Hamano [Mon, 13 Jul 2026 15:27:27 +0000 (08:27 -0700)]
Merge branch 'kk/prio-queue-get-put-fusion'
The lazy priority queue optimization pattern (deferring actual removal
in 'prio_queue_get()' to allow get+put fusion) has been folded
directly into 'prio_queue' itself, speeding up commit traversal
workflows and simplifying callers.
* kk/prio-queue-get-put-fusion:
prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion
prio-queue: rename .nr to .nr_ and add accessor helpers
Junio C Hamano [Mon, 13 Jul 2026 15:27:27 +0000 (08:27 -0700)]
Merge branch 'hn/branch-push-slip-advice'
When 'git push origin/main' or 'git branch origin main' is run, the
command is now recognized as a potential typo, and advice has been
added to offer a typo fix.
* hn/branch-push-slip-advice:
push: suggest <remote> <branch> for a slash slip
branch: suggest <remote>/<branch> on upstream slip
Junio C Hamano [Mon, 13 Jul 2026 15:27:26 +0000 (08:27 -0700)]
Merge branch 'jk/format-patch-leakfix'
A memory leak in the '--base' handling of 'git format-patch' has been
plugged, and the leak reporting of the test suite when running under a
TAP harness has been improved.
* jk/format-patch-leakfix:
format-patch: fix leak of rev_info in prepare_bases()
t: move LSan errors from stdout to stderr
Junio C Hamano [Mon, 13 Jul 2026 15:27:26 +0000 (08:27 -0700)]
Merge branch 'jk/reftable-leakfix'
A memory leak in the 'reftable_writer_new()' initialization function
has been fixed by delaying the allocation of 'struct reftable_writer'
until after input options are validated.
* jk/reftable-leakfix:
reftable: fix unlikely leak on API error
Junio C Hamano [Mon, 13 Jul 2026 15:27:26 +0000 (08:27 -0700)]
Merge branch 'ad/gpg-strip-cr-before-lf'
The GPG and SSH signature parsing code has been corrected to strip
carriage return characters only when they immediately precede line
feeds, instead of unconditionally stripping all carriage returns.
* ad/gpg-strip-cr-before-lf:
gpg-interface: fix strip_cr_before_lf to only remove CR before LF
t9811: replace 'test -f' and '! test -f' with 'test_path_*'
Replace the basic shell commands 'test -f', with more modern test
helpers 'test_path_is_file' and 'test_path_is_missing'.
These modern helpers emit useful information when the corresponding
tests fail, unlike 'test -f' and '! test -f'.
The occurrences of '! test -f filename' were replaced by
'file_path_is_missing filename', a stronger guarantee equivalent to
'! test -e filename'.
Co-authored-by: Vinicius Lira de Freitas <vinilira@usp.br> Signed-off-by: Vinicius Lira de Freitas <vinilira@usp.br> Signed-off-by: Marcelo Machado Lage <marcelomlage@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rewrite single-line && chains by breaking them into multiple lines.
Co-authored-by: Vinicius Lira de Freitas <vinilira@usp.br> Signed-off-by: Vinicius Lira de Freitas <vinilira@usp.br> Signed-off-by: Marcelo Machado Lage <marcelomlage@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 10a6762719 (object-file: adapt `stream_object_signature()` to take a
stream, 2026-02-23), we have refactored `stream_object_signature()` so
that it doesn't create the stream ad-hoc anymore. Instead, callers are
expected to pass in a stream, which allows them to construct the streams
from different sources.
While the stream was previously managed by `stream_object_signature()`,
the full lifecycle is now owned by the caller. Hence, it's the caller's
responsibility to close the stream, and the called function shouldn't do
that anymore.
And while the mentioned commit did drop one call that closed the stream,
there's a second such call that was missed when reading from the stream
fails. The consequence of this can be a double free of the stream.
Fix the bug by dropping that leftover call to `odb_read_stream_close()`.
Note that it was originally discussed whether this should be treated as
a security vulnerability. But there are only two callers: once via
`parse_object_with_flags()`, and once via `verify_packfile()`. Neither
of these callers plays any role on the transport layer, so this issue is
only relevant for objects that are already available via the local
object database. Furthermore, a packfile that is corrupted in this way
would be detected when receiving the packfile, so it's not easy for an
adversary to plant such a packfile, either. Consequently, we decided
that this is not covered as part of our threat model.
Reported-by: xuqing yang <rigelyoung@icloud.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/receive-pack: stage incoming objects via ODB transactions
Objects received by git-receive-pack(1) are quarantined in a temporary
"incoming" directory and migrated into the object database prior to the
reference updates. The quarantine is currently managed through
`tmp_objdir` directly. In a pluggable ODB future, how exactly an object
gets written to a transaction may vary for a given ODB source. Refactor
git-receive-pack(1) to use the ODB transaction interfaces to manage the
object staging area in a more agnostic manner accordingly.
Note that the ODB transaction is now responsible for managing the
primary and alternate ODBs for the repository. One small change as a
result is that the temporary directory is now applied as the primary ODB
in the main process instead of an alternate. This does not change
anything for git-receive-pack(1) though because it only needs access to
the newly written objects and doesn't care how exactly it is set up.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When performing the connectivity checks for a shallow ref in
`update_shallow_ref()`, the child process environment variables are
populated via `tmp_objdir_env()`. This is unnecessary though as
`update_shallow_ref()` is only reached after `tmp_objdir_migrate()` has
been performed which means there is no longer a temporary directory that
needs to be shared with child processes.
Drop the call to `tmp_objdir_env()` accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The temporary directory used by git-receive-pack(1) to write objects is
managed slightly differently than how it is done via ODB transactions:
- The temporary directory is eagerly created upfront, instead of
waiting for the first object write.
- The prefix name of the temporary directory is "incoming" instead of
"bulk-fsync".
In a subsequent commit, git-receive-pack(1) will use ODB transactions
instead of `tmp_objdir` directly. To provide a means to configure the
same transaction behavior, introduce `enum odb_transaction_flags` and
the ODB_TRANSACTION_RECEIVE flag intended as a signal for ODB
transactions using the "files" backend to be set up for
git-receive-pack(1). Transaction call sites are updated accordingly to
provide the required flag parameter.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ODB transaction backend is responsible for creating/managing its own
staging area for writing objects. Other child processes spawned by Git
may need access to uncommitted objects or write new objects in the
staging area though.
Introduce `odb_transaction_env()` which is expected to provide the set
of environment variables needed by a child process to access the
transaction's staging area.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When `odb_transaction_commit()` is invoked, the return value of the
backend commit callback is silently discarded. A backend has no way
to signal that committing failed, such as when the "files" backend
cannot migrate its temporary object directory into the permanent
ODB.
In a subsequent commit, git-receive-pack(1) starts using ODB transaction
to stage objects and consequently cares about such failures so it can
handle the error appropriately. Change the commit callback signature to
return an int error code and have `odb_transaction_commit()` forward it
accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When `odb_transaction_begin()` is invoked, the function returns the
transaction pointer directly. There is no way for the backend to
signal that it failed to set up its state, such as when creating the
temporary object directory backing the transaction.
In a subsequent commit, git-receive-pack(1) starts using ODB
transactions and needs to be able to report such failures rather
than silently ignore them. Refactor `odb_transaction_begin()` to
return an int error code and write the resulting transaction into an
out parameter. Also introduce `odb_transaction_begin_or_die()` as a
convenience for callsites that do not need to handle errors
explicitly.
Note that `odb_transaction_begin()` now returns an error when the ODB
already has an inflight transaction pending. ODB transaction call sites
that may encounter an inflight transaction are updated to explicitly
handle this case.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "files" transaction backend may encounter errors related to managing
the temporary directory used to stage objects, but silently ignores
these errors. Instead return errors encountered in the
`odb_transaction_files_{prepare,begin,commit}()` interfaces to allow
callers to handle them as needed.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ODB transactions are started via `odb_transaction_begin()` and contain
validation to avoid starting multiple transactions at the same time. The
"files" backend also has the same logic, but is redundant due to the
generic layer already handling it. Drop this validation from the "files"
backend accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
object-file: embed transaction flush logic in commit function
When a "files" transaction is committed,
`flush_loose_object_transaction()` is invoked to handle performing a
hardware flush along with migrating the temporary object directory into
the primary and configuring the repository ODB source accordingly. The
function name here is a bit misleading because the helper is doing a bit
more than just "flushing" the transaction contents. Also, in a
subsequent commit, the transaction temporary directory is used to stage
packfiles and not just loose objects anymore.
Lift the helper function logic into `odb_transaction_files_commit()` to
more accurately signal to readers the operation being performed.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
object-file: rename files transaction fsync function
When writing an object to a "files" ODB transaction, a full hardware
flush is not initially performed during the fsync in
`fsync_loose_object_transaction()` and instead delayed until the
transaction is later committed.
To be more consistent with other "files" ODB transaction helpers, rename
the function to `odb_transaction_files_fsync()` accordingly. The
conditional in the helper is also slightly restructured to improve
clarity to readers.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
object-file: rename files transaction prepare function
The "files" ODB transaction backend lazily creates a temporary object
directory when the first loose object is written to the transaction via
`prepare_loose_object_transaction()`. In a subsequent commit, the
temporary directory is used to also write packfiles to.
Rename the function to `odb_transaction_files_prepare()` accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
With b4 0.15.2, I hit a local failure after sending a series with the
in-tree cover template. The generated sent/<change-id>-vN tag contained
base-commit, but did not contain change-id, and later b4 commands failed
when trying to read it:
CRITICAL: Tag sent/... does not contain change-id info
Looking at b4's source, the sent tag message is derived from the rendered
cover letter. The same code later parses that tag and expects both
base-commit and change-id to be present. The default b4 cover template
has both trailers, but our in-tree template only has base-commit.
Add the missing change-id trailer next to base-commit so sent tags
produced from the project template remain readable by b4's reroll and
comparison logic.
Signed-off-by: Chen Linxuan <me@black-desk.cn> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
config: add "worktree" and "worktree/i" includeIf conditions
The includeIf mechanism already supports matching on the .git
directory path (gitdir) and the currently checked out branch
(onbranch). But in multi-worktree setups the .git directory of a
linked worktree points into the main repository's .git/worktrees/
area, which makes gitdir patterns cumbersome when one wants to
include config based on the working tree's checkout path instead.
Introduce two new condition keywords:
- worktree:<pattern> matches the realpath of the current worktree's
working directory (i.e. repo_get_work_tree()) against a glob
pattern. This is the path returned by git rev-parse
--show-toplevel.
- worktree/i:<pattern> is the case-insensitive variant.
The implementation reuses the include_by_path() helper introduced in
the previous commit, passing the worktree path in place of the
gitdir. The condition never matches in bare repositories (where
there is no worktree) or during early config reading (where no
repository is available).
Add documentation describing the new conditions, including a comparison
with extensions.worktreeConfig and a note that worktree matching currently
uses the realpath-resolved worktree location. Add tests covering bare
repositories, multiple worktrees, realpath-resolved symlinked worktree
paths, case-sensitive and case-insensitive matching, early config reading,
and non-repository scenarios.
Signed-off-by: Chen Linxuan <me@black-desk.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>
config: refactor include_by_gitdir() into include_by_path()
The include_by_gitdir() function matches the realpath of a given
path against a glob pattern, but its interface is tightly coupled to
the gitdir condition: it takes a struct config_options *opts and
extracts opts->git_dir internally.
Refactor it into a more generic include_by_path() helper that takes
a const char *path parameter directly, and update the gitdir and
gitdir/i callers to pass opts->git_dir explicitly. No behavior
change, just preparing for the addition of a new worktree condition
that will reuse the same path-matching logic with a different path.
Signed-off-by: Chen Linxuan <me@black-desk.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable: fix quadratic behavior in the presence of tombstones
When many tombstones are present in a reftable, operations that need
to look up or iterate over refs exhibit quadratic behavior. With
8000 refs deleted and re-created, update-ref takes ~15s, quadrupling
for each doubling of input size.
The root cause is the merged iterator's suppress_deletions flag.
When set, merged_iter_next_void() silently consumes tombstone records
in a tight internal loop before returning to the caller. This
prevents higher-level code from checking iteration bounds (such as
prefix or refname comparisons) until after all tombstones have been
scanned.
This affects any code path that seeks into a range containing
tombstones, including:
- refs_verify_refnames_available() seeks to "refs/tags/foo-1/" to
check for D/F conflicts and must scan through all subsequent
tombstones before the caller can see that they are past the prefix
of interest.
- reftable_backend_read_ref() seeks to a specific refname and must
scan through all subsequent tombstones before returning "not
found", because the merged iterator skips the matching tombstone
and searches for the next live record.
Fix this by making suppress_deletions configurable via
reftable_stack_options instead of unconditionally enabling it. Git
no longer sets the flag, so tombstones are now returned to callers in
the reftable backend, which skip them after their existing bounds
checks. This allows iteration to terminate as soon as a tombstone
past the relevant bound is encountered.
Downstream users of the reftable library (e.g. libgit2) can still
enable suppress_deletions through the stack options to retain the
previous behavior.
This also requires adding deletion checks to the log iteration paths,
since suppress_deletions applied to both ref and log iterators.
Both tests in p1401 go from ~13s to ~0.2s with this change.
Reported-by: Jeff King <peff@peff.net> Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add performance tests for update-ref when many tombstones are present
in a reftable.
The first test exercises the scenario where all refs are deleted
(creating tombstones) and then re-created with the same names, which
currently exhibits quadratic behavior.
The second test uses a separate repository with an asymmetric variant
where refs are deleted and then new, differently-named refs are
created. When the tombstones sort after the new refs, every create
scans all tombstones, making this case even worse than re-creating
the same refs.
Helped-by: Jeff King <peff@peff.net> Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
shallow: give write_one_shallow() its own hex buffer
The previous fix reuses the local `hex` variable that is already
computed at the top of `write_one_shallow()`. That works today, but
`oid_to_hex()` returns a pointer into a small rotating buffer, so it is
not stable across an unrelated call to `oid_to_hex()` from the same
thread. A future edit that adds such a call between the assignment and
the last user of `hex` would silently corrupt the output.
Move `write_one_shallow()` off the rotating buffer entirely by using a
local buffer instead. The current users of that `hex` variable are
unchanged.
Suggested-by: Junio C Hamano <gitster@pobox.com> Assisted-by: Claude Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
After `write_one_shallow()` calls `lookup_commit()` to find the commit
object for a shallow graft entry, it then checks `if (!c || ...)`.
Inside that block, when the VERBOSE flag is set, it prints the OID being
removed, via `c->object.oid`. But `c` can be NULL (the first condition
in the `||` check).
This happens when a shallow graft entry references a commit object that
is not in the object store (e.g., after a partial fetch or in a
corrupted repository). In that case, `lookup_commit()` returns NULL
because the object cannot be found, the SEEN_ONLY check correctly
decides to remove this entry from .git/shallow, but the verbose message
crashes before the removal can complete.
Use `graft->oid` instead of `c->object.oid` for the message. The graft
entry's OID is the same value (it was used as the lookup key) and is
always available regardless of whether the commit object exists.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When `refs_resolve_ref_unsafe()` is called to resolve HEAD, and returns
NULL (e.g., HEAD does not exist as a proper ref), the code falls back to
`repo_get_oid("HEAD")` to try to resolve the OID directly. If that
succeeds, execution continues with `head` still set to NULL.
Later, that variable is passed to `repo_get_oid()` and `starts_with()`,
both of which would dereference the NULL pointer.
A concrete trigger for `refs_resolve_ref_unsafe()` returning NULL while
`repo_get_oid()` succeeds could not be constructed against the ref
backends currently in the tree; the naive case (a symbolic HEAD pointing
at a nonexistent branch, in either the files or the reftable backend)
fails in both calls consistently and returns via the existing
`error(_("bad HEAD - I need a HEAD"))` path. Coverity, however, flags
the leftover use of `head` after the outer `if (!head)` on a formal
reading: `head` is still NULL at that point, and both `starts_with(head,
...)` and the second `repo_get_oid(..., head, ...)` in the else-branch
would dereference it if that state were ever reached.
Removing the outer check would risk regressing to a crash if a future
ref backend ever manages to hit the "returns NULL for HEAD but has a
valid OID for HEAD" state. Assigning the literal string "HEAD" as a
safe fallback documents the intent and satisfies the analyzer without
changing behavior in any code path we can currently reach.
Assisted-by: Claude Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When `prepare_midx_bitmap_git()` is called to load the bitmap for a
chained MIDX's base layer, if the base MIDX does not have an associated
bitmap file (e.g., it was not generated, or was deleted by gc), the
return value is NULL. It is then stored in `bitmap_git->base` and
immediately dereferenced on the next line.
This can happen in practice with incremental MIDX chains: the base MIDX
may have been written without `--write-bitmap-index`, or the bitmap may
have been pruned while the incremental layer's bitmap still references
it.
Check the return value and go to the cleanup label (which unmaps the
current bitmap and returns -1) so the caller falls back to non-bitmap
object enumeration, matching the handling of other bitmap loading
failures in the same function.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
revision: avoid dereferencing NULL in `add_parents_only()`
This function resolves revision suffixes like commit^@ (all parents),
commit^! (commit minus parents), and commit^-N (exclude Nth parent). It
calls `get_reference()` in a loop to peel through tag objects until it
reaches a commit.
The existing NULL check after `get_reference()` only handles the
ignore_missing case, but get_reference() can return NULL through three
distinct paths:
1. revs->ignore_missing: the caller asked to silently skip missing
objects.
2. revs->exclude_promisor_objects: the object is a lazy promisor
object that should be excluded from the walk.
3. revs->do_not_die_on_missing_objects: the caller wants to record
missing OIDs for later reporting (used by `git rev-list
--missing=print`) rather than dying.
In the latter two instances, the code falls through to dereference the
NULL pointer.
Handle all three cases explicitly:
- ignore_missing: return 0, matching the existing behavior and
the pattern in `handle_revision_arg()`.
- do_not_die_on_missing_objects: return 0. The missing OID has already
been recorded in `revs->missing_commits` by `get_reference()`.
Returning 0 is consistent with `handle_revision_arg()` and
`process_parents()`, both of which continue without error when this flag
is set. The broader codebase pattern for this flag is "record and
continue": list-objects.c, builtin/rev-list.c, and process_parents
all skip the die/error and keep walking.
- everything else (only the `exclude_promisor_objects` case in
practice): return -1, consistent with `handle_revision_arg()` where
the condition only matches `ignore_missing` or
`do_not_die_on_missing_objects`, falling through to ret = -1 for the
promisor case.
Note: the callers of `add_parents_only()` in
`handle_revision_pseudo_opt()` treat any nonzero return as "handled"
(`if (add_parents_only(...)) { ret = 0; }`), so the -1 for the promisor
case is indistinguishable from success there. This means a
promisor-excluded tag target referenced via commit^@ would be silently
skipped rather than producing an error. This is a pre-existing
limitation of the caller's return value handling and not made worse by
this change; the alternative (a NULL dereference crash) _would be_
strictly worse.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `peel_committish()` function calls `repo_peel_to_type()` to convert
the given object to a commit, but does not check the return value. When
the object exists but cannot be peeled to a commit (e.g., a tree or blob
OID is passed as --onto), the return value is NULL. Add an explicit NULL
check and die with a descriptive message in that case.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
bisect: handle NULL commit in `bisect_successful()`
When `lookup_commit_reference_by_name()` is called to find the first bad
commit, the result is passed to `repo_format_commit_message()`
immediately, which dereferences commit without checking for NULL.
However, the commit could be NULL, even though in practice this is
unlikely because `bisect_successful()` is only called after a successful
bisect run has identified the bad commit, but the ref could still become
dangling due to a concurrent gc or repository corruption.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
mailsplit: move NULL check before first use of file handle
The `split_mbox()` function calls fileno(f) to check whether the input
is a terminal, but the NULL check for f (from `fopen()`) does not happen
until later. When the file cannot be opened, f is NULL, and
`fileno(NULL)` is undefined behavior, typically crashing with a
segmentation fault.
Move the NULL check above the `isatty()`/`fileno()` call so the error
path is taken before any use of the potentially-NULL handle.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
reftable/stack: guard against NULL list_file in stack_destroy
When reftable_new_stack() fails partway through initialization
(e.g., reftable_buf_addstr returns an OOM error before
reftable_buf_detach assigns p->list_file), it jumps to the error
path which calls reftable_stack_destroy(p). At that point,
p->list_file is still NULL because the detach never happened.
reftable_stack_destroy() passes st->list_file unconditionally to
read_lines(), which calls open(filename, O_RDONLY). Passing NULL
to open() is undefined behavior and will typically crash.
Guard the read_lines() call with a NULL check on st->list_file.
When list_file is NULL, there are no table files to clean up
anyway, so skipping read_lines is the correct behavior.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
remote: guard `remote_tracking()` against NULL remote
The `remote_tracking()` function unconditionally dereferences
`remote->fetch` without checking whether remote is NULL.
In practice, this never happens because the only caller (`apply_cas()`)
guards the calls to this function by checking the `use_tracking` and
`use_tracking_for_rest` attributes.
However, it requires quite involved reasoning to reach that conclusion,
and is therefore fragile. Just return -1 ("no tracking ref") when there
is no remote to work with.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff: handle NULL return from repo_get_commit_tree()
The `repo_get_commit_tree()` function can return NULL when a commit's
tree object is not available (e.g., the commit was parsed but its
maybe_tree field is unset and the commit is not in the commit-graph). In
cmd_diff(), the return value is immediately dereferenced via ->object
without a NULL check, which would crash if the tree cannot be loaded.
Add an explicit NULL check and die with a descriptive message.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diffcore-break: guard against NULLed queue entries in merge loop
The outer loop in `diffcore_merge_broken()` sets `q->queue[j]` to NULL
when it merges a broken pair back together, and has a NULL check to skip
such entries on subsequent iterations. The inner loop, however, lacks
this guard: when it scans forward looking for a matching peer, it can
encounter a slot that was NULLed by a previous outer-loop iteration and
dereference it unconditionally.
In practice this requires at least two broken pairs whose peers
both survive rename/copy detection and appear later in the queue,
which is rare but not impossible.
Add the same `if (!pp) continue` guard to the inner loop.
Pointed out by Coverity.
Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit-graph: propagate topo_levels slab to all chain layers
The topo_levels slab is only propagated to the topmost graph
layer instead of all layers in the chain. Commits from lower
layers appear to have no generation numbers, so the DFS
re-walks the entire ancestry.
Fix by making topo_levels visible to all layers, not just
the first one.
Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit-graph: add trace2 instrumentation for generation DFS
Count the number of steps taken in
compute_reachable_generation_numbers() and expose it via
trace2 to make it easier to detect performance regressions.
Add a failing test for such a regression, introduced in 199d452758 (commit-graph: return the prepared commit graph
from `prepare_commit_graph()`, 2025-09-04), where incremental
commit-graph writes do not see existing generation numbers
from lower graph layers and fall back to walking the full
ancestry.
Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 9 Jul 2026 17:12:15 +0000 (10:12 -0700)]
Merge branch 'ps/odb-drop-whence' into ps/odb-for-each-object-filter
* ps/odb-drop-whence:
odb: document object info fields
odb: drop `whence` field from object info
treewide: convert users of `whence` to the new source field
odb: add `source` field to struct object_info_source
odb: make backend-specific fields optional
packfile: thread odb_source_packed through packed_object_info()
t1410-reflog.sh: avoid suppressing git's exit code in pipelines
Piping git commands directly to wc -l suppresses the exit code of
git, hiding potential failures from the test suite. Use
test_stdout_line_count instead, which handles exit code preservation
internally while keeping the test logic clean and readable.
Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
unpack-trees: avoid quadratic index scan in next_cache_entry()
Diffing the working tree against a commit with a pathspec can take
time quadratic in the size of the index when the pathspec matches a
subtree whose entries are the first entries of the index. Fix it by
having next_cache_entry() record how far it scanned in cache_bottom,
so repeated calls no longer rescan the growing prefix of
already-unpacked entries. On a Chromium checkout (~500k index
entries),
git diff HEAD -- .agents/OWNERS
took about 8 minutes before this change and 0.07 seconds after it.
The same diff without the commit, without the pathspec, or with
--cached was already instant.
Add p0009-diff-pathspec.sh, which builds a 10,000-entry index whose
first path lives in a subtree (100,000 entries under --long-tests),
to guard against the regression. Comparing v2.55.0 with this change
using GIT_TEST_LONG=t:
Test v2.55.0 HEAD
------------------------------------------------------------------------
0009.2: diff pathspec subtree 7.16(7.12+0.01) 0.02(0.01+0.00) -99.7%
Signed-off-by: Henrique Ferreiro <hferreiro@igalia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When bundle-URI info is requested by the client, the server responds
with all "bundle.*" config lines as key=value packet lines. On the
client-side, the received bundle config packet lines are always expected
to contain both a key and a value otherwise the client errors out during
parsing. The server performs no validation of the read bundle
configuration though which results in any misconfiguration on the
server-side, such as bundle configuration with an empty value, being
blindly sent to the client.
To avoid having the server transmit invalid configuration to clients,
only send bundle configuration that has non-empty values.
This change makes bundle-URI information sent by the server
syntactically correct, but semantically it still can be invalid. For
example the server may end up sending `bundle.bundle-1.creationToken`,
but be lacking a `bundle.bundle-1.uri` for that bundle. The `uri` is
mandatory, thus the client cannot process this bundle and will error
with the message:
error: bundle 'bundle-1' has no uri
Fixing this would require a more complex solution, because bundles need
to be validated as a whole and not line-by-line. This is considered
outside the scope of this change.
Co-authored-by: Toon Claes <toon@iotcl.com> Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
bundle-uri: drain remaining response on invalid bundle-uri lines
On clone, when the client sends the `bundle-uri` command, the server
might respond with invalid data. For example if it sends information
about a bundle where the 'uri' is empty, it produces the following
error:
Cloning into 'foo'...
error: bundle-uri: line has empty key or value
error: error on bundle-uri response line 4: bundle.bundle-1.uri=
error: could not retrieve server-advertised bundle-uri list
This error is bubbled up to `transport_get_remote_bundle_uri()`, which
is called by `cmd_clone()` in builtin/clone.c. Over here, the return
value is ignored, so clone continues.
Despite this, it still dies with this error:
fatal: expected 'packfile'
This happens because `get_remote_bundle_uri()` exited early, leaving
some unprocessed packet data behind in the read buffer. This is
misleading to the user, because it suggests a problem with the packfile
exchange, when in reality it's caused by a misconfigured bundle-URI on
the server-side.
Fix this by continuing to read packets when an error was encountered,
but without processing the remaining lines. This drains the protocol
stream so no stale data is left behind and the caller can use it if they
like.
With this, clone now continues successfully if invalid bundle-URI data
was sent by the server. This is intentional, because since the inception
of `transport_get_remote_bundle_uri()` in 0cfde740f0 (clone: request the
'bundle-uri' command when available, 2022-12-22) the return value of
that function is ignored in `cmd_clone()` so the clone can continue
without bundles.
Signed-off-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 8 Jul 2026 17:09:49 +0000 (10:09 -0700)]
SubmittingPatches: document how to retract a topic
While this document outlines an idealized lifecycle where an author
develops a patch, refines it with reviewer feedback, and
successfully merges it into Git, reality is rarely so seamless.
Sometimes, a topic must be abandoned. Doing so explicitly is far
better than leaving it in limbo, especially since topics can always
be resurrected later.
Clearly state that we encourage contributors to retract any topic
that does not pan out.
submodule--helper: accept '-i' shorthand for update --init
commit 3ad0ba722744 ("git-submodule.sh: improve variables readability")
made `git submodules update -i` pass `-i` as is to submodule--helper,
but it fails with `error: unknown switch `i'` because the helper does
not accept the short option.
All other short options supported by git-submodule.sh are properly
handle in the helper, so also add the alias for --init
Fixes: 3ad0ba722744 ("git-submodule.sh: improve variables readability") Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Shardul Natu [Wed, 8 Jul 2026 03:21:19 +0000 (03:21 +0000)]
contrib: wire up osxkeychain in contrib/Makefile on macOS
When running "make test" with TEST_CONTRIB_TOO=yes (which is default in
macOS CI workflows), $(MAKE) -C contrib/ test is invoked. However,
contrib/Makefile only invoked tests for diff-highlight and subtree,
meaning git-credential-osxkeychain was never built or verified during
standard CI test runs.
Add a "test" target to contrib/credential/osxkeychain/Makefile that
depends on building git-credential-osxkeychain. Additionally, wire up
credential/osxkeychain in contrib/Makefile under "all", "test", and
"clean" whenever running on macOS (Darwin).
This ensures that running "make test" or "make all" in contrib on macOS
automatically builds and links git-credential-osxkeychain, preventing
future build or symbol linking regressions from slipping through CI.
Signed-off-by: Shardul Natu <snatu@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Shardul Natu [Wed, 8 Jul 2026 03:21:18 +0000 (03:21 +0000)]
Makefile: support universal macOS builds via RUST_TARGETS
On macOS, Universal Binaries contain native executable code for
multiple architectures (such as Intel x86_64 and Apple Silicon arm64)
bundled into a single file. This is standard practice for macOS
distribution and CI packaging (such as internal distribution packages
or tooling like Burrito/Homebrew), allowing a single build artifact
to run natively across all Macs without Rosetta emulation or
maintaining separate packages.
When building Git C code for multiple architectures on macOS, the
Apple toolchain (clang) natively supports universal builds via
CFLAGS/LDFLAGS. When "-arch x86_64 -arch arm64" is passed, clang
automatically compiles and links universal binaries for all C object
files and executables out of the box.
Cargo and rustc, however, do not support multiple "-arch" flags or
emitting universal binaries in a single invocation. Instead, Cargo
requires invoking each target triple independently (e.g., passing
"--target x86_64-apple-darwin" and "--target aarch64-apple-darwin").
To bridge this gap when Rust is enabled:
1. Allow specifying space-separated target triples in RUST_TARGETS.
2. Introduce declarative pattern rules (target/%/...) to compile
each target-specific library slice via Cargo.
3. On macOS, if multiple targets are specified, use "lipo" (part of
the mandatory Xcode Command Line Tools) to combine the resulting
static libraries into target/release/libgitcore.a.
Once $(RUST_LIB) is compiled into a universal static archive, the
standard C linker seamlessly links it with the C object files to
produce universal Git executables.
Signed-off-by: Shardul Natu <snatu@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Shardul Natu [Wed, 8 Jul 2026 03:21:17 +0000 (03:21 +0000)]
Makefile: add $(RUST_LIB) prerequisite to osxkeychain
When Rust is enabled, the git-credential-osxkeychain helper depends on
Rust symbols compiled into $(RUST_LIB). While commit 522ea8ef7d
("osxkeychain: fix build with Rust") updated the linker command line to
use $(LIBS), it omitted $(RUST_LIB) from the target prerequisite list.
Without this prerequisite, running a parallel build ("make -j") from a
clean working tree can fail because Make does not know to invoke Cargo
to build libgitcore.a before linking git-credential-osxkeychain.
Note that we depend explicitly on $(LIB_FILE) and $(RUST_LIB) rather
than $(GITLIBS). Unlike standard Git builtins and programs like scalar
(which define cmd_main() and rely on common-main.o to supply main()),
git-credential-osxkeychain.c defines its own standalone int main().
If $(GITLIBS) were used, $(filter %.o,$^) in the link recipe would
match both git-credential-osxkeychain.o and common-main.o, causing a
duplicate symbol linking error for _main on macOS.
Additionally, wrap the definitions of $(RUST_LIB) and the "rust" build
target in "ifndef NO_RUST". This ensures that when NO_RUST=1 is
specified, $(RUST_LIB) evaluates to empty, making the Rust dependency a
clean no-op without needing intermediate variables.
Signed-off-by: Shardul Natu <snatu@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 8 Jul 2026 03:53:05 +0000 (23:53 -0400)]
hash: check ctx->active flag in all wrapper functions
It only makes sense to call git_hash_update(), etc, on a hash context
that has been initialized but not yet finalized or discarded. This is an
unlikely error to make, but it's easy for us to catch it and complain.
It's especially important because it would quietly "work" for many hash
backends (like sha1dc, which is just manipulating some bytes) but would
cause undefined behavior with others (like OpenSSL, which puts the
context onto the heap). Checking the flag lets us catch problems
consistently on every build.
Note that we can't do the same for git_hash_init(). Even though it would
cause a leak to call it twice (without an intervening final/discard),
the point of the function is that the contents of the struct are
undefined before the call. But calling it twice is an even less likely
error to make, so not covering it is OK.
We leave git_hash_discard() alone, as its idempotent behavior is
convenient for callers. We _could_ try to do something similar for
git_hash_final(), allowing:
but it does not make much sense. After the first final() call we have
thrown away the state, so we cannot produce the same output. We could
come up with some sensible output (the null hash, or the empty hash),
but double-calls like this are more likely a bug, so our best bet is to
complain loudly (whereas the current code produces either nonsense
output or undefined behavior, depending on the backend).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 8 Jul 2026 03:53:02 +0000 (23:53 -0400)]
http: use idempotent git_hash_discard()
Now that it is OK to call git_hash_discard() even after finalizing the
hash, we no longer need the ctx_valid bool added by a2d8ea5a76 (http:
discard hash in dumb-http http_object_request, 2026-07-02).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 8 Jul 2026 03:53:00 +0000 (23:53 -0400)]
csum-file: use idempotent git_hash_discard()
Now that it is safe to call git_hash_discard() even after finalizing it,
we can simplify our cleanup logic a bit. This is mostly undoing a few
bits of 64337aecde (csum-file: always finalize or discard hash,
2026-07-02):
- We no longer need a separate free_hashfile_memory() function for
finalize_hashfile(). It can just call free_hashfile(), which will
now discard (or not) the hash as appropriate.
- When f->skip_hash is set, we don't need to discard; we can rely on
free_hashfile() to do it.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 8 Jul 2026 03:52:57 +0000 (23:52 -0400)]
hash: make git_hash_discard() idempotent
You must always either finalize or discard a hash context to release any
resources, but you must call only one such function. This creates extra
work for some callers, since their cleanup code paths need to know
whether they got there via their happy path (and the finalization
happened) or due to an error (in which case they need to discard).
Let's add an "active" flag that turns a redundant discard into a noop.
That lets you safely do this:
git_hash_init(&ctx, algo);
...
if (some_error)
goto out;
...
git_hash_final(result, &ctx);
out:
git_hash_discard(&ctx);
This should avoid future errors, and will also let us simplify a few
existing callers (in future patches).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 8 Jul 2026 03:52:55 +0000 (23:52 -0400)]
hash: document function pointers and wrappers
We want people to use the git_hash_*() wrappers rather than the bare
function pointers in the git_hash_algo struct. Let's document them
rather than the bare pointers, and warn people away from the pointers.
Coccinelle will eventually force the use of the wrappers, but it's
helpful to lead readers in the right direction from the start.
While we're here we can document a few other bits of wisdom I've turned
up while working in this area:
- You have to initialize the destination of a git_hash_clone(). This
is something we may eventually change for efficiency, but we should
definitely document the requirement for now.
- You must eventually finalize or discard a hash, since some backends
may allocate resources during initialization.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 8 Jul 2026 03:52:53 +0000 (23:52 -0400)]
hash: convert remaining direct function calls
The previous patch added a coccinelle rule to make sure callers always
use git_hash_init() rather than direct function pointers from the algo
struct.
Let's do the same for the rest of the git_hash_*() wrappers. I split
these out because they're a bit different: they implicitly use the algop
pointer in the git_hash_ctx. So when we convert:
we drop the reference to algo entirely! But this is always going to be
the right thing. If "algo" does not match what is in ctx.algop, then
we'd already be invoking undefined behavior.
So in addition to making it possible to add more logic to the
git_hash_*() functions, we're avoiding the need to pass around the extra
algo pointer and make sure that it matches what's in "ctx".
The rest of the patch is the mechanical application of that coccinelle
patch, plus a minor cleanup in test-synthesize.c to drop a now-unused
function parameter (since we don't have to pass around the algo
separately anymore).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 8 Jul 2026 03:52:49 +0000 (23:52 -0400)]
hash: use git_hash_init() consistently
We'd like to add more logic to git_hash_init(), but many callers skip it
and call algop->init_fn() directly. Let's make sure we're consistently
using the wrapper by adding a coccinelle rule.
Besides the coccinelle file itself, this is a purely mechanical
conversion based on the patch it generates. There should be no bare
init_fn() calls left (except for the one in the wrapper).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The standard philosophy for Unix software when a help option (such as
--help) is specified is that the software should exit 0, printing the
help output to standard output, since the standard output is for
user-requested output and the program performed the requested task
successfully. If the user specifies an incorrect option, then the help
output should be printed to standard error (since the user has made a
mistake) and it should exit unsuccessfully.
Most of our commands currently exit 129 on receiving the -h option to
print the short help, which does not line up with the standard
philosophy above. Let's change that to exit 0 instead.
This requires changes to a variety of tests which previously wanted the
129 exit code, so update them. Note that because git diff does its own
option parsing, it still exits with 129, so update some of the tests to
expect either exit status.
Some commands also now pass with -h but not --help-all, so handle those
cases differently for those commands.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
rev-parse: have --parseopt callers exit 0 on --help
The standard philosophy for Unix software when a help option (such as
--help) is specified is that the software should exit 0, printing the
help output to standard output, since the standard output is for
user-requested output and the program performed the requested task
successfully. If the user specifies an incorrect option, then the help
output should be printed to standard error (since the user has made a
mistake) and it should exit unsuccessfully.
git rev-parse --parseopt properly directs the output in both of these
cases, but it currently exits 129 when it receives a --help or -h option
on the command line, which causes its invoking script to do the same.
This is not in line with the usual behavior and it causes scripts using
this command to exit unsuccessfully on --help as well.
Note that Git subcommands implemented using scripts, such as git
submodule, don't have this problem because Git itself intercepts the
--help option and runs man (or a similar tool), which then exits 0.
However, this still affects the myriad scripts that use this
functionality because Git is widespread and the --parseopt functionality
is a good way to get sensible option parsing across shells in a portable
way.
Because git rev-parse --parseopt is intended to be eval'd by the shell,
when help output is to be printed to standard output, Git actually
prints a cat command with a heredoc since the standard output is being
evaluated by the shell. Thus, to do the right thing, simply add an
"exit 0" right after the end of the heredoc, which will cause the
invoking program to exit successfully.
The usual invocation recommended by the manual page is this:
Thus, the fact that git rev-parse --parseopt still exits 129 in this
case is irrelevant, since the "echo exit $?" will print "exit 129", but
that will be after the "exit 0" printed by Git—and thus ignored, since
the shell will have already exited successfully.
Update the tests for this case. Note that we no longer need to delete
only the first and last lines in some tests, so add a command to delete
the end of the heredoc as well. We could do something clever with sed
to delete all but the last two lines or switch to head and tail, but
those would be more complicated and less readable, so just stick with
the simple approach.
In t1517, add three shell scripts to the failure case because they no
longer return 129 as expected. In a future commit, we'll change the
expected result to exit 0 and these will become successful again.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
parse-options: add a separate case for help output on error
When we parse a command line option such as -h or --help, we currently
exit 129, since that is the exit code when help output is printed. In a
future commit, we'll change this to exit 0 instead, since we're doing
what the user wanted successfully.
However, there are some cases where we print help output because the
user has provided ambiguous or invalid input, such as an ambiguous
option, and we'll want to exit unsuccessfully there. Make this easier
by defining a new return code, PARSE_OPT_HELP_ERROR, that can be used in
this case, while reserving PARSE_OPT_HELP for those cases where the user
has requested help directly.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The svn tests currently assume that git-svn's option parsing will always
fail the tests because it exits 0 on --help, not 129. However, in a
future commit, we'll expect it to exit 0 and the tests will then need to
be updated to succeed in some cases and fail in others.
We therefore need to have t1517 determine whether the Subversion Perl
modules are present, since if they are not, git-svn will die on start
and then it needs to continue to expect failure. Add a stripped down
version of the tests in t/lib-git-svn.sh as a prerequisite we can use
here for our svn tests.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Mantas Mikulėnas [Wed, 13 May 2026 07:08:03 +0000 (10:08 +0300)]
sideband: allow ANSI SGR with colon-separated subfields
The SGR values used for 256-color formatting are officially defined to
be a single field with :-separated subfields (e.g. "\e[1;38:5:XX;40m")
despite the more common but kludgy use of separate values (which then
become context-dependent and lead to misinterpretation by incompatible
terminals).
Signed-off-by: Mantas Mikulėnas <grawity@gmail.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commit we have removed the last callers of
`set_git_work_tree()` that is located outside of "setup.c". Remove its
declaration and mark the function as file-local.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commits we have refactored how we discover and set up
repositories so that we cannot end up with partially-configured repos.
Instead, we apply the gitdir, worktree and repository format in a single
location, only.
Initializing a new repository has the same antipattern though: while
most of the information for the new repository is passed via parameters,
the work tree is instead propagated by configuring the repository's work
tree.
Refactor the code so that we also pass the work tree as an explicit
parameter. Like this, configuration fo the repository happens in a
single spot, too, just as with repository discovery.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
setup: drop redundant configuration of `startup_info->have_repository`
In `init_db()` we set `startup_info->have_repository` twice: once before
reading and applying the repository format and once after. This is
redundant though, as configuring the repository format does not rely on
this variable at all.
Remove the first such site. While at it, fix up formatting a bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commits we have started to propagate all information
required for the configuration of the repository via a new `struct
repo_discovery`. The only exception is the repository's prefix, which we
still return via the return parameter.
This is conceptually fine, but somewhat inconsistent. Refactor this to
instead propagate the prefix via the repository discovery, too.
While at it, drop a static variable in `repo_discover_bare_gitdir()`.
We apply its value to the repository discovery anyway, so we don't have
to keep it around afterwards anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current working directory is stored as part of a static strbuf
variable. This variable had to have a lifetime longer than its
containing function because the value we return typically points into
that buffer.
In the preceding commit we have moved the prefix into the repository
though. Consequently, we can now return the repository's prefix instead
of the local one and thus properly manage the lifecycle of this local
variable.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The repository prefix is currently stored in the startup info. This
feels somewhat awkward though, as it is inherently a property of a given
repository.
Move the prefix into the repository accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
All functions related to repository discovery receive both a `struct
repository_discovery` and `struct repository_format` as input, and the
expectation is that both will be populated. Refactor this so that the
repository format is part of the discovery result.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When setting up the global repository we intermix repository discovery
and repository configuration: we repeatedly call `set_git_work_tree()`
and `apply_and_export_relative_gitdir()` until we're happy with the
result. The result of this is then a partially-configured repository
that we use for further setup.
This process is quite hard to follow, as it's never quite clear which
parts of the repository have been configured already and which haven't.
Furthermore, it means that the repository configuration is distributed
across many different places instead of having it neatly contained in a
single location. Ultimately, this is the reason that we cannot use a
central function like `repo_init()`.
Refactor the logic so that we stop partially-configuring a repository
and instead populate a new `struct repo_discovery`. This allow us to
essentially split repository setup into two phases:
- The first phase only figures out parameters required to configure
the repository.
- The second phase then takes these parameters and applies them to the
repository.
Like this, we'll never end up with a partially-configured repository and
can eventually extend `repo_init()` to handle the full initialization
for us.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
setup: split up concerns of `setup_git_env_internal()`
The function `setup_git_env_internal()` does two completely unrelated
things:
- It configures the repository's gitdir and propagates environment
variables into it.
- It configures a couple of global parameters via environment
variables.
The function is called when we initialize the repository's path, but
it's also called via `chdir_notify_register()` whenever we change the
current working directory. While we indeed have to reconfigure the
gitdir in case it's a relative path, it doesn't make sense to reapply
the global environment variables.
Split up concerns of this function along the above delineation. Handling
of the global environment variables is moved into `init_git()`, as they
can be considered part of our setup procedure.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
It is possible to configure an arbitrary "shallow" file via two
mechanisms, and the respective logic to handle these is split across two
locations:
- Via the "GIT_SHALLOW_FILE" environment variable, which is handled in
`setup_git_env_internal()`.
- Via the global "--shallow-file=" command line option, which is
handled in `handle_options()`.
We can rather easily unify this logic by not configuring the shallow
file in `handle_options()`, but instead overwriting the environment
variable. The environment variable itself is then handled inside of
`apply_repository_format()`, which is responsible for configuring a
discovered Git directory.
This new logic is similar in nature to how we handle the other global
options already, all of which end up setting an environment variable.
So for one this gives us more consistency. But more importantly, this
change means that `the_repository` will not contain any relevant state
anymore before we hit `apply_repository_format()` once we're at the end
of this patch series. Consequently, it will become possible for us to
completely discard `the_repository` and populate it anew.
Note that on first sight, this change looks like it might change the
precedence order. Before this change, we used to configure the shallow
file in the arguments handler first, and then it looks like we override
it via the environment variable. What's important to note though is the
last parameter to `set_alternate_shallow_file()`, which tells us whether
we want to overwrite a preexisting value, and when applying the value
from the environment we tell it not to overwrite preexisting values. So
in effect, the command line has precedence over the environment. After
this change, we now overwrite preexisting environment variables when we
see the argument, and consequently we keep the precedence order in tact.
With this change though we don't need the final parameter anymore that
tells `set_alternate_shallow_file()` whether or not to overwrite. We
only have a single callsite for this function now, and that function is
itself only ever called exactly once. Remove that parameter.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
setup: mark bogus worktree in `apply_repository_format()`
When a repository is configured to have both "core.worktree" and
"core.bare" we emit a warning and mark the worktree configuration as
bogus so that the next call to `setup_work_tree()` will cause us to die.
This allows us to still use the misconfigured repository, at least as
long as we don't try to use its worktree.
This condition is handled in `setup_explicit_git_dir()`. In a subsequent
commit we'll refactor this function so that it doesn't receive a repo as
input anymore though, and consequently we cannot set the "bogus" bit
anymore.
Move the logic into `apply_repository_format()` instead to prepare for
this. While at it, fix up formatting a bit.
Note that this change requires us to also explicitly unset the value of
"core.worktree" in case we have the "GIT_WORK_TREE" environment variable
set. This is because the environment variable overrides the repository's
configuration, and we don't want to warn or die in case the work tree
has been configured explicitly regardless of whether or not "core.bare"
is set.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `check_repository_format_gently()` receives a format as
input. An unknowing reader may thus suspect that this function actually
checks the passed-in format for consistency. While the function indeed
checks the repository format, it actually serves two purposes:
- It reads the repository's format and populates the passed-in format
with that information.
- It then indeed checks whether the format is consistent.
Rename the function to `read_and_verify_repository_format()` to clarify
its functionality. While at it, reorder the parameters so that the
format comes first to better match other functions that pass around the
format.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A test checking interactions between git rebase --quit and
autostash in t3420-rebase-autostash.sh has been corrected to use
test_path_is_missing instead of ! grep on a file that shouldn't
exist in the conflicted state.
* sg/t3420-do-not-grep-in-missing-file:
t3420-rebase-autostash: don't try to grep non-existing files
The connectivity check has been refactored to search for promisor
objects in a generic way using the object database interface,
rather than iterating packfiles directly. This allows connectivity
checks to work properly in repositories that do not use packfiles.
* ps/connected-generic-promisor-checks:
connected: search promisor objects generically
connected: split out promisor-based connectivity check
odb/source-packed: support flags when iterating an object prefix
odb/source-packed: extract logic to skip certain packs
Junio C Hamano [Mon, 6 Jul 2026 22:50:24 +0000 (15:50 -0700)]
Merge branch 'ps/refs-onbranch-fixes'
Reference backend configuration has been updated to load lazily to
avoid recursive calls during repository initialization when 'onbranch'
configuration conditions are evaluated. This has also fixed a memory
leak and allowed the unused `chdir_notify_reparent()` machinery to be
dropped.
* ps/refs-onbranch-fixes:
refs: protect against chicken-and-egg recursion
refs/reftable: lazy-load configuration to fix chicken-and-egg
reftable: split up write options
refs/files: lazy-load configuration to fix chicken-and-egg
refs: move parsing of "core.logAllRefUpdates" back into ref stores
repository: free main reference database
chdir-notify: drop unused `chdir_notify_reparent()`
refs: unregister reference stores from "chdir_notify"
setup: don't apply "GIT_REFERENCE_BACKEND" without a repository
setup: stop applying repository format twice
setup: inline `check_and_apply_repository_format()`
Junio C Hamano [Mon, 6 Jul 2026 22:50:23 +0000 (15:50 -0700)]
Merge branch 'mv/log-follow-mergy'
"git log --follow" has been updated to better handle non-linear
history, in which the path being tracked gets renamed differently in
multiple history lines.
* mv/log-follow-mergy:
log: improve --follow following renames for non-linear history
Junio C Hamano [Mon, 6 Jul 2026 22:50:23 +0000 (15:50 -0700)]
Merge branch 'wy/doc-clarify-review-replies'
Documentation on community contribution guidelines has been updated to
encourage replying to review comments before rerolling, and to advise
a default limit of at most one reroll per day to give reviewers across
different time zones enough time to participate.
Junio C Hamano [Mon, 6 Jul 2026 22:50:23 +0000 (15:50 -0700)]
Merge branch 'pw/status-rebase-todo'
The display of the rebase todo list in "git status" has been
improved to correctly abbreviate object IDs for more commands and
avoid misinterpreting refs as object IDs.
* pw/status-rebase-todo:
status: improve rebase todo list parsing
sequencer: factor out parsing of todo commands
Junio C Hamano [Mon, 6 Jul 2026 22:50:23 +0000 (15:50 -0700)]
Merge branch 'jk/repo-info-path-keys'
The "git repo info" command has been taught new keys to output both
absolute and relative paths for "gitdir" and "commondir", supported by
a new path-formatting helper extracted from "git rev-parse".
* jk/repo-info-path-keys:
repo: add path.gitdir with absolute and relative suffix formatting
repo: add path.commondir with absolute and relative suffix formatting
path: extract format_path() and use in rev-parse
projects / thirdparty / git.git / log
