git/builtin/diff-tree.c, branch v2.46.2

Merge branch 'xx/diff-tree-remerge-diff-fix' into maint-2.46

2024-08-26T18:10:25Z

"git rev-list ... | git diff-tree -p --remerge-diff --stdin" should behave more or less like "git log -p --remerge-diff" but instead it crashed, forgetting to prepare a temporary object store needed. * xx/diff-tree-remerge-diff-fix: diff-tree: fix crash when used with --remerge-diff

diff-tree: fix crash when used with --remerge-diff

2024-08-09T15:07:44Z

When using "git-diff-tree" to get the tree diff for merge commits with the diff format set to `remerge`, a bug is triggered as shown below: $ git diff-tree -r --remerge-diff 363337e6eb 363337e6eb812d0c0d785ed4261544f35559ff8b BUG: log-tree.c:1006: did a remerge diff without remerge_objdir?!? This bug is reported by `log-tree.c:do_remerge_diff`, where a bug check added in commit 7b90ab467a (log: clean unneeded objects during log --remerge-diff, 2022-02-02) detects the absence of `remerge_objdir` when attempting to clean up temporary objects generated during the remerge process. After some further digging, I find that the remerge-related diff options were introduced in db757e8b8d (show, log: provide a --remerge-diff capability, 2022-02-02), which also affect the setup of `rev_info` for "git-diff-tree", but were not accounted for in the original implementation (inferred from the commit message). Elijah Newren, the author of the remerge diff feature, notes that other callers of `log-tree.c:log_tree_commit` (the only caller of `log-tree.c:do_remerge_diff`) also exist, but: `builtin/am.c`: manually sets all flags; remerge_diff is not among them `sequencer.c`: manually sets all flags; remerge_diff is not among them so `builtin/diff-tree.c` really is the only caller that was overlooked when remerge-diff functionality was added. This commit resolves the crash by adding `remerge_objdir` setup logic to `builtin/diff-tree.c`, mirroring `builtin/log.c:cmd_log_walk_no_free`. It also includes the necessary cleanup for `remerge_objdir`. Reviewed-by: Elijah Newren Signed-off-by: Xing Xin Signed-off-by: Junio C Hamano

builtin: stop using `the_index`

2024-04-18T19:30:42Z

Convert builtins to use `the_repository->index` instead of `the_index`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

treewide: remove unnecessary includes in source files

2023-12-26T20:04:31Z

Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

diff: drop useless "status" parameter from diff_result_code()

2023-08-21T22:33:24Z

Many programs use diff_result_code() to get a user-visible program exit code from a diff result (e.g., checking opts.found_changes if --exit-code was requested). This function also takes a "status" parameter, which seems at first glance that it could be used to propagate an error encountered when computing the diff. But it doesn't work that way: - negative values are passed through as-is, but are not appropriate as program exit codes - when --exit-code or --check is in effect, we _ignore_ the passed-in status completely. So a failed diff which did not have a chance to set opts.found_changes would erroneously report "success, no changes" instead of propagating the error. After recent cleanups, neither of these bugs is possible to trigger, as every caller just passes in "0". So rather than fixing them, we can simply drop the useless parameter instead. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

revisions: drop unused "opt" parameter in "tweak" callbacks

2023-07-14T00:24:00Z

The setup_revision_opt struct has a "tweak" function pointer, which can be used to adjust parameters after setup_revisions() parses arguments, but before it finalizes setup. In addition to the rev_info struct, the callback receives a pointer to the setup_revision_opt, as well. But none of the existing callbacks looks at the extra "opt" parameter, leading to -Wunused-parameter warnings. We could mark it as UNUSED, but instead let's remove it entirely. It's conceivable that it could be useful for a callback to have access to the "opt" struct. But in the 13 years that this mechanism has existed, nobody has used it. So let's just drop it in the name of simplifying. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

log-tree: replace include of revision.h with simple forward declaration

2023-06-21T20:39:53Z

Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

cache.h: remove this no-longer-used header

2023-06-21T20:39:53Z

Since this header showed up in some places besides just #include statements, update/clean-up/remove those other places as well. Note that compat/fsmonitor/fsm-path-utils-darwin.c previously got away with violating the rule that all files must start with an include of git-compat-util.h (or a short-list of alternate headers that happen to include it first). This change exposed the violation and caused it to stop building correctly; fix it by having it include git-compat-util.h first, as per policy. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

read-cache*.h: move declarations for read-cache.c functions from cache.h

2023-06-21T20:39:53Z

For the functions defined in read-cache.c, move their declarations from cache.h to a new header, read-cache-ll.h. Also move some related inline functions from cache.h to read-cache.h. The purpose of the read-cache-ll.h/read-cache.h split is that about 70% of the sites don't need the inline functions and the extra headers they include. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

diff-tree: integrate with sparse index

2023-05-18T17:40:33Z

The index is read in 'cmd_diff_tree' at two points: 1. The first index read was added in fd66bcc31ff (diff-tree: read the index so attribute checks work in bare repositories, 2017-12-06) to deal with reading '.gitattributes' content. 77efbb366ab (attr: be careful about sparse directories, 2021-09-08) established that, in a sparse index, we do _not_ try to load a '.gitattributes' file from within a sparse directory. 2. The second index access point is involved in rename detection, specifically when reading from stdin.This was initially added in f0c6b2a2fd9 ([PATCH] Optimize diff-tree -[CM]--stdin, 2005-05-27), where 'setup' was set to 'DIFF_SETUP_USE_SIZE_CACHE |DIFF_SETUP_USE_CACHE'. That assignment was later modified to drop the'DIFF_SETUP_USE_CACHE' in ff7fe37b053 (diff.c: move read_index() code back to the caller, 2018-08-13).However, 'DIFF_SETUP_USE_SIZE_CACHE' seems to be unused as of 6e0b8ed6d35 (diff.c: do not use a separate "size cache"., 2007-05-07) and nothing about 'detect_rename' otherwise indicates index usage. Hence we can just set the requires-full-index to false for "diff-tree". Add tests that verify that 'git diff-tree' behaves correctly when the sparse index is enabled and test to ensure the index is not expanded. The `p2000` tests demonstrate a ~98% execution time reduction for 'git diff-tree' using a sparse index: Test before after ----------------------------------------------------------------------- 2000.94: git diff-tree HEAD (full-v3) 0.05 0.04 -20.0% 2000.95: git diff-tree HEAD (full-v4) 0.06 0.05 -16.7% 2000.96: git diff-tree HEAD (sparse-v3) 0.59 0.01 -98.3% 2000.97: git diff-tree HEAD (sparse-v4) 0.61 0.01 -98.4% 2000.98: git diff-tree HEAD -- f2/f4/a (full-v3) 0.05 0.05 +0.0% 2000.99: git diff-tree HEAD -- f2/f4/a (full-v4) 0.05 0.04 -20.0% 2000.100: git diff-tree HEAD -- f2/f4/a (sparse-v3) 0.58 0.01 -98.3% 2000.101: git diff-tree HEAD -- f2/f4/a (sparse-v4) 0.55 0.01 -98.2% Helped-by: Victoria Dye Signed-off-by: Shuqi Liang Signed-off-by: Junio C Hamano