summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2025-11-18ci(windows-meson-test): handle options and output like other test jobsJeff King2-1/+24
The GitHub windows-meson-test jobs directly run "meson test" with the --slice option. This means they skip all of the ci/lib.sh infrastructure, and in particular: 1. They do not actually set any GIT_TEST_OPTS like --verbose-log or -x. 2. They do not do the usual handle_failed_tests() magic to print test failures or tar up failed directories. As a result, you get almost no feedback at all when a test fails in this job, making debugging rather tricky. Let's try to make this behave more like the other CI jobs. Because we're on Windows, we can't just use the normal run-build-and-tests.sh script. Our build runs as a separate job (like the non-meson Windows job), and then we parallelize the tests across several job slices. So we need something like the run-test-slice.sh script that the "windows-test" job uses. In theory we could just swap out the "make" invocation there for "meson". But it doesn't quite work, because "make" knows how to pull GIT_TEST_OPTS out of GIT-BUILD-OPTIONS automatically. But for meson, we have to extract them into the --test-args option ourselves. I tried making the logic in run-test-slice.sh conditional, but there ended up being hardly any common code at all (and there are some tricky ordering constraints). So I added up with a new meson-specific test-slice runner. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18unit-test: ignore --no-chain-lintJeff King1-0/+1
In the same spirit as 9faf3963b6 (t: introduce compatibility options to clar-based tests, 2024-12-13), we should ignore --no-chain-lint passed to our clar tests, since it may appear in GIT_TEST_OPTS to be used with other tests. This is particularly important on Windows CI, where --no-chain-lint is added to the test options by default, and the meson build will pass all options to the unit tests. The only reason our meson Windows CI job does not run into this currently is that it is not respecting GIT_TEST_OPTS at all! So ignoring this option is a prerequisite to fixing that situation. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18t: enable ASan's strict_string_checks optionJeff King1-0/+1
ASan has an option to enable strict string checking, where any pointer passed to a function that expects a NUL-terminated string will be checked for that NUL termination. This can sometimes produce false positives. E.g., it is not wrong to pass a buffer with { '1', '2', '\n' } into strtoul(). Even though it is not NUL-terminated, it will stop at the newline. But in trying it out, it identified two problematic spots in our test suite (which have now been adjusted): 1. The strtol() parsing in cache-tree.c was a real potential problem, which would have been very hard to find otherwise (since it required constructing a very specific broken index file). 2. The use of string functions in fsck_ident() were false positives, because we knew that there was always a trailing newline which would stop the functions from reading off the end of the buffer. But the reasoning behind that is somewhat fragile, and silencing those complaints made the code easier to reason about. So even though this did not find any earth-shattering bugs, and even had a few false positives, I'm sufficiently convinced that its complaints are more helpful than hurtful. Let's turn it on by default (since the test suite now runs cleanly with it) and see if it ever turns up any other instances. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18fsck: avoid parse_timestamp() on buffer that isn't NUL-terminatedJeff King1-4/+19
In fsck_ident(), we parse the timestamp with parse_timestamp(), which is really an alias for strtoumax(). But since our buffer may not be NUL-terminated, this can trigger a complaint from ASan's strict_string_checks mode. This is a false positive, since we know that the buffer contains a trailing newline (which we checked earlier in the function), and that strtoumax() would stop there. But it is worth working around ASan's complaint. One is because that will let us turn on strict_string_checks by default, which has helped catch other real problems. And two is that the safety of the current code is very hard to reason about (it subtly depends on distant code which could change). One option here is to just parse the number left-to-right ourselves. But we care about the size of a timestamp_t and detecting overflow, since that's part of the point of these checks. And doing that correctly is tricky. So we'll instead just pull the digits into a separate, NUL-terminated buffer, and use that to call parse_timestamp(). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18fsck: remove redundant date timestamp checkJeff King1-1/+1
After calling "parse_timestamp(p, &end, 10)", we complain if "p == end", which would imply that we did not see any digits at all. But we know this cannot be the case, since we would have bailed already if we did not see any digits, courtesy of extra checks added by 8e4309038f (fsck: do not assume NUL-termination of buffers, 2023-01-19). Since then, checking "p == end" is redundant and we can drop it. This will make our lives a little easier as we refactor further. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18fsck: avoid strcspn() in fsck_ident()Jeff King1-10/+22
We may be operating on a buffer that is not NUL-terminated, but we use strcspn() to parse it. This is OK in practice, as discussed in 8e4309038f (fsck: do not assume NUL-termination of buffers, 2023-01-19), because we know there is at least a trailing newline in our buffer, and we always pass "\n" to strcspn(). So we know it will stop before running off the end of the buffer. But this is a subtle point to hang our memory safety hat on. And it confuses ASan's strict_string_checks mode, even though it is technically a false positive (that mode complains that we have no NUL, which is true, but it does not know that we have verified the presence of the newline already). Let's instead open-code the loop. As a bonus, this makes the logic more obvious (to my mind, anyway). The current code skips forward with strcspn until it hits "<", ">", or "\n". But then it must check which it saw to decide if that was what we expected or not, duplicating some logic between what's in the strcspn() and what's in the domain logic. Instead, we can just check each character as we loop and act on it immediately. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18fsck: assert newline presence in fsck_ident()Jeff King1-7/+9
The fsck code purports to handle buffers that are not NUL-terminated, but fsck_ident() uses some string functions. This works OK in practice, as explained in 8e4309038f (fsck: do not assume NUL-termination of buffers, 2023-01-19). Before calling fsck_ident() we'll have called verify_headers(), which makes sure we have at least a trailing newline. And none of our string-like functions will walk past that newline. However, that makes this code at the top of fsck_ident() very confusing: *ident = strchrnul(*ident, '\n'); if (**ident == '\n') (*ident)++; We should always see that newline, or our memory safety assumptions have been violated! Further, using strchrnul() is weird, since the whole point is that if the newline is not there, we don't necessarily have a NUL at all, and might read off the end of the buffer. So let's have callers pass in the boundary of our buffer, which lets us safely find the newline with memchr(). And if it is not there, this is a BUG(), because it means our caller did not validate the input with verify_headers() as it was supposed to (and we are better off bailing rather than having memory-safety problems). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18cache-tree: avoid strtol() on non-string bufferJeff King1-13/+37
A cache-tree extension entry in the index looks like this: <name> NUL <entry_nr> SPACE <subtree_nr> NEWLINE <binary_oid> where the "_nr" items are human-readable base-10 ASCII. We parse them with strtol(), even though we do not have a NUL-terminated string (we'd generally have an mmap() of the on-disk index file). For a well-formed entry, this is not a problem; strtol() will stop when it sees the newline. But there are two problems: 1. A corrupted entry could omit the newline, causing us to read further. You'd mostly get stopped by seeing non-digits in the oid field (and if it is likewise truncated, there will still be 20 or more bytes of the index checksum). So it's possible, though unlikely, to read off the end of the mmap'd buffer. Of course a malicious index file can fake the oid and the index checksum to all (ASCII) 0's. This is further complicated by the fact that mmap'd buffers tend to be zero-padded up to the page boundary. So to run off the end, the index size also has to be a multiple of the page size. This is also unlikely, though you can construct a malicious index file that matches this. The security implications aren't too interesting. The index file is a local file anyway (so you can't attack somebody by cloning, but only if you convince them to operate in a .git directory you made, at which point attacking .git/config is much easier). And it's just a read overflow via strtol(), which is unlikely to buy you much beyond a crash. 2. ASan has a strict_string_checks option, which tells it to make sure that options to string functions (like strtol) have some eventual NUL, without regard to what the function would actually do (like stopping at a newline here). This option sometimes has false positives, but it can point to sketchy areas (like this one) where the input we use doesn't exhibit a problem, but different input _could_ cause us to misbehave. Let's fix it by just parsing the values ourselves with a helper function that is careful not to go past the end of the buffer. There are a few behavior changes here that should not matter: - We do not consider overflow, as strtol() would. But nor did the original code. However, we don't trust the value we get from the on-disk file, and if it says to read 2^30 entries, we would notice that we do not have that many and bail before reading off the end of the buffer. - Our helper does not skip past extra leading whitespace as strtol() would, but according to gitformat-index(5) there should not be any. - The original quit parsing at a newline or a NUL byte, but now we insist on a newline (which is what the documentation says, and what Git has always produced). Since we are providing our own helper function, we can tweak the interface a bit to make our lives easier. The original code does not use strtol's "end" pointer to find the end of the parsed data, but rather uses a separate loop to advance our "buf" pointer to the trailing newline. We can instead provide a helper that advances "buf" as it parses, letting us read strictly left-to-right through the buffer. I didn't add a new test here. It's surprisingly difficult to construct an index of exactly the right size due to the way we pad entries. But it is easy to trigger the problem in existing tests when using ASan's strict string checking, coupled with a recent change to use NO_MMAP with ASan builds. So: make SANITIZE=address cd t ASAN_OPTIONS=strict_string_checks=1 ./t0090-cache-tree.sh triggers it reliably. Technically it is not deterministic because there is ~8% chance (it's 1-(255/256)^20, or ^32 for sha256) that the trailing checksum hash has a NUL byte in it. But we compute enough cache-trees in the course of that script that we are very likely to hit the problem in one of them. We can look at making strict_string_checks the default for ASan builds, but there are some other cases we'd want to fix first. Reported-by: correctmost <cmlists@sent.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18Makefile: turn on NO_MMAP when building with ASanJeff King2-1/+8
Git often uses mmap() to access on-disk files. This leaves a blind spot in our SANITIZE=address builds, since ASan does not seem to handle mmap at all. Nor does the OS notice most out-of-bounds access, since it tends to round up to the nearest page size (so depending on how big the map is, you might have to overrun it by up to 4095 bytes to trigger a segfault). The previous commit demonstrates a memory bug that we missed. We could have made a new test where the out-of-bounds access was much larger, or where the mapped file ended closer to a page boundary. But the point of running the test suite with sanitizers is to catch these problems without having to construct specific tests. Let's enable NO_MMAP for our ASan builds by default, which should give us better coverage. This does increase the memory usage of Git, since we're copying from the filesystem into heap. But the repositories in the test suite tend to be small, so the overhead isn't really noticeable (and ASan already has quite a performance penalty). There are a few other known bugs that this patch will help flush out. However, they aren't directly triggered in the test suite (yet). So it's safe to turn this on now without breaking the test suite, which will help us add new tests to demonstrate those other bugs as we fix them. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18pack-bitmap: handle name-hash lookups in incremental bitmapsJeff King1-4/+25
If a bitmap has a name-hash cache, it is an array of 32-bit integers, one per entry in the bitmap, which we've mmap'd from the .bitmap file. We access it directly like this: if (bitmap_git->hashes) hash = get_be32(bitmap_git->hashes + index_pos); That works for both regular pack bitmaps and for non-incremental midx bitmaps. There is one bitmap_index with one "hashes" array, and index_pos is within its bounds (we do the bounds-checking when we load the bitmap). But for an incremental midx bitmap, we have a linked list of bitmap_index structs, and each one has only its own small slice of the name-hash array. If index_pos refers to an object that is not in the first bitmap_git of the chain, then we'll access memory outside of the bounds of its "hashes" array, and often outside of the mmap. Instead, we should walk through the list until we find the bitmap_index which serves our index_pos, and use its hash (after adjusting index_pos to make it relative to the slice we found). This is exactly what we do elsewhere for incremental midx lookups (like the pack_pos_to_midx() call a few lines above). But we can't use existing helpers like midx_for_object() here, because we're walking through the chain of bitmap_index structs (each of which refers to a midx), not the chain of incremental multi_pack_index structs themselves. The problem is triggered in the test suite, but we don't get a segfault because the out-of-bounds index is too small. The OS typically rounds our mmap up to the nearest page size, so we just end up accessing some extra zero'd memory. Nor do we catch it with ASan, since it doesn't seem to instrument mmaps at all. But if we build with NO_MMAP, then our maps are replaced with heap allocations, which ASan does check. And so: make NO_MMAP=1 SANITIZE=address cd t ./t5334-incremental-multi-pack-index.sh does show the problem (and this patch makes it go away). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18compat/mmap: mark unused argument in git_munmap()Jeff King1-1/+1
Our mmap compat code emulates mapping by using malloc/free. Our git_munmap() must take a "length" parameter to match the interface of munmap(), but we don't use it (it is up to the allocator to know how big the block is in free()). Let's mark it as UNUSED to avoid complaints from -Wunused-parameter. Otherwise you cannot build with "make DEVELOPER=1 NO_MMAP=1". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-18ci: bump actions/setup-go from 5 to 6Johannes Schindelin1-1/+1
Bumps actions/setup-go from 5 to 6. This upgrade includes dependency updates that incorporate a fix for a critical vulnerability. [Originally opened at https://github.com/git-for-windows/git/pull/5811] - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](https://github.com/actions/setup-go/compare/v5...v6) Originally-authored-by: dependabot[bot] <support@github.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jiang Xin <worldhello.net@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17mingw: avoid the comma operatorJohannes Schindelin1-20/+28
The pattern `return errno = ..., -1;` is observed several times in `compat/mingw.c`. It has served us well over the years, but now clang starts complaining: compat/mingw.c:723:24: error: possible misuse of comma operator here [-Werror,-Wcomma] 723 | return errno = ENOSYS, -1; | ^ See for example this failing workflow run: https://github.com/git-for-windows/git-sdk-arm64/actions/runs/15457893907/job/43513458823#step:8:201 Let's appease clang (and also reduce the use of the no longer common comma operator). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17cmake: stop trying to build the reftable and xdiff librariesJohannes Schindelin1-13/+1
In the `en/make-libgit-a` topic branch, more precisely in the commits f3b4c89d59f1 (make: delete REFTABLE_LIB, add reftable to LIB_OBJS, 2025-10-02) and cf680cdb9543 (make: delete XDIFF_LIB, add xdiff to LIB_OBJS, 2025-10-02), the strategy to build three static libraries was rethought, and instead only one static library is now built. This is good. However, the CMake definition was not changed accordingly, and now CMake-based builds fail thusly: [...] Generating hook-list.h CMake Error at CMakeLists.txt:122 (string): string sub-command REPLACE requires at least four arguments. Call Stack (most recent call first): CMakeLists.txt:711 (parse_makefile_for_sources) CMake Error at CMakeLists.txt:122 (string): string sub-command REPLACE requires at least four arguments. Call Stack (most recent call first): CMakeLists.txt:717 (parse_makefile_for_sources) -- Configuring incomplete, errors occurred! Fix that by removing the parts that expect the reftable and xdiff objects to be defined separately in the Makefile, still. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17wincred: avoid memory corruptionDavid Macek1-1/+1
`wcsncpy_s()` wants to write the terminating null character so we need to allocate one more space for it in the target memory block. This should fix crashes when trying to read passwords. When this happened, the password/token wouldn't print out and Git would therefore ask for a new password every time. Signed-off-by: David Macek <david.macek.0@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17merge-ort: fix failing merges in special corner caseElijah Newren2-1/+106
At GitHub, we had a repository that was triggering git: merge-ort.c:3032: process_renames: Assertion `newinfo && !newinfo->merged.clean` failed. during git replay. This sounds similar to the somewhat recent f6ecb603ff8a (merge-ort: fix directory rename on top of source of other rename/delete, 2025-08-06), but the cause is different. Unlike that case, there are no rename-to-self situations arising in this case, and new to this case it can only be triggered during a replay operation on the 2nd or later commit being replayed, never on the first merge in the sequence. To trigger, the repository needs: * an upstream which: * renames a file to a different directory, e.g. old/file -> new/file * leaves other files remaining in the original directory (so that e.g. "old/" still exists upstream even though file has been removed from it and placed elsewhere) * a topic branch being rebased where: * a commit in the sequence: * modifies old/file * a subsequent commit in the sequence being replayed: * does NOT touch *anything* under new/ * does NOT touch old/file * DOES modify other paths under old/ * does NOT have any relevant renames that we need to detect _anywhere_ elsewhere in the tree (meaning this interacts interestingly with both directory renames and cached renames) In such a case, the assertion will trigger. The fix turns out to be surprisingly simple. I have a very vague recollection that I actually considered whether to add such an if-check years ago when I added the very similar one for oldinfo in 1b6b902d95a5 (merge-ort: process_renames() now needs more defensiveness, 2021-01-19), but I think I couldn't figure out a possible way to trigger it and was worried at the time that if I didn't know how to trigger it then I wasn't so sure that simply skipping it was correct. Waiting did give me a chance to put more thorough tests and checks into place for the rename-to-self cases a few months back, which I might not have found as easily otherwise. Anyway, put the check in place now and add a test that demonstrates the fix. Note that this bug, as demonstrated by the conditions listed above, runs at the intersection of relevant renames, trivial directory resolutions, and cached renames. All three of those optimizations are ones that unfortunately make the code (and testcases!) a bit more complex, and threading all three makes it a bit more so. However, the testcase isn't crazy enough that I'd expect no one to ever hit it in practice, and was confused why we didn't see it before. After some digging, I discovered that merge.directoryRenames=false is a workaround to this bug, and GitHub used that setting until recently (it was a "temporary" match-what-libgit2-does piece of code that lasted years longer than intended). Since the conditions I gave above for triggering this bug rule out the possibility of there being directory renames, one might assume that it shouldn't matter whether you try to detect such renames if there aren't any. However, due to commit a16e8efe5c2b (merge-ort: fix merge.directoryRenames=false, 2025-03-13), the heavy hammer used there means that merge.directoryRenames=false ALSO turns off rename caching, which is critical to triggering the bug. This becomes a bit more than an aside since... Re-reading that old commit, a16e8efe5c2b (merge-ort: fix merge.directoryRenames=false, 2025-03-13), it appears that the solution to this latest bug might have been at least a partial alternative solution to that old commit. And it may have been an improved alternative (or at least help implement one), since it may be able to avoid the heavy-handed disabling of rename cache. That might be an interesting future thing to investigate, but is not critical for the current fix. However, since I spent time digging it all up, at least leave a small comment tweak breadcrumb to help some future reader (myself or others) who wants to dig further to connect the dots a little quicker. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17merge-ort: remove debugging crudElijah Newren1-1/+1
While developing commit a16e8efe5c2b (merge-ort: fix merge.directoryRenames=false, 2025-03-13), I was testing things out and had an extra condition on one of the if-blocks that I occasionally swapped between '&& 0' and '&& 1' to see the effects of the changes. I forgot to remove it before submitting and it wasn't caught in review. Remove it now. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17t6429: update comment to mention correct toolElijah Newren1-8/+7
A comment at the top of t6429 mentions why the test doesn't exercise git rebase or git cherry-pick. However, it claims that it uses `test-tool fast-rebase`. That was true when the comment was written, but commit f920b0289ba3 (replay: introduce new builtin, 2023-11-24) changed it to use git replay without updating this comment. We could potentially just strike this second comment, since git replay is a bona fide built-in, but perhaps the explanation about why it focuses on git replay is still useful. Update the comment to make it accurate again. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17make strip: include `scalar`Johannes Schindelin1-1/+1
When Scalar was made a canonical part of Git in 7b5c93c6c68 (scalar: include in standard Git build & installation, 2022-09-02), it was added to all relevant Makefile targets except for the `strip` target. Let's correct that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17wrapper: simplify xmkstemp()René Scharfe1-18/+1
Call xmkstemp_mode() instead of duplicating its error handling code. This switches the implementation from the system's mkstemp(3) to our own git_mkstemp_mode(), which works just as well. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17blame: make diff algorithm configurableAntonin Delpeuch6-21/+278
The diff algorithm used in 'git-blame(1)' is set to 'myers', without the possibility to change it aside from the `--minimal` option. There has been long-standing interest in changing the default diff algorithm to "histogram", and Git 3.0 was floated as a possible occasion for taking some steps towards that: https://lore.kernel.org/git/xmqqed873vgn.fsf@gitster.g/ As a preparation for this move, it is worth making sure that the diff algorithm is configurable where useful. Make it configurable in the `git-blame(1)` command by introducing the `--diff-algorithm` option and make honor the `diff.algorithm` config variable. Keep Myers diff as the default. Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17xdiff: add 'minimal' to XDF_DIFF_ALGORITHM_MASKAntonin Delpeuch3-3/+1
The XDF_DIFF_ALGORITHM_MASK bit mask only includes bits for the patience and histogram diffs, not for the minimal one. This means that when reseting the diff algorithm to the default one, one needs to separately clear the bit for the minimal diff. There are places in the code that fail to do that: merge-ort.c and builtin/merge-file.c. Add the XDF_NEED_MINIMAL bit to the bit mask, and remove the separate clearing of this bit in the places where it hasn't been forgotten. Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17Git 2.52v2.52.0Junio C Hamano2-4/+7
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-17Merge branch 'jc/ci-use-arm64-p4-on-macos'Junio C Hamano1-1/+1
We replaced deprecated macos-13 with macos-14 image in GitHub Actions CI, but we forgot that the image is for arm64. We have been seeing a lot of test failures ever since. Switch to arm64 binary for Perforce tests. * jc/ci-use-arm64-p4-on-macos: Use Perforce arm64 binary on macOS CI jobs
2025-11-16commit: refactor verify_commit_buffer()Christian Couder2-2/+22
In a following commit, we are going to check commit signatures, but we won't have a commit yet, only a commit buffer, and we are going to discard this commit buffer if the signature is invalid. So it would be wasteful to create a commit that we might discard, just to be able to check a commit signature. It would be simpler instead to be able to check commit signatures using only a commit buffer instead of a commit. To be able to do that, let's extract some code from the check_commit_signature() function into a new verify_commit_buffer() function, and then let's make check_commit_signature() call verify_commit_buffer(). Note that this doesn't fundamentally change how check_commit_signature() works. It used to call parse_signed_commit() which calls repo_get_commit_buffer(), parse_buffer_signed_by_header() and repo_unuse_commit_buffer(). Now these 3 functions are called directly by verify_commit_buffer(). Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-16fast-import: refactor finalize_commit_buffer()Christian Couder1-4/+13
In a following commit we are going to finalize commit buffers with or without signatures in order to check the signatures and possibly drop them. To do so easily and without duplication, let's refactor the current code that finalizes commit buffers into a new finalize_commit_buffer() function. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-16builtin/repo: fix table alignment for UTF-8 charactersJiang Xin2-4/+54
The output table from "git repo structure" is misaligned when displaying UTF-8 characters (e.g., non-ASCII glyphs). E.g.: | 仓库结构 | 值 | | -------------- | ---- | | * 引用 | | | * 计数 | 67 | The previous implementation used simple width formatting with printf() which didn't properly handle multi-byte UTF-8 characters, causing misaligned table columns when displaying repository structure information. This change modifies the stats_table_print_structure function to use strbuf_utf8_align() instead of basic printf width specifiers. This ensures proper column alignment regardless of the character encoding of the content being displayed. Also add test cases for strbuf_utf8_align(), a function newly introduced in "builtin/repo.c". Signed-off-by: Jiang Xin <worldhello.net@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-16t/unit-tests: add UTF-8 width tests for CJK charsJiang Xin3-0/+99
The file "builtin/repo.c" uses utf8_strwidth() to calculate the display width of UTF-8 characters in a table, but the resulting output is still misaligned. Add test cases for both utf8_strwidth and utf8_strnwidth to verify that they correctly compute the display width for UTF-8 characters. Also updated the build configuration in Makefile and meson.build to include the new test suite in the build process. Signed-off-by: Jiang Xin <worldhello.net@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-16Use Perforce arm64 binary on macOS CI jobsJunio C Hamano1-1/+1
The previous step replaced deprecated macos-13 image with macos-14 image on GitHub Actions CI. While x86-64 binaries can work there, because macos-14 images are arm64 based (we could replace it with macos-14-large that is x86-64), it makes more sense to use arm64 binary there. Without this change, we have been getting unusually higher rate of failures from random macOS CI jobs railing to run t98xx series of tests. Helped-by: Koji Nakamaru <koji.nakamaru@gree.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-16Merge tag 'l10n-2.52.0-v1' of https://github.com/git-l10n/git-poJunio C Hamano10-8985/+13309
l10n-2.52.0-v1 * tag 'l10n-2.52.0-v1' of https://github.com/git-l10n/git-po: l10n: zh_CN: updated translation for 2.52 l10n: uk: add 2.52 translation l10n: zh_TW.po: update Git 2.52 translation l10n: Updated translation for vi-2.52 l10n: tr: Update Turkish translations l10n: po-id for 2.52 l10n: ga.po: Update Irish translation for Git 2.52 l10n: bg.po: Updated Bulgarian translation (6065t) l10n: fr: version 2.52 l10n: sv.po: Update Swedish translation
2025-11-16l10n: zh_CN: updated translation for 2.52Teng Long1-297/+1401
Reviewed-by: 依云 <lilydjwg@gmail.com> Signed-off-by: Teng Long <dyroneteng@gmail.com> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2025-11-15read-cache: drop submodule check from add_to_cache()Jeff King3-4/+2
In add_to_cache(), we treat any directories as submodules, and complain if we can't resolve their HEAD. This call to resolve_gitlink_ref() was added by f937bc2f86 (add: error appropriately on repository with no commits, 2019-04-09), with the goal of improving the error message for empty repositories. But we already resolve the submodule HEAD in index_path(), which is where we find the actual oid we're going to use. Resolving it again here introduces some downsides: 1. It's more work, since we have to open up the submodule repository's files twice. 2. There are call paths that get to index_path() without going through add_to_cache(). For instance, we'd want a similar informative message if "git diff empty" finds that it can't resolve the submodule's HEAD. (In theory we can also get there through update-index, but AFAICT it refuses to consider directories as submodules at all, and just complains about them). 3. The resolution in index_path() catches more errors that we don't handle here. In particular, it will validate that the object format for the submodule matches that of the superproject. This isn't a bug, since our call in add_to_cache() throws away the oid it gets without looking at it. But it certainly caused confusion for me when looking at where the object-format check should go. So instead of resolving the submodule HEAD in add_to_cache(), let's just teach the call in index_path() to actually produce an error message (which it already does for other cases). That's probably what f937bc2f86 should have done in the first place, and it gives us a single point of resolution when adding a submodule to the index. The resulting output is slightly more verbose, as we propagate the error up the call stack, but I think that's OK (and again, matches many other errors we get when indexing fails). I've left the text of the error message as-is, though it is perhaps overly specific. There are many reasons that resolving the submodule HEAD might fail, though outside of corruption or system errors it is probably most likely that the submodule HEAD is simply on an unborn branch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-16Merge branch '2.52-uk' of github.com:arkid15r/git-ukrainian-l10nJiang Xin1-231/+1161
* '2.52-uk' of github.com:arkid15r/git-ukrainian-l10n: l10n: uk: add 2.52 translation
2025-11-15object-file: disallow adding submodules of different hash algobrian m. carlson3-1/+55
The design of the hash algorithm transition plan is that objects stored must be entirely in one algorithm since we lack any way to indicate a mix of algorithms. This also includes submodules, but we have traditionally not enforced this, which leads to various problems when trying to clone or check out the the submodule from the remote. Since this cannot work in the general case, restrict adding a submodule of a different algorithm to the index. Add tests for git add and git submodule add that these are rejected. Note that we cannot check this in git fsck because the malformed submodule is stored in the tree as an object ID which is either truncated (when a SHA-256 submodule is added to a SHA-1 repository) or padded with zeros (when a SHA-1 submodule is added to a SHA-256 repository). We cannot detect even the latter case because someone could have an actual submodule that actually ends in 24 zeros, which would be a false positive. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-15l10n: uk: add 2.52 translationArkadii Yakovets1-231/+1161
Co-authored-by: Kate Golovanova <kate@kgthreads.com> Signed-off-by: Arkadii Yakovets <ark@cho.red> Signed-off-by: Kate Golovanova <kate@kgthreads.com>
2025-11-15Merge branch 'vi-2.52' of github.com:Nekosha/git-poJiang Xin1-242/+1140
* 'vi-2.52' of github.com:Nekosha/git-po: l10n: Updated translation for vi-2.52
2025-11-15Merge branch 'l10n/zh-TW/git-2-52' of github.com:l10n-tw/git-poJiang Xin1-415/+1568
* 'l10n/zh-TW/git-2-52' of github.com:l10n-tw/git-po: l10n: zh_TW.po: update Git 2.52 translation
2025-11-15Merge branch 'po-id' of github.com:bagasme/git-poJiang Xin1-283/+1420
* 'po-id' of github.com:bagasme/git-po: l10n: po-id for 2.52
2025-11-15Merge branch 'master' of github.com:alshopov/git-poJiang Xin1-230/+1177
* 'master' of github.com:alshopov/git-po: l10n: bg.po: Updated Bulgarian translation (6065t)
2025-11-15Merge branch 'fr_v2.52' of github.com:jnavila/gitJiang Xin1-228/+1208
* 'fr_v2.52' of github.com:jnavila/git: l10n: fr: version 2.52
2025-11-15Merge branch 'l10n-ga-2.52' of github.com:aindriu80/git-poJiang Xin1-6603/+1938
* 'l10n-ga-2.52' of github.com:aindriu80/git-po: l10n: ga.po: Update Irish translation for Git 2.52
2025-11-15Merge branch 'master' of github.com:nafmo/git-l10n-svJiang Xin1-235/+1151
* 'master' of github.com:nafmo/git-l10n-sv: l10n: sv.po: Update Swedish translation
2025-11-15l10n: zh_TW.po: update Git 2.52 translationYi-Jyun Pan1-415/+1568
Reviewed-by: hms5232 <hms5232@hhming.moe> Co-authored-by: Lumynous <lumynou5.tw@gmail.com> Signed-off-by: Yi-Jyun Pan <pan93412@gmail.com>
2025-11-15l10n: Updated translation for vi-2.52Vũ Tiến Hưng1-242/+1140
Signed-off-by: Vũ Tiến Hưng <newcomerminecraft@gmail.com>
2025-11-15l10n: tr: Update Turkish translationsEmir SARI1-221/+1145
Signed-off-by: Emir SARI <emir_sari@icloud.com>
2025-11-14doc: commit: link to git-status(1) on all format optionsKristoffer Haugsbakk1-4/+6
`--branch` and `--long` refer to git-status(1) options but they don’t tell us what `short-format` and `long-format` are, respectively. And `--null` mentions “status” but does not link to the command. Refer to git-config(1) on `--branch` like `--short` does. `long-format` is the git-status(1) output. So we can just say that directly. Replace “status” with a `linkgit` on `--null`. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-14osxkeychain: avoid incorrectly skipping store operationKoji Nakamaru3-30/+132
git-credential-osxkeychain skips storing a credential if its "get" action sets "state[]=osxkeychain:seen=1". This behavior was introduced in e1ab45b2 (osxkeychain: state to skip unnecessary store operations, 2024-05-15), which appeared in v2.46. However, this state[] persists even if a credential returned by "git-credential-osxkeychain get" is invalid and a subsequent helper's "get" operation returns a valid credential. Another subsequent helper (such as [1]) may expect git-credential-osxkeychain to store the valid credential, but the "store" operation is incorrectly skipped because it only checks "state[]=osxkeychain:seen=1". To solve this issue, "state[]=osxkeychain:seen" needs to contain enough information to identify whether the current "store" input matches the output from the previous "get" operation (and not a credential from another helper). Set "state[]=osxkeychain:seen" to a value encoding the credential output by "get", and compare it with a value encoding the credential input by "store". [1]: https://github.com/hickford/git-credential-oauth Reported-by: Petter Sælen <petter@saelen.eu> Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Koji Nakamaru <koji.nakamaru@gree.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-14attr: enable incomplete-line whitespace error for this projectJunio C Hamano1-3/+3
Now "git diff --check" and "git apply --whitespace=warn/fix" learned incomplete line is a whitespace error, enable them for this project to prevent patches to add new incomplete lines to our source to both code and documentation files. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-13RelNotes: fix typo in release notes for 2.52.0Taylor Blau1-1/+1
Introduced via aea86cf00f (The nineteenth batch, 2025-10-14). Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2025-11-13l10n: po-id for 2.52Bagas Sanjaya1-283/+1420
Update following components: - add-patch.c - builtin/bisect.c - builtin/describe.c - builtin/fast-export.c - builtin/fast-import.c - builtin/fetch.c - builtin/for-each-ref.c - builtin/gc.c - builtin/log.c - builtin/pack-refs.c - builtin/range-diff.c - builtin/reflog.c - builtin/refs.c - builtin/remote.c - builtin/repo.c - builtin/sparse-checkout.c - command-list.h - config.c - diff-lib.c - diff.c - gpg-interface.c - midx-write.c - promisor-remote.c - range-diff.c - refs.c - refs/files-backend.c - refs/reftable-backend.c - remote.c - usage.c - git-send-email.perl Translate following new components: - builtin/last-modified.c - http.h Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>