Philippe Blain [Fri, 15 May 2026 15:48:11 +0000 (15:48 +0000)]
diff-format.adoc: mode and hash are 0* for unmerged paths from index only
In the "Raw output format" section, we mention that the 'mode' and
'sha1' for "src" and "dst" are 0* if "(creation|deletion) or unmerged".
For unmerged entries, 'mode' and 'sha1' are in fact 0* only when we are
looking at the index, i.e. on the left side for 'git diff-files' and on
the right side for 'git diff-index --cached'. Be more precise by
mentioning this, and while at it uniformize the wording of the "work
tree out of sync with the index" case.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Fri, 15 May 2026 15:48:10 +0000 (15:48 +0000)]
diff-format.adoc: 'git diff-files' prints two lines for unmerged files
Since 10637b84d9 (diff-files: -1/-2/-3 to diff against unmerged stage.,
2005-11-29), for unmerged entries 'git diff-files' print both an
"unmerged" line ('U'), as well as an "in-place edit" line ('M')
comparing stage 2 (by default) with the working tree. The "Raw output
format" documentation however mentions that all commands print a single
line per changed file. Adjust diff-format.adoc to also mention this
special case, for completeness.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Philippe Blain [Fri, 15 May 2026 15:48:09 +0000 (15:48 +0000)]
diff-format.adoc: remove mention of diff-tree specific output
In the "Raw output format" section, we start by mentioning that 'git
diff-tree' prints the hashes of what is being compared. This is only
true in --stdin mode, and is already mentioned in the description of
'--stdin' in git-diff-tree.adoc. Remove this sentence such that we only
focus on the common output between diff-tree, diff-index, diff-files and
These rules exist to ensure Make won't fail with the following error if
one of the .adoc files is renamed or removed:
make: *** No rule to make target 'Documentation/config.adoc', needed by 'config-list.h'.
With the no-op targets defined in `config-list.h.d`, Make knows there's
no work to be done to generate these files, so it doesn't error out if
it doesn't exist.
For the Makefile build system this works great. And since ebeea3c471 (build: regenerate config-list.h when Documentation changes,
2026-02-24) this script is also called from the Meson build system.
Nevertheless, on AlmaLinux 8 the following build failure is seen:
This version of this distro uses Ninja 1.8.2 and it seems to have some
issues with the format of the `config-list.h.d` file.
Ninja versions before 1.10.0 do not reset the depfile parser state on
newlines. This causes issues when the depfile has one dependency per
line, like we have in `config-list.h.d`:
The parser only recognizes the first "config-list.h:" as a target. On
subsequent lines it is still in dependency-parsing mode, so the repeated
output name is recorded as an input. This causes the error mentioned
above.
The bug in Ninja is fixed in 1.10, with commit
ninja-build/ninja@1daa7470ab7e (depfile_parser: remove restriction on
multiple outputs, 2019-11-20).
To be compatible with older versions of Ninja, collapse the dependencies
for `config-list.h` into a single line like:
This works around the bug in older versions of Ninja, and is fully
compatible Make and with more recent versions of Ninja. And while the
no-op targets are not needed for Ninja, they also don't do any harm.
Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ethan Dickson [Fri, 15 May 2026 06:39:54 +0000 (06:39 +0000)]
connected: close err_fd in promisor fast-path
connected.h documents that err_fd is closed before check_connected()
returns. It is, on three of four exit paths. The promisor-pack fast
path added in 50033772d (connected: verify promisor-ness of partial
clone, 2020-01-30) returns 0 without closing it.
receive-pack uses err_fd as the write end of an async sideband
muxer's pipe, and the muxer thread waits for EOF. The same omission
has caused deadlocks there twice before: 49ecfa13f (receive-pack:
close sideband fd on early pack errors, 2013-04-19) and 6cdad1f13
(receive-pack: fix deadlock when we cannot create tmpdir,
2017-03-07).
Signed-off-by: Ethan Dickson <ethanndickson@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Fri, 15 May 2026 07:33:53 +0000 (09:33 +0200)]
trailer: change strbuf in-place in unfold_value()
Avoid an allocation by doing s/\n\s*/ /g (replacing NL and any following
whitespace with a SP) right in the strbuf instead of copying the result
to a temporary one and swapping them in the end. We can safely do that
because the replacement is never longer than the original string.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sat, 16 May 2026 02:23:10 +0000 (22:23 -0400)]
commit: handle large commit messages in utf8 verification
Running t4205 under UBSan with the EXPENSIVE prereq enabled triggers an
error when we try to create a commit message that is over 2GB:
commit.c:1574:6: runtime error: signed integer overflow:
-2147483648 - 1 cannot be represented in type 'int'
The problem is that find_invalid_utf8() is not prepared to handle
large buffers, as it uses an "int" to represent buffer sizes and
offsets.
We can fix this with a few changes:
1. We'll take in "len" as a size_t (which is what the caller has
anyway, since it's working with a strbuf).
2. We need to return a size_t to give the offset to the invalid utf8,
but we also need a sentinel value for "no invalid value"
(previously "-1"). Let's split these to return a bool for "found
invalid utf8" and then pass back the offset as an out-parameter.
We'll switch the function name to match the new semantics.
3. The caller in verify_utf8() uses a "long" to store buffer
positions, which is a bit funny. This goes back to 08a94a145c
(commit/commit-tree: correct latin1 to utf-8, 2012-06-28) and is
perhaps trying to match our use of "unsigned long" for object sizes
(though we don't care about it ever becoming negative here). This
should be a size_t, too, as some platforms (like Windows) still use
a 32-bit long on machines with 64-bit pointers.
4. The "bytes" field within find_invalid_utf() does not have range
problems. It is the number of bytes the utf8 sequence claims to
have, so is limited by how many bits can be set in a single 8-bit
byte. However, if we leave it as an "int" then the compiler will
complain about the sign mismatch when comparing it to "len". So
let's make it unsigned, too.
All of this is a little silly, of course, because 2GB text commit
messages are clearly nonsense. So we might consider rejecting them
outright, but it is easy enough to make these helper functions more
robust in the meantime.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Sat, 16 May 2026 02:16:22 +0000 (22:16 -0400)]
apply: plug leak on "patch too large" error
In apply_patch(), we return immediately if read_patch_file() returns an
error. Traditionally this was OK, since an error from strbuf_read()
would restore the strbuf to its unallocated state.
But since f1c0e3946e (apply: reject patches larger than ~1 GiB,
2022-10-25), we may also return an error if we successfully read the
patch but it is too large. In this case we leak the strbuf contents when
apply_patch() returns.
You can see it in action by running t4141 under LSan with the EXPENSIVE
prereq enabled.
We can fix this in one of two places:
1. In read_patch_file(), we could release the buffer before returning
the error, behaving more like a raw strbuf_read() call.
2. In apply_patch(), we can release the strbuf ourselves before
returning.
I picked the latter, since it future proofs us against read_patch_file()
getting new error modes. We also have a cleanup label in that function
already, so now our error handling at this spot matches the rest of the
function (and all of the variables are initialized such that the rest of
the cleanup is correctly a noop at this point).
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/unit-tests: add tests for the in-memory object source
While the in-memory object source is a full-fledged source, our code
base only exercises parts of its functionality because we only use it in
git-blame(1). Implement unit tests to verify that the yet-unused
functionality of the backend works as expected.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The in-memory source stores its objects in a simple array that we grow as
needed. This has a couple of downsides:
- The object lookup is O(n). This doesn't matter in practice because
we only store a small number of objects.
- We don't have an easy way to iterate over all objects in
lexicographic order.
- We don't have an easy way to compute unique object ID prefixes.
Refactor the code to use an oidtree instead. This is the same data
structure used by our loose object source, and thus it means we get a
bunch of functionality for free.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The oidtree data structure is currently only used to store object IDs,
without any associated data. So consequently, it can only really be used
to track which object IDs exist, and we can use the tree structure to
efficiently operate on OID prefixes.
But there are valid use cases where we want to both:
- Store object IDs in a sorted order.
- Associated arbitrary data with them.
Refactor the oidtree interface so that it allows us to store arbitrary
payloads within the respective nodes. This will be used in the next
commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
cbtree: allow using arbitrary wrapper structures for nodes
The cbtree subsystem allows the user to store arbitrary data in a
prefix-free set of strings. This is used by us to store object IDs in a
way that we can easily iterate through them in lexicograph order, and so
that we can easily perform lookups with shortened object IDs.
In its current form, it is not easily possible to store arbitrary data
with the tree nodes. There are a couple of approaches such a caller
could try to use, but none of them really work:
- One may embed the `struct cb_node` in a custom structure. This does
not work though as `struct cb_node` contains a flex array, and
embedding such a struct in another struct is forbidden.
- One may use a `union` over `struct cb_node` and ones own data type,
which _is_ allowed even if the struct contains a flex array. This
does not work though, as the compiler may align members of the
struct so that the node key would not immediately start where the
flex array starts.
- One may allocate `struct cb_node` such that it has room for both its
key and the custom data. This has the downside though that if the
custom data is itself a pointer to allocated memory, then the leak
checker will not consider the pointer to be alive anymore.
Refactor the cbtree to drop the flex array and instead take in an
explicit offset for where to find the key, which allows the caller to
embed `struct cb_node` is a wrapper struct.
Note that this change has the downside that we now have a bit of padding
in our structure, which grows the size from 60 to 64 bytes on a 64 bit
system. On the other hand though, it allows us to get rid of the memory
copies that we previously had to do to ensure proper alignment. This
seems like a reasonable tradeoff.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
odb: fix unnecessary call to `find_cached_object()`
The function `odb_pretend_object()` writes an object into the in-memory
object database source. The effect of this is that the object will now
become readable, but it won't ever be persisted to disk.
Before storing the object, we first verify whether the object already
exists. This is done by calling `odb_has_object()` to check all sources,
followed by `find_cached_object()` to check whether we have already
stored the object in our in-memory source.
This is unnecessary though, as `odb_has_object()` already checks the
in-memory source transitively via:
Implement the `free()` callback function for the "in-memory" source.
Note that this requires us to define `struct cached_object_entry` in
"odb/source-inmemory.h", as it is accessed in both "odb.c" and
"odb/source-inmemory.c" now. This will be fixed in subsequent commits
though.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Next to our typical object database sources, each object database also
has an implicit source of "cached" objects. These cached objects only
exist in memory and some use cases:
- They contain evergreen objects that we expect to always exist, like
for example the empty tree.
- They can be used to store temporary objects that we don't want to
persist to disk, which is used by git-blame(1) to create a fake
worktree commit.
Overall, their use is somewhat restricted though. For example, we don't
provide the ability to use it as a temporary object database source that
allows the user to write objects, but discard them after Git exists. So
while these cached objects behave almost like a source, they aren't used
as one.
This is about to change over the following commits, where we will turn
cached objects into a new "in-memory" source. This will allow us to use
it exactly the same as any other source by providing the same common
interface as the "files" source.
For now, the in-memory source only hosts the cached objects and doesn't
provide any logic yet. This will change with subsequent commits, where
we move respective functionality into the source.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Thu, 14 May 2026 18:37:40 +0000 (13:37 -0500)]
odb/transaction: make `write_object_stream()` pluggable
How an ODB transaction handles writing objects is expected to vary
between implementations. Introduce a new `write_object_stream()`
callback in `struct odb_transaction` to make this function pluggable.
Rename `index_blob_packfile_transaction()` to
`odb_transaction_files_write_object_stream()` and wire it up for use
with `struct odb_transaction_files` accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Thu, 14 May 2026 18:37:39 +0000 (13:37 -0500)]
object-file: generalize packfile writes to use odb_write_stream
The `index_blob_packfile_transaction()` function streams blob data
directly from an fd. This makes it difficult to reuse as part of a
generic transactional object writing interface.
Refactor the packfile write path to operate on a `struct
odb_write_stream`, allowing callers to supply data from arbitrary
sources.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Thu, 14 May 2026 18:37:38 +0000 (13:37 -0500)]
object-file: avoid fd seekback by checking object size upfront
In certain scenarios, Git handles writing blobs that exceed
"core.bigFileThreshold" differently by streaming the object directly
into a packfile. When there is an active ODB transaction, these blobs
are streamed to the same packfile instead of using a separate packfile
for each. If "pack.packSizeLimit" is configured and streaming another
object causes the packfile to exceed the configured limit, the packfile
is truncated back to the previous object and the object write is
restarted in a new packfile.
This works fine, but requires the fd being read from to save a
checkpoint so it becomes possible to rewind the input source via seeking
back to a known offset at the beginning. In a subsequent commit, blob
streaming is converted to use `struct odb_write_stream` as a more
generic input source instead of an fd which doesn't provide a mechanism
for rewinding.
For this use case though, rewinding the fd is not strictly necessary
because the inflated size of the object is known and can be used to
approximate whether writing the object would cause the packfile to
exceed the configured limit prior to writing anything. These blobs
written to the packfile are never deltified thus the size difference
between what is written versus the inflated size is due to zlib
compression. While this does prevent packfiles from being filled to the
potential maximum is some cases, it should be good enough and still
prevents the packfile from exceeding any configured limit.
Use the inflated blob size to determine whether writing an object to a
packfile will exceed the configured "pack.packSizeLimit".
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Thu, 14 May 2026 18:37:37 +0000 (13:37 -0500)]
object-file: remove flags from transaction packfile writes
The `index_blob_packfile_transaction()` function handles streaming a
blob from an fd to compute its object ID and conditionally writes the
object directly to a packfile if the INDEX_WRITE_OBJECT flag is set. A
subsequent commit will make these packfile object writes part of the
transaction interface. Consequently, having the object write be
conditional on this flag is a bit awkward.
In preparation for this change, introduce a dedicated
`hash_blob_stream()` helper that only computes the OID from a `struct
odb_write_stream`. This is invoked by `index_fd()` instead when the
INDEX_WRITE_OBJECT is not set. The object write performed via
`index_blob_packfile_transaction()` is made unconditional accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `read()` callback used by `struct odb_write_stream` currently
returns a pointer to an internal buffer along with the number of bytes
read. This makes buffer ownership unclear and provides no way to report
errors.
Update the interface to instead require the caller to provide a buffer,
and have the callback return the number of bytes written to it or a
negative value on error. While at it, also move the `struct
odb_write_stream` definition to "odb/streaming.h". Call sites are
updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Thu, 14 May 2026 18:37:35 +0000 (13:37 -0500)]
odb/transaction: use pluggable `begin_transaction()`
Each ODB source is expected to provide an ODB transaction implementation
that should be used when starting a transaction. With d6fc6fe6f8
(odb/source: make `begin_transaction()` function pluggable, 2026-03-05),
the `struct odb_source` now provides a pluggable callback for beginning
transactions. Use the callback provided by the ODB source accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Justin Tobler [Thu, 14 May 2026 18:37:34 +0000 (13:37 -0500)]
odb: split `struct odb_transaction` into separate header
The current ODB transaction interface is colocated with other ODB
interfaces in "odb.{c,h}". Subsequent commits will expand `struct
odb_transaction` to support write operations on the transaction
directly. To keep things organized and prevent "odb.{c,h}" from becoming
more unwieldy, split out `struct odb_transaction` into a separate
header.
Signed-off-by: Justin Tobler <jltobler@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
limit_list() maintains a date-sorted work queue of commits using a
linked list with commit_list_insert_by_date() for insertion. Each
insertion walks the list to find the right position — O(n) per insert.
In repositories with merge-heavy histories, the symmetric difference
can contain thousands of commits, making this O(n) insertion the
dominant cost.
Replace the sorted linked list with a prio_queue (binary heap). This
gives O(log n) insertion and O(log n) extraction instead of O(n)
insertion and O(1) extraction, which is a net win when the queue is
large.
The still_interesting() and everybody_uninteresting() helpers are
updated to scan the prio_queue's contiguous array instead of walking a
linked list. process_parents() already accepts both a commit_list and
a prio_queue parameter, so the change in limit_list() simply switches
which one is passed.
Benchmark: git rev-list --left-right --count HEAD~N...HEAD
Repository: 2.3M commits, merge-heavy DAG (monorepo)
Best of 5 runs, times in seconds:
Elijah Newren [Thu, 14 May 2026 16:25:28 +0000 (16:25 +0000)]
grep: prefetch necessary blobs
In partial clones, `git grep` fetches necessary blobs on-demand one
at a time, which can be very slow. In partial clones, add an extra
preliminary walk over the tree similar to grep_tree() which collects
the blobs of interest, and then prefetches them.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Thu, 14 May 2026 16:25:27 +0000 (16:25 +0000)]
builtin/log: prefetch necessary blobs for `git cherry`
In partial clones, `git cherry` fetches necessary blobs on-demand one
at a time, which can be very slow. We would like to prefetch all
necessary blobs upfront. To do so, we need to be able to first figure
out which blobs are needed.
`git cherry` does its work in a two-phase approach: first computing
header-only IDs (based on file paths and modes), then falling back to
full content-based IDs only when header-only IDs collide -- or, more
accurately, whenever the oidhash() of the header-only object_ids
collide.
patch-ids.c handles this by creating an ids->patches hashmap that has
all the data we need, but the problem is that any attempt to query the
hashmap will invoke the patch_id_neq() function on any colliding objects,
which causes the on-demand fetching.
Insert a new prefetch_cherry_blobs() function before checking for
collisions. Use a temporary replacement on the ids->patches.cmpfn
in order to enumerate the blobs that would be needed without yet
fetching them, and then fetch them all at once, then restore the old
ids->patches.cmpfn.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
promisor_remote_get_direct() does not, on its happy path, filter out
OIDs that are already present in the local object store: every OID
the caller supplies is written to the fetch subprocess's stdin and
ends up in the response pack. The only filtering it performs is in
remove_fetched_oids(), and that only runs after a fetch failure when
falling back to a different configured promisor remote.
Almost every existing caller already filters locally-present OIDs out
itself (typically with odb_read_object_info_extended() and
OBJECT_INFO_FOR_PREFETCH, or odb_has_object() with no fetch flag). But
the existing API comment does not state this expectation, so a new
caller is easy to write incorrectly (I missed this originally and wrote
two problematic callers). Omitting the filter still "works" in the
sense that the desired objects end up local, but it silently makes the
fetch request -- and the response pack -- larger than necessary,
defeating part of the point of batching.
Spell the contract out so future callers know to filter (and
deduplicate) themselves, and point them at the helpers they should
use to check local presence without accidentally triggering a lazy
fetch.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonas Rebmann [Thu, 14 May 2026 09:07:06 +0000 (11:07 +0200)]
rev-parse: use selected alternate terms to look up refs
git rev-parse --bisect does not work when alternate bisect terms are
used, simply listing no revisions at all.
This is because a such bisect using e.g. "old" and "new" in place of
"good" and "bad" will name refs "refs/bisect/old" (or new) accordingly
so the hardcoded "refs/bisect/bad" (and good) yields no results in a
bisect using alternate terms.
Use the current bisect_terms to make rev-parse --bisect work in an
alternate term bisect.
Signed-off-by: Jonas Rebmann <kernel@schlaraffenlan.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonas Rebmann [Thu, 14 May 2026 09:07:05 +0000 (11:07 +0200)]
bisect: print bisect terms in single quotes
As bisect terms can be arbitrarily chosen, they have been quoted in some
status messages, and in even more by translators.
To make the role of bisect terms more clear, including in translations,
and for consistency, 'enquote' all occurrences of bisect terms in status
messages.
Signed-off-by: Jonas Rebmann <kernel@schlaraffenlan.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonas Rebmann [Thu, 14 May 2026 09:07:04 +0000 (11:07 +0200)]
bisect: use selected alternate terms in status output
Alternate bisect terms are helpful when the terms "good" and "bad" are
confusing such as when bisecting for the resolution of an issue (the
first good commit) rather than the introduction of a regression.
These terms must be used when marking a commit (e.g. `git bisect new`),
they will be used in reference names (e.g. refs/bisect/new) and they are
used in parts of git's log output such as "<sha> was both old and new"
in git bisect skip's output.
However, hardcoded "good"/"bad" terms are still used in a few status
messages and can cause confusion about the status of the bisect such as:
$ git bisect old
[sha] is the first new commit
or about the required action such as:
status: waiting for bad commit, 1 good commit known
$ git bisect bad
error: Invalid command: you're currently in a new/old bisect
fatal: unknown command: 'bad'
This commit updates all remaining output messages which use hardcoded
"good" and "bad" terms to use the selected terms consistently across the
bisect output and adds tests.
Signed-off-by: Jonas Rebmann <kernel@schlaraffenlan.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Wed, 13 May 2026 15:49:11 +0000 (17:49 +0200)]
hex: add and use strbuf_add_oid_hex()
Add a function for adding the full hexadecimal hash value of an object
ID to a strbuf. It's thread-safe and slightly more efficient than using
strbuf_addstr() with oid_to_hex() because it doesn't have to determine
the length of the string or copy it from the intermediate static buffer.
Add and apply a semantic patch to use it throughout the code base.
I get a tiny speedup for git log showing a single hash per commit:
Benchmark 1: ./git_main log --format=%H
Time (mean ± σ): 91.2 ms ± 0.7 ms [User: 51.9 ms, System: 38.6 ms]
Range (min … max): 89.8 ms … 92.6 ms 31 runs
Benchmark 2: ./git log --format=%H
Time (mean ± σ): 90.5 ms ± 0.7 ms [User: 51.0 ms, System: 38.8 ms]
Range (min … max): 89.2 ms … 92.3 ms 32 runs
Summary
./git log --format=%H ran
1.01 ± 0.01 times faster than ./git_main log --format=%H
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "gc.auto" configuration has traditionally been used to turn off
running git-gc(1) as part of our auto-maintenance. We have eventually
switched over to git-maintenance(1) in a95ce12430 (maintenance: replace
run_auto_gc(), 2020-09-17), and with 1942d48380 (maintenance: optionally
skip --auto process, 2020-08-28) we have introduced "maintenance.auto"
to control whether or not to run auto-maintenance.
At that point though we still shelled out to git-gc(1) internally. So
if "gc.auto=0" was set we would still _execute_ git-maintenance(1), but
the command would have exited fast because git-gc(1) itself knew to
honor the config key.
This has recently changed though, as we have adapted the default
maintenance strategy to not use git-gc(1) anymore. The consequence is
that "gc.auto=0" doesn't have an effect anymore, which is a somewhat
surprising change in behaviour for our users.
Adapt `run_auto_maintenance()` so that it knows to also read "gc.auto",
similar to how it also reads both "maintenance.autoDetach" and
"gc.autoDetach".
Reported-by: Jean-Christophe Manciot <actionmystique@gmail.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When running git-maintenance(1), we create a lockfile that is supposed
to keep other maintenance processes from running at the same time. This
lockfile is broken though in case the "--detach" flag is passed: the
lockfile is created by the parent process and will be cleaned up either
manually or on exit. But when detaching, the parent will exit before all
of the background maintenance tasks have been run, and consequently the
lock only covers a smaller part of the whole maintenance process.
Fix this bug by reassigning all tempfiles from the parent process to the
child process when daemonizing so that it becomes the responsibility of
the child to clean them up.
Note that this is a broader fix, as we now always reassign tempfiles
when daemonizing. This is a natural consequence of the semantics of
`daemonize()` though, as it essentially promises to continue running the
current process in the background. It is thus sensible to have that
function perform the whole dance of assigning resources to the child
process, including tempfiles.
There's only a single other caller in "daemon.c", but that process
doesn't create any tempfiles before the call to `daemonize()` and is
thus not impacted by this change.
Reported-by: Jean-Christophe Manciot <actionmystique@gmail.com> Helped-by: Jeff King <peff@peff.net> Helped-by: Derrick Stolee <stolee@gmail.com> Co-authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael Montalbo [Tue, 12 May 2026 18:10:23 +0000 (18:10 +0000)]
parse-options: clarify what "negated" means for PARSE_OPT_NONEG
The documentation says the flag prevents an option from being
"negated" without specifying what that means. Add a parenthetical
to clarify that it rejects the "--no-<option>" form.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael Montalbo [Tue, 12 May 2026 18:10:22 +0000 (18:10 +0000)]
xdiff: guard against negative context lengths
The xdemitconf_t fields ctxlen and interhunkctxlen are typed as long
(signed), but negative values are not meaningful for context line
counts. Unlike the diff_options fields changed in the previous two
commits, these cannot be converted to unsigned because the xdiff
arithmetic relies on signed subtraction:
s1 = XDL_MAX(xch->i1 - xecfg->ctxlen, 0);
If ctxlen were unsigned long, the signed operand would be implicitly
converted to unsigned, and the subtraction would wrap to a large
positive value when i1 < ctxlen, defeating the XDL_MAX clamp. The
signed type is required for correct context-window calculations.
The previous two commits reject negative values at the parse layer
for --inter-hunk-context and -U/--unified, so negative values should
no longer reach xdiff in normal use. Add BUG() guards at the top of
xdl_get_hunk() as defense in depth to catch programming errors in
current or future callers that bypass option parsing.
xdl_get_hunk() is called by both xdl_emit_diff() and
xdl_call_hunk_func(), so a single guard covers all xdiff consumers.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Line 503 of a 106-line file, count "999-" is not a valid integer.
The config variable diff.context already rejects negative values, but
the command line callback diff_opt_unified() uses strtol() with no
range check.
Change the type of diff_options.context and its static default from
int to unsigned int, matching the change to interhunkcontext in the
previous commit. The type change requires reworking the callback and
config parsing to validate in a local variable before assigning to
the now-unsigned field.
Unlike --inter-hunk-context which could be converted to OPT_UNSIGNED,
-U needs OPT_CALLBACK_F for PARSE_OPT_OPTARG (bare -U with no value
enables patch output). Add a range check in the callback instead.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Hunk 1 covers lines 110-115, hunk 2 starts at 115 (overlap), hunk 3
starts at 116 (overlaps both). The resulting patch cannot be applied.
The config variable diff.interHunkContext already rejects negative
values, but the command line option does not.
Change the type of diff_options.interhunkcontext and its static
default from int to unsigned int, and switch the option parser from
OPT_INTEGER_F to OPT_UNSIGNED. This rejects negative values at parse
time via git_parse_unsigned() and enforces the correct type at compile
time via BARF_UNLESS_UNSIGNED.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 12 May 2026 16:26:19 +0000 (12:26 -0400)]
http: handle absolute-path alternates from server root
When a dumb http server reports alternates with an absolute path, we try
to paste that onto the root of the URL we're trying to fetch from. So if
we go to "http://example.com/path/to/child.git" and it tells us about an
alternate at "/parent.git", we'll hit "http://example.com/parent.git".
But there's a bug in computing the base when the URL does not have any
path component at all, like "http://example.com". When looking for the
first slash after the host, strchr() returns NULL, and we compute a
nonsense value for the length of the host portion. And then when we use
that length to copy the base of the URL into a strbuf, we're likely to
fail.
The security implications are minimal here. We store the nonsense length
("serverlen") as an int, so on a 64-bit system it may effectively be
anything (it is zero minus a 64-bit heap pointer, then truncated to
32-bits and stuffed into a signed value). When we feed that length to
strbuf_add(), it is cast into a size_t and one of four things will
happen:
1. If serverlen was negative, it will turn into a very large positive
value and strbuf_add() will fail to allocate, ending the program.
Ditto if serverlen was positive but just very large.
This doesn't really get an attacker anything; the victim will just
fail to clone their evil repo.
2. If serverlen was small enough, we'll successfully extend the target
strbuf, and then copy an arbitrary set of bytes from "base". And
then one of these is true:
a. That set of bytes is much larger than the length of the "base"
string. This is an out-of-bounds read, but there's no
out-of-bounds write, since the strbuf code both allocates and
copies using the same size_t. This is likely to cause a
segfault as we try to read unmapped pages of memory.
b. Like (2a), but if the set of bytes is small enough we might
not segfault. We might read random memory from the process and
copy it into the "target" strbuf.
What happens then? We know that "base" ends with a NUL
terminator, which will be copied into "target" as well. So
even though target.len might be 1000 bytes (or whatever), when
interpreted as a NUL-terminated string, target.buf is still
the exact same string as "base".
And that's all we ever do with target: pass it around as a C
string, and then eventually strbuf_detach() it to become a C
string. So even though there was arbitrary memory copied into
the strbuf, we never access it.
c. The other interesting case is when serverlen is actually
_shorter_ than the length of base. And there we truncate the
string. Probably in a way that makes it totally invalid, but
if you were very unlucky you could turn something like:
http://victim.com.evil.domain:8000
into:
http://victim.com
Which looks like the start of a redirect attack, except that
the attacker could just have written "http://victim.com" in
the first place! Either way we feed it to
is_alternate_allowed(), which is where we check redirect and
protocol rules.
I think we can just treat this like a regular bug.
And it's quite a weird setup in the first place, as it implies that the
root of the web server is serving a repository (i.e., that you can get
something useful from "http://example.com/info/refs"). The bug has been
there since b3661567cf ([PATCH] Add support for alternates in HTTP,
2005-09-14) without anybody noticing.
I kind of doubt anybody really cares about making this work, but it's
easy enough to do so: the host-portion of the URL ends at either the
first slash or the end-of-string. So we can just replace strchr() with
strchrnul().
The test setup is a little gross, as we take over the httpd document
root by shoving our bare-repo components into it. But it demonstrates
the problem and shows that our solution actually allows the alternate to
function, if the server is configured to allow it.
Reported-by: slonkazoid <slonkazoid@slonk.ing> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Tue, 12 May 2026 16:20:22 +0000 (12:20 -0400)]
pretty: drop strbuf pre-sizing from add_rfc2047()
At the top of add_rfc2047() we do this:
strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
where "len" is the size of the header (like an author name) we are about
to encode into the buffer. This pre-sizing is purely an optimization; we
use strbuf_addf() and friends to actually write into the buffer, and
they will grow the buffer as necessary.
But there's a problem with the code above: the input can be arbitrarily
large, so we might overflow a size_t while doing that computation,
ending up with a too-small allocation request. Overflowing requires an
impractically large input on a 64-bit system, but is easy to demonstrate
on a 32-bit system with a commit whose author name is ~1.4GB.
Because this pre-sizing is just an optimization, there's no real harm.
We'll start with a smaller buffer and grow it as necessary. But it
_looks_ like a vulnerability, since some other code may pre-size a
strbuf and then write directly into its buffer. So it's worth avoiding
the overflow in the first place.
The obvious way to do that is via checked operations like st_add() and
friends. But taking a step back, is this pre-sizing actually helping
anything?
The computation goes all the way back to 4234a76167 (Extend
--pretty=oneline to cover the first paragraph,, 2007-06-11), but back
then we really were sizing the array to write into directly! In 674d172730 (Rework pretty_print_commit to use strbufs instead of custom
buffers., 2007-09-10) that switched to a strbuf, and at that point it
was a pure optimization.
Is the optimization helping? I don't think so. Even for a gigantic case
like the 1.4GB author name, I couldn't measure any slowdown when
removing it. And most input will be much smaller, and added to a running
strbuf containing the rest of the email-header output. We can just rely
on strbuf's usual amortized-linear growth.
So deleting the line seems like the best way to go. It eliminates the
integer overflow and makes the code a tiny bit simpler.
Reported-by: Luke Martin <lmartin@paramenoeng.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Tue, 12 May 2026 11:56:03 +0000 (13:56 +0200)]
ls-tree: use strbuf_add_uint()
Speed up printing of objectsize values by using the specialized function
strbuf_add_uint() as well as strbuf_insert() for padding instead of the
general-purpose function strbuf_addf(). Here are the numbers I get when
listing objects in the Linux kernel repo:
Benchmark 1: ./git_main -C ../linux ls-tree -r --format='%(objectsize)' HEAD
Time (mean ± σ): 294.4 ms ± 0.4 ms [User: 231.5 ms, System: 59.4 ms]
Range (min … max): 293.9 ms … 295.0 ms 10 runs
Benchmark 2: ./git -C ../linux ls-tree -r --format='%(objectsize)' HEAD
Time (mean ± σ): 291.2 ms ± 0.4 ms [User: 227.9 ms, System: 62.1 ms]
Range (min … max): 290.6 ms … 292.0 ms 10 runs
Benchmark 3: ./git_main -C ../linux ls-tree -r --format='%(objectsize:padded)' HEAD
Time (mean ± σ): 295.3 ms ± 0.6 ms [User: 232.0 ms, System: 59.6 ms]
Range (min … max): 294.3 ms … 296.3 ms 10 runs
Benchmark 4: ./git -C ../linux ls-tree -r --format='%(objectsize:padded)' HEAD
Time (mean ± σ): 291.9 ms ± 0.4 ms [User: 228.5 ms, System: 61.5 ms]
Range (min … max): 291.2 ms … 292.3 ms 10 runs
Summary
./git -C ../linux ls-tree -r --format='%(objectsize)' HEAD ran
1.00 ± 0.00 times faster than ./git -C ../linux ls-tree -r --format='%(objectsize:padded)' HEAD
1.01 ± 0.00 times faster than ./git_main -C ../linux ls-tree -r --format='%(objectsize)' HEAD
1.01 ± 0.00 times faster than ./git_main -C ../linux ls-tree -r --format='%(objectsize:padded)' HEAD
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Tue, 12 May 2026 11:56:02 +0000 (13:56 +0200)]
ls-files: use strbuf_add_uint()
Speed up printing of objectsize values by using the specialized function
strbuf_add_uint() as well as strbuf_insert() for padding instead of the
general-purpose function strbuf_addf(). Here are the numbers I get when
listing files in the Linux kernel repo:
Benchmark 1: ./git_main -C ../linux ls-files --format='%(objectsize)'
Time (mean ± σ): 257.3 ms ± 0.4 ms [User: 197.4 ms, System: 56.7 ms]
Range (min … max): 256.7 ms … 258.1 ms 11 runs
Benchmark 2: ./git -C ../linux ls-files --format='%(objectsize)'
Time (mean ± σ): 253.4 ms ± 0.3 ms [User: 193.6 ms, System: 56.6 ms]
Range (min … max): 253.0 ms … 253.8 ms 11 runs
Benchmark 3: ./git_main -C ../linux ls-files --format='%(objectsize:padded)'
Time (mean ± σ): 257.9 ms ± 0.3 ms [User: 198.0 ms, System: 56.6 ms]
Range (min … max): 257.3 ms … 258.5 ms 11 runs
Benchmark 4: ./git -C ../linux ls-files --format='%(objectsize:padded)'
Time (mean ± σ): 254.6 ms ± 1.0 ms [User: 194.6 ms, System: 56.7 ms]
Range (min … max): 253.7 ms … 256.8 ms 11 runs
Summary
./git -C ../linux ls-files --format='%(objectsize)' ran
1.00 ± 0.00 times faster than ./git -C ../linux ls-files --format='%(objectsize:padded)'
1.02 ± 0.00 times faster than ./git_main -C ../linux ls-files --format='%(objectsize)'
1.02 ± 0.00 times faster than ./git_main -C ../linux ls-files --format='%(objectsize:padded)'
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Tue, 12 May 2026 11:56:00 +0000 (13:56 +0200)]
strbuf: add strbuf_add_uint()
strbuf_addf() calls vsnprintf(3) underneath, which supports a plethora
of formatting options. We can avoid its overhead in basic cases by
providing specialized functions like strbuf_addstr() for strings. Add
another one, strbuf_add_uint(), for unsigned integers.
Prepare the number string in a temporary buffer. Make it big enough for
any unsigned integer value: A decimal digit can represent ln(10)/ln(2) ≈
3.32 bits; dividing the number of bits of uintmax_t by 3.3 and rounding
up gives a sufficiently close conservative size estimate.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge: use repo_in_merge_bases for octopus up-to-date check
The octopus merge path checks whether each remote head is already
an ancestor of HEAD by computing all merge-bases via
repo_get_merge_bases() and comparing the first result's OID to
the remote head. This is more expensive than necessary:
repo_get_merge_bases() calls paint_down_to_common() with
min_generation=0, performs the full STALE drain, and may run
remove_redundant(), when all we need is a yes/no reachability
answer.
Replace this with repo_in_merge_bases(), which answers the
is-ancestor question directly. When generation numbers are
available, repo_in_merge_bases() uses can_all_from_reach() -- a
DFS bounded by generation number that stops as soon as the target
is found or ruled out, without entering paint_down_to_common() at
all. Without generation numbers, it still benefits from a tighter
min_generation floor.
Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
To help Windows 10 installations, avoid removing files whose
contents are still mmap()'ed.
* js/maintenance-fix-deadlock-on-win10:
maintenance(geometric): do release the `.idx` files before repacking
mingw: optionally use legacy (non-POSIX) delete semantics
Junio C Hamano [Tue, 12 May 2026 02:04:44 +0000 (11:04 +0900)]
Merge branch 'js/ci-github-actions-update'
Update various GitHub Actions versions.
* js/ci-github-actions-update:
l10n: bump mshick/add-pr-comment from v2 to v3
ci: bump git-for-windows/setup-git-for-windows-sdk from v1 to v2
ci: bump actions/checkout from v5 to v6
ci: bump actions/github-script from v8 to v9
ci: bump actions/{upload,download}-artifact to v7 and v8
ci: bump microsoft/setup-msbuild from v2 to v3
Taylor Blau [Tue, 12 May 2026 00:47:12 +0000 (20:47 -0400)]
pack-bitmap: prevent pattern leak on pseudo-merge re-assignment
When "bitmapPseudoMerge.*.pattern" appears more than once for the same
group, `pseudo_merge_config()` frees the old `regex_t *` pointer
but does not call `regfree()` on it first. This leaks whatever internal
state `regcomp()` allocated.
The final cleanup path in `pseudo_merge_group_release()` does call
`regfree()` before `free()`, so only the intermediate replacement is
affected.
Fix this by guarding the replacement with a NULL check and calling
`regfree()` before `free()` when the pointer is non-NULL.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:47:09 +0000 (20:47 -0400)]
Documentation: fix broken `sampleRate` in gitpacking(7)
The documentation explaining some sample configurations for bitmap
pseudo-merges incorrectly uses a sample rate outside of the allowed
(0,1] range.
This dates back to faf558b23ef (pseudo-merge: implement support for
selecting pseudo-merge commits, 2024-05-23), and was likely written when
the allowable range for this configuration was the integral values
between (0,100].
Fix this to conform to the actual allowable range for this
configuration.
Noticed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:47:06 +0000 (20:47 -0400)]
pack-bitmap: reject pseudo-merge "sampleRate" of 0
The "bitmapPseudoMerge.*.sampleRate" configuration controls what
fraction of unstable commits are included in each pseudo-merge group.
The config validation accepts values in the range `[0, 1]`, but a value
of exactly 0 causes a division by zero in `select_pseudo_merges_1()`:
if (j % (uint32_t)(1.0 / group->sample_rate))
When `sample_rate` is 0, `1.0 / 0.0` produces `+inf`, and casting
infinity to `uint32_t` is undefined behavior in C. On most platforms
this yields 0, making the subsequent modulo operation (`j % 0`) a
fatal arithmetic trap.
This path was not previously reachable because an earlier bug caused
all pseudo-merge candidates to be classified as "stable" (where the
sampling rate is not used), regardless of their actual commit date. Now
that the date classification is fixed, the unstable path is exercised
and the division by zero can fire.
Fix this by changing the validation to require a strict lower bound and
thus reject 0.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:47:03 +0000 (20:47 -0400)]
pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
`find_pseudo_merge_group_for_ref()` uses the commit's date to classify
it as either "stable" (older than the stable threshold) or "unstable"
(otherwise).
However, to find the relevant commit from a given OID, the function
`find_pseudo_merge_group_for_ref()` uses `lookup_commit()` which does
not parse commits.
Because an unparsed commit has its "date" set to zero, every candidate
is placed in the "stable" bucket regardless of its actual committer
timestamp. This means the `bitmapPseudoMerge.*.threshold` and
`stableThreshold` configuration options have no effect: the
stable/unstable split is always determined by comparing against zero
rather than the real commit date.
The net result is that pseudo-merge groups are partitioned by
`stableSize` instead of the intended decay-based sizing, and the
`sampleRate` knob (which only applies to the unstable path) is never
exercised.
Fix this by calling `repo_parse_commit()` after `lookup_commit()`,
bailing out of the callback if parsing fails.
The corresponding test configures two pseudo-merge groups that both
match all tags. The "stable" group uses `threshold=1.month.ago`, and the
"all" group uses `threshold=now`. The test use our custom
"GIT_TEST_DATE_NOW" environment variable by setting it to the value of
"$test_tick" to align Git's notion of "now" (and therefore
"1.month.ago") with the `test_tick` timestamps, so the commits appear to
be younger than one month: only the "all" group matches them, producing
exactly one pseudo-merge.
Without the fix every commit has `date == 0`, which satisfies `date <=
threshold` for both groups (since 0 is older than one month ago), and
the "stable" group erroneously matches as well.
Now that commits are correctly classified as "unstable", the bug
described in the test exercising the "sampleRate=0" test is reachable,
and the test is marked as failing. It will be fixed in a following
commit.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:47:00 +0000 (20:47 -0400)]
pack-bitmap: fix pseudo-merge lookup for shared commits
When a commit appears in more than one pseudo-merge group, its entry in
the commit lookup table has the high bit set in its offset field,
indicating that the offset points to an "extended" table containing the
set of pseudo-merges for that commit.
There are three bugs in this path:
* The `next_ext` offset in `write_pseudo_merges()` undercounts the
per-entry size of the lookup table (8 vs. 12 bytes).
* `nth_pseudo_merge_ext()` calls `read_pseudo_merge_commit_at()` on a
pseudo-merge bitmap offset, misinterpreting it as a 12-byte commit
table entry.
* The error check after `pseudo_merge_ext_at()` in
`apply_pseudo_merges_for_commit()` tests `< -1` instead of `< 0`,
silently swallowing errors from `error()`.
The first bug is on the write side: each commit lookup entry contains a
4- and 8-byte unsigned value for a total of 12 bytes, but the
calculation assumes that the entry only contains 8 bytes of data. This
makes `next_ext` too small, so the extended-table offsets that get
written point into the middle of the non-extended lookup table rather
than past it. The reader then interprets non-extended lookup data as
extended entries, producing garbage.
The second bug is on the read side and is independently fatal: even with
a correctly positioned extended table, `nth_pseudo_merge_ext()` feeds
the offset it reads (which points at pseudo-merge bitmap data) to
`read_pseudo_merge_commit_at()`. That function tries to parse 12 bytes
as a `pseudo_merge_commit` struct, clobbering `merge->pseudo_merge_ofs`
with whatever happens to be at that location. The caller only needs
`pseudo_merge_ofs`, so the fix is to store the offset directly rather
than re-parsing a commit table entry. The `commit_pos` field is left
untouched, retaining the value that `find_pseudo_merge()` set earlier.
The third bug is latent. With the first two fixes applied, the extended
table is correctly written and read, so `pseudo_merge_ext_at()` does not
fail during normal operation. The `< -1` vs `< 0` distinction only
matters when the bitmap file is corrupt or truncated, in which case the
error would be silently ignored and the code would proceed with
uninitialized data.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:46:57 +0000 (20:46 -0400)]
pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
The binary search in `pseudo_merge_at()` has its "lo" and "hi" updates
swapped: when the midpoint's offset is less than the target, it sets `hi
= mi` (searching left) instead of `lo = mi + 1` (searching right), and
vice versa.
This means that lookups for pseudo-merges whose offset is not near the
midpoint of the pseudo-merge table are likely to fail.
In practice, with a single pseudo-merge group this is masked because the
lone entry is always at the midpoint. With multiple groups, the inverted
comparisons cause lookups to search in the wrong direction, potentially
missing entries.
Swap the "lo" and "hi" assignments to search in the correct direction,
making it possible to apply pseudo-merges during fill-in when more than
one pseudo-merge exists in a group.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:46:54 +0000 (20:46 -0400)]
pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
The pseudo-merge commit lookup table stores each commit's position in
the pack- or pseudo-pack order, and is used to perform a binary search
in order to determine which pseudo-merge(s) a given commit belongs to.
However, the table was previously sorted in lexical order (via
`oid_array_sort()`), causing the binary search to fail.
While this causes pseudo-merge bitmaps to be de-facto broken for fill-in
traversal, there are a couple of important points to keep in mind:
* Pseudo-merge application during the initial phases of a bitmap-based
traversal are applied via `cascade_pseudo_merges_1()`. This function
enumerates the known pseudo-merges and determines if its parents are
a subset of the traversal roots.
This is a different path than the fill-in traversal, where we are
looking for any pseudo-merges which may be satisfied after visiting
some commit along an object walk, which involves the aforementioned
(broken) binary search.
As a consequence, any pseudo-merges we apply at this stage are done
so correctly.
* While this bug makes applying pseudo-merges during fill-in traversal
effectively broken, it does not produce wrong results. Instead of
applying the *wrong* pseudo-merge, we will simply fail to find
satisfied pseudo-merges, leaving the traversal to use the existing
fill-in routines.
Fix this by sorting the table by bit position before writing, matching
the order that the reader's binary search expects.
This does produce a change the on-disk format insofar as the actual code
now complies with the documented format (for more details, refer to:
Documentation/technical/bitmap-format.adoc). Given that this never
worked in the first place, such a change should be OK to perform.
If an out-of-tree implementation of pseudo-merges happened to generate
bitmaps that comply with the documented format, they will continue to be
read and interpreted as normal.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:46:51 +0000 (20:46 -0400)]
t5333: demonstrate various pseudo-merge bugs
Using the test helper introduced via the previous commit, add various
failing tests demonstrating bugs in the pseudo-merge implementation.
These are all marked as failing with one exception. The "sampleRate=0"
test describes a latent bug, which is only reachable through a code path
that is itself masked by a separate bug. A future commit will fix that
bug, and, in turn, cause the aforementioned test to fail. Accordingly,
that commit will mark the test as failing, and it will be re-marked as
passing in a separate commit which fixes the once-latent bug.
For the rest: the following commits will explain and fix the underlying
bugs in detail.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Taylor Blau [Tue, 12 May 2026 00:46:48 +0000 (20:46 -0400)]
t/helper: add 'test-tool bitmap write' subcommand
In f16eb1c091 (pseudo-merge: fix disk reads from find_pseudo_merge(),
2026-03-31), we noted that `apply_pseudo_merges_for_commit()` is never
triggered by the existing test suite, and that this bears further
investigation.
This patch is the first one to begin that investigation. The following
patches will expose and fix a variety of bugs in the implementation of
pseudo-merge bitmaps.
In order to do so, however, many of these tests require very precise
selection of which commits receive bitmaps and which do not. To date,
there isn't a standard approach to easily facilitate this. Address this
by introducing a `test-tool bitmap write` subcommand that writes a
bitmap for a given packfile, reading the set of commits which should
receive individual bitmaps from stdin like so:
, where "<pack-basename>" is the filename for a specific packfile (e.g.,
"pack-abc123.pack"), and "/path/to/commits.list" is a list of commit
OIDs which will receive bitmaps.
The helper respects `bitmapPseudoMerge.*` configuration for creating
pseudo-merge bitmaps alongside the regular commit bitmaps.
Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Abhinav Gupta [Mon, 11 May 2026 12:21:53 +0000 (12:21 +0000)]
sequencer: remove todo_add_branch_context.commit
The 'commit' field in 'struct todo_add_branch_context' is unused.
It's written to, but never read from.
add_decorations_to_list() gets the commit passed to it explicitly
as an argument.
Signed-off-by: Abhinav Gupta <mail@abhinavg.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit-reach: early exit paint_down_to_common for single merge-base
Commits not in the commit-graph get GENERATION_NUMBER_INFINITY and
sort to the top of the priority queue. After those, commits with
finite generation numbers are popped in non-increasing order.
When MERGE_BASE_FIND_ALL is not set the first doubly-painted commit
with a finite generation is therefore a best merge-base: no commit
still in the queue can be a descendant of it. Skip the expensive
STALE drain in this case.
Add MERGE_BASE_FIND_ALL to the merge_base_flags enum. Callers that
need every merge-base (repo_get_merge_bases_many, repo_get_merge_bases,
repo_in_merge_bases_many, remove_redundant_no_gen) pass the flag to
preserve existing behavior. git merge-base (without --all) passes 0,
triggering the early exit.
On a 2.2M-commit merge-heavy monorepo with commit-graph:
HEAD vs ~500: 5,229ms -> 24ms
HEAD vs ~1000: 4,214ms -> 39ms
HEAD vs ~5000: 3,799ms -> 46ms
HEAD vs ~10000: 3,827ms -> 61ms
Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Replace the boolean ignore_missing_commits parameter in
paint_down_to_common() with an enum merge_base_flags, and thread
the flags through merge_bases_many(), get_merge_bases_many_0(),
and the public repo_get_merge_bases_many_dirty() API.
This makes callsites with boolean parameters easier to read and
prepares the function for additional flags in a subsequent commit.
No functional change: the single caller that used
ignore_missing_commits (repo_in_merge_bases_many) now sets
MERGE_BASE_IGNORE_MISSING_COMMITS in the flags word, and all
other callers pass 0.
Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
format-rev: introduce builtin for on-demand pretty formatting
Introduce a new builtin for pretty formatting one revision expression
per line or commit object names found in running text.
Sometimes you want to format commits. Most of the time you’re
walking the graph, e.g. getting a range of commits like
`master..topic`. That’s a job for git-log(1).
But there are times when you want to format commits that you encounter
on demand:
• Full hashes in running text that you might want to pretty-print
• git-last-modified(1) outputs full hashes that you can do the same
with
• git-cherry(1) has `-v` for commit subject, but maybe you want
something else?
But now you can’t use git-log(1), git-show(1), or git-rev-list(1):
• You can’t feed commits piecemeal to these commands, one input
for one output; they block until standard in is closed
• You can’t feed a list of possibly duplicate commits, like the output
of git-last-modified(1); they effectively deduplicate the output
Beyond these two points there’s also the input massage problem: you
cannot feed mixed input (revisions mixed with arbitrary text).
One might hope that git-cat-file(1) can save us. But it doesn’t
support pretty formats.
But there is one command that already both handles revisions as
arguments, revisions on standard input, and even revisions mixed in
with arbitrary text. Namely git-name-rev(1): the command for outputting
symbolic names for commits.
We made some room in `builtin/name-rev.c` two commits ago. Let’s
now add this new git-format-rev(1) command. Taking inspiration from
git-name-rev(1), there are two modes:
• revs: like git-name-rev(1) in argv mode, but one revision per line
on standard in
• text: like git-name-rev(1) with `--annotate-stdin`
***
We need to add this command to the exception list in
`t/t1517-outside-repo.sh` because it uses “EXPERIMENTAL!”
in the usage line.
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
name-rev: make dedicated --annotate-stdin --name-only test
The previous commit split the `--name-only` handling:
1. `--annotate-stdin`: uses the new `struct command`
2. The rest: uses `struct name_ref_data`
But there is no dedicated test for the option combination in (1). That
means that the following tests will fail if you neglect to set
`command.u.name_only` properly:
name-rev --annotate-stdin works with commitGraph
name-rev --annotate-stdin works with non-monotonic timestamps
even though it has nothing to do with what these tests are supposed
to test.
Let’s add another regression test now that it is relevant.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
name-rev: factor code for sharing with a new command
We are about to introduce a new command git-format-rev(1) to this
file. Let’s factor some code so that we can share it with the new
command.
We want to be able to format commits found in freeform text, and
git-name-rev(1) already has a function for that but for symbolic
names. Let’s use a tagged union for the command-specific payload.
No functional changes.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Samo Pogačnik [Mon, 11 May 2026 19:20:42 +0000 (21:20 +0200)]
shallow: fix relative deepen on non-shallow repositories
The commit "3ef68ff40e (shallow: handling fetch relative-deepen,
2026-02-15)" introduced a bug where using --deepen=<n> on a non-
shallow repository incorrectly treated the value as an absolute
depth, resulting in a shallow fetch and truncated history.
This patch prevents any modification when a relative deepen is
requested on a non-shallow repository.
A test is added to ensure that history is not changed when
--deepen is used on a non-shallow repository.
Reported-by: Owen Stephens <owen@owenstephens.co.uk> Signed-off-by: Samo Pogačnik <samo_pogacnik@t-2.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
build: tolerate use of _Generic from glibc 2.43 with Clang
When building with `make DEVELOPER=1` we explicitly pass "-std=gnu99" to
the compiler so that we don't start leaning on features exposed by more
recent versions of the C standard. Unfortunately though, glibc 2.43
started to use type-generic expressions. This works alright with GCC,
but when compiling with Clang this leads to errors:
$ make DEVELOPER=1 CC=clang
CC daemon.o
In file included from daemon.c:3:
./git-compat-util.h:344:11: error: '_Generic' is a C11 extension [-Werror,-Wc11-extensions]
344 | return !!strchr(path, '/');
| ^
/usr/include/string.h:265:3: note: expanded from macro 'strchr'
265 | __glibc_const_generic (S, const char *, strchr (S, C))
| ^
/usr/include/x86_64-linux-gnu/sys/cdefs.h:838:3: note: expanded from macro '__glibc_const_generic'
838 | _Generic (0 ? (PTR) : (void *) 1, \
| ^
In theory, the `__glibc_const_generic` macro does have feature gating:
But this feature gating isn't effective because `_has_extension()` will
always evaluate to true as C generics _are_ available as a language
extension to GNU C99 when using Clang. This would have been different if
`_has_feature()` was used instead, in which case it would have properly
evaluated to `false`.
GCC has a workaround to squelch this warning from standard system
headers, but because clang fails due to [-Werror,-Wc11-extensions],
as it lacks the corresponding workaround.
For both meson and Makefile, pass -Wno-c11-extensions when we are
building with clang.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Helped-by: Shardul Natu <snatu@google.com>
[jc: replaced Makefile side with Shardul's approach] Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 11 May 2026 04:49:05 +0000 (13:49 +0900)]
Merge branch 'jc/neuter-sideband-fixup'
Try to resurrect and reboot a stalled "avoid sending risky escape
sequences taken from sideband to the terminal" topic by Dscho. The
plan is to keep it in 'next' long enough to see if anybody screams
with the "everything dropped except for ANSI color escape sequences"
default.
* jc/neuter-sideband-fixup:
sideband: drop 'default' configuration
sideband: offer to configure sanitizing on a per-URL basis
sideband: add options to allow more control sequences to be passed through
sideband: do allow ANSI color sequences by default
sideband: introduce an "escape hatch" to allow control characters
sideband: mask control characters
Junio C Hamano [Mon, 11 May 2026 01:05:54 +0000 (10:05 +0900)]
Merge branch 'ps/test-set-e-clean'
The test suite harness and many individual test scripts have been
updated to work correctly when 'set -e' is in effect, which helps
detect misspelled test commands.
* ps/test-set-e-clean:
t: detect errors outside of test cases
t9902: fix use of `read` with `set -e`
t6002: fix use of `expr` with `set -e`
t1301: don't fail in case setfacl(1) doesn't exist or fails
t0008: silence error in subshell when using `grep -v`
t: prepare `test_when_finished ()`/`test_atexit()` for `set -e`
t: prepare execution of potentially failing commands for `set -e`
t: prepare conditional test execution for `set -e`
t: prepare `git config --unset` calls for `set -e`
t: prepare `stop_git_daemon ()` for `set -e`
t: prepare `test_must_fail ()` for `set -e`
t: prepare `test_match_signal ()` calls for `set -e`
Junio C Hamano [Mon, 11 May 2026 01:05:53 +0000 (10:05 +0900)]
Merge branch 'ar/parallel-hooks'
Hook scripts defined via the configuration system can now be
configured to run in parallel.
* ar/parallel-hooks:
t1800: test SIGPIPE with parallel hooks
hook: allow hook.jobs=-1 to use all available CPU cores
hook: add hook.<event>.enabled switch
hook: move is_known_hook() to hook.c for wider use
hook: warn when hook.<friendly-name>.jobs is set
hook: add per-event jobs config
hook: add -j/--jobs option to git hook run
hook: mark non-parallelizable hooks
hook: allow pre-push parallel execution
hook: allow parallel hook execution
hook: parse the hook.jobs config
config: add a repo_config_get_uint() helper
repository: fix repo_init() memleak due to missing _clear()
Junio C Hamano [Mon, 11 May 2026 01:05:53 +0000 (10:05 +0900)]
Merge branch 'cc/promisor-auto-config-url'
Promisor remote handling has been refactored and fixed in
preparation for auto-configuration of advertised remotes.
* cc/promisor-auto-config-url:
t5710: use proper file:// URIs for absolute paths
promisor-remote: remove the 'accepted' strvec
promisor-remote: keep accepted promisor_info structs alive
promisor-remote: refactor accept_from_server()
promisor-remote: refactor has_control_char()
promisor-remote: refactor should_accept_remote() control flow
promisor-remote: reject empty name or URL in advertised remote
promisor-remote: clarify that a remote is ignored
promisor-remote: pass config entry to all_fields_match() directly
promisor-remote: try accepted remotes before others in get_direct()
Junio C Hamano [Mon, 11 May 2026 01:05:51 +0000 (10:05 +0900)]
Merge branch 'sp/refs-reduce-the-repository'
Code clean-up to use the right instance of a repository instance in
calls inside refs subsystem.
* sp/refs-reduce-the-repository:
refs/reftable-backend: drop uses of the_repository
refs: remove the_hash_algo global state
refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
Junio C Hamano [Sun, 10 May 2026 23:51:15 +0000 (08:51 +0900)]
ci: enable EXPENSIVE for contributor builds
Earlier, we enabled EXPENSIVE tests for pushes to integration
branches. As we didn't have any CI jobs that run these tests, this
was a step in the right direction.
It however is an ineffective and inefficient use of the maintainer
time, which does not scale, to allow contributors to send changes
that are less tested at the list, only to force the maintainer
notice breakages caused by their changes but only after these
changes are mixed with changes from other contributors. The
problematic topic needs to be isolated by bisecting, and it
historically has been done by the maintainer alone.
It is far better to let the problem identified early, preferably
before the problematic code leaves the hands of the original
developer. In order for it to happen, the test coverage of the
contributor tests must be at least as wide as the coverage of the
integration tests.
Enable expensive tests for CI jobs triggered by pull requests. This
will make each contributor take care of their own, which scales much
better.
Keep the expensive tests also enabled for the pushes of integration
branches, as that is the only place we can notice problems stemming
from mismerges and inter-topic interactions, even if the topics from
the contributors in isolation all passes these tests.
Junio C Hamano [Mon, 11 May 2026 00:05:28 +0000 (09:05 +0900)]
Merge branch 'js/objects-larger-than-4gb-on-windows' into jc/ci-enable-expensive
* js/objects-larger-than-4gb-on-windows:
ci: run expensive tests on push builds to integration branches
t5608: mark >4GB tests as EXPENSIVE
test-tool synthesize: add precomputed SHA-256 pack for 4 GiB + 1
test-tool synthesize: precompute pack for 4 GiB + 1
test-tool synthesize: use the unsafe hash for speed
t5608: add regression test for >4GB object clone
test-tool: add a helper to synthesize large packfiles
delta, packfile: use size_t for delta header sizes
odb, packfile: use size_t for streaming object sizes
git-zlib: handle data streams larger than 4GB
index-pack, unpack-objects: use size_t for object size
Abhinav Gupta [Sun, 10 May 2026 22:41:11 +0000 (15:41 -0700)]
rebase: ignore non-branch update-refs
The following Git configuration breaks git rebase --update-refs:
[rebase]
instructionFormat = %s%d
The '%d' format requests all available decorations for a commit,
filling the global decoration table with all of them,
which --update-refs then uses to populate 'update-ref' instructions
in the rebase todo list.
Specifically, this results in the following instruction:
update-ref HEAD
The todo parser then rejects the instruction:
error: update-ref requires a fully qualified refname e.g. refs/heads/HEAD
error: invalid line 3: update-ref HEAD
To fix, ignore decorations that are not local branches
when scanning through the table.
This matches the documented contract:
it moves branch refs under refs/heads/
and leaves display-only decorations (HEAD, tags, etc.) alone.
Verification:
A regression test that fails without this fix is included.
Signed-off-by: Abhinav Gupta <mail@abhinavg.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sun, 10 May 2026 12:42:04 +0000 (14:42 +0200)]
sideband: clear full line when printing remote messages
demultiplex_sideband() can write its remote output over active local
progress lines. That's why it has been using ANSI code Erase in Line on
smart terminals to clear the remainder of lines it writes since ebe8fa738d (fix display overlap between remote and local progress,
2007-11-04).
This erases the last character of remote lines that span the full width
of the terminal, though, as the cursor is stuck at the rightmost column
for them. It's the same effect as in the following command, which
clears the 1 and shows just the leading zeros:
$ EL="\033[K"
$ printf "%0${COLUMNS}d${EL}\n" 1
If we move the ANSI code to the start we get to see the 1 as well:
$ printf "${EL}%0${COLUMNS}d\n" 1
So do the same in demultiplex_sideband() and emit the ANSI code as a
prefix instead of a suffix to show messages in full even if they happen
to fill the whole width of a smart terminal.
Reported-by: Hugo Osvaldo Barrera <hugo@whynothugo.nl> Suggested-by: Chris Torek <chris.torek@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Saagar Jha [Sun, 10 May 2026 03:50:22 +0000 (03:50 +0000)]
submodule-config: fix reading submodule.fetchJobs
update_clone_config_from_gitmodules() passes &max_jobs to
config_from_gitmodules(), but max_jobs is already a pointer. This causes
the config value to be written to the wrong address and get dropped.
Pass max_jobs directly.
Signed-off-by: Saagar Jha <saagar@saagarjha.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ci: run expensive tests on push builds to integration branches
Derrick Stolee suggested [1] that expensive tests should be run at a
regular cadence rather than on every PR iteration. Gate GIT_TEST_LONG
on push builds to the integration branches (next, master, main, maint)
so that the EXPENSIVE prereq is satisfied there but not during PR
validation, where the extra minutes of wall-clock time do not justify
themselves.
Helped-by: Derrick Stolee <derrickstolee@github.com> Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
