diff options
| author | Toon Claes <toon@iotcl.com> | 2025-09-18 10:00:08 +0200 |
|---|---|---|
| committer | Junio C Hamano <gitster@pobox.com> | 2025-09-18 08:00:41 -0700 |
| commit | e6c06e87a255995d2e7ead2b8e49e46e29a724fb (patch) | |
| tree | d98350747cdc7bb2a1beab7c2f73909215cd8250 | |
| parent | 215033b3ac599432a17d58f18a92b356d98354a9 (diff) | |
| download | git-e6c06e87a255995d2e7ead2b8e49e46e29a724fb.tar.gz | |
last-modified: fix bug when some paths remain unhandled
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 <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
| -rw-r--r-- | builtin/last-modified.c | 1 | ||||
| -rw-r--r-- | combine-diff.c | 3 | ||||
| -rw-r--r-- | diff.h | 7 | ||||
| -rwxr-xr-x | t/t8020-last-modified.sh | 16 |
4 files changed, 26 insertions, 1 deletions
diff --git a/builtin/last-modified.c b/builtin/last-modified.c index 886ba12cb5..ae8b36a2c3 100644 --- a/builtin/last-modified.c +++ b/builtin/last-modified.c @@ -265,6 +265,7 @@ static int last_modified_init(struct last_modified *lm, struct repository *r, lm->rev.boundary = 1; lm->rev.no_commit_id = 1; lm->rev.diff = 1; + lm->rev.diffopt.flags.no_recursive_diff_tree_combined = 1; lm->rev.diffopt.flags.recursive = lm->recursive; lm->rev.diffopt.flags.tree_in_recursive = lm->show_trees; diff --git a/combine-diff.c b/combine-diff.c index 3878faabe7..e779b86e0b 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1515,8 +1515,9 @@ void diff_tree_combined(const struct object_id *oid, diffopts = *opt; copy_pathspec(&diffopts.pathspec, &opt->pathspec); - diffopts.flags.recursive = 1; diffopts.flags.allow_external = 0; + if (!opt->flags.no_recursive_diff_tree_combined) + diffopts.flags.recursive = 1; /* find set of paths that everybody touches * @@ -126,6 +126,13 @@ struct diff_flags { unsigned recursive; unsigned tree_in_recursive; + /* + * Historically diff_tree_combined() overrides recursive to 1. To + * suppress this behavior, set the flag below. + * It has no effect if recursive is already set to 1. + */ + unsigned no_recursive_diff_tree_combined; + /* Affects the way how a file that is seemingly binary is treated. */ unsigned binary; unsigned text; diff --git a/t/t8020-last-modified.sh b/t/t8020-last-modified.sh index 5eb4cef035..e13aad1439 100755 --- a/t/t8020-last-modified.sh +++ b/t/t8020-last-modified.sh @@ -128,6 +128,22 @@ test_expect_success 'only last-modified files in the current tree' ' EOF ' +test_expect_success 'last-modified with subdir and criss-cross merge' ' + git checkout -b branch-k1 1 && + mkdir -p a k && + test_commit k1 a/file2 && + git checkout -b branch-k2 && + test_commit k2 k/file2 && + git checkout branch-k1 && + test_merge km2 branch-k2 && + test_merge km3 3 && + check_last_modified <<-\EOF + km3 a + k2 k + 1 file + EOF +' + test_expect_success 'cross merge boundaries in blaming' ' git checkout HEAD^0 && git rm -rf . && |