git/diffcore-rename.c, branch jch

merge,diff: remove the_repository check before prefetching blobs

2026-02-22T02:34:06Z

Prefetching of blobs from promisor remotes was added to diff in 7fbbcb21b162 (diff: batch fetching of missing blobs, 2019-04-05). In that commit, https://lore.kernel.org/git/20190405170934.20441-1-jonathantanmy@google.com/ was squashed into https://lore.kernel.org/git/44de02e584f449481e6fb00cf35d74adf0192e9d.1553895166.git.jonathantanmy@google.com/ without the extra explanation about the squashed changes being added to the commit message; in particular, this explanation from that first link is absent: > Also, prefetch only if the repository being diffed is the_repository > (because we do not support lazy fetching for any other repository > anyway). Then, later, this checking was spread from diff.c to diffcore-rename.c and diffcore-break.c by 95acf11a3dc3 (diff: restrict when prefetching occurs, 2020-04-07) and then further split in d331dd3b0c82 (diffcore-rename: allow different missing_object_cb functions, 2021-06-22). I also copied the logic from prefetching blobs from diff.c to merge-ort.c in 2bff554b23e8 (merge-ort: add prefetching for content merges, 2021-06-22). The reason for all these checks was noted above -- we only supported lazy fetching for the_repository. However, that changed with ef830cc43412 (promisor-remote: teach lazy-fetch in any repo, 2021-06-17), so these checks are now unnecessary. Remove them. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

global: constify some pointers that are not written to

2026-02-06T01:52:49Z

The recent glibc 2.43 release had the following change listed in its NEWS file: For ISO C23, the functions bsearch, memchr, strchr, strpbrk, strrchr, strstr, wcschr, wcspbrk, wcsrchr, wcsstr and wmemchr that return pointers into their input arrays now have definitions as macros that return a pointer to a const-qualified type when the input argument is a pointer to a const-qualified type. When compiling with GCC 15, which defaults to -std=gnu23, this causes many warnings like this: merge-ort.c: In function ‘apply_directory_rename_modifications’: merge-ort.c:2734:36: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 2734 | char *last_slash = strrchr(cur_path, '/'); | ^~~~~~~ This patch fixes the more obvious ones by making them const when we do not write to the returned pointer. Signed-off-by: Collin Funk Signed-off-by: Junio C Hamano

object-store: move function declarations to their respective subsystems

2025-04-29T17:08:12Z

We carry declarations for a couple of functions in "object-store.h" that are not defined in "object-store.c", but in a different subsystem. Move these declarations to the respective headers whose matching code files carry the corresponding definition. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'ps/object-file-cleanup' into ps/object-store-cleanup

2025-04-24T18:37:21Z

* ps/object-file-cleanup: object-store: merge "object-store-ll.h" and "object-store.h" object-store: remove global array of cached objects object: split out functions relating to object store subsystem object-file: drop `index_blob_stream()` object-file: split up concerns of `HASH_*` flags object-file: split out functions relating to object store subsystem object-file: move `xmmap()` into "wrapper.c" object-file: move `git_open_cloexec()` to "compat/open.c" object-file: move `safe_create_leading_directories()` into "path.c" object-file: move `mkdir_in_gitdir()` into "path.c"

object-store: merge "object-store-ll.h" and "object-store.h"

2025-04-15T15:24:37Z

The "object-store-ll.h" header has been introduced to keep transitive header dependendcies and compile times at bay. Now that we have created a new "object-store.c" file though we can easily move the last remaining additional bit of "object-store.h", the `odb_path_map`, out of the header. Do so. As the "object-store.h" header is now equivalent to its low-level alternative we drop the latter and inline it into the former. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'en/assert-wo-side-effects'

2025-04-08T18:43:12Z

Ensure what we write in assert() does not have side effects, and introduce ASSERT() macro to mark those that cannot be mechanically checked for lack of side effects. * en/assert-wo-side-effects: treewide: replace assert() with ASSERT() in special cases ci: add build checking for side-effects in assert() calls git-compat-util: introduce ASSERT() macro

treewide: replace assert() with ASSERT() in special cases

2025-03-21T10:32:10Z

When the compiler/linker cannot verify that an assert() invocation is free of side effects for us (e.g. because the assertion includes some kind of function call), replace the use of assert() with ASSERT(). Signed-off-by: Elijah Newren Acked-by: Taylor Blau Signed-off-by: Junio C Hamano

diffcore-rename: fix BUG when break detection and --follow used together

2025-03-15T01:43:28Z

Prior to commit 9db2ac56168e (diffcore-rename: accelerate rename_dst setup, 2020-12-11), the function add_rename_dst() resulted in quadratic runtime since each call inserted the new entry into the array in sorted order. The reason for the sorted order requirement was so that locate_rename_dst(), used when break detection is turned on, could find the appropriate entry in logarithmic time via bisection on string comparisons. (It's better to be quadratic in moving pointers than quadratic in string comparisons, so this made some sense.) However, since break detection always sticks the broken pairs adjacent to each other, that commit decided to simply append entries to rename_dst, and record the mapping of (filename) -> (index within rename_dst) via a strintmap. Doing this relied on the fact that when adding the source of a broken pair via register_rename_src(), that the next item we'd process was the other half of the same broken pair and would be added to rename_dst via add_rename_dst(). This assumption was fine under break detection alone, but the combination of break detection and single_follow violated that assumption because of this code: else if (options->single_follow && strcmp(options->single_follow, p->two->path)) continue; /* not interested */ which would end up skipping calling add_rename_dst() below that point. Since I knew I was assuming that the dst pair of a break would always be added right after the src pair of a break, I added a new BUG() directive as part of that commit later on at time of use that would check my assumptions held. That BUG() didn't trip for nearly 4 years...which sadly meant I had long since forgotten the related details. Anyway... When the dst half of a broken pair is skipped like this, it means that not only could my recorded index be invalid (just past the end of the array), it could also point to some unrelated dst that just happened to be the next one added to the array. So, to fix this, we need to add a little more safety around the checks for the recorded break_idx. It turns out that making a testcase to trigger this is quite the challenge. I actually added two testscases: * One testcase which uses --follow incorrectly (it uses its single pathspec to specifying something other than a single filename), and which triggers the same bug reported-by Olaf. This triggers a special case within locate_rename_dst() where idx evaluates to 0 and rename_dst is NULL, meaning that our return value of &rename_dst[idx] happens to evaluate to NULL as well. This addressing of an index into a NULL array hints at deeper problems, which are raised in the next testcase... * A second testcase which when run under valgrind shows that the code actually depends upon unintialized memory, in particular the entry just after the end of the rename_dst array. In short, when the two rare options -B and --follow are used together, fix the accidental find of the wrong dst entry (which would often be uninitialized memory just past the end of the array, but also could have just been a dst for an unrelated path if no dst was recorded for the expected path). Do so by adding a little more care around checking the recorded indices in break_idx. Reported-by: Olaf Hering Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

progress: stop using `the_repository`

2024-12-18T18:44:30Z

Stop using `the_repository` in the "progress" subsystem by passing in a repository when initializing `struct progress`. Furthermore, store a pointer to the repository in that struct so that we can pass it to the trace2 API when logging information. Adjust callers accordingly by using `the_repository`. While there may be some callers that have a repository available in their context, this trivial conversion allows for easier verification and bubbles up the use of `the_repository` by one level. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: trivial conversions to fix `-Wsign-compare` warnings

2024-12-06T11:20:04Z

We have a bunch of loops which iterate up to an unsigned boundary using a signed index, which generates warnigs because we compare a signed and unsigned value in the loop condition. Address these sites for trivial cases and enable `-Wsign-compare` warnings for these code units. This patch only adapts those code units where we can drop the `DISABLE_SIGN_COMPARE_WARNINGS` macro in the same step. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano