wincred: avoid memory corruption when erasing a credential
The earlier d22a488482 (wincred: avoid memory corruption, 2025-11-17)
repaired only get_credential(); match_cred_password() has the same
defect and is reached on `git credential reject`. When Git asks the
helper to erase a stored credential whose password was supplied by
the caller, the helper copies the candidate's password into a freshly
allocated buffer for comparison. That copy overruns the allocation
by one WCHAR of NUL, which on uninstrumented Windows manifests as
process termination with status 0xC0000374. Because the helper can
die before reaching CredDeleteW(), `git credential reject` masks the
failure and the rejected credential remains stored.
CredentialBlobSize is documented as a byte count, so for an N-WCHAR
blob it equals N * sizeof(WCHAR). The pre-fix code allocated that
many bytes and asked wcsncpy_s to copy N wide characters, but
wcsncpy_s always appends a terminating NUL WCHAR, writing one WCHAR
past the allocation. The destination-capacity argument was also
passed in bytes rather than in WCHAR elements as the API requires,
so the safe-CRT runtime never rejected the copy.
See GHSA-rxqw-wxqg-g7hw.
Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since dd4bc01c0a (revision: use priority queue for non-limited
streaming walks, 2026-05-27), "git rev-list --no-walk <commit>
-- <path>" ignores the path arguments and outputs all commits
regardless of whether they touch the given paths.
That commit introduced a REV_WALK_NO_WALK enum value to separate
--no-walk from the streaming walk in get_revision_1(). The new
case skips process_parents(), which is correct for not enqueuing
parents, but also skips try_to_simplify_commit() which
process_parents() calls to evaluate whether each commit touches
the given paths.
Add a call to try_to_simplify_commit() for the
REV_WALK_NO_WALK case, folding it into the existing
REV_WALK_REFLOG case which already does the same.
Add tests for --no-walk path filtering to t6017. The
"single commit, match" test is defensive and passes without
the fix, while the other two fail without it.
Reported-by: Peter Colberg <pcolberg@redhat.com> Signed-off-by: Kristofer Karlsson <krka@spotify.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Thu, 16 Jul 2026 17:48:52 +0000 (10:48 -0700)]
Merge branch 'master' of https://github.com/j6t/git-gui
* 'master' of https://github.com/j6t/git-gui:
git-gui: allow larger width for the commit message field
git-gui: reduce complexity of the quiet msgfmt rule
git-gui: drop msgfmt --statistics output
git-gui i18n: Update Bulgarian translation (562t)
There are still a couple of callsites that use `the_repository`. Convert
these to instead use a repository injected by the caller. This allows us
to remove `USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor remaining public functions that still depend on
`the_repository` to instead receive a repository as parameter. This
allows us to get rid of `USE_THE_REPOSITORY_VARIABLE`.
Adapt callers accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have a bunch of file-local functions that use `the_repository`.
Adapt them so that the repository is instead passed as a parameter so
that we can get rid of this dependency.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
worktree: refactor code to use available repositories
In "worktree.c" we have lots of users of `the_repository` that already
have a repository available to them. Convert all of them to use that
repository instead.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have a bunch of users of `the_repository` in the "files" backend, all
of which are trivial to convert to instead use the backend's own repo.
Do so.
There is one more dependency on global state though via `ignore_case`,
and thus we can't trivially remove `USE_THE_REPOSITORY_VARIABLE`. But
this is the only use of global state, and we want to ensure that we
don't unwittingly reintroduce a dependency on `the_repository` going
forward.
Add an extern declaration for `ignore_case` so that it becomes
accessible even without `USE_THE_REPOSITORY_VARIABLE` and drop the
define itself.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs/packed: de-globalize handling of "core.packedRefsTimeout"
When locking the "packed-refs" file we allow the user to configure a
timeout for how long we try taking the lock. This is configurable via
"core.packedRefsTimeout", which we parse in `packed_refs_lock()`.
The parsed value is stored in function-static variables though, which of
course has the effect that we'll only ever use the timeout configured in
the first packed reference store that we see. Consequently, if we ever
were to handle stores from different repositories, then we'd use the
same configuration for both stores even if they diverge.
This is of course a somewhat theoretical concern -- we don't typically
handle multiple packed stores, and even if we did it's very unlikely
that the user has configured different timeout values for each of them.
But still, this is a code smell, and an unnecessary one, too.
Fix the issue by moving the value into `struct packed_ref_store` so that
it can be parsed per store.
This removes the last callsite that still used `the_repository`, so drop
the `USE_THE_REPOSITORY_VARIABLE` define.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Wed, 15 Jul 2026 04:41:17 +0000 (06:41 +0200)]
remote-curl: simplify passing of push specs
The push specs are kept in a strvec, whose array is NULL-terminated.
Pass only that to the protocol handlers, which avoids dealing with item
counts and their conversions from size_t to int, slightly simplifying
the code.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 15 Jul 2026 20:24:19 +0000 (13:24 -0700)]
Merge branch 'rs/blame-abbrev-marks'
The alignment of commit object name abbreviations in 'git blame'
output has been optimized to reserve a column for marks (caret,
question mark, or asterisk) only when such marks are actually shown.
* rs/blame-abbrev-marks:
blame: reserve mark column only if necessary
Junio C Hamano [Wed, 15 Jul 2026 20:24:19 +0000 (13:24 -0700)]
Merge branch 'mg/meson-hook-list-buildfix'
A racy build failure under Meson has been corrected by ensuring that
the generated header file 'hook-list.h' is built before compiling
files in 'builtin_sources' that depend on it.
* mg/meson-hook-list-buildfix:
meson: restore hook-list.h to builtin_sources
Junio C Hamano [Wed, 15 Jul 2026 20:24:19 +0000 (13:24 -0700)]
Merge branch 'jk/bloom-leak-fixes'
Various memory leaks in the Bloom-filter code paths that are exposed
when running tests with the 'GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1'
environment variable have been plugged.
* jk/bloom-leak-fixes:
line-log: drop extra copy of range with bloom filters
revision: avoid leaking bloom keyvecs with multiple traversals
bloom: make bloom-filter slab initialization idempotent
Junio C Hamano [Wed, 15 Jul 2026 20:24:18 +0000 (13:24 -0700)]
Merge branch 'ps/history-drop'
The experimental 'git history' command has been taught a new 'drop'
subcommand to remove a commit, with its descendants replayed onto its
parent.
* ps/history-drop:
builtin/history: implement "drop" subcommand
builtin/history: split handling of ref updates into two phases
replay: expose `replay_result_queue_update()`
reset: stop assuming that the caller passes in a clean index
reset: allow the caller to specify the current HEAD object
reset: introduce ability to skip updating HEAD
reset: introduce dry-run mode
reset: modernize flags passed to `reset_working_tree()`
reset: rename `reset_head()`
reset: drop `USE_THE_REPOSITORY_VARIABLE`
read-cache: split out function to drop unmerged entries to stage 0
Junio C Hamano [Wed, 15 Jul 2026 20:24:18 +0000 (13:24 -0700)]
Merge branch 'ps/odb-drop-whence'
The 'whence' field in 'struct object_info' has been removed. The
backend-specific object information retrieval has been refactored into
an opt-in 'struct object_info_source' structure.
* 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()
Junio C Hamano [Wed, 15 Jul 2026 20:24:18 +0000 (13:24 -0700)]
Merge branch 'ps/refs-writing-subcommands'
The 'git refs' toolbox has been extended with new 'create', 'delete',
'update', and 'rename' subcommands to create, delete, update, and
rename references, respectively.
Junio C Hamano [Wed, 15 Jul 2026 20:10:06 +0000 (13:10 -0700)]
submodule--helper: avoid use of %zu for now
Since d7d850e2b9 (CodingGuidelines: mention C99 features we can't
use, 2022-10-10), our CodingGuidelines document has explicitly
forbidden the use of '%z' and '%zu' printf() format specifiers,
even though C99 does support them. However, a new instance crept
in via 82c36fa0a9 (submodule: hash the submodule name for the
gitdir path, 2026-01-12).
We could claim that this is an unintentional weather balloon that
nobody has complained about for the past six months since Git 2.54,
proving that it is now safe to use these format specifiers. But
(1) it is probably too early to make that claim, as distributions
often stick to a stale version for several releases, and (2) it is
unlikely that a failure in this code path would manifest as a
major user-visible breakage that would trigger a failure report to
percolate down to us.
Instead, let's stick to the established workaround recommended by
our CodingGuidelines, which is to cast the value to (uintmax_t) and
format it with PRIuMAX, at least for now. Even if we eventually
perform a bulk update using a Coccinelle script to transition to %z
and %zu in the future, adding one more instance to the pile that
will need such a conversion is hardly a tragedy.
The exit code of the upstream command in a pipe is ignored, so in
git cat-file commit HEAD | sed -e "1,/^\$/d" >actual
a crash of "git cat-file" would go unnoticed: the exit code of the
pipeline is that of "sed", which happily succeeds on empty input. The
test would thus pass even though "git cat-file" failed.
Write the output of "git cat-file" to a file first and run "sed" on
that file, so that the exit codes of both commands are checked by the
&&-chain.
Signed-off-by: Shlok Kulshreshtha <diy2903@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sequencer: do not record dropped commits as rewritten
If a commit gets dropped because its changes are already upstream
then we should not record it as rewritten. As well as confusing any
post-rewrite hooks, it means we end up copying the notes from the
dropped commit to the commit that was picked immediately before the
one that was dropped.
While we do not want to record the dropped commit as rewritten, if
it is the final commit in a chain of fixups then we need to flush
the list of rewritten commits. The behavior of an "edit" command
where the commit is dropped is changed so that "rebase --continue"
will not amend the previous pick. However, as the code comment notes
it will still be erroneously recorded as rewritten when the rebase
continues. That will need to be addressed separately along with not
recording skipped commits as rewritten.
The initialization of "drop_commit" is moved to ensure it is initialized
when rewording a fast-forwarded commit.
Reported-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Tested-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sequencer: use an enum to represent result of picking a commit
Rather than using an integer where -1 is an error, 0 is success and 1
indicates there were conflicts, use an enum. This is clearer and lets
us add a separate return value for commits that are dropped because
they become empty in the next commit.
Note we continue to use "return error(...)" to return errors and
take advantage of C's lax typing of enums
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Unless we're rebasing, all we do in pick_one_commit() is call
do_pick_commit() and return its result. Simplify the code by returning
early if we're not rebasing so that we don't have to repeatedly call
is_rebase_i() in the rest of the function. Note that there are a couple
of conditions that do not call is_rebase_i() but they check for either
an "edit" or a "fixup" command, both of which imply we're rebasing.
The only block that does not return early is the one guarded by
"!res". Move the return into that block to make it clear that after
recording the commit as rewritten, all we do is return from the
function.
As the conditional blocks are all mutually exclusive (either the
conditions are mutually exclusive, or an earlier conditional block
that would match a later one contains a "return" statement) chain
them together with "else if" to make that clear.
While we could remove "res" from the conditions below "if (!res)"
they are left alone because, when we start using an enum in the next
commit, it makes it clear that these clauses are handling cases where
there are conflicts.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sequencer: remove unnecessary condition in pick_one_commit()
item->commit holds the commit to be picked and so it must be non-NULL
otherwise pick_one_commit() would not know which commit to pick.
It is also unconditionally dereferenced in do_pick_commit() which is
called at the top of this function. Therefore the check to see if it
is non-NULL is superfluous.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sequencer: simplify handling of fixup with conflicts
Commit e032abd5a0 (rebase: fix rewritten list for failed pick,
2023-09-06) introduced an early return when res == -1, so if
we enter this conditional block then res is positive. After the
last couple of commits the only possible positive value is 1. That
means we can simplify the code by removing the conditional call to
intend_to_amend() and have error_failed_squash() request that it is
called in error_with_patch() instead.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sequencer: remove unnecessary "or" in pick_one_commit()
If error_with_patch(..., res, ...) succeeds then it returns "res", if
it fails then it returns -1. This means that or-ing the return value
with "res" is pointless as the result is the same as the return value.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If "git commit" fails to run then run_git_commit() returns -1 which
causes the current command to be rescheduled. This is incorrect as
we have successfully picked the commit and have written all the state
files we need to successfully commit when the user continues. Fix this
by converting -1 to 1 which matches what do_merge() does.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If an external merge strategy cannot merge (for example because it
would overwrite an untracked file) it exits with a non-zero exit
code other than 1. This should be treated differently from a merge
with conflicts, which is signaled by an exit code of 1, because, as
the merge failed, we need to reschedule the last pick. The caller
expects us to return -1 in this case. Also reschedule without trying
to merge if the commit message cannot be written as that prevents us
from successfully picking the commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t3400: restore coverage for note copying with apply backend
Now that the merge backend is the default, we have lost coverage for
"git rebase --apply" copying notes. Fix this by replacing "-m" with
"--apply" as the previous test which uses the default backend now
checks the merge backend.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/cat-file: filter objects via object database
When batching all objects, git-cat-file(1) reaches into the internals of
the object database and manually manages bitmaps to apply object
filters. This creates coupling between the command and the internals of
the respective backend.
Refactor git-cat-file(1) to use the new object filter option when
batching all objects. This significantly simplifies the logic and
ensures that we don't have to reach into internals of the "files" source
anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
odb: introduce object filters to `odb_for_each_object()`
The function `for_each_bitmapped_object()` can be used to iterate
through all objects covered by a bitmap. The benefit of this function is
that it allows the caller to efficiently handle some object filters. For
example, this can be used to filter out objects of a specific type with
some simple bitmap operations. But callers are currently required to
manually wire up the use of bitmaps though, and to do so they have to
reach into internals of a given object database source.
Introduce a new `struct odb_for_each_object_options::filter` field so
that the interface becomes generic. When set, then a backend may
optionally use the filter to skip some objects that it would have
otherwise yielded.
Note that the respective backends are free to ignore this field if they
cannot meaningfully optimize for a given filter, and consequently
callers need to verify whether they actually want the returned objects.
While annoying, we cannot easily lift this restriction anyway as the
object filter infrastructure supports some filters that cannot be
answered by the object database alone.
An alternative might be to limit the filters to only those that _can_ be
answered by backends. But ultimately, the filters that can be answered
efficiently by the "packed" backend are completely disjunct from those
that can be answered by the "loose" backend, and consequently the set of
filters supported by all backends would be empty. Furthermore, it would
require us to make assumptions about capabilities of future backends,
which may be able to efficiently handle more filters than current ones.
So in the end, this alternative would only limit us artificially.
Implement the logic for the "packed" source. Note that we use the new
function `prepare_bitmap_git_for_source()` to open the bitmap: as the
backend operates on a single object source, we must only use bitmaps
that belong to that specific source. Otherwise we might yield objects
that are not part of the source at all, and with multiple sources we
would enumerate the same bitmap once per source.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-bitmap: introduce function to open bitmap for a single source
The function `prepare_bitmap_git()` opens the first bitmap it can find
in any of the object sources connected to the repository. In a
subsequent commit, the "packed" object database backend will learn to
use bitmaps to answer object filters when enumerating objects. That
backend operates on a single object source though, so using a bitmap
that potentially belongs to a different source would be wrong:
- The source would yield objects that are not part of the source
itself.
- The object source info would be attributed to the wrong source.
- With multiple sources, each source would enumerate the same bitmap
another time.
Introduce a new function `prepare_bitmap_git_for_source()` that only
opens bitmaps belonging to the given object source.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-bitmap: drop `_1` suffix from functions that open bitmaps
In the preceding commit we've refactored how we open bitmaps. As part of
the refactoring we have consolidated `open_pack_bitmap()` as well as
`open_midx_bitmap()` into `open_bitmap_for_source()`. Consequently, we
only have their `open_pack_bitmap_1()` and `open_midx_bitmap_1()`
variants left over, where the `_1` suffix doesn't really make much sense
anymore.
Drop the suffix.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-bitmap: iterate object sources when opening bitmaps
When opening a bitmap for a repository we perform two steps:
- We first look for a multi-pack index bitmap in any of the object
sources connected to the repository.
- We then look for a packfile bitmap in any of the packfiles of any of
the object sources.
Both of these steps thus iterate through object sources themselves, one
via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This
layout makes it hard to introduce a way to open the bitmap of one
specific object source, which is functionality that we'll require in a
subsequent commit.
Reverse the loop so that we instead loop through all sources in the
outer loop, and then for each source we try to load its bitmap via
either the multi-pack index or via a packfile.
Note that this changes the precedence of bitmaps in one specific edge
case: when an earlier object source only has a packfile bitmap, but a
later source has a multi-pack index bitmap, we now pick the packfile
bitmap of the earlier source. Previously, a multi-pack index bitmap from
any source would have taken precedence over all packfile bitmaps. Given
that object sources are ordered such that the local source comes first,
this arguably is an improvement, as we now prefer local bitmaps over
bitmaps in alternates. Furthermore, we already warn about repositories
that have multiple bitmaps, so this setup is broken and thus arguably
not worth worrying about too much.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
pack-bitmap: allow aborting iteration of bitmapped objects
In a subsequent commit we'll lift iteration of bitmapped objects into
the "packed" backend and make it accessible via `odb_for_each_object()`.
The calling convention for that function is that the callback may return
a non-zero exit code, and if so we'll abort iteration. This is currently
impossible to realize though, as `for_each_bitmapped_object()` will
ignore any return value and just churn through all objects completely.
This doesn't matter to the callers of `for_each_bitmapped_object()`, as
there's only one of them in git-cat-file(1), and the callbacks we pass
always return zero. But once we move the logic into the generic
infrastructure it becomes a latent bug waiting to happen.
Refactor the code so that the return value of the `show_reach` callback
is not ignored anymore. Instead, returning a non-zero value will cause
us to abort iteration in both `show_objects_for_type()` and in
`for_each_bitmapped_object()`.
Note though that there's a second user of `show_objects_for_type()` with
`traverse_bitmap_commit_list()`, and that function does indeed invoke
callbacks that may return non-zero. This non-zero return value never had
any effect at all though, and the callbacks that return non-zero values
are only ever invoked via `traverse_bitmap_commit_list()`. Consequently,
we adapt them to always return 0.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King [Wed, 15 Jul 2026 06:22:33 +0000 (08:22 +0200)]
pack-objects: drop unused return value from add_object_entry()
This function returns 0/1 to its caller to tell them whether we actually
added a new entry (or if we considered it redundant). But nobody has
relied on that behavior since 5379a5c5ee (Thin pack generation:
optimization., 2006-04-05).
The extra return does not hurt much, but it is a bit confusing. We have
a sister function, add_object_entry_from_bitmap(), which has the same
return value semantics. That function is about to change to always return
0 (not void, because it must conform to a callback function interface).
So with that change, we'd have two related functions which both return
an "int" but with different semantics.
Let's drop the unused "int" return from add_object_entry() entirely,
which makes it more clear that the two functions have diverged.
Signed-off-by: Jeff King <peff@peff.net>
[ps: slightly massaged the commit message] Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
repository: adjust the comment of config_values_private_
The configurations in 'struct config_values_private_' are not all
parsed in 'git_default_config()'. For example, 'pager_program' is
now parsed in 'pager.c'. Therefore, update the comment.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: move object_creation_mode into repo_config_values
The global variable 'object_creation_mode' controls how Git creates
object files, specifically determining whether to use hardlinks or
renames when moving temporary files into the object database. Move
it into 'struct repo_config_values' to continue the libification
effort.
Move the 'enum object_creation_mode' definition higher up in
'environment.h' to ensure it is visible to the structure. Initialize
the per-repository value to its default macro value
OBJECT_CREATION_MODE inside 'repo_config_values_init()'.
Update configuration parsing in 'git_default_core_config()' to write
directly to the repository-specific configuration structure.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: move autorebase into repo_config_values
The global variable 'autorebase' dictates whether a newly created
branch should be configured to automatically rebase by default.
Move it into 'struct repo_config_values' to continue the
libification effort.
The 'enum rebase_setup_type' definition is moved higher up in
'environment.h' so that it is visible to the repository-specific
structure. The default state AUTOREBASE_NEVER is now correctly
initialized in 'repo_config_values_init()'.
Configuration parsing in 'git_default_branch_config()' is updated to
write directly to the repository's configuration instance.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: move push_default into repo_config_values
The global variable 'push_default' specifies the default behavior of
'git push' when no explicit refspec is provided. Move 'push_default'
into 'struct repo_config_values' to continue the libification effort.
While 'enum push_default_type' ideally belongs in 'remote.h', moving it
there introduces a circular dependency chain:
Therefore, the enum definition is kept in 'environment.h' just above
'struct repo_config_values' with a NEEDSWORK comment for future cleanup.
Modify the configuration parsing in environment.c to update the
per-repository structure directly, and update caller across the
codebase to access the value via 'repo_config_values()'.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
The global variables 'apply_default_whitespace' and
'apply_default_ignorewhitespace' are used to store the default
whitespace configuration for 'git apply'. Move these variables
into 'struct repo_config_values' to continue the libification
effort.
Dynamically allocated strings fetched via 'repo_config_get_string()'
are now tracked per-repository and safely freed in
'repo_config_values_clear()'.
As part of this transition, update 'git_apply_config()' to accept a
'struct repository *' argument rather than relying on the
'the_repository' global.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: move askpass_program into repo_config_values
The global variable 'askpass_program' stores the path to the program
used to prompt the user for credentials. Move it into repo_config_values
to continue the libification effort.
While it is uncommon for a single process to require different askpass
programs for different repositories, maintaining this value as a mutable
global string is a blocker for libification. Global heap-allocated
strings introduce thread-safety issues in a multi-repo environment.
Move 'askpass_program' into 'struct repo_config_values' to eliminate
this global state. The memory is now safely managed and freed via
'repo_config_values_clear()'.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: move pager_program into repo_config_values
The 'pager_program' variable is currently defined as a file-scoped
static string in pager.c. Move it into 'struct repo_config_values'.
The configuration parsing logic remains strictly within pager.c to
respect subsystem boundaries. The read/write operations are simply
redirected to the repository-specific structure using
'repo_config_values()'. All current callers indeed pass
'the_repository', so this new enforcement does not harm them.
Similar to the recent editor_program migration, no standalone getter
is introduced to keep the code minimal. The dynamically allocated
memory is now managed by 'repo_config_values_clear()'.
On top of that, fix memory leaks in pager.c while we are at it.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: move editor_program into repo_config_values
The global variable 'editor_program' holds the path to the user's
preferred editor. Move 'editor_program' into
'struct repo_config_values' to continue the libification effort.
There have been discussions on whether external programs like
editors truly need to be configured on a per-repository basis within
the same process. While a single process might rarely invoke
different editors, this migration is necessary for two reasons:
1. Developers frequently use different toolchains for different
projects. Per-repo configuration respects this.
2. Moving this string into 'repo_config_values' eliminates mutable
global state. As the codebase moves toward becoming a long-running
processes, managing multiple repositories concurrently must
not overwrite each other's program configurations.
No standalone getter function is introduced. Callers directly access
the field via 'repo_config_values()'. Heap memory is safely reclaimed
in 'repo_config_values_clear()'.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
environment: move excludes_file into repo_config_values
The global variable 'excludes_file' is used to track the path to the
global ignore file. If this variable is NULL,
'setup_standard_excludes()' in 'dir.c' forcefully evaluates and assigns
the XDG default path to it.
Continue the libification effort by encapsulating this lazy-loading
fallback logic into a proper getter and moving the variable into
'struct repo_config_values'.
Since 'excludes_file' is a dynamically allocated string, it requires
proper heap memory management. It is safely freed using the newly
introduced 'repo_config_values_clear()' function when the repository
is torn down.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> Signed-off-by: Junio C Hamano <gitster@pobox.com>
As part of the ongoing libification effort, dynamically allocated
global configuration variables are being moved into
'struct repo_config_values'. To prevent memory leaks, we need a
destructor to free these heap-allocated variables when a repository
instance is torn down.
Introduce 'repo_config_values_clear()' in environment.c and invoke it
from 'repo_clear()' in repository.c. As a starting point, update this
new function to handle the cleanup of 'attributes_file'.
Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev> 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>
projects / thirdparty / git.git / log
