summaryrefslogtreecommitdiff
path: root/t/t6042-merge-rename-corner-cases.sh
AgeCommit message (Collapse)AuthorFilesLines
2018-11-08merge-recursive: fix rename/add conflict handlingLibravatar Elijah Newren1-2/+2
This makes the rename/add conflict handling make use of the new handle_file_collision() function, which fixes several bugs and improves things for the rename/add case significantly. Previously, rename/add would: * Not leave any higher order stage entries in the index, making it appear as if there were no conflict. * Would place the rename file at the colliding path, and move the added file elsewhere, which combined with the lack of higher order stage entries felt really odd. It's not clear to me why the rename should take precedence over the add; if one should be moved out of the way, they both probably should. * In the recursive case, it would do a two way merge of the added file and the version of the renamed file on the renamed side, completely excluding modifications to the renamed file on the unrenamed side of history. Use the new handle_file_collision() to fix all of these issues. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-08t6036, t6042: testcases for rename collision of already conflicting filesLibravatar Elijah Newren1-0/+118
When a single file is renamed, it can also be modified, yielding the possibility of that renamed file having content conflicts. If two different such files are renamed into the same location, then two-way merging those files may result in nested conflicts. Add a testcase that makes sure we get this case correct, and uses different lengths of conflict markers to differentiate between the different nestings. Also add another case with an extra (i.e. third) level of conflict markers due to using merge.conflictstyle=diff3 and the virtual merge base also having conflicts present. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-08t6042: add tests for consistency in file collision conflict handlingLibravatar Elijah Newren1-0/+162
Add testcases dealing with file collisions for the following types of conflicts: * add/add * rename/add * rename/rename(2to1) All these conflict types simplify down to two files "colliding" and should thus be handled similarly. This means that rename/add and rename/rename(2to1) conflicts need to be modified to behave the same as add/add conflicts currently do: the colliding files should be two-way merged (instead of the current behavior of writing the two colliding files out to separate temporary unique pathnames). Add testcases which check this; subsequent commits will fix the conflict handling to make these tests pass. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-17Merge branch 'en/abort-df-conflict-fixes'Libravatar Junio C Hamano1-1/+0
"git merge --abort" etc. did not clean things up properly when there were conflicted entries in the index in certain order that are involved in D/F conflicts. This has been corrected. * en/abort-df-conflict-fixes: read-cache: fix directory/file conflict handling in read_index_unmerged() t1015: demonstrate directory/file conflict recovery failures
2018-08-02Merge branch 'en/t6042-insane-merge-rename-testcases'Libravatar Junio C Hamano1-0/+245
Various glitches in the heuristics of merge-recursive strategy have been documented in new tests. * en/t6042-insane-merge-rename-testcases: t6042: add testcase covering long chains of rename conflicts t6042: add testcase covering rename/rename(2to1)/delete/delete conflict t6042: add testcase covering rename/add/delete conflict type
2018-07-31read-cache: fix directory/file conflict handling in read_index_unmerged()Libravatar Elijah Newren1-2/+0
read_index_unmerged() has two intended purposes: * return 1 if there are any unmerged entries, 0 otherwise * drops any higher-stage entries down to stage #0 There are several callers of read_index_unmerged() that check the return value to see if it is non-zero, all of which then die() if that condition is met. For these callers, dropping higher-stage entries down to stage #0 is a waste of resources, and returning immediately on first unmerged entry would be better. But it's probably only a very minor difference and isn't the focus of this series. The remaining callers ignore the return value and call this function for the side effect of dropping higher-stage entries down to stage #0. As mentioned in commit e11d7b596970 ("'reset --merge': fix unmerged case", 2009-12-31), The _only_ reason we want to keep a previously unmerged entry in the index at stage #0 is so that we don't forget the fact that we have corresponding file in the work tree in order to be able to remove it when the tree we are resetting to does not have the path. In fact, prior to commit d1a43f2aa4bf ("reset --hard/read-tree --reset -u: remove unmerged new paths", 2008-10-15), read_index_unmerged() did just remove unmerged entries from the cache immediately but that had the unwanted effect of leaving around new untracked files in the tree from aborted merges. So, that's the intended purpose of this function. The problem is that when directory/files conflicts are present, trying to add the file to the index at stage 0 fails (because there is still a directory in the way), and the function returns early with a -1 return code to signify the error. As noted above, none of the callers who want the drop-to-stage-0 behavior check the return status, though, so this means all remaining unmerged entries remain in the index and the callers proceed assuming otherwise. Users then see errors of the form: error: 'DIR-OR-FILE' appears as both a file and as a directory error: DIR-OR-FILE: cannot drop to stage #0 and potentially also messages about other unmerged entries which came lexicographically later than whatever pathname was both a file and a directory. Google finds a few hits searching for those messages, suggesting there were probably a couple people who hit this besides me. Luckily, calling `git reset --hard` multiple times would workaround this bug. Since the whole purpose here is to just put the entry *temporarily* into the index so that any associated file in the working copy can be removed, we can just skip the DFCHECK and allow both the file and directory to appear in the index. The temporary simultaneous appearance of the directory and file entries in the index will be removed by the callers by calling unpack_trees(), which excludes these unmerged entries marked with CE_CONFLICTED flag from the resulting index, before they attempt to write the index anywhere. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-16t6000-t6999: fix broken &&-chainsLibravatar Eric Sunshine1-4/+4
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03t6042: add testcase covering long chains of rename conflictsLibravatar Elijah Newren1-0/+111
Each rename is a lego: the source side could be connected to a delete or another rename, and the destination side could be connected to a rename or a conflicting add. Previous tests combined these to get e.g. rename/rename(1to2)/add/add, rename/rename(2to1)/delete/delete, and rename/add/delete. But we can also build bigger chains of conflicts. Add a testcase demonstrating this. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03t6042: add testcase covering rename/rename(2to1)/delete/delete conflictLibravatar Elijah Newren1-0/+68
If either side of a rename/rename(2to1) conflict is itself also involved in a rename/delete conflict, then the conflict is a little more complex; we can even have what I'd call a rename/rename(2to1)/delete/delete conflict. (In some ways, this is similar to a rename/rename(1to2)/add/add conflict, as added in commit 3672c9714830 ("merge-recursive: Fix working copy handling for rename/rename/add/add", 2011-08-11)). Add a testcase for such a conflict. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03t6042: add testcase covering rename/add/delete conflict typeLibravatar Elijah Newren1-0/+66
If a file is renamed on one side of history, and the other side of history both deletes the original file and adds a new unrelated file in the way of the rename, then we have what I call a rename/add/delete conflict. Add a testcase covering this scenario. Reported-by: Robert Dailey <rcdailey.lists@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-28t6036, t6042: prefer test_cmp to sequences of testLibravatar Elijah Newren1-35/+59
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-28t6036, t6042: prefer test_path_is_file, test_path_is_missingLibravatar Elijah Newren1-20/+20
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-28t6036, t6042: use test_line_count instead of wc -lLibravatar Elijah Newren1-33/+66
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-28t6036, t6042: use test_create_repo to keep tests independentLibravatar Elijah Newren1-413/+474
These tests used pretty strong measures to get a clean slate: git rm -rf . && git clean -fdqx && rm -rf .git && git init && It's easier, safer (what if a previous test has a bug and accidentally changes into a directory outside the test path?), and allows re-inspecting test setup later if we instead just use test_create_repo to put different tests into separate sub-repositories. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-07-26i18n: merge-recursive: mark strings for translationLibravatar Jiang Xin1-1/+1
Mark strings in merge-recursive for translation. Some tests would start to fail with GETTEXT_POISON turned on after this update. Use test_i18ncmp and test_i18ngrep where appropriate to mark strings that should only be checked in the C locale output to avoid such issues. Signed-off-by: Jiang Xin <worldhello.net@gmail.com> Reviewed-by: Stefano Lattarini <stefano.lattarini@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-11tests: modernise style: more uses of test_line_countLibravatar Stefano Lattarini1-1/+1
Prefer: test_line_count <OP> COUNT FILE over: test $(wc -l <FILE) <OP> COUNT (or similar usages) in several tests. Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-08-14merge-recursive: Fix working copy handling for rename/rename/add/addLibravatar Elijah Newren1-1/+10
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>
2011-08-14merge-recursive: add handling for rename/rename/add-dest/add-destLibravatar Elijah Newren1-1/+1
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>
2011-08-14merge-recursive: Consider modifications in rename/rename(2to1) conflictsLibravatar Elijah Newren1-1/+1
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>
2011-08-14merge-recursive: Provide more info in conflict markers with file renamesLibravatar Elijah Newren1-1/+1
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>
2011-08-14merge-recursive: Improve handling of rename target vs. directory additionLibravatar Elijah Newren1-1/+1
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>
2011-08-14merge-recursive: Fix deletion of untracked file in rename/delete conflictsLibravatar Elijah Newren1-1/+1
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>
2011-08-14t6042: Add failing testcases for rename/rename/add-{source,dest} conflictsLibravatar Elijah Newren1-0/+125
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>
2011-08-14t6042: Ensure rename/rename conflicts leave index and workdir in sane stateLibravatar Elijah Newren1-0/+102
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>
2011-08-14t6042: Add tests for content issues with modify/rename/directory conflictsLibravatar Elijah Newren1-0/+141
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>
2011-08-14t6042: Add a testcase where undetected rename causes silent file deletionLibravatar Elijah Newren1-0/+65
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>
2011-08-14t6042: Add a pair of cases where undetected renames cause issuesLibravatar Elijah Newren1-0/+61
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>
2011-08-14t6042: Add failing testcase for rename/modify/add-source conflictLibravatar Elijah Newren1-0/+39
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>
2011-08-14t6042: Add a testcase where git deletes an untracked fileLibravatar Elijah Newren1-0/+36
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>