]> git.ipfire.org Git - thirdparty/git.git/commit
t6423: document two bugs with rename-to-self testcases
authorElijah Newren <newren@gmail.com>
Wed, 6 Aug 2025 23:15:18 +0000 (23:15 +0000)
committerJunio C Hamano <gitster@pobox.com>
Thu, 7 Aug 2025 20:23:58 +0000 (13:23 -0700)
commit034b2fde7ff6e73759a649177ab466651dbf2b34
tree63ce67cf1bebe27ae78e568b97a479b9e4829901
parentedbe2abcd8411e5c6598db37ea75b5ab20027da9
t6423: document two bugs with rename-to-self testcases

When commit 98a1a00d5301 (t6423: add a testcase causing a failed
assertion in process_renames, 2025-03-06) was added, I tweaked
the commit message, and moved the test into t6423.  However, that
still left two other things missing that made this test unlike the
others in the same testfile:

  * It didn't have an English description of the test setup like
    all other tests in t6423

  * It didn't check that the right number of files were present at
    the end

The former issue is a minor detail that isn't that critical, but the
latter feels more important.  If it had been done, I might have noticed
another bug.  In particular, this testcase involves
   Side A: rename world -> tools/world
and
   Side B: rename tools/ -> <the toplevel>
   Side B: remove world
The tools/ -> <toplevel> rename turns the world -> tools/world rename
into world -> world, i.e. a rename-to-self case.  But, it's a path
conflict because merge.directoryRenames defaults to false.  There's
no content conflict because Side A didn't modify world, so we should
just take the content of world from Side B -- i.e. delete it.  So, we
have a conflict on the path, but not on its content.  We could consider
letting the content trump since it is unconflicted, but if we are going
to leave a conflict, it should certainly represent that 'world' existed
both in the base version and on Side A.  Currently it doesn't.

Add a description of this test, add some checking of the number of
entries in the index at the end of the merge, and mark the test as
expecting to fail for now.  A subsequent commit will fix this bug.

While at it, I found another related bug from a nearly identical setup
but setting merge.directoryRenames=true.  Copy testcase 12n into 12n2,
changing it to use merge instead of cherry-pick, and turn on directory
renames for this test.  In this case, since there is no content conflict
and no path conflict, it should be okay to delete the file.
Unfortunately, the code resolves without conflict but silently leaves
world despite the fact it should be deleted.  It might also be okay if
the code spuriously thought there was a modify/delete conflict here;
that would at least notify users to look closer and then when they
notice there was no change since the base version, they can easily
resolve.  A conflict notice is much better than silently providing the
wrong resolution.  Cover this with the 12n2 testcase, which for now is
marked as expecting to fail as well.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/t6423-merge-rename-directories.sh