Re: [PATCH 1/3] t6423: add tests of dual directory rename plus add/add conflict
- From
Elijah Newren <newren@gmail.com>
- Date
- Jun 30, 2022, 00:06 UTC
- Message-ID
- <CABPp-BGtKQVgaf9ttwRMAE6AvH1QC3TqFcFwiAOpnm-SqKnNCQ@mail.gmail.com>
- In-Reply-To
- <20220627182024.1358144-1-jonathantanmy@google.com>
On Mon, Jun 27, 2022 at 11:20 AM Jonathan Tan <jonathantanmy@google.com> wrote:
Show 12 quoted lines
> > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > From: Elijah Newren <newren@gmail.com> > > > > This is an attempt at minimalizing a testcase reported by Glen Choo > > with tensorflow where merge-ort would report an assertion failure: > > > > Assertion failed: (ci->filemask == 2 || ci->filemask == 4), function apply_directory_rename_modifications, file merge-ort.c, line 2410 > > First of all, thanks for this fix - I've verified with Glen Choo's test > cases and it does fix it.
:-)
Show 22 quoted lines
> > diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
> > index 479db32cd62..296c04f8046 100755
> > --- a/t/t6423-merge-rename-directories.sh
> > +++ b/t/t6423-merge-rename-directories.sh
> > @@ -5199,6 +5199,108 @@ test_expect_success '12k: Directory rename with sibling causes rename-to-self' '
> > )
> > '
> >
> > +# Testcase 12l, Both sides rename a directory into the other side, both add
> > +# a file with after directory renames are the same filename
> > +# Commit O: sub1/file, sub2/other
> > +# Commit A: sub3/file, sub2/{other, new_add_add_file_1}
> > +# Commit B: sub1/{file, newfile}, sub1/sub2/{other, new_add_add_file_2}
> > +#
> > +# In words:
> > +# A: sub1/ -> sub3/, add sub2/new_add_add_file_1
> > +# B: sub2/ -> sub1/sub2, add sub1/newfile, add sub1/sub2/new_add_add_file_2
> > +#
> > +# Expected: sub3/{file, newfile, sub2/other}
> > +# CONFLICT (add/add): sub3/sub2/new_add_add_file
>
> The "expected" might need to be updated.Oops! Yes, I kept changing and editing the testcase and the comments...but didn't quite get all the updates I needed when I was revising.
Show 13 quoted lines
> After all patches are applied, > this is the tree I get (note that it's "sub1/sub2/new_add_add_file, not > "sub3/sub2/new_add_add_file"): > > . > |-- sub1 > | `-- sub2 > | `-- new_add_add_file > `-- sub3 > |-- file > |-- newfile > `-- sub2 > `-- other
Yes, that's right.
> Also, at first glance, "newfile" shouldn't belong in a minimal > reproduction
Ah, I can see you've looked at this very closely. Thanks for digging in! I know it's counter-intuitive at first, but the file is necessary in order to get the sub1/ -> sub3/ rename. The reasoning is as follows: We don't need to detect a directory rename for a directory where the other side added no new files into that directory...because the whole point of directory renames is to move new files in a directory to the new location. Therefore, no new files in the directory on one side, means no need to detect a directory rename for it on the other side. For a deeper discussion of this, see commit c64432aacd (t6423: more involved rules for renaming directories into each other, 2020-10-15).
Show 46 quoted lines
>, but it somehow changes the output. When I apply this diff
> (to the state after all patches are applied):
>
> diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh
> index 4286ae987c..9472fb7233 100755
> --- a/t/t6423-merge-rename-directories.sh
> +++ b/t/t6423-merge-rename-directories.sh
> @@ -5237,7 +5237,6 @@ test_setup_12l () {
>
> git checkout B &&
> echo dissimilar >sub2/new_add_add_file &&
> - echo brand >sub1/newfile &&
> git add sub1 sub2 &&
> git mv sub2 sub1 &&
> test_tick &&
> @@ -5255,14 +5254,14 @@ test_expect_merge_algorithm failure success '12l (B into A): Rename into each ot
> test_must_fail git -c merge.directoryRenames=true merge -s recursive B^0 &&
>
> git ls-files -s >out &&
> - test_line_count = 5 out &&
> + test_line_count = 4 out &&
>
> git rev-parse >actual \
> - :0:sub3/file :0:sub3/newfile :0:sub3/sub2/other \
> + :0:sub3/file :0:sub3/sub2/other \
> :2:sub1/sub2/new_add_add_file \
> :3:sub1/sub2/new_add_add_file &&
> git rev-parse >expect \
> - O:sub1/file B:sub1/newfile O:sub2/other \
> + O:sub1/file O:sub2/other \
> A:sub2/new_add_add_file \
> B:sub1/sub2/new_add_add_file &&
> test_cmp expect actual &&
>
> I get:
>
> .
> |-- sub1
> | `-- sub2
> | |-- new_add_add_file
> | `-- other
> `-- sub3
> `-- file
>
> (Note the path to "other".) I haven't figured out what's going on,
> though.Yeah, this is the result of having no directory rename due to having no new files that need to be moved by a directory rename.