Re: [PATCH 07/48] t6039: Ensure rename/rename conflicts leave index and workdir in sane state

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 18, 2011, 23:40 UTC
Message-ID: <7vaacbb1jn.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1307518278-23814-8-git-send-email-newren@gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 22 quoted lines

> +test_expect_success 'setup simple rename/rename (1to2) conflict' '
> +	git rm -rf . &&
> +	git clean -fdqx &&
> +	rm -rf .git &&
> +	git init &&
> +
> +	echo stuff >a &&
> +	git add a &&
> +	test_tick &&
> +	git commit -m A &&
> +	git tag A &&
> +
> +	git checkout -b B A &&
> +	git mv a b &&
> +	test_tick &&
> +	git commit -m B &&
> +
> +	git checkout -b C A &&
> +	git mv a c &&
> +	test_tick &&
> +	git commit -m C
> +'

This looks like a simpler variant of the last test in 06/48; shouldn't it come before the more complex one?

Show 13 quoted lines

> +
> +test_expect_success 'merge has correct working tree contents' '
> +	git checkout C^0 &&
> +
> +	test_must_fail git merge -s recursive B^0 &&
> +
> +	test 3 -eq $(git ls-files -s | wc -l) &&
> +	test 3 -eq $(git ls-files -u | wc -l) &&
> +	test 0 -eq $(git ls-files -o | wc -l) &&
> +
> +	test -f b &&
> +	test -f c
> +'

This test is being a bit too lazy, compared to the better ones in 06/48, no? What do we want to see here? Perhaps

 - "a" at stage #1 with "stuff", not in the working tree;
 - "b" at stage #3 with "stuff", same in the working tree; and
 - "c" at stage #2 with "stuff", same in the working tree

Show 11 quoted lines

> +# Test for all kinds of things that can go wrong with rename/rename (2to1):
> +#   Commit A: new files: a & b
> +#   Commit B: rename a->c, modify b
> +#   Commit C: rename b->c, modify a
> +#
> +# Merging of B & C should NOT be clean.  Questions:
> +#   * Both a & b should be removed by the merge; are they?
> +#   * The two c's should contain modifications to a & b; do they?
> +#   * The index should contain two files, both for c; does it?
> +#   * The working copy should have two files, both of form c~<unique>; does it?
> +#   * Nothing else should be present.  Is anything?

What is the most useful thing to leave in the index and in the working tree for the person who needs to resolve such a merge using the working tree, starting from B and merging C? The above "Questions" lists what the current code might try to do but I am not sure if it is really useful. For example, in the index, you would have to stuff two stage #1 entries ("a" from A and "b" from A) for path "c", with stage #2 ("c" from B) and stage #3 ("c" from C) entries, and represent what B tried to do to "a" (in the above you said "rename a->c" but it does not have to be a rename without content change) and what C tried to do to "b" in the half-conflicted result that is in a single file "c". Because the result are totally unrelated files (one side wants a variant of original "a" there, the other side wants a variant of "b"), such a half-merge result is totally useless to help the person to come up with anything.

Also renaming "c" to "c~<unique>", if they do not have corresponding entries in the index to let you check with "git diff", would make the result _harder_ to use, not easier. So if you are going to rename "c" to "c-B" and "c-C", at least it would make much more sense to have in the index:

 - "c-B", with stage #1 ("a" from A), stage #2 ("c" from B) and stage #3
   ("a" from C);
 - "c-C", with stage #1 ("b" from A), stage #2 ("b" from B) and stage #3
   ("c" from C); and
 - No "a" nor "b" in the index nor in the working tree.

no?

That way, you could run "git diff" to get what happened to the two variants of "a" and "b" at the content level, and decide to clean things up with:

    $ git diff ;# view content level merge
    $ edit c-B c-C; git add c-B c-C
    $ git mv c-B c-some-saner-name
    $ git mv c-C c-another-saner-name
    $ edit other files that refer to c like Makefile
    $ git commit

To take it one step further to the extreme, it might give us a more reasonable and useful conflicted state if we deliberately dropped some information instead in a case like this, e.g.:

 - We may want to have "a" at stage #1 (from A) in the index;
 - No "a" remains in the working tree;
 - "b" at stage #1 (from A), stage #2 (from B) and stage #3 ("c" from C);
 - "b" in the working tree a conflicted-merge of the above three;
 - "c" at stage #1 ("a" from A), stage #2 (from B), and stage #3 ("a" from
   C); and
 - "c" in the working tree a conflicted-merge of the above three.

Note that unlike the current merge-recursive that tries to come up with a temporary pathname to store both versions of C, this would ignore "mv b c" on the A->C branch, and make the conflicted tentative merge asymmetric (merging B into C and merging C into B would give different conflicts), but I suspect that the asymmetry may not hurt us.

Whether the merger wants to keep "c" that was derived from "a" (in line with the HEAD) or "c" that was derived from "b" (in line with MERGE_HEAD), if the result were to keep both files in some shape, the content level edit, renaming of at least one side, and adjusting other files that refer to it, are all required anyway, e.g.

    $ git diff ;# view content level merge
    $ edit b c; git add b c
    $ edit other files that refer to c line Makefile (the content C's
      change wants is now in "b").
    $ git commit

would be a way to pick "c" as "c-some-saner-name" and "b" as "c-another-saner-name" in the previous workflow, but needs much less typing. The complexity of the workflow would be the same if the final resolution is to take what one side did and dropping the other's work, I think.