If either side of a rename/rename(1to2) conflict is itself also involved
in a rename/add-dest conflict, then we need to make sure both the rename
and the added file appear in the working copy.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Each side of the rename in rename/rename(1to2) could potentially also be
involved in a rename/add conflict. Ensure stages for such conflicts are
also recorded.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our previous conflict resolution for renaming two different files to the
same name ignored the fact that each of those files may have modifications
from both sides of history to consider. We need to do a three-way merge
for each of those files, and then handle the conflict of both sets of
merged contents trying to be recorded with the same name.
It is important to note that this changes our strategy in the recursive
case. After doing a three-way content merge of each of the files
involved, we still are faced with the fact that we are trying to put both
of the results (including conflict markers) into the same path. We could
do another two-way merge, but I think that becomes confusing. Also,
taking a hint from the modify/delete and rename/delete cases we handled
earlier, a more useful "common ground" would be to keep the three-way
content merge but record it with the original filename. The renames can
still be detected, we just allow it to be done in the o->call_depth=0
case. This seems to result in simpler & easier to understand merge
conflicts as well, as evidenced by some of the changes needed in our
testsuite in t6036. (However, it should be noted that this change will
cause problems those renames also occur along with a file being added
whose name matches the source of the rename. Since git currently cannot
detect rename/add-source situations, though, this codepath is not
currently used for those cases anyway.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Whenever there are merge conflicts in file contents, we would mark the
different sides of the conflict with the two branches being merged.
However, when there is a rename involved as well, the branchname is not
sufficient to specify where the conflicting content came from. In such
cases, mark the two sides of the conflict with branchname:filename rather
than just branchname.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When dealing with file merging and renames and D/F conflicts and possible
criss-cross merges (how's that for a corner case?), we did not do a
thorough job ensuring the index and working directory had the correct
contents. Fix the logic in merge_content() to handle this. Also,
correct some erroneous tests in t6022 that were expecting the wrong number
of unmerged index entries. These changes fix one of the tests in t6042
(and almost fix another one from t6042 as well).
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the recursive case (o->call_depth > 0), we do not modify the working
directory. However, when o->call_depth==0, file renames can mean we need
to delete the old filename from the working copy. Since there have been
lots of changes and mistakes here, let's go through the details. Let's
start with a simple explanation of what we are trying to achieve:
Original goal: If a file is renamed on the side of history being merged
into head, the filename serving as the source of that rename needs to be
removed from the working directory.
The path to getting the above statement implemented in merge-recursive took
several steps. The relevant bits of code may be instructive to keep in
mind for the explanation, especially since an English-only description
involves double negatives that are hard to follow. These bits of code are:
int remove_file(..., const char *path, int no_wd)
{
...
int update_working_directory = !o->call_depth && !no_wd;
and
remove_file(o, 1, ren1_src, <expression>);
Where the choice for <expression> has morphed over time:
65ac6e9 (merge-recursive: adjust to loosened "working file clobbered"
check 2006-10-27), introduced the "no_wd" parameter to remove_file() and
used "1" for <expression>. This meant ren1_src was never deleted, leaving
it around in the working copy.
In 8371234 (Remove uncontested renamed files during merge. 2006-12-13),
<expression> was changed to "index_only" (where index_only ==
!!o->call_depth; see b7fa51da). This was equivalent to using "0" for
<expression> (due to the early logic in remove_file), and is orthogonal to
the condition we actually want to check at this point; it resulted in the
source file being removed except when index_only was false. This was
problematic because the file could have been renamed on the side of history
including head, in which case ren1_src could correspond to an untracked
file that should not be deleted.
In 183d797 (Keep untracked files not involved in a merge. 2007-02-04),
<expression> was changed to "index_only || stage == 3". While this gives
correct behavior, the "index_only ||" portion of <expression> is
unnecessary and makes the code slightly harder to follow.
There were also two further changes to this expression, though without
any change in behavior. First in b7fa51d (merge-recursive: get rid of the
index_only global variable 2008-09-02), it was changed to "o->call_depth
|| stage == 3". (index_only == !!o->call_depth). Later, in 41d70bd6
(merge-recursive: Small code clarification -- variable name and comments),
this was changed to "o->call_depth || renamed_stage == 2" (where stage was
renamed to other_stage and renamed_stage == other_stage ^ 1).
So we ended with <expression> being "o->call_depth || renamed_stage == 2".
But the "o->call_depth ||" piece was unnecessary. We can remove it,
leaving us with <expression> being "renamed_stage == 2". This doesn't
change behavior at all, but it makes the code clearer. Which is good,
because it's about to get uglier.
Corrected goal: If a file is renamed on the side of history being merged
into head, the filename serving as the source of that rename needs to be
removed from the working directory *IF* that file is tracked in head AND
the file tracked in head is related to the original file.
Note that the only difference between the original goal and the corrected
goal is the two extra conditions added at the end. The first condition is
relevant in a rename/delete conflict. If the file was deleted on the
HEAD side of the merge and an untracked file of the same name was added to
the working copy, then without that extra condition the untracked file
will be erroneously deleted. This changes <expression> to "renamed_stage
== 2 || !was_tracked(ren1_src)".
The second additional condition is relevant in two cases.
The first case the second condition can occur is when a file is deleted
and a completely different file is added with the same name. To my
knowledge, merge-recursive has no mechanism for detecting deleted-and-
replaced-by-different-file cases, so I am simply punting on this
possibility.
The second case for the second condition to occur is when there is a
rename/rename/add-source conflict. That is, when the original file was
renamed on both sides of history AND the original filename is being
re-used by some unrelated (but tracked) content. This case also presents
some additional difficulties for us since we cannot currently detect these
rename/rename/add-source conflicts; as long as the rename detection logic
"optimizes" by ignoring filenames that are present at both ends of the
diff, these conflicts will go unnoticed. However, rename/rename conflicts
are handled by an entirely separate codepath not being discussed here, so
this case is not relevant for the line of code under consideration.
In summary:
Change <expression> from "o->call_depth || renamed_stage == 2" to
"renamed_stage == 2 || !was_tracked(ren1_src)", in order to remove
unnecessary code and avoid deleting untracked files.
96 lines of explanation in the changelog to describe a one-line fix...
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add testcases that cover three failures with current git merge, all
involving renaming one file on both sides of history:
Case 1:
If a single file is renamed to two different filenames on different sides
of history, there should be a conflict. Adding a new file on one of those
sides of history whose name happens to match the rename source should not
cause the merge to suddenly succeed.
Case 2:
If a single file is renamed on both sides of history but renamed
identically, there should not be a conflict. This works fine. However,
if one of those sides also added a new file that happened to match the
rename source, then that file should be left alone. Currently, the
rename/rename conflict handling causes that new file to become untracked.
Case 3:
If a single file is renamed to two different filenames on different sides
of history, there should be a conflict. This works currently. However,
if those renames also involve rename/add conflicts (i.e. there are new
files on one side of history that match the destination of the rename of
the other side of history), then the resulting conflict should be recorded
in the index, showing that there were multiple files with a given filename.
Currently, git silently discards one of file versions.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
rename/rename conflicts, both with one file being renamed to two different
files and with two files being renamed to the same file, should leave the
index and the working copy in a sane state with appropriate conflict
recording, auxiliary files, etc. Git seems to handle one of the two cases
alright, but has some problems with the two files being renamed to one
case. Add tests for both cases.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add testcases that cover a variety of merge issues with files being
renamed and modified on different sides of history, when there are
directories possibly conflicting with the rename location.
Case 1:
On one side of history, a file is modified and a new directory is added.
On the other side of history, the file is modified in a non-conflicting
way but is renamed to the location of the new directory.
Case 2:
[Same as case 1, but there is also a content conflict. In detail:]
On one side of history, a file is modified and a new directory is added.
On the other side of history, the file is modified in a conflicting way
and it is renamed to the location of the new directory.
Case 3:
[Similar to case 1, but the "conflicting" directory is the directory
where the file original resided. In detail:]
On one side of history, a file is modified. On the other side of history,
the file is modified in a non-conflicting way, but the directory it was
under is removed and the file is renamed to the location of the directory
it used to reside in (i.e. 'sub/file' gets renamed to 'sub'). This is
flagged as a directory/rename conflict, but should be able to be resolved
since the directory can be cleanly removed by the merge.
One branch renames a file and makes a file where the directory the renamed
file used to be in, and the other branch updates the file in
place. Merging them should resolve it cleanly as long as the content level
change on the branches do not overlap and rename is detected, or should
leave conflict without losing information.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are cases where history should merge cleanly, and which current git
does merge cleanly despite not detecting a rename; however the merge
currently nukes files that should not be removed.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An undetected rename can cause a silent success where a conflict should
have been detected, or can cause an erroneous conflict state where the
merge should have been resolvable. Add testcases for both.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
If there is a cleanly resolvable rename/modify conflict AND there is a new
file introduced on the renamed side of the merge whose name happens to
match that of the source of the rename (but is otherwise unrelated to the
rename), then git fails to cleanly resolve the merge despite the fact that
the new file should not cause any problems.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Current git will nuke an untracked file during a rename/delete conflict if
(a) there is an untracked file whose name matches the source of a rename
and (b) the merge is done in a certain direction. Add a simple testcase
demonstrating this bug.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
