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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
Michael Montalbo [Wed, 15 Jul 2026 19:29:52 +0000 (19:29 +0000)]
revision: make get_commit_action() a pure predicate
get_commit_action() reads as a predicate that decides whether a commit
is shown or ignored, but for a line-level log without parent rewriting
it also calls line_log_process_ranges_arbitrary_commit(), which
mutates the tracked line ranges. That hidden side effect makes it unsafe
to evaluate ahead of the walk, the way a lookahead would.
get_commit_action() was split out of simplify_commit() in beb5af43a6
(graph API: fix bug in graph_is_interesting(), 2009-08-18) as the
show/ignore decision minus the parent rewriting, so the graph renderer
could reuse it; line-level log later routed its filtering through it as
well, in 3cb9d2b6 (line-log: more responsive, incremental 'git log -L',
2020-05-11). Besides simplify_commit(), the walk driver,
graph_is_interesting() is its only other caller, and it runs only under
--graph, which sets rewrite_parents and therefore want_ancestry(); the
"-L without ancestry" branch that holds the side effect never fires
there, so it is dormant today.
The line-level processing folds a commit's tracked ranges onto its
parents, which must happen even for a commit that get_commit_action()
filters from the output, or the ranges never reach the parents. Move it
to simplify_commit() and run it before get_commit_action(), gated by
get_commit_action()'s leading checks (already shown, uninteresting, and
the like) so a commit ignored by those is not folded, as before; factor
those checks out as commit_early_ignore(). get_commit_action() is then
side-effect free.
commit_early_ignore() runs twice on the -L path, once for that gate and
once inside get_commit_action(), but it reads only object flags and pack
membership, disjoint from the TREESAME flag the fold sets, so the repeat
is harmless.
Add a "line-log-peek" subcommand to the revision-walking test helper
that evaluates get_commit_action() on a commit the walk has not reached
yet, plus a t4211 check that the call leaves the commit's flags
unchanged. The flags are compared rather than the commit list because
add_line_range() merges ranges by union, which is idempotent, so the
side effect never changed which commits a linear -L history shows.
Suggested-by: Junio C Hamano <redacted> Signed-off-by: Michael Montalbo <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Tested-by: Uwe Kleine-König <redacted> Signed-off-by: Phillip Wood <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
Jeff King [Wed, 15 Jul 2026 06:05:23 +0000 (02:05 -0400)]
diff: ignore unmerged paths outside prefix with --relative --cached
A diff using --relative ignores entries outside the current directory.
This results in a segfault when we try to process an unmerged entry
that's outside of our prefix, since we end up with a NULL diff_filepair
and use it without checking that it's valid.
I think this bug goes back to 76399c0195 (diff.c: return filepair from
diff_unmerge(), 2011-04-22). Prior to that, diff_unmerge() knew to skip
entries outside of our prefix, due to cd676a5136 (diff --relative:
output paths as relative to the current subdirectory, 2008-02-12). Back
then the caller didn't care that we hadn't added anything to the queue.
In 76399c0195 that changed; we now returned the pair (or NULL), and the
caller in do_oneway_diff() was then called fill_filespec() itself. And
it does so without checking for NULL, causing a segfault.
The obvious fix is to skip the fill_filespec() call (after which we just
return), which this patch does.
There's another call to diff_unmerge() in run_diff_files(). That case
was already fixed by 8174627b3d (diff-lib: ignore paths that are outside
$cwd if --relative asked, 2021-08-22), but of course it didn't help us
for --cached.
That commit also claims that checking the result of diff_unmerge() is
not enough, as we'd want other code paths to skip the entry, too (even
if they wouldn't segfault). But as far as I can tell, that is not true
for --cached. We eventually end up in diff_queue_addremove() or in
diff_queue_change(), both of which know to return early when we're
outside of the prefix.
Arguably we could be checking at the top of oneway_diff() whether the
path is interesting at all. That would not only avoid this code path
entirely, but would also possibly save a small amount of work. But since
everything else appears to work OK, I went for the smallest fix here to
avoid any regression.
Specifically, a comment in oneway_diff() claims we're supposed to
advance o->pos, which we might fail to do if we return early. Though
that "advance" seems to have gone away in da165f470e (unpack-trees.c:
prepare for looking ahead in the index, 2010-01-07), so it is possible
the comment is simply out of date. We can explore that separately;
checking for a NULL return from diff_unmerge() seems like a sensible
thing to do regardless.
We can piggy-back on the tests added by 8174627b3d; we're just checking
the --cached variant.
Signed-off-by: Jeff King <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted>
[ps: slightly massaged the commit message] Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Pablo Sabater <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Chandra Pratap <redacted> Signed-off-by: Pablo Sabater <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Mentored-by: Ayush Chandekar <redacted> Mentored-by: Olamide Caleb Bello <redacted> Signed-off-by: Tian Yuchen <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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
Move `odb_optimize()` and `odb_optimize_required()` from "builtin/gc.c"
into the "files" source and wire them up via newly introduced vtable
pointers for the object database sources. This makes the logic pluggable
and thus allows other backends to have their own, custom implementation.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
builtin/gc: fix signedness issues in ODB-related functionality
There are a couple of signedness issues in ODB-related functionality.
These are not a problem because we disable -Wsign-compare in this file,
but once we move these functions into "odb/source-files.c" they will
result in warnings.
Fix those issues:
- In `too_many_loose_objects()` we receive a signed limit, but compare
it with the unsigned actual number of loose objects. This is fixed
by bailing out immediately when the limit is smaller than or equal
to zero, which we also do similarly in other places. The warning is
then squelched via a cast.
- In `find_base_packs()` we compare the signed size of the pack
against the unsigned limit. As the pack size is always going to be a
positive file size it's safe to cast it to an unsigned value.
- In `odb_optimize()` we compare the unsigned `keep_pack.nr` value
against the signed `gc_auto_pack_limit`. We only reach this code
when `too_many_packs()` returns true-ish, and that can only happen
when `gc_auto_pack_limit > 0`. Consequently, we can fix the warning
by casting the limit to an unsigned value.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
builtin/gc: refactor ODB optimizations to operate on "files" source
We have a couple of functions that are implementation details of how the
"files" object database source performs optimizations. These functions
often use global state like `the_repository` and implicitly derive the
source they are supposed to optimize.
Refactor these interfaces to accept a "files" source directly. This will
make it easier to move around the whole logic into "odb/source-files.c"
in a subsequent step.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
When invoking either git-gc(1) or git-maintenance(1) with the "--auto"
flag then we only perform those maintenance tasks that are actually
required. This logic is inherently an implementation detail of the
object database backend that's in use. But the logic is scattered around
multiple different functions, which makes it hard to make the logic
pluggable.
Introduce a new `odb_optimize_required()` function that allows us to
check these conditions in a generic way.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
builtin/gc: move geometric repacking into `odb_optimize()`
We have two major object database optimization strategies:
- The legacy strategy used by git-gc(1), which absorbs loose objects
into packfiles, and eventually merges all packfiles once we have too
many of them.
- The more recent "geometric" strategy used by git-maintenance(1),
which merges packfiles using a geometric sequence.
These two strategies are still using completely separate code paths. In
a subsequent commit we'll want to make both strategies pluggable though.
Prepare for this change by merging the "geometric" strategy into
`odb_optimize()`. This also allows us to reuse some of the logic we have
in that function.
Note that this change requires us to adapt tests because we're now using
"-q" instead of "--quiet". Naturally though, these invocations are of
course equivalent to one another.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
Introduce `struct odb_optimize_options` to decouple the options that are
specific to optimizing the object database from `struct gc_config`. This
structure will be moved into the object database layer in a subsequent
commit.
Note that there are a small set of backend-specific options in this
structure. In an ideal world those of course wouldn't exist, but as
we're introducing the object database abstractions retroactively we are
somewhat forced to keep them.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
builtin/gc: inline config values specific to the "files" backend
The `struct gc_config` contains a set of values that we read via the Git
repository's configuration. Several of those values that are consumed by
the object database optimization logic are inherently specific to the
"files" config.
In a later commit we'll make the logic to optimize object databases
pluggable. So by carrying these "files"-backend specific values in the
generic config struct means that other backends would have to worry
about these values, too. This feels somewhat dirty, as implementation-
specific details should live with the backends themselves.
Inline these values directly at the call sites that need them instead.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
When optimizing the object database most of the heavy-lifting is done by
git-repack(1). The arguments we pass to this function are assembled in
global scope, which is hard to follow.
Refactor the logic by moving the vector into `maintenance_task_odb()`.
While that means we have to pass more arguments to this function, it has
the upside that the logic becomes self-contained without any kind of
global interdependencies.
This is a pure refactoring with no intended functional change.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
builtin/gc: extract object database optimizations into separate function
Extract the object database optimization logic from `cmd_gc()` into a
new `maintenance_task_odb()` helper function. This is a pure refactoring
with no intended functional change.
Note that the message that notifies the user about too many loose
objects is moved into the new function, as well. It is inherently an
implementation detail of how the "files" source works, and as a
consequence we'll move it around in a later commit, as well. This
reordering means that the warning may now be printed at a different
point in time, but it's not expected that this will have any practical
implications.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
builtin/gc: move worktree and rerere tasks before object optimizations
In subsequent patches we'll consolidate all tasks that relate to
maintenance of the object database and move it into the "files" backend.
The relevant code is somewhat scattered though, as several other tasks
are interspersed between.
Refactor the code so that all object database optimizations are grouped
together, which requires us to move worktree pruning and rerere garbage
collection around. In theory, rearranging this code can have an effect
on the object database optimizations:
- Rerere entries really shouldn't impact garbage collection at all, as
these entries are not stored in the object database.
- The index and HEAD reference of pruned worktrees may reference
objects that become unreachable.
That being said, the impact should be overall rather negligible. If the
user was asking us to prune objects with immediate expiration time then
we might now prune objects that were previously still kept alive by the
worktree. But besides being a very specific edge case, it's arguably not
even the wrong thing to also prune any potentially-unreachable objects
immediately.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
odb: run "pre-auto-gc" hook for all maintenance tasks
The "pre-auto-gc" hook is supposed to run before auto-maintenance
starts. The intent of this is to give users the ability to intercept
running maintenance in case there's for example an event that is not
supposed to run in parallel with repository maintenance.
This hook runs via `need_to_gc()`, which is invoked via two paths:
- It is called directly by git-gc(1).
- It is called indirectly by git-maintenance(1) via the "gc" task.
While the former makes sense, the latter is somewhat off. While the hook
is indeed strongly tied to gc'ing a repository, the original intent of
the hook is rather to inhibit any kind of automated garbage collection.
That noticeably also includes all the other maintenance tasks that our
new infrastructure may run, but those aren't getting intercepted at all.
The move towards our new maintenance strategy has thus somewhat neutered
the effectiveness of the hook.
Fix this issue by running the hook before the first auto-maintenance
task that would run as determined by the tasks's auto condition. Note
that this requires us to lift the call to `run_hooks()` out of
`needs_to_gc()`, as the hook would otherwise potentially run multiple
times.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
t7900: simplify how we check for maintenance tasks
We have several tests in t7900 that verify whether specific maintenance
tasks did or did not run. This is done rather ad-hoc by checking for
spawned Git commands, which is awfully fragile:
- We have to adjust tests whenever arguments to the spawned Git
commands change.
- We don't have a way to verify that negative matches are still
working as expected.
- We rely on maintenance tasks spawning a Git command in the first
place.
We can do much better though, as we already have trace2 regions for each
of the maintenance tasks. Introduce a helper function that extracts all
such regions so that we can get a direct list of all maintenance tasks
that a certain command ran.
Adapt tests that care about whether or not a specific task ran to use
this new helper. Note that many tests still use `test_subcommand`
though, as they really care about the exact command that was executed.
Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Vinicius Lira de Freitas <redacted> Signed-off-by: Marcelo Machado Lage <redacted> Signed-off-by: Junio C Hamano <redacted>
Rewrite single-line && chains by breaking them into multiple lines.
Co-authored-by: Vinicius Lira de Freitas <redacted> Signed-off-by: Vinicius Lira de Freitas <redacted> Signed-off-by: Marcelo Machado Lage <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Helped-by: Jeff King <redacted> Signed-off-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Acked-by: Patrick Steinhardt <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
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 <redacted> Signed-off-by: Junio C Hamano <redacted>
git.99rst.org / git.git / log