git/combine-diff.c, branch jch

Merge branch 'tc/last-modified-recursive-fix'

2025-09-29T18:40:35Z

"git last-modified" operating in non-recursive mode used to trigger a BUG(), which has been corrected. * tc/last-modified-recursive-fix: last-modified: fix bug when some paths remain unhandled

Merge branch 'jk/color-variable-fixes'

2025-09-29T18:40:35Z

Some places in the code confused a variable that is *not* a boolean to enable color but is an enum that records what the user requested to do about color. A couple of bugs of this sort have been fixed, while the code has been cleaned up to prevent similar bugs in the future. * jk/color-variable-fixes: config: store want_color() result in a separate bool add-interactive: retain colorbool values longer color: return bool from want_color() color: use git_colorbool enum type to store colorbools pretty: use format_commit_context.auto_color as colorbool diff: stop passing ecbdata->use_color as boolean diff: pass o->use_color directly to fill_metainfo() diff: don't use diff_options.use_color as a strict bool diff: simplify color_moved check when flushing grep: don't treat grep_opt.color as a strict bool color: return enum from git_config_colorbool() color: use GIT_COLOR_* instead of numeric constants

last-modified: fix bug when some paths remain unhandled

2025-09-18T15:00:41Z

The recently introduced new subcommand git-last-modified(1) runs into an error in some scenarios. It then would exit with the message: BUG: paths remaining beyond boundary in last-modified This seems to happens for example when criss-cross merges are involved. In that scenario, the function diff_tree_combined() gets called. The function diff_tree_combined() copies the `struct diff_options` from the input `struct rev_info` to override some flags. One flag is `recursive`, which is always set to 1. This has been the case since the inception of this function in af3feefa1d (diff-tree -c: show a merge commit a bit more sensibly., 2006-01-24). This behavior is incompatible with git-last-modified(1), when called non-recursive (which is the default). The last-modified machinery uses a hashmap for all the paths it wants to get the last-modified commit for. Through log_tree_commit() the callback mark_path() is called. The diff machinery uses diff_tree_combined() internally, and due to it's recursive behavior the callback receives entries inside subtrees, but not the subtree entries themselves. So a directory is never expelled from the hashmap, and the BUG() statement gets hit. Because there are many callers calling into diff_tree_combined(), both directly and indirectly, we cannot simply change it's behavior. Instead, add a flag `no_recursive_diff_tree_combined` which supresses the behavior of diff_tree_combined() to override `recursive` and set this flag in builtin/last-modified.c. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano

color: use git_colorbool enum type to store colorbools

2025-09-17T00:59:53Z

We traditionally used "int" to store and pass around the values defined by "enum git_colorbool" (which were originally just #define macros). Using an int doesn't produce incorrect results, but using the actual enum makes the intent of the code more clear. It would be nice if the compiler could catch cases where we used the enum and an int interchangeably, since it's very easy to accidentally check the boolean true/false of a colorbool like: if (branch_use_color) This is wrong because GIT_COLOR_UNKNOWN and GIT_COLOR_AUTO evaluate to true in C, even though we may ultimately decide not to use color. But C is pretty happy to convert between ints and enums (even with various -Wenum-* warnings). So this sadly doesn't protect us from such mistakes, but it hopefully does make the code easier to read. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'tc/diff-tree-max-depth'

2025-08-25T21:22:01Z

"git diff-tree" learned "--max-depth" option. * tc/diff-tree-max-depth: diff: teach tree-diff a max-depth parameter within_depth: fix return for empty path combine-diff: zero memory used for callback filepairs

combine-diff: zero memory used for callback filepairs

2025-08-07T22:29:34Z

In commit 25e5e2bf85 (combine-diff: support format_callback, 2011-08-19), the combined-diff code learned how to make a multi-sourced `diff_filepair` to pass to a diff callback. When we create each filepair, we do not bother to fill in many of the fields, because they would make no sense (e.g. there can be no rename score or broken_pair flag because we do not go through the diffcore filters). However, we did not even bother to zero them, leading to random values. Let's make sure everything is blank with xcalloc(), just as the regular diff code does. We would potentially want to set the `status` flag to something non-zero, but it is not clear to what. Possibly a new DIFF_STATUS_COMBINED would make sense, as this is not strictly a modification, nor does it fit any other category. Since it is not yet clear what callers would want, this patch simply leaves it as `0`, the same empty flag that is seen when `diffcore_std` is not used at all. Signed-off-by: Jeff King Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano

odb: rename `repo_read_object_file()`

2025-07-01T21:46:38Z

Rename `repo_read_object_file()` to `odb_read_object()` to match other functions related to the object database and our modern coding guidelines. Introduce a compatibility wrapper so that any in-flight topics will continue to compile. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

object-store: rename files to "odb.{c,h}"

2025-07-01T21:46:34Z

In the preceding commits we have renamed the structures contained in "object-store.h" to `struct object_database` and `struct odb_backend`. As such, the code files "object-store.{c,h}" are confusingly named now. Rename them to "odb.{c,h}" accordingly. 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

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