Re: [PATCH 11/30] directory rename detection: testcases exploring possibly suboptimal merges

[Elijah Newren <newren@gmail.com>]

On Tue, Nov 14, 2017 at 12:33 PM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Nov 10, 2017 at 11:05 AM, Elijah Newren <newren@gmail.com> wrote:

>> +# Possible Resolutions:
>> +#   Previous git: y/{a,b,f},   z/{c,d},   x/e
>> +#   Expected:     y/{a,b,e,f}, z/{c,d}
>> +#   Preferred:    y/{a,b,e},   z/{c,d,f}
>
> it might be tricky in the future to know what "previous git" is;
> "Previous git" means without directory renames enabled;
>
> "expected" means we expect the algorithm presented in this series to produce
> this output, preferred is what we actually expect.

Yes, how about using:
  "Without dir rename detection:"
  "Currently expected:"
and
  "Optimal:"
?

>> +# Testcase 8b, Dual-directory rename, one into the others' way, with conflicting filenames
>> +#   Commit A. x/{a_1,b_1},     y/{a_2,b_2}
>> +#   Commit B. x/{a_1,b_1,e_1}, y/{a_2,b_2,e_2}
>> +#   Commit C. y/{a_1,b_1},     z/{a_2,b_2}
>> +#
>> +# Possible Resolutions:
>> +#   Previous git: y/{a_1,b_1,e_2}, z/{a_2,b_2}, x/e_1
>> +#   Scary:        y/{a_1,b_1},     z/{a_2,b_2}, CONFLICT(add/add, e_1 vs. e_2)
>> +#   Preferred:    y/{a_1,b_1,e_1}, z/{a_2,b_2,e_2}
>
> It may be common to have sub directories with the same path having different
> blobs, e.g. when having say multiple hardware configurations in different sub
> directories configured. Then renaming becomes a pain when they overlap.

Sure, agreed.  Although, the one nice thing about this particular
testcase is that despite showing suboptimal merge behavior, it's at
least the exact same suboptimal behavior as before when we didn't have
directory rename detection.

>> +# moves directories.  Implment directory rename detection suboptimally, and
>
> Implement

Thanks.

> ok, so add "Expected" as well? (repeating "Previous git", or so?)

Yeah, I should make that more explicit.

>> +# Testcase 8d, rename/delete...or not?
>> +#   (Related to testcase 5b; these may appear slightly inconsistent to users;
>> +#    Also related to testcases 7d and 7e)
>
>> +#   Commit A: z/{b,c,d}
>> +#   Commit B: y/{b,c}
>> +#   Commit C: z/{b,c,d,e}
>> +#   Expected: y/{b,c,e}
>
> Why this?
> * d is deleted in B and not found in the result
> * the rename detection also worked well in z->y  for adding e
>
> I do not see the confusion, yet.

Um...yaay?  If you don't see it as confusing, then maybe others don't?
 I was wondering if folks would expect a rename/delete conflict (x/d
either deleted or renamed to y/d via directory rename detection), and
be annoyed if the merge succeeded and didn't even give so much as a
warning about what happened to 'd'.

>> +#   In this case, I'm leaning towards: commit B was the one that deleted z/d
>> +#   and it did the rename of z to y, so the two "conflicts" (rename vs.
>> +#   delete) are both coming from commit B, which is non-sensical.  Conflicts
>> +#   during merging are supposed to be about opposite sides doing things
>> +#   differently.
>
>   "Sensical has not yet become an "official" word in the English language, which
>   would be why you can't use it. Nonsense is a word, therefore nonsensical can
>   used to describe something of nonsense. However, sense has different meanings
>   and doesn't have an adjective for something of sense"
>
> from https://english.stackexchange.com/questions/38582/antonym-of-nonsensical
> I don't mind it, the spell checker just made me go on a detour. Maybe illogical?

Illogical works for me.

>> +# Testcase 8e, Both sides rename, one side adds to original directory
>> +#   Commit A: z/{b,c}
>> +#   Commit B: y/{b,c}
>> +#   Commit C: w/{b,c}, z/d
>> +#
>> +# Possible Resolutions:
>> +#   Previous git: z/d, CONFLICT(z/b -> y/b vs. w/b), CONFLICT(z/c -> y/c vs. w/c)
>> +#   Expected:     y/d, CONFLICT(z/b -> y/b vs. w/b), CONFLICT(z/c -> y/c vs. w/c)
>> +#   Preferred:    ??
>> +#
>> +# Notes: In commit B, directory z got renamed to y.  In commit C, directory z
>> +#        did NOT get renamed; the directory is still present; instead it is
>> +#        considered to have just renamed a subset of paths in directory z
>> +#        elsewhere.  Therefore, the directory rename done in commit B to z/
>> +#        applies to z/d and maps it to y/d.
>> +#
>> +#        It's possible that users would get confused about this, but what
>> +#        should we do instead?   Silently leaving at z/d seems just as bad or
>> +#        maybe even worse.  Perhaps we could print a big warning about z/d
>> +#        and how we're moving to y/d in this case, but when I started thinking
>> +#        abouty the ramifications of doing that, I didn't know how to rule out
>> +#        that opening other weird edge and corner cases so I just punted.
>
> s/about/abouty

I think you mean the other direction?  Thanks for catching, I'll fix that up.

> It sort of makes sense from a users POV.

I'm afraid I'm unsure what the antecedent of "It" is here.  (Are you
just saying that my rationale for what I listed as "Expected" makes
sense, or something else?)