When fetching references into a repository, if a lock for a particular
reference exists, then `lock_raw_ref()` throws:
- REF_TRANSACTION_ERROR_CASE_CONFLICT: when there is a conflict
because the transaction contains conflicting references while being
on a case-insensitive filesystem.
- REF_TRANSACTION_ERROR_GENERIC: for all other errors.
The latter causes the entire set of batched updates to fail, even in
case sensitive filessystems.
Instead, return a 'REF_TRANSACTION_ERROR_CREATE_EXISTS' error. This
allows batched updates to reject the individual update which conflicts
with the existing file, while updating the rest of the references.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
During the 'prepare' phase of a reference transaction in the files
backend, we create the lock files for references to be created. When
using batched updates on case-insensitive filesystems, the entire
batched updates would be aborted if there are conflicting names such as:
refs/heads/Foo
refs/heads/foo
This affects all commands which were migrated to use batched updates in
Git 2.51, including 'git-fetch(1)' and 'git-receive-pack(1)'. Before
that, reference updates would be applied serially with one transaction
used per update. When users fetched multiple references on
case-insensitive systems, subsequent references would simply overwrite
any earlier references. So when fetching:
refs/heads/foo: 5f34ec0bfeac225b1c854340257a65b106f70ea6
refs/heads/Foo: ec3053b0977e83d9b67fc32c4527a117953994f3
refs/heads/sample: 2eefd1150e06d8fca1ddfa684dec016f36bf4e56
The user would simply end up with:
refs/heads/foo: ec3053b0977e83d9b67fc32c4527a117953994f3
refs/heads/sample: 2eefd1150e06d8fca1ddfa684dec016f36bf4e56
This is buggy behavior since the user is never informed about the
overrides performed and missing references. Nevertheless, the user is
left with a working repository with a subset of the references. Since
Git 2.51, in such situations fetches would simply fail without updating
any references. Which is also buggy behavior and worse off since the
user is left without any references.
The error is triggered in `lock_raw_ref()` where the files backend
attempts to create a lock file. When a lock file already exists the
function returns a 'REF_TRANSACTION_ERROR_GENERIC'. When this happens,
the entire batched updates, not individual operation, is aborted as if
it were in a transaction.
Change this to return 'REF_TRANSACTION_ERROR_CASE_CONFLICT' instead to
aid the batched update mechanism to simply reject such errors. The
change only affects batched updates since batched updates will reject
individual updates with non-generic errors. So specifically this would
only affect:
1. git fetch
2. git receive-pack
3. git update-ref --batch-updates
This bubbles the error type up to `files_transaction_prepare()` which
tries to lock each reference update. So if the locking fails, we check
if the rejection type can be ignored, which is done by calling
`ref_transaction_maybe_set_rejected()`.
As the error type is now 'REF_TRANSACTION_ERROR_CASE_CONFLICT',
the specific reference update would simply be rejected, while other
updates in the transaction would continue to be applied. This allows
partial application of references in case-insensitive filesystems when
fetching colliding references.
While the earlier implementation allowed the last reference to be
applied overriding the initial references, this change would allow the
first reference to be applied while rejecting consequent collisions.
This should be an okay compromise since with the files backend, there is
no scenario possible where we would retain all colliding references.
Let's also be more proactive and notify users on case-insensitive
filesystems about such problems by providing a brief about the issue
while also recommending using the reftable backend, which doesn't have
the same issue.
Reported-by: Joe Drew <joe.drew@indexexchange.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Squelch false-positive compiler warning.
* dl/squelch-maybe-uninitialized:
t/unit-tests/clar: fix -Wmaybe-uninitialized with -Og
remote: bail early from set_head() if missing remote name
The case where a new submodule takes a path where used to be a
completely different subproject is now dealt a bit better than
before.
* kj/renamed-submodule:
fixup! submodule: skip redundant active entries when pattern covers path
fixup! submodule: prevent overwriting .gitmodules on path reuse
submodule: skip redundant active entries when pattern covers path
submodule: prevent overwriting .gitmodules on path reuse
When building with -Og on gcc 15.1.1, the build produces a warning. In
practice, though, this cannot be hit because `exact` acts as a guard and
that variable can only be set after `matchlen` is already initialized
Assign a default value to `matchlen` so that the warning is silenced.
Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git add/etc -p" now honor the diff.context configuration variable,
and also they learn to honor the -U<n> command-line option.
* lm/add-p-context:
add-patch: add diff.context command line overrides
add-patch: respect diff.context configuration
t: use test_config in t4055
t: use test_grep in t3701 and t4055
The config API had a set of convenience wrapper functions that
implicitly use the_repository instance; they have been removed and
inlined at the calling sites.
* ps/config-wo-the-repository: (21 commits)
config: fix sign comparison warnings
config: move Git config parsing into "environment.c"
config: remove unused `the_repository` wrappers
config: drop `git_config_set_multivar()` wrapper
config: drop `git_config_get_multivar_gently()` wrapper
config: drop `git_config_set_multivar_in_file_gently()` wrapper
config: drop `git_config_set_in_file_gently()` wrapper
config: drop `git_config_set()` wrapper
config: drop `git_config_set_gently()` wrapper
config: drop `git_config_set_in_file()` wrapper
config: drop `git_config_get_bool()` wrapper
config: drop `git_config_get_ulong()` wrapper
config: drop `git_config_get_int()` wrapper
config: drop `git_config_get_string()` wrapper
config: drop `git_config_get_string()` wrapper
config: drop `git_config_get_string_multi()` wrapper
config: drop `git_config_get_value()` wrapper
config: drop `git_config_get_value()` wrapper
config: drop `git_config_get()` wrapper
config: drop `git_config_clear()` wrapper
...
Code clean-up.
* kn/for-each-ref-skip-updates:
ref-filter: use REF_ITERATOR_SEEK_SET_PREFIX instead of '1'
t6302: add test combining '--start-after' with '--exclude'
for-each-ref: reword the documentation for '--start-after'
for-each-ref: fix documentation argument ordering
ref-cache: use 'size_t' instead of int for length
A new test to ensure that a recent change will keep working.
* jb/t7510-gpg-program-path:
t7510: use $PWD instead of $(pwd) inside PATH
t7510: add test cases for non-absolute gpg program
A few file descriptors left unclosed upon program completion in a
few test helper programs are now closed.
* hl/test-helper-fd-close:
test-delta: close output descriptor after use
test-delta: use strbufs to hold input files
test-delta: handle errors with die()
t/helper/test-truncate: close file descriptor after truncation
"git rebase -i" with bogus rebase.instructionFormat configuration
failed to produce the todo file after recording the state files,
leading to confused "git status"; this has been corrected.
* ow/rebase-verify-insn-fmt-before-initializing-state:
rebase: write script before initializing state
"git for-each-ref" learns "--start-after" option to help
applications that want to page its output.
* kn/for-each-ref-skip:
ref-cache: set prefix_state when seeking
for-each-ref: introduce a '--start-after' option
ref-filter: remove unnecessary else clause
refs: selectively set prefix in the seek functions
ref-cache: remove unused function 'find_ref_entry()'
refs: expose `ref_iterator` via 'refs.h'
The reftable unit tests are now ported to the "clar" unit testing
framework.
* sk/reftable-clarify-tests:
t/unit-tests: finalize migration of reftable-related tests
t/unit-tests: convert reftable stack test to use clar
t/unit-tests: convert reftable record test to use clar
t/unit-tests: convert reftable readwrite test to use clar
t/unit-tests: convert reftable table test to use clar
t/unit-tests: convert reftable pq test to use clar
t/unit-tests: convert reftable merged test to use clar
t/unit-tests: convert reftable block test to use clar
t/unit-tests: convert reftable basics test to use clar test framework
t/unit-tests: implement clar specific reftable test helper functions
Prior to 05e9cd64 (config: quote values containing CR character,
2025-05-19), a repository can trick "clone --recurse-submodules"
into running a post-checkout hook shipped with the project. The
test was written to make sure the trick would no longer run the
hook with the fix in the commit.
However, the test did not check for the path the hook would
create; correct the path to the expected one if the bug were
still with us.
Signed-off-by: chenjianhu <chenjianhu@kylinos.cn>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As I ended up wasting a few dozen minutes looking for the reason why
this is still here, help future developers by saving them from
wasting their time by documenting why this code that apparently is
not used by anybody is still here.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This patch compliments the previous commit, where builtins that use
add-patch infrastructure now respect diff.context and
diff.interHunkContext file configurations.
In particular, this patch helps users who don't want to set persistent
context configurations or just want a way to override them on a one-time
basis, by allowing the relevant builtins to accept corresponding command
line options that override the file configurations.
This mimics commands such as diff and log, which allow for both context
file configuration and command line overrides.
Signed-off-by: Leon Michalak <leonmichalak6@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Various builtins that use add-patch infrastructure do not respect
the user's diff.context and diff.interHunkContext file configurations.
The user may be used to seeing their diffs with customized context size,
but not in the patches "git add -p" shows them to pick from.
Teach add-patch infrastructure to read these configuration variables and
pass their values when spawning the underlying plumbing commands as
their command line option.
Signed-off-by: Leon Michalak <leonmichalak6@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use the modern "test_config" test utility instead of manual"git config"
as the former provides clean up on test completion.
This is a prerequisite to the commits that follow which add to this test
file.
Signed-off-by: Leon Michalak <leonmichalak6@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
As a preparatory clean-up, use the "test_grep" test utility instead of
regular "grep" which provides better debug information if tests fail.
Signed-off-by: Leon Michalak <leonmichalak6@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "clar-decls.h" header gets generated by us to extract prototypes of
unit test functions from our clar-based tests. This generated file is
then written into "t/unit-tests/" and included via "unit-test.h". The
intent of all this is that we can keep "-Wmissing-prototype" warnings
enabled. If we had that warning disabled, it would be easy to miss in
case any of the non-static functions had a typo in its name and thus
wasn't picked up by our test case extractor.
Including the file directly has a big downside though: if a source tree
was built both with our Makefile and with Meson, then the Meson build
would include the "clar-decls.h" file from our Makefile. And if those
are out of sync we get compiler errors.
We already fixed a similar issue in 4771501c0a (meson: ensure correct
version-def.h is used, 2025-01-14). Let's do the same and pass the
absolute path to "clar-decls.h" via a preprocessor define.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
On Windows, $(pwd) will give us a Windows-style path like "D:/foo".
Putting that into $PATH confuses anybody parsing that variable, since
colon is a separator character in $PATH. Instead, we should use the
Unix-style value we get from $PWD ("/d/foo").
This is similar to the cases fixed by 71dd50472d (t0021, t5615: use $PWD
instead of $(pwd) in PATH-like shell variables, 2016-11-11).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The '--start-after' doesn't explicitly mention being compatible with the
'--exclude' flag, generally only incompatibility is explicitly called
out. However, it would be nice to test the compatibility between the
two to avoid future regressions. Let's do that.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git commit" that concludes a conflicted merge failed to notice and remove
existing comment added automatically (like "# Conflicts:") when the
core.commentstring is set to 'auto'.
* ac/auto-comment-char-fix:
config: set comment_line_str to "#" when core.commentChar=auto
commit: avoid scanning trailing comments when 'core.commentChar' is "auto"
The pop_most_recent_commit() function can have quite expensive
worst case performance characteristics, which has been optimized by
using prio-queue data structure.
* rs/pop-recent-commit-with-prio-queue:
commit: use prio_queue_replace() in pop_most_recent_commit()
prio-queue: add prio_queue_replace()
commit: convert pop_most_recent_commit() to prio_queue
A number of tests in "t9350-fast-export.sh" are using sub-shells to
redirect content to a number of commands instead of only
`git fast-import`.
This is confusing and possibly error-prone, so let's change those tests
so that no sub-shell is used and the content goes only to
`git fast-import`.
Reported-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
GIT_TEST_INSTALLED was not honored in the recent topic related to
SHA256 hashes, which has been corrected.
* kl/test-installed-fix:
test-lib: respect GIT_TEST_INSTALLED when querying default hash
configure_added_submodule always writes an explicit
submodule.<name>.active entry, even when the new
path is already matched by submodule.active
patterns. This leads to unnecessary and cluttered configuration.
change the logic to centralize wildmatch-based pattern lookup,
in configure_added_submodule. Wrap the active-entry write in a conditional
that only fires when that helper reports no existing pattern covers the
submodule’s path.
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adding a submodule at a path that previously hosted
another submodule (e.g., 'child') reuses the submodule
name derived from the path. If the original submodule
was only moved (e.g., to 'child_old') and not renamed,
this silently overwrites its configuration in .gitmodules.
This behavior loses user configuration and causes
confusion when the original submodule is expected
to remain intact. It assumes that the path-derived
name is always safe to reuse, even though the name
might still be in use elsewhere in the repository.
Teach module_add() to check if the computed submodule
name already exists in the repository's submodule config,
and if so, refuse the operation unless the user explicitly
renames the submodule or uses the --force option,
which will automatically generate a unique name by
appending a number (e.g., child1).
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The old `lib-reftable.{c,h}` implemented helper functions for our
homegrown unit-testing framework. As part of migrating reftable-related
tests to the Clar framework, Clar-specific versions of these functions
in `lib-reftable-clar.{c,h}` were introduced.
Now that all test files using these helpers have been converted to Clar,
we can safely remove the original `lib-reftable.{c,h}` and rename the
Clar- specific versions back to `lib-reftable.{c,h}`. This restores a
clean and consistent naming scheme for shared test utilities.
Finally, update our build system to reflect the changes made and remove
redundant code related to the reftable tests and our old homegrown
unit-testing setup. `test-lib.{c,h}` remains unchanged in our build
system as some files particularly `t/helper/test-example-tap.c` depends
on it in order to run, and removing that would be beyond the scope of
this patch.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable stack test file to use clar by using clar assertions
where necessary.
This marks the end of all unit tests migrated away from the
`unit-tests/t-*.c` pattern, there are no longer any files matching that
glob. Remove the sanity check for `t-*.c` files to prevent Meson
configuration errors during CI and local builds.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable record test file to use clar by using clar assertions
where necessary.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable readwrite test file to use clar by using clar assertions
where necessary.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable table test file to use clar by using clar assertions
where necessary.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable priority queue test file to use clar by using clar
assertions where necessary.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable merged test file to use clar testing framework by using
clar assertions where necessary.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable block test file to use clar testing framework by using
clar assertions where necessary.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt reftable basics test file to clar by using clar assertions
where necessary.Break up test edge case to improve modularity and
clarity.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Helper functions defined in `t/unit-tests/lib-reftable.{c,h}` are
required for the reftable-related test files to run. In the current
implementation these functions are designed to conform with our
homegrown unit-testing structure. So in other to convert the reftable
test files, there is need for a clar specific implementation of these
helper functions.
Implement equivalent helper functions in `lib-reftable-clar.{c,h}` to
use clar. These functions conform with the clar testing framework and
become available for all reftable-related test files implemented using
the clar testing framework, which requires them. This will be used by
subsequent commits.
Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
After we write to the output file, the program exits. This naturally
closes the descriptor. But we should do an explicit close for two
reasons:
1. It's possible to hit an error on close(), which we should detect
and report via our exit code.
2. Leaking descriptors is a bad practice in general. Even if it isn't
meaningful here, it sets a bad example.
It is tempting to write:
if (write_in_full(fd, ...) < 0 || close(fd) < 0)
die_errno(...);
But that pattern contains a subtle problem that has resulted in
descriptor leaks before. If write_in_full() fails, we'll short-circuit
and never call close(), leaking the descriptor.
That's not a problem here, since our error path dies instead of
returning up the stack. But since we're trying to set a good example,
let's write it out as two separate conditions. As a bonus, that lets us
produce a slightly more specific error message.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We want to read the whole contents of two files into memory. If we
switch from raw ptr/len pairs to strbufs, we can use strbuf_read_file()
to shorten the code.
This incidentally fixes two small bugs:
1. We stat() the files and allocate our buffers based on st.st_size.
But that is an off_t which may be larger than the size_t we'd use
to allocate. We should use xsize_t() to do a checked conversion.
Otherwise integer truncation (on a file >4GB) could cause us to
under-allocate (though in practice this does not result in a buffer
overflow because the same truncation happens when read_in_full()
also takes a size_t).
2. We get the size from st.st_size, and then try to read_in_full()
that many bytes. But it may return fewer bytes than expected (if
the file changed racily and we get an early EOF), leading us to
read uninitialized bytes in the allocated buffer. We don't notice
because we only check the value for error, not that we got the
expected number of bytes.
The strbuf code doesn't run into this, because it just reads to EOF,
expanding the buffer dynamically as necessary. Neither bug is a big deal
for a test helper, but fixing them is a nice bonus on top of simplifying
the code.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is a short test helper that does all of its work in the main
function. When we encounter an error, we try to clean up memory and
descriptors and then jump to an error return, which exits the program.
We can get the same effect by just calling die(), which means we do not
have to bother with cleaning up. This simplifies the code, and also
removes some inconsistencies where a few code paths forgot to clean up
descriptors (though in practice it was not a big deal since we were
exiting anyway).
In addition to die() and die_errno(), we'll also use a few of our usual
helpers like xopen() and usage() that make things more ergonomic.
This does change the exit code in these cases from 1 to 128, but I
don't think it matters (and arguably is better, as we'd already exit 128
for other errors like xmalloc() failure).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Clean up the way how signature on commit objects are exported to
and imported from fast-import stream.
* cc/fast-import-export-signature-names:
fast-(import|export): improve on commit signature output format
Lift the limitation to use changed-path filter in "git log" so that
it can be used for a pathspec with multiple literal paths.
* ly/changed-paths-traversal:
bloom: optimize multiple pathspec items in revision
revision: make helper for pathspec to bloom keyvec
bloom: replace struct bloom_key * with struct bloom_keyvec
bloom: rename function operates on bloom_key
bloom: add test helper to return murmur3 hash
