mirror of
https://github.com/git/git.git
synced 2026-03-09 16:52:33 +01:00
line-log: fix crash when combined with pickaxe options
queue_diffs() passes the caller's diff_options, which may carry
user-specified pickaxe state, to diff_tree_oid() and diffcore_std()
when detecting renames for line-level history tracking. When pickaxe
options are present on the command line (-G and -S to filter by text
pattern, --find-object to filter by object identity), diffcore_std()
also runs diffcore_pickaxe(), which may discard diff pairs that are
relevant for rename detection. Losing those pairs breaks rename
following.
Before a2bb801f6a (line-log: avoid unnecessary full tree diffs,
2019-08-21), this silently truncated history at rename boundaries.
That commit moved filter_diffs_for_paths() inside the rename-
detection block, so it only runs when diff_might_be_rename() returns
true. When pickaxe discards a rename pair, the rename goes
undetected, and a deletion pair at a subsequent commit passes
through uncleaned, reaching process_diff_filepair() with an invalid
filespec and triggering an assertion failure.
Fix this by building a private diff_options for the rename-detection
path inside queue_diffs(), following the same pattern used by blame's
find_rename(). This isolates the rename machinery from unrelated
user-specified options.
Reported-by: Matthew Hughes <matthewhughes934@gmail.com>
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
committed by
Junio C Hamano
parent
795c338de7
commit
984677c96b
24
line-log.c
24
line-log.c
@@ -858,15 +858,29 @@ static void queue_diffs(struct line_log_data *range,
|
||||
diff_queue_clear(&diff_queued_diff);
|
||||
diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
|
||||
if (opt->detect_rename && diff_might_be_rename()) {
|
||||
/* must look at the full tree diff to detect renames */
|
||||
clear_pathspec(&opt->pathspec);
|
||||
diff_queue_clear(&diff_queued_diff);
|
||||
struct diff_options rename_opts;
|
||||
|
||||
diff_tree_oid(parent_tree_oid, tree_oid, "", opt);
|
||||
/*
|
||||
* Build a private diff_options for rename detection so
|
||||
* that any user-specified options on the original opts
|
||||
* (e.g. pickaxe) cannot discard diff pairs needed for
|
||||
* rename tracking. Similar to blame's find_rename().
|
||||
*/
|
||||
repo_diff_setup(opt->repo, &rename_opts);
|
||||
rename_opts.flags.recursive = 1;
|
||||
rename_opts.detect_rename = opt->detect_rename;
|
||||
rename_opts.rename_score = opt->rename_score;
|
||||
rename_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
|
||||
diff_setup_done(&rename_opts);
|
||||
|
||||
/* must look at the full tree diff to detect renames */
|
||||
diff_queue_clear(&diff_queued_diff);
|
||||
diff_tree_oid(parent_tree_oid, tree_oid, "", &rename_opts);
|
||||
|
||||
filter_diffs_for_paths(range, 1);
|
||||
diffcore_std(opt);
|
||||
diffcore_std(&rename_opts);
|
||||
filter_diffs_for_paths(range, 0);
|
||||
diff_free(&rename_opts);
|
||||
}
|
||||
move_diff_queue(queue, &diff_queued_diff);
|
||||
}
|
||||
|
||||
@@ -367,4 +367,53 @@ test_expect_success 'show line-log with graph' '
|
||||
test_cmp expect actual
|
||||
'
|
||||
|
||||
test_expect_success 'setup for -L with -G/-S/--find-object and a merge with rename' '
|
||||
git checkout --orphan pickaxe-rename &&
|
||||
git reset --hard &&
|
||||
|
||||
echo content >file &&
|
||||
git add file &&
|
||||
git commit -m "add file" &&
|
||||
|
||||
git checkout -b pickaxe-rename-side &&
|
||||
git mv file renamed-file &&
|
||||
git commit -m "rename file" &&
|
||||
|
||||
git checkout pickaxe-rename &&
|
||||
git commit --allow-empty -m "diverge" &&
|
||||
git merge --no-edit pickaxe-rename-side &&
|
||||
|
||||
git mv renamed-file file &&
|
||||
git commit -m "rename back"
|
||||
'
|
||||
|
||||
test_expect_success '-L -G does not crash with merge and rename' '
|
||||
git log --format="%s" --no-patch -L 1,1:file -G "." >actual
|
||||
'
|
||||
|
||||
test_expect_success '-L -S does not crash with merge and rename' '
|
||||
git log --format="%s" --no-patch -L 1,1:file -S content >actual
|
||||
'
|
||||
|
||||
test_expect_success '-L --find-object does not crash with merge and rename' '
|
||||
git log --format="%s" --no-patch -L 1,1:file \
|
||||
--find-object=$(git rev-parse HEAD:file) >actual
|
||||
'
|
||||
|
||||
test_expect_failure '-L -G should filter commits by pattern' '
|
||||
git log --format="%s" --no-patch -L 1,1:file -G "nomatch" >actual &&
|
||||
test_must_be_empty actual
|
||||
'
|
||||
|
||||
test_expect_failure '-L -S should filter commits by pattern' '
|
||||
git log --format="%s" --no-patch -L 1,1:file -S "nomatch" >actual &&
|
||||
test_must_be_empty actual
|
||||
'
|
||||
|
||||
test_expect_failure '-L --find-object should filter commits by object' '
|
||||
git log --format="%s" --no-patch -L 1,1:file \
|
||||
--find-object=$ZERO_OID >actual &&
|
||||
test_must_be_empty actual
|
||||
'
|
||||
|
||||
test_done
|
||||
|
||||
Reference in New Issue
Block a user