René Scharfe [Sat, 6 Dec 2025 13:29:43 +0000 (14:29 +0100)]
banned.h: ban mktemp(3)
Older versions of mktemp(3) generate easily guessable file names. The
function checks if the generated name is used, which is unreliable, as
a file with that name might then be created by some other process before
we can do it ourselves. The function was dropped from POSIX due to its
security problems. Forbid its use.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 6 Dec 2025 13:27:47 +0000 (14:27 +0100)]
compat: use git_mkdtemp()
A file might appear at the path returned by mktemp(3) before we call
mkdir(2). Use the more robust git_mkdtemp() instead, which retries a
number of times and doesn't need to call lstat(2).
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Sat, 6 Dec 2025 13:27:39 +0000 (14:27 +0100)]
wrapper: add git_mkdtemp()
Extend git_mkstemps_mode() to optionally call mkdir(2) instead of
open(2), then use that ability to create a mkdtemp(3) replacement,
git_mkdtemp(). We'll start using it in the next commit.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
gitattributes: disable blank-at-eof errors for clar test expectations
The clar unit testing framework carries a couple of files that contain
expected output for its self-tests. Some of these files expectedly end
with a blank line at the end of the file, which Git would consider to be
a whitespace error by default.
Teach our gitattributes to ignore those errors.
Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/unit-tests: demonstrate use of integer comparison assertions
The clar project has introduced a couple of new assertions that perform
relative integer comparisons, like "greater than" or "less or equal".
Adapt the reftable-record unit tests to demonstrate their usage.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Update clar to commit 39f11fe (Merge pull request #131 from
pks-gitlab/pks-integer-double-evaluation, 2025-12-05). This commit
includes the following changes relevant to Git:
- There are now typesafe integer comparison functions. Furthermore,
the range of comparison functions has been included to also have
relative comparisons, like "greater than".
- There is a new `cl_failf()` macro that allows the caller to specify
an error message with formatting directives.
- The TAP format has been fixed to correctly terminate YAML blocks
with "...\n" instead of "---\n".
Note that we already had a `cl_failf()` function declared in our own
sources. This function is equivalent to the upstreamed function, so we
can simply drop it now.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Fri, 5 Dec 2025 12:23:32 +0000 (21:23 +0900)]
Merge branch 'ps/object-source-management' into ps/odb-misc-fixes
* ps/object-source-management:
odb: handle recreation of quarantine directories
odb: handle changing a repository's commondir
chdir-notify: add function to unregister listeners
odb: handle initialization of sources in `odb_new()`
http-push: stop setting up `the_repository` for each reference
t/helper: stop setting up `the_repository` repeatedly
builtin/index-pack: fix deferred fsck outside repos
oidset: introduce `oidset_equal()`
odb: move logic to disable ref updates into repo
odb: refactor `odb_clear()` to `odb_free()`
odb: adopt logic to close object databases
setup: convert `set_git_dir()` to have file scope
path: move `enter_repo()` into "setup.c"
The error message given by "git config set", when the variable
being updated has more than one values defined, used old style "git
config" syntax with an incorrect option in its hint, both of which
have been corrected.
* rs/config-set-multi-error-message-fix:
config: fix suggestion for failed set of multi-valued option
Junio C Hamano [Fri, 5 Dec 2025 05:49:59 +0000 (14:49 +0900)]
Merge branch 'rs/config-unset-opthelp-fix'
The option help text given by "git config unset -h" described
the "--all" option to "replace", not "unset", multiple variables,
which has been corrected.
* rs/config-unset-opthelp-fix:
config: fix short help of unset flags
Junio C Hamano [Fri, 5 Dec 2025 05:49:58 +0000 (14:49 +0900)]
Merge branch 'ps/object-source-management'
Code refactoring around object database sources.
* ps/object-source-management:
odb: handle recreation of quarantine directories
odb: handle changing a repository's commondir
chdir-notify: add function to unregister listeners
odb: handle initialization of sources in `odb_new()`
http-push: stop setting up `the_repository` for each reference
t/helper: stop setting up `the_repository` repeatedly
builtin/index-pack: fix deferred fsck outside repos
oidset: introduce `oidset_equal()`
odb: move logic to disable ref updates into repo
odb: refactor `odb_clear()` to `odb_free()`
odb: adopt logic to close object databases
setup: convert `set_git_dir()` to have file scope
path: move `enter_repo()` into "setup.c"
Junio C Hamano [Fri, 5 Dec 2025 05:49:56 +0000 (14:49 +0900)]
Merge branch 'jc/optional-path'
"git config get --path" segfaulted on an ":(optional)path" that
does not exist, which has been corrected.
* jc/optional-path:
config: really treat missing optional path as not configured
config: really pretend missing :(optional) value is not there
config: mark otherwise unused function as file-scope static
Junio C Hamano [Fri, 5 Dec 2025 05:49:56 +0000 (14:49 +0900)]
Merge branch 'en/xdiff-cleanup-2'
Code clean-up.
* en/xdiff-cleanup-2:
xdiff: rename rindex -> reference_index
xdiff: change rindex from long to size_t in xdfile_t
xdiff: make xdfile_t.nreff a size_t instead of long
xdiff: make xdfile_t.nrec a size_t instead of long
xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash
xdiff: use unambiguous types in xdl_hash_record()
xdiff: use size_t for xrecord_t.size
xdiff: make xrecord_t.ptr a uint8_t instead of char
xdiff: use ptrdiff_t for dstart/dend
doc: define unambiguous type mappings across C and Rust
repo: add -z as an alias for --format=nul to git-repo-structure
Other Git commands that have nul-terminated output, such as git-config,
git-status, git-ls-files, and git-repo-info have a flag `-z` for using
the null character as the record separator.
Add the `-z` flag to git-repo-structure as an alias for `--format=nul`,
making it consistent with the behavior of the other commands.
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
repo: remove blank line from Documentation/git-repo.adoc
There was an extra blank line in git-repo-structure documentation, which
led to an unwawnted '+' character after generating an HTML or PDF from
that page. This can be seen, for example, in Git 2.52.0 online docs [1].
Remove that extra line.
[1] https://git-scm.com/docs/git-repo/2.52.0
Signed-off-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Toon Claes [Wed, 3 Dec 2025 14:53:31 +0000 (15:53 +0100)]
meson: use is_cross_build() where possible
In previous commit the first use of meson.can_run_host_binaries() was
introduced. This is a guard around compiler.run() to ensure it's
actually possible to execute the provided.
In other places we've been having the same issue, but here `not
meson.is_cross_build()` is used as guard. This does the trick, but it
also prevents the code from running even when an exe_wrapper is
configured.
Switch to using meson.can_run_host_binaries() here as well.
There is another place left that still uses `not
meson.is_cross_build()`, but here it's a guard around fs.exists(). That
function will always run on the build machine, so checking for
cross-compilation is still in place here.
Signed-off-by: Toon Claes <toon@iotcl.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a sparse checkout, a user might want to run `last-modified` on a
directory outside the worktree.
And even in non-sparse checkouts, a user might need to run that command
on a directory that does not exist in the worktree.
These use cases should be supported via the `--` separator between
revision and file arguments, which is even advertised in the
documentation. This patch fixes a tiny bug that prevents that from
working.
This fixes https://github.com/git-for-windows/git/issues/5978
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Acked-by: Derrick Stolee <stolee@gmail.com> Acked-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Julia Evans [Wed, 3 Dec 2025 15:34:55 +0000 (15:34 +0000)]
doc: git-pull: fix 'git --rebase abort' typo
An earlier commit e9d221b0 (doc: git-pull: clarify how to exit a
conflicted merge, 2025-10-15) misspelt `git rebase --abort` to
`git --rebase abort`. Fix it.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Julia Evans [Tue, 2 Dec 2025 18:11:24 +0000 (18:11 +0000)]
doc: remove stray text in Git data model
I meant to delete this sentence fragment when rewriting this paragraph,
but accidentally left it in. It's repetitive (since it was meant to be
deleted) and it's causing some formatting issues with the note.
Signed-off-by: Julia Evans <julia@jvns.ca> Signed-off-by: Junio C Hamano <gitster@pobox.com>
branch: advice using git-help(1) instead of man(1)
8fbd903e (branch: advise about ref syntax rules, 2024-03-05) added
an advice about checking git-check-ref-format(1) for the ref syntax
rules. The advice uses man(1). But git(1) is a multi-platform tool and
man(1) may not be available on some platforms. It might also be slightly
jarring to see a suggestion for running a command which is not from
the Git suite.
Let’s instead use git-help(1) in order to stay inside the land of
git(1). This also means that `help.format` (for `man`, `html` or other
formats) will be used if set.
Also change to using single quotes (') to quote the command since that
is more conventional.
While here let’s also update the test to use `{SQ}`, which is more
readable and easier to edit.
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Mon, 1 Dec 2025 02:31:41 +0000 (18:31 -0800)]
Merge branch 'jk/asan-bonanza'
Various issues detected by Asan have been corrected.
* jk/asan-bonanza:
t: enable ASan's strict_string_checks option
fsck: avoid parse_timestamp() on buffer that isn't NUL-terminated
fsck: remove redundant date timestamp check
fsck: avoid strcspn() in fsck_ident()
fsck: assert newline presence in fsck_ident()
cache-tree: avoid strtol() on non-string buffer
Makefile: turn on NO_MMAP when building with ASan
pack-bitmap: handle name-hash lookups in incremental bitmaps
compat/mmap: mark unused argument in git_munmap()
Junio C Hamano [Mon, 1 Dec 2025 02:31:40 +0000 (18:31 -0800)]
Merge branch 'jc/whitespace-incomplete-line'
Both "git apply" and "git diff" learn a new whitespace error class,
"incomplete-line".
* jc/whitespace-incomplete-line:
attr: enable incomplete-line whitespace error for this project
diff: highlight and error out on incomplete lines
apply: check and fix incomplete lines
whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE
apply: revamp the parsing of incomplete lines
diff: update the way rewrite diff handles incomplete lines
diff: call emit_callback ecbdata everywhere
diff: refactor output of incomplete line
diff: keep track of the type of the last line seen
diff: correct suppress_blank_empty hack
diff: emit_line_ws_markup() if/else style fix
whitespace: correct bit assignment comments
René Scharfe [Sun, 30 Nov 2025 11:47:17 +0000 (12:47 +0100)]
diff-index: don't queue unchanged filepairs with diff_change()
diff_cache() queues unchanged filepairs if the flag find_copies_harder
is set, and uses diff_change() for that. This function allocates a
filespec for each side, does a few other things that are unnecessary for
unchanged filepairs and always sets the diff_flag has_changes, which is
simply misleading in this case.
Add a new streamlined function for queuing unchanged filepairs and
use it in show_modified(), which is called by diff_cache() via
oneway_diff() and do_oneway_diff(). It allocates only a single filespec
for each filepair and uses it twice with reference counting. This has a
measurable effect if there are a lot of them, like in the Linux repo:
Benchmark 1: ./git_v2.52.0 -C ../linux diff --cached --find-copies-harder
Time (mean ± σ): 31.8 ms ± 0.2 ms [User: 24.2 ms, System: 6.3 ms]
Range (min … max): 31.5 ms … 32.3 ms 85 runs
Benchmark 2: ./git -C ../linux diff --cached --find-copies-harder
Time (mean ± σ): 23.9 ms ± 0.2 ms [User: 18.1 ms, System: 4.6 ms]
Range (min … max): 23.5 ms … 24.4 ms 111 runs
Summary
./git -C ../linux diff --cached --find-copies-harder ran
1.33 ± 0.01 times faster than ./git_v2.52.0 -C ../linux diff --cached --find-copies-harder
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Toon Claes [Fri, 28 Nov 2025 16:37:13 +0000 (17:37 +0100)]
last-modified: fix use of uninitialized memory
git-last-modified(1) uses a scratch bitmap to keep track of paths that
have been changed between commits. To avoid reallocating a bitmap on
each call of process_parent(), the scratch bitmap is kept and reused.
Although, it seems an incorrect length is passed to memset(3).
`struct bitmap` uses `eword_t` to for internal storage. This type is
typedef'd to uint64_t. To fully zero the memory used by the bitmap,
multiply the length (saved in `struct bitmap::word_alloc`) by the size
of `eword_t`.
Reported-by: Anders Kaseorg <andersk@mit.edu> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Elijah Newren [Sat, 29 Nov 2025 04:44:24 +0000 (04:44 +0000)]
Documentation/git-replay.adoc: fix errors around revision range
There was significant confusion in the git-replay manual about what
constitutes a revision range. As noted in f302c1e4aa09 (revisions(7):
clarify that most commands take a single revision range, 2021-05-18):
Commands that are specifically designed to take two distinct ranges
(e.g. "git range-diff R1 R2" to compare two ranges) do exist, but they
are exceptions. Unless otherwise noted, all "git" commands that operate
on a set of commits work on a single revision range.
`git replay` is not an exception, but a few places in the manual were
written as though it were. These appear to have come in revisions to
the original series, between v3->v4 (see
https://lore.kernel.org/git/CAP8UFD3bpLrVW97DH7j=V9H2GsTSAkksC9L3QujQERFk_kLnZA@mail.gmail.com/
, "More than one <revision-range> can be passed") and between v6->v7
(https://lore.kernel.org/git/20231115143327.2441397-1-christian.couder@gmail.com/,
"Takes ranges of commits"), and I missed both of these revisions when
reviewing. Fix them now.
There was also a reference to the "Commit Limiting options below", but
this page has no such section of options; strike the misleading
reference.
It is worth noting that we are documenting existing behavior, rather
than optimal behavior. Junio has multiple times suggested introducing
alternative ways to walk revisions and use them in `git replay
--advance`, e.g. at
* https://lore.kernel.org/git/xmqqy1mqo6kv.fsf@gitster.g/
* https://lore.kernel.org/git/xmqq8rb3is8c.fsf@gitster.g/
* https://lore.kernel.org/git/xmqqtsydj2zk.fsf@gitster.g/ (item (2))
If/when we introduce some new revision walking flag that implements one
of these alternate types of revision walks, we can update the --advance
option and this manual appropriately.
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The find_longest_common_sequence() function in patience diff is
inefficient as it calls binary_search() for every unique line it
encounters when deciding where to put it in the sequence. From
instrumentation (using xctrace) on popular repositories, binary_search()
takes up 50-60% of the run time within patience_diff() when performing a
diff.
To optimize this, add a boundary condition check before binary_search()
is called to see if the encountered unique line is located after the
entire currently tracked longest subsequence. If so, skip the
unnecessary binary search and simply append the entry to the end of
sequence. Given that most files compared in a diff are usually quite
similar to each other, this condition is very common, and should be hit
much more frequently than the binary search.
Below are some end-to-end performance results by timing `git log
--shortstat --oneline -500 --patience` on different repositories with
the old and new code. Generally speaking this seems to give at least
8-10% speed up. The "binary search hit %" column describes how often the
algorithm enters the binary search path instead of the new faster path.
Even in the WebKit case we can see that it's quite rare (1.46%).
brian m. carlson [Fri, 28 Nov 2025 01:21:06 +0000 (01:21 +0000)]
t5564: fix test hang under zsh's sh mode
This test starts a SOCKS server in Perl in the background and then kills
it after the tests are done. However, when using zsh (in sh mode) in
the tests, the start_socks function hangs until the background process
is killed.
Note that this does not reproduce in a simple shell script, so there is
likely some interaction between job handling, our heavy use of eval in
the test framework, and possibly other complexities of our test
framework. What is clear, however, is that switching from a compound
statement to a subshell fixes the problem entirely and the test passes
with no problem, so do that.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
brian m. carlson [Fri, 28 Nov 2025 01:21:05 +0000 (01:21 +0000)]
t0614: use numerical comparison with test_line_count
In this comparison, we want to know whether the number of lines is
greater than 1. Our test_line_count function passes the first argument
as the comparison operator to test, so what we want is a numerical
comparison, not a string comparison. While this does not produce a
functional problem now, it could very well if we expected two or more
items, in which case the value "10" would not match when it should.
Furthermore, the "<" and ">" comparisons are new in POSIX 1003.1-2024
and we don't want to require such a new version of POSIX since many
popular and supported operating systems were released before that
version of POSIX was released.
Finally, zsh's builtin test operator does not like the greater-than sign
in "test", since it is only supported in the double-bracket extension.
This has been reported and will be addressed in a future version, but
since our code is also technically incorrect, as well as not very
compatible, let's fix it by using a numeric comparison.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 26 Nov 2025 18:32:42 +0000 (10:32 -0800)]
Merge branch 'pw/worktree-list-display-width-fix'
"git worktree list" attempts to show paths to worktrees while
aligning them, but miscounted display columns for the paths when
non-ASCII characters were involved, which has been corrected.
Junio C Hamano [Wed, 26 Nov 2025 18:32:42 +0000 (10:32 -0800)]
Merge branch 'js/cmake-libgit-fix'
Makefile based build have recently been updated to build a
libgit.a that also has reftable and xdiff objects; CMake based
build procedure has been updated to match.
* js/cmake-libgit-fix:
cmake: stop trying to build the reftable and xdiff libraries
Junio C Hamano [Wed, 26 Nov 2025 18:32:41 +0000 (10:32 -0800)]
Merge branch 'js/mingw-assign-comma-fix'
The "return errno = EFOO, -1" construct, which is heavily used in
compat/mingw.c and triggers warnings under "-Wcomma", has been
rewritten to avoid the warnings.
* js/mingw-assign-comma-fix:
mingw: avoid the comma operator
ci(dockerized): do show the result of failing tests again
The quality of tests and test suites is most apparent not when
everything passes, but in how quickly bugs can be identified,
analyzed, and resolved after test failures occur.
As such, it is an unfortunate side effect of 2a21098b98a (github: adapt
containerized jobs to be rootless, 2025-01-10) that the output of failed
test cases, which was shown before that change directly in the build
logs, is now no longer shown at all.
The reason is a side effect of trying to run the build and the tests
with permissions other than the `root` user, but without providing the
prerequisite permissions to signal what tests failed and whose output
hence needs to be included in the logs.
The way this signaling works is for the workflow to write into
special-purpose files whose path is specific to the current workflow
step and which can be accessed via the `$GITHUB_ENV` environment
variable, which differs between workflow steps. It is this file that is
missing write permission for the `builder` user that was introduced in
above-mentioned commit.
The solution is simple: make the file world-writable.
Technically, this write permission should be removed after the step has
completed, if proper security practices were to be upheld, but since
nothing uses that file again, it does not matter, and the fix is more
succinct this way.
This commit is best viewed with `--color-words`.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
[jc: squashed Elijah's rewrite of the first paragraph of the log message]
[jc: updated chmod to match "world-writable" in the log message] Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Wed, 26 Nov 2025 17:35:09 +0000 (09:35 -0800)]
Merge branch 'master' of https://github.com/j6t/gitk
* 'master' of https://github.com/j6t/gitk:
gitk: add external diff file rename detection
gitk: show unescaped file names on 'rename' and 'copy' lines
gitk: fix a 'continue' statement outside a loop to 'return'
gitk: persist position and size of the Tags and Heads window
Revert "gitk: Only restore window size from ~/.gitk, not position"
Phillip Wood [Wed, 26 Nov 2025 14:33:37 +0000 (14:33 +0000)]
replay: do not copy "gpgsign-sha256" header
When "git replay" replays a commit it copies the extended headers
across from the original commit. However, if the original commit
was signed, we do not want to copy the header associated with the
signature is it wont be valid for the new commit. The code already
knows to avoid coping the "gpgsig" header but does not know to avoid
copying the "gpgsig-sha256" header. Add that header to the list of
exclusions to match what "git commit --amend" does.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Christian Couder [Mon, 17 Nov 2025 04:34:50 +0000 (05:34 +0100)]
fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>
Tools like `git filter-repo`[1] use `git fast-export` and
`git fast-import` to rewrite repository history. When rewriting
history using one such tool though, commit signatures might become
invalid because the commits they sign changed due to the changes
in the repository history made by the tool between the fast-export
and the fast-import steps.
Note that as far as signature handling goes:
* Since fast-export doesn't know what changes filter-repo may make
to the stream, it can't know whether the signatures will still be
valid.
* Since filter-repo doesn't know what history canonicalizations
fast-export performed (and it performs a few), it can't know whether
the signatures will still be valid.
* Therefore, fast-import is the only process in the pipeline that
can know whether a specified signature remains valid.
Having invalid signatures in a rewritten repository could be
confusing, so users rewritting history might prefer to simply
discard signatures that are invalid at the fast-import step.
For example a common use case is to rewrite only "recent" history.
While specifying commit ranges corresponding to "recent" commits
could work, users worry about getting it wrong and want to just
automatically rewrite everything, expecting older commit signatures
to be untouched.
To let them do that, let's add a new 'strip-if-invalid' mode to the
`--signed-commits=<mode>` option of `git fast-import`.
It would be interesting for the `--signed-tags=<mode>` option to
have this mode too, but we leave that for a future improvement.
It might also be possible for `git fast-export` to have such a mode
in its `--signed-commits=<mode>` and `--signed-tags=<mode>`
options, but the use cases for it are much less clear, so we also
leave that for possible future improvements.
For now let's just die() if 'strip-if-invalid' is passed to these
options where it hasn't been implemented yet.
[1]: https://github.com/newren/git-filter-repo
Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Sixt [Wed, 26 Nov 2025 15:02:23 +0000 (16:02 +0100)]
Merge branch 'js/persist-ref-window-geometry'
* js/persist-ref-window-geometry:
gitk: persist position and size of the Tags and Heads window
Revert "gitk: Only restore window size from ~/.gitk, not position"
In the preceding commit we have moved the logic that reparents object
database sources on chdir(3p) from "setup.c" into "odb.c". Let's also do
the same for any temporary quarantine directories so that the complete
reparenting logic is self-contained in "odb.c".
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `repo_set_gitdir()` is called in two situations:
- To initialize the repository with its discovered location. As part
of this we also set up the new object database.
- To update the repository's discovered location in case the process
changes its working directory so that we update relative paths. This
means we also have to update any relative paths that are potentially
used in the object database.
In the context of the object database we ideally wouldn't ever have to
worry about the second case: if all paths used by our object database
sources were absolute, then we wouldn't have to update them. But
unfortunately, the paths aren't only used to locate files owned by the
given source, but we also use them for reporting purposes. One such
example is `repo_get_object_directory()`, where we cannot just change
semantics to always return absolute paths, as that is likely to break
tooling out there.
One solution to this would be to have both a "display path" and an
"internal path". This would allow us to use internal paths for all
internal matters, but continue to use the potentially-relative display
paths so that we don't break compatibility. But converting the codebase
to honor this split is quite a messy endeavour, and it wouldn't even
help us with the goal to get rid of the need to update the display path
on chdir(3p).
Another solution would be to rework "setup.c" so that we never have to
update paths in the first place. In that case, we'd only initialize the
repository once we have figured out final locations for all directories.
This would be a significant simplification of that subsystem indeed, but
the current logic is so messy that it would take significant investments
to get there.
Meanwhile though, while object sources may still use relative paths, the
best thing we can do is to handle the reparenting of the object source
paths in the object database itself. This can be done by registering one
callback for each object database so that we get notified whenever the
current working directory changes, and we then perform the reparenting
ourselves.
Ideally, this wouldn't even happen on the object database level, but
instead handled by each object database source. But we don't yet have
proper pluggable object database sources, so this will need to be
handled at a later point in time.
The logic itself is rather simple:
- We register the callback when creating the object database.
- We unregister the callback when releasing it again.
- We split up `set_git_dir_1()` so that it becomes possible to skip
recreating the object database. This is required because the
function is called both when the current working directory changes,
but also when we set up the repository. Calling this function
without skipping creation of the ODB will result in a bug in case
it's already created.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
chdir-notify: add function to unregister listeners
While we (obviously) have a way to register new listeners that get
called whenever we chdir(3p), we don't have an equivalent that can be
used to unregister such a listener again.
Add one, as it will be required in a subsequent commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
odb: handle initialization of sources in `odb_new()`
The logic to set up a new object database is currently distributed
across two functions in "repository.c":
- In `initialize_repository()` we initialize an empty object database.
This object database is not fully initialized and doesn't have any
sources attached to it.
- The primary object database source is then created in
`repo_set_gitdir()`.
Ideally though, the logic should be entirely self-contained so that we
can iterate more readily on how exactly the sources themselves get set
up.
Refactor `odb_new()` to handle both allocation and setup of the object
database. This ensures that the object database is always initialized
and ready for use, and it allows us to change how the sources get set up
eventually.
Note that `repo_set_gitdir()` still reaches into the sources when the
function gets called with an already-initialized object database. This
will be fixed in the next commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
http-push: stop setting up `the_repository` for each reference
When pushing references via HTTP we call `repo_init_revisions()` in a
loop for each reference that we're about to push. As third argument we
pass the result of `setup_git_directory()`, which causes us to
reinitialize the repository every single time.
This is an obvious waste of compute, as the repository that we're
working in will never change across any of the initializations. The only
reason that we do this is to retrieve the directory of the repository.
Furthermore, this is about to create issues in a subsequent commit,
where reinitializing the repository will cause a `BUG()`.
Address this by storing the Git directory in a variable instead so that
we don't have to call the function repeatedly.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/helper: stop setting up `the_repository` repeatedly
The "repository" test helper sets up `the_repository` twice. In fact
though, we don't even have to set it up even once: all we need is to set
up its hash algorithm, because we still depend on some subsystems that
aren't free of `the_repository`.
Refactor the code accordingly. This prepares for a subsequent change,
where setting up the repository repeatedly will lead to a `BUG()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When asked to perform object consistency checks via the `--fsck-objects`
flag we verify that each object part of the pack is valid. In general,
this check can even be performed outside of a Git repository: we don't
need an initialized object database as we simply read the object from
the packfile directly.
But there's one exception: a subset of the object checks may be deferred
to a later point in time. For now, this only concerns ".gitmodules" and
".gitattributes" files: whenever we see a tree referencing these files
we queue them for a deferred check. This is done because we need to do
some extra checks for those files to ensure that they are well-formed,
and these checks need to be done regardless of whether the corresponding
blobs are part of the packfile or not.
This works inside a repository, but unfortunately the logic leads to a
segfault when running outside of one. This is because we eventually call
`odb_read_object()`, which will crash because the object database has
not been initialized.
There's multiple options here:
- We could in theory create a purely in-memory database with only a
packfile store that contains the single packfile. We don't really
have the infrastructure for this yet though, and it would end up
being quite hacky.
- We could refuse to perform consistency checks outside of a
repository. But most of the checks work alright, so this would be a
regression.
- We can skip the finalizing consistency checks when running outside
of a repository. This is not as invasive as skipping all checks,
but it's not great to randomly skip a subset of tests, either.
None of these options really feel perfect. The first one would be the
obvious choice if easily possible.
There's another option though: instead of skipping the final object
checks, we can die if there are any queued object checks. With this
change we now die exactly if and only if we would have previously
segfaulted. Like this we ensure that objects that _may_ fail the
consistency checks won't be silently skipped, and at the same time we
give users a much better error message.
Refactor the code accordingly and add a test that would have triggered
the segfault. Note that we also move down the logic to add the packfile
to the store. There is no point doing this any earlier than right before
we execute `fsck_finish()`, and it ensures that the logic to set up and
perform the consistency check is self-contained.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our object database sources have a field `disable_ref_updates`. This
field can obviously be set to disable reference updates, but it is
somewhat curious that this logic is hosted by the object database.
The reason for this is that it was primarily added to keep us from
accidentally updating references while an ODB transaction is ongoing.
Any objects part of the transaction have not yet been committed to disk,
so new references that point to them might get corrupted in case we
never end up committing the transaction. As such, whenever we create a
new transaction we set up a new temporary ODB source and mark it as
disabling reference updates.
This has one (and only one?) upside: once we have committed the
transaction, the temporary source will be dropped and thus we clean up
the disabled reference updates automatically. But other than that, it's
somewhat misdesigned:
- We can have multiple ODB sources, but only the currently active
source inhibits reference updates.
- We're mixing concerns of the refdb with the ODB.
Arguably, the decision of whether we can update references or not should
be handled by the refdb. But that wouldn't be a great fit either, as
there can be one refdb per worktree. So we'd again have the same problem
that a "global" intent becomes localized to a specific instance.
Instead, move the setting into the repository. While at it, convert it
into a boolean.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano [Sun, 16 Nov 2025 07:02:57 +0000 (23:02 -0800)]
submodule add: sanity check existing .gitmodules
"git submodule add" tries to find if a submodule with the same name
already exists at a different path, by looking up an entry in the
.gitmodules file. If the entry in the file is incomplete, e.g.,
when the submodule.<name>.something variable is defined but there is
no definition of submodule.<name>.path variable, it accesses the
missing .path member of the submodule structure and triggers a
segfault.
A brief audit was done to make sure that the code does not assume
members other than those that are absolutely certain to exist: a
submodule obtained by submodule_from_name() should have .name
member, while a submodule obtained by submodule_from_path() should
also have .path as well as .name member, and we cannot assume
anything else. Luckily, the module_add() codepath was the only
problematic one. It is fairly recent code that comes from 1fa06ced
(submodule: prevent overwriting .gitmodules on path reuse,
2025-07-24).
A helper used by update_submodule() seems to assume that its call to
submodule_from_path() always yields a submodule object without a
failure, which seems to rely on the caller making sure it is the
case. Leave an assert() with a NEEDSWORK comment there for future
developers to make sure the assumption actually holds.
Junio C Hamano [Thu, 20 Nov 2025 19:45:35 +0000 (11:45 -0800)]
config: really treat missing optional path as not configured
These callers expect that git_config_pathname() that returns 0 is a
signal that the variable they passed has a string they need to act
on. But with the introduction of ":(optional)path" earlier, that is
no longer the case. If the path specified by the configuration
variable is missing, their variable will get a NULL in it, and they
need to act on it (often, just refraining from copying it elsewhere).
Junio C Hamano [Thu, 20 Nov 2025 19:34:50 +0000 (11:34 -0800)]
config: really pretend missing :(optional) value is not there
Earlier we added support for a value spelled as ":(optional)path"
for configuration variables whose values are of type "path", with
the documented semantics "if the path is missing, behave as if such
a variable definition is not even there."
This has worked OK for code paths that reads configuration files and
stores the configured value as a string, where NULL in such a string
is treated as if the setting is not there, left as the default.
However, there are other code paths that do not _ignore_ such NULL
values and misbehave. "git config get --path" is one of them.
When git_config_pathname() helper function finds that the value of
the variable is an optional path *and* the path is missing, it
leaves the destination pointer intact (which usually is left to
NULL) and returns 0 to signal a success. format_config() helper
however assumed that the destination pointer always gets a string,
which no longer is the case, and segfaulted.
Make sure that git_config_pathname() clears the destination pointer
in such a case, and teach format_config() to react to the condition
by returning 1 (which is different from 0 that is a normal success
and negative that is an error) to its callers. Adjust the callers
to react to this new return value that tells them to pretend as if
they did not even see this partcular <key, value> pair.
Reported-by: Han Jiang <jhcarl0814@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
An earlier check added to osx keychain credential helper to avoid
storing the credential itself supplied was overeager and rejected
credential material supplied by other helper backends that it would
have wanted to store, which has been corrected.
* kn/osxkeychain-idempotent-store-fix:
osxkeychain: avoid incorrectly skipping store operation
Junio C Hamano [Mon, 24 Nov 2025 23:46:40 +0000 (15:46 -0800)]
Merge branch 'sa/replay-atomic-ref-updates'
"git replay" (experimental) learned to perform ref updates itself
in a transaction by default, instead of emitting where each refs
should point at and leaving the actual update to another command.
* sa/replay-atomic-ref-updates:
replay: add replay.refAction config option
replay: make atomic ref updates the default behavior
replay: use die_for_incompatible_opt2() for option validation
René Scharfe [Mon, 24 Nov 2025 21:00:05 +0000 (22:00 +0100)]
config: fix short help of unset flags
The flags --all and --value of "git config unset" don't make the command
"replace" or "show" anything, they are about selecting what to unset.
Change their help text accordingly.
Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe [Mon, 24 Nov 2025 20:33:24 +0000 (21:33 +0100)]
config: fix suggestion for failed set of multi-valued option
The command "git config set <name> <value>" fails for an option that has
multiple values. List the "git config set" flags that can be used,
instead of old-style "git config" actions.
Reported-by: Paul Wintz <pwintz@ucsc.edu> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the preceding commits we have turned `struct odb_read_stream` into a
publicly visible structure. Furthermore, this structure now contains the
type and size of the object that we are about to stream. Consequently,
the out-pointers that we used before to propagate the type and size of
the streamed object are now somewhat redundant with the data contained
in the structure itself.
Drop these out-pointers and adapt callers accordingly.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "streaming" terminology is somewhat generic, so it may not be
immediately obvious that "streaming.{c,h}" is specific to the object
database. Rectify this by moving it into the "odb/" directory so that it
can be immediately attributed to the object subsystem.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: make the `odb_read_stream` definition public
Subsequent commits will move the backend-specific logic of setting up an
object read stream into the specific subsystems. As the backends are now
the ones that are responsible for allocating the stream they'll need to
have the stream definition available to them.
Make the stream definition public to prepare for this.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Subsequent commits will move the backend-specific logic of object
streaming into their respective subsystems. These subsystems have gotten
rid of `the_repository` already, but we still use it in two locations in
the streaming subsystem.
Prepare for the move by fixing those two cases. Converting the logic in
`open_istream_pack_non_delta()` is trivial as we already got the object
database as input.
But for `stream_blob_to_fd()` we have to add a new parameter to make it
accessible. So, as we already have to adjust all callers anyway, rename
the function to `odb_stream_blob_to_fd()` to indicate it's part of the
object subsystem.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: rely on object sources to create object stream
When creating an object stream we first look up the object info and, if
it's present, we call into the respective backend that contains the
object to create a new stream for it.
This has the consequence that, for loose object source, we basically
iterate through the object sources twice: we first discover that the
file exists as a loose object in the first place by iterating through
all sources. And, once we have discovered it, we again walk through all
sources to try and map the object. The same issue will eventually also
surface once the packfile store becomes per-object-source.
Furthermore, it feels rather pointless to first look up the object only
to then try and read it.
Refactor the logic to be centered around sources instead. Instead of
first reading the object, we immediately ask the source to create the
object stream for us. If the object exists we get stream, otherwise
we'll try the next source.
Like this we only have to iterate through sources once. But even more
importantly, this change also helps us to make the whole logic
pluggable. The object read stream subsystem does not need to be aware of
the different source backends anymore, but eventually it'll only have to
call the source's callback function.
Note that at the current point in time we aren't fully there yet:
- The packfile store still sits on the object database level and is
thus agnostic of the sources.
- We still have to call into both the packfile store and the loose
object source.
But both of these issues will soon be addressed.
This refactoring results in a slight change to semantics: previously, it
was `odb_read_object_info_extended()` that picked the source for us, and
it would have favored packed (non-deltified) objects over loose objects.
And while we still favor packed over loose objects for a single source
with the new logic, we'll now favor a loose object from an earlier
source over a packed object from a later source.
Ultimately this shouldn't matter though: the stream doesn't indicate to
the caller which source it is from and whether it was created from a
packed or loose object, so such details are opaque to the caller. And
other than that we should be able to assume that two objects with the
same object ID should refer to the same content, so the streamed data
would be the same, too.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
packfile: introduce function to read object info from a store
Extract the logic to read object info for a packed object from
`do_oid_object_into_extended()` into a standalone function that operates
on the packfile store. This function will be used in a subsequent
commit.
Note that this change allows us to make `find_pack_entry()` an internal
implementation detail. As a consequence though we have to move around
`packfile_store_freshen_object()` so that it is defined after that
function.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: create structure for filtered object streams
As explained in a preceding commit, we want to get rid of the union of
stream-type specific data in `struct odb_read_stream`. Create a new
structure for filtered object streams to move towards this design.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: create structure for packed object streams
As explained in a preceding commit, we want to get rid of the union of
stream-type specific data in `struct odb_read_stream`. Create a new
structure for packed object streams to move towards this design.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: create structure for loose object streams
As explained in a preceding commit, we want to get rid of the union of
stream-type specific data in `struct odb_read_stream`. Create a new
structure for loose object streams to move towards this design.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: create structure for in-core object streams
As explained in a preceding commit, we want to get rid of the union of
stream-type specific data in `struct odb_read_stream`. Create a new
structure for in-core object streams to move towards this design.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: allocate stream inside the backend-specific logic
When creating a new stream we first allocate it and then call into
backend-specific logic to populate the stream. This design requires that
the stream itself contains a `union` with backend-specific members that
then ultimately get populated by the backend-specific logic.
This works, but it's awkward in the context of pluggable object
databases. Each backend will need its own member in that union, and as
the structure itself is completely opaque (it's only defined in
"streaming.c") it also has the consequence that we must have the logic
that is specific to backends in "streaming.c".
Ideally though, the infrastructure would be reversed: we have a generic
`struct odb_read_stream` and some helper functions in "streaming.c",
whereas the backend-specific logic sits in the backend's subsystem
itself.
This can be realized by using a design that is similar to how we handle
reference databases: instead of having a union of members, we instead
have backend-specific structures with a `struct odb_read_stream base`
as its first member. The backends would thus hand out the pointer to the
base, but internally they know to cast back to the backend-specific
type.
This means though that we need to allocate different structures
depending on the backend. To prepare for this, move allocation of the
structure into the backend-specific functions that open a new stream.
Subsequent commits will then create those new backend-specific structs.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: explicitly pass packfile info when streaming a packed object
When streaming a packed object we first populate the stream with
information about the pack that contains the object before calling
`open_istream_pack_non_delta()`. This is done because we have already
looked up both the pack and the object's offset, so it would be a waste
of time to look up this information again.
But the way this is done makes for a somewhat awkward calling interface,
as the caller now needs to be aware of how exactly the function itself
behaves.
Refactor the code so that we instead explicitly pass the packfile info
into `open_istream_pack_non_delta()`. This makes the calling convention
explicit, but more importantly this allows us to refactor the function
so that it becomes its responsibility to allocate the stream itself in a
subsequent patch.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
streaming: propagate final object type via the stream
When opening the read stream for a specific object the caller is also
expected to pass in a pointer to the object type. This type is passed
down via multiple levels and will eventually be populated with the type
of the looked-up object.
The way we propagate down the pointer though is somewhat non-obvious.
While `istream_source()` still expects the pointer and looks it up via
`odb_read_object_info_extended()`, we also pass it down even further
into the format-specific callbacks that perform another lookup. This is
quite confusing overall.
Refactor the code so that the responsibility to populate the object type
rests solely with the format-specific callbacks. This will allow us to
drop the call to `odb_read_object_info_extended()` in `istream_source()`
entirely in a subsequent patch.
Furthermore, instead of propagating the type via an in-pointer, we now
propagate the type via a new field in the object stream. It already has
a `size` field, so it's only natural to have a second field that
contains the object type.
Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
projects / thirdparty / git.git / log
