]> git.ipfire.org Git - thirdparty/git.git/commitdiff
t8020: fix test failure due to indeterministic tag sorting
authorPatrick Steinhardt <ps@pks.im>
Thu, 2 Oct 2025 11:04:40 +0000 (13:04 +0200)
committerJunio C Hamano <gitster@pobox.com>
Thu, 2 Oct 2025 16:44:58 +0000 (09:44 -0700)
In e6c06e87a2 (last-modified: fix bug when some paths remain unhandled,
2025-09-18), we have fixed a bug where under certain circumstances,
git-last-modified(1) would BUG because there's still some unhandled
paths. The fix claims that the root cause here is criss-cross merges,
and it adds a test case that seemingly exercises this.

Curiously, this test case fails on some systems because the actual
output does not match our expectations:

    diff --git a/expect b/actual
    index 5271607..bdc620e 100644
    --- a/expect
    --- b/actual
    @@ -1,3 +1,3 @@
     km3 a
    -k2 k
    +km2 k
     1 file
    error: last command exited with $?=1
    not ok 15 - last-modified with subdir and criss-cross merge

The output we see is git-name-rev(1) with `--annotate-stdin`. What it
does is to take the output of git-last-modified(1), which contains
object IDs of the blamed commits, and convert those object IDs into the
names of the corresponding tags. Interestingly, we indeed have both "k2"
and "km2" as tags, and even more interestingly both of these tags point
to the same commit. So the output we get isn't _wrong_, as the tags are
ambiguous.

But why do both of these tags point to the same commit? "km2" really is
supposed to be a merge, but due to the way the test is constructed the
merge turns into a fast-forward merge. Which means that the resulting
commit-graph does not even contain a criss-cross merge in the first place!
A quick test though shows that the test indeed triggers the bug, so
the initial analysis that the behaviour is triggered by such merges
must be wrong.

And it is: seemingly, the issue isn't with criss-cross merges, but
rather with a graph where different files in the same directory were
modified on both sides of a merge.

Refactor the test so that we explicitly test for this specific situation
instead of mentioning the "criss-cross merge" red herring. As the test
is very specific to the actual layout of the repository we also adapt it
to use its own standalone repository.

Note that this requires us to drop the `test_when_finished` call in
`check_last_modified` because it's not supported to execute that
function in a subshell.

This refactoring also fixes the original tag ambiguity that caused us to
fail on some platforms.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/t8020-last-modified.sh

index e13aad14398dd9055ff45ab57700329e7aab37c4..61f00bc15c3b2d170d981038f0ebb48fd4bf31c2 100755 (executable)
@@ -33,7 +33,6 @@ check_last_modified() {
        done &&
 
        cat >expect &&
-       test_when_finished "rm -f tmp.*" &&
        git ${indir:+-C "$indir"} last-modified "$@" >tmp.1 &&
        git name-rev --annotate-stdin --name-only --tags \
                <tmp.1 >tmp.2 &&
@@ -128,20 +127,25 @@ 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 'subdirectory modified via merge' '
+       test_when_finished rm -rf repo &&
+       git init repo &&
+       (
+               cd repo &&
+               test_commit base &&
+               git switch --create left &&
+               mkdir subdir &&
+               test_commit left subdir/left &&
+               git switch --create right base &&
+               mkdir subdir &&
+               test_commit right subdir/right &&
+               git switch - &&
+               test_merge merge right &&
+               check_last_modified <<-\EOF
+               merge subdir
+               base base.t
+               EOF
+       )
 '
 
 test_expect_success 'cross merge boundaries in blaming' '