git/diff.c, branch v2.50.0

Merge branch 'js/diff-codeql-false-positive-workaround'

2025-05-08T19:36:32Z

Work around false positive given by CodeQL. * js/diff-codeql-false-positive-workaround: diff: check range before dereferencing an array element

diff: check range before dereferencing an array element

2025-04-29T19:38:34Z

Before accessing an array element at a given index, it should be verified that the index is within the desired bounds, not afterwards, otherwise it may not make sense to even access the array element in the first place. This is the point of CodeQL's `cpp/offset-use-before-range-check` rule. This CodeQL rule unfortunately is also triggered by the `fill_es_indent_data()` code, even though the condition `off < len - 1` does not even need to guarantee that the offset is in bounds (`s` points to a NUL-terminated string, for which `s[off] == '\r'` would fail before running out of bounds). Let's work around this rare false positive to help us use an otherwise mostly useful tool is a worthy thing to do. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano

Merge branch 'ps/parse-options-integers'

2025-04-25T00:25:34Z

Update parse-options API to catch mistakes to pass address of an integral variable of a wrong type/size. * ps/parse-options-integers: parse-options: detect mismatches in integer signedness parse-options: introduce precision handling for `OPTION_UNSIGNED` parse-options: introduce precision handling for `OPTION_INTEGER` parse-options: rename `OPT_MAGNITUDE()` to `OPT_UNSIGNED()` parse-options: support unit factors in `OPT_INTEGER()` global: use designated initializers for options parse: fix off-by-one for minimum signed values

global: use designated initializers for options

2025-04-17T15:15:15Z

While we expose macros for most of our different option types understood by the "parse-options" subsystem, not every combination of fields that has one as that would otherwise quickly lead to an explosion of macros. Instead, we just initialize structures manually for those variants of fields that don't have a macro. Callsites that open-code these structure initialization don't use designated initializers though and instead just provide values for each of the fields that they want to initialize. This has three significant downsides: - Callsites need to specify all values up to the last field that they care about. This often includes fields that should simply be left at their default zero-initialized state, which adds distraction. - Any reader not deeply familiar with the layout of the structure has a hard time figuring out what the respective initializers mean. - Reordering or introducing new fields in the middle of the structure is impossible without adapting all callsites. Convert all sites to instead use designated initializers, which we have started using in our codebase quite a while ago. This allows us to skip any default-initialized fields, gives the reader context by specifying the field names and allows us to reorder or introduce new fields where we want to. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

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 'ps/object-wo-the-repository' into ps/object-file-cleanup

2025-04-08T21:28:17Z

* ps/object-wo-the-repository: hash: stop depending on `the_repository` in `null_oid()` hash: fix "-Wsign-compare" warnings object-file: split out logic regarding hash algorithms delta-islands: stop depending on `the_repository` object-file-convert: stop depending on `the_repository` pack-bitmap-write: stop depending on `the_repository` pack-revindex: stop depending on `the_repository` pack-check: stop depending on `the_repository` environment: move access to "core.bigFileThreshold" into repo settings pack-write: stop depending on `the_repository` and `the_hash_algo` object: stop depending on `the_repository` csum-file: stop depending on `the_repository`

hash: stop depending on `the_repository` in `null_oid()`

2025-03-10T20:16:20Z

The `null_oid()` function returns the object ID that only consists of zeroes. Naturally, this ID also depends on the hash algorithm used, as the number of zeroes is different between SHA1 and SHA256. Consequently, the function returns the hash-algorithm-specific null object ID. This is currently done by depending on `the_hash_algo`, which implicitly makes us depend on `the_repository`. Refactor the function to instead pass in the hash algorithm for which we want to retrieve the null object ID. Adapt callsites accordingly by passing in `the_repository`, thus bubbling up the dependency on that global variable by one layer. There are a couple of trivial exceptions for subsystems that already got rid of `the_repository`. These subsystems instead use the repository that is available via the calling context: - "builtin/grep.c" - "grep.c" - "refs/debug.c" There are also two non-trivial exceptions: - "diff-no-index.c": Here we know that we may not have a repository initialized at all, so we cannot rely on `the_repository`. Instead, we adapt `diff_no_index()` to get a `struct git_hash_algo` as parameter. The only caller is located in "builtin/diff.c", where we know to call `repo_set_hash_algo()` in case we're running outside of a Git repository. Consequently, it is fine to continue passing `the_repository->hash_algo` even in this case. - "builtin/ls-files.c": There is an in-flight patch series that drops `USE_THE_REPOSITORY_VARIABLE` in this file, which causes a semantic conflict because we use `null_oid()` in `show_submodule()`. The value is passed to `repo_submodule_init()`, which may use the object ID to resolve a tree-ish in the superproject from which we want to read the submodule config. As such, the object ID should refer to an object in the superproject, and consequently we need to use its hash algorithm. This means that we could in theory just not bother about this edge case at all and just use `the_repository` in "diff-no-index.c". But doing so would feel misdesigned. Remove the `USE_THE_REPOSITORY_VARIABLE` preprocessor define in "hash.c". Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

environment: move access to "core.bigFileThreshold" into repo settings

2025-03-10T20:16:18Z

The "core.bigFileThreshold" setting is stored in a global variable and populated via `git_default_core_config()`. This may cause issues in the case where one is handling multiple different repositories in a single process with different values for that config key, as we may or may not see the correct value in that case. Furthermore, global state blocks our path towards libification. Refactor the code so that we instead store the value in `struct repo_settings`, where the value is computed as-needed and cached. Note that this change requires us to adapt one test in t1050 that verifies that we die when parsing an invalid "core.bigFileThreshold" value. The exercised Git command doesn't use the value at all, and thus it won't hit the new code path that parses the value. This is addressed by using git-hash-object(1) instead, which does read the value. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

diff: add option to skip resolving diff statuses

2025-03-03T16:17:47Z

By default, `diffcore_std()` resolves the statuses for queued diff file pairs by calling `diff_resolve_rename_copy()`. If status information is already manually set, invoking `diffcore_std()` may change the status value. Introduce the `skip_resolving_statuses` diff option that prevents `diffcore_std()` from resolving file pair statuses when enabled. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano

diff: return diff_filepair from diff queue helpers

2025-03-03T16:17:47Z

The `diff_addremove()` and `diff_change()` functions set up and queue diffs, but do not return the `diff_filepair` added to the queue. In a subsequent commit, modifications to `diff_filepair` need to occur in certain cases after being queued. Since the existing `diff_addremove()` and `diff_change()` are also used for callbacks in `diff_options` as types `add_remove_fn_t` and `change_fn_t`, modifying the existing function signatures requires further changes. The diff options for pruning use `file_add_remove()` and `file_change()` where file pairs do not even get queued. Thus, separate functions are implemented instead. Split out the queuing operations into `diff_queue_addremove()` and `diff_queue_change()` which also return a handle to the queued `diff_filepair`. Both `diff_addremove()` and `diff_change()` are reimplemented as thin wrappers around the new functions. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano