Web lists-archives.com

Re: [PATCH 10/30] directory rename detection: more involved edge/corner testcases




On Fri, Nov 10, 2017 at 11:05 AM, Elijah Newren <newren@xxxxxxxxx> wrote:

> +# In my opinion, testcases that are difficult to understand from this
> +# section is due to difficulty in the testcase rather than the directory
> +# renaming (similar to how t6042 and t6036 have difficult resolutions due
> +# to the problem setup itself being complex).  And I don't think the
> +# error messages are a problem.

"In my opinion" ... sounds like commit message?

> +# On the other hand, the testcases in section 8 worry me slightly more...

The aforementioned section 8... :)

> +# Testcase 7a, rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file
> +#   Commit A: z/{b,c}
> +#   Commit B: y/{b,c}
> +#   Commit C: w/b, x/c, z/d
> +#   Expected: y/d, CONFLICT(rename/rename for both z/b and z/c)
> +#   NOTE: There's a rename of z/ here, y/ has more renames, so z/d -> y/d.

But the creator of C intended to have z/d, not {w,x}/d, and as {w,x} == y,
I am not sure I like this result. (I have no concrete counter example, just
messy logic)


> +# Testcase 7b, rename/rename(2to1), but only due to transitive rename
> +#   (Related to testcase 1d)
> +#   Commit A: z/{b,c},     x/d_1, w/d_2
> +#   Commit B: y/{b,c,d_2}, x/d_1
> +#   Commit C: z/{b,c,d_1},        w/d_2
> +#   Expected: y/{b,c}, CONFLICT(rename/rename(2to1): x/d_1, w/d_2 -> y_d)

Makes sense.

> +# Testcase 7c, rename/rename(1to...2or3); transitive rename may add complexity
> +#   (Related to testcases 3b and 5c)
> +#   Commit A: z/{b,c}, x/d
> +#   Commit B: y/{b,c}, w/d
> +#   Commit C: z/{b,c,d}
> +#   Expected: y/{b,c}, CONFLICT(x/d -> w/d vs. y/d)

CONFLICT(x/d -> y/d vs w/d) ?

> +#   NOTE: z/ was renamed to y/ so we do not want to report
> +#         either CONFLICT(x/d -> w/d vs. z/d)
> +#         or CONFLiCT x/d -> w/d vs. y/d vs. z/d)

"neither ... nor" instead of "not either or"?

> +# Testcase 7d, transitive rename involved in rename/delete; how is it reported?
> +#   (Related somewhat to testcases 5b and 8d)
> +#   Commit A: z/{b,c}, x/d
> +#   Commit B: y/{b,c}
> +#   Commit C: z/{b,c,d}
> +#   Expected: y/{b,c}, CONFLICT(delete x/d vs rename to y/d)
> +#   NOTE: z->y so NOT CONFLICT(delete x/d vs rename to z/d)


> +# Testcase 7e, transitive rename in rename/delete AND dirs in the way
> +#   (Very similar to 'both rename source and destination involved in D/F conflict' from t6022-merge-rename.sh)
> +#   (Also related to testcases 9c and 9d)
> +#   Commit A: z/{b,c},     x/d_1
> +#   Commit B: y/{b,c,d/g}, x/d/f
> +#   Commit C: z/{b,c,d_1}
> +#   Expected: rename/delete(x/d_1->y/d_1 vs. None) + D/F conflict on y/d
> +#             y/{b,c,d/g}, y/d_1~C^0, x/d/f
> +#   NOTE: x/d/f may be slightly confusing here.  x/d_1 -> z/d_1 implies
> +#         there is a directory rename from x/ -> z/, performed by commit C.
> +#         However, on the side of commit B, it renamed z/ -> y/, thus
> +#         making a rename from x/ -> z/ when it was getting rid of z/ seems
> +#         non-sensical.  Further, putting x/d/f into y/d/f also doesn't
> +#         make a lot of sense because commit B did the renaming of z to y
> +#         and it created x/d/f, and it clearly made these things separate,
> +#         so it doesn't make much sense to push these together.

This is confusing.