* [PATCHv2 09/56] t6036: criss-cross with weird content can fool git into clean merge
From: Elijah Newren @ 2011-08-12 5:19 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
t/t6036-recursive-corner-cases.sh | 83 +++++++++++++++++++++++++++++++++++++
1 files changed, 83 insertions(+), 0 deletions(-)
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index 90b50bb..991c56d 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -314,4 +314,87 @@ test_expect_failure 'git detects conflict merging criss-cross+modify/delete, rev
test $(git rev-parse :3:file) = $(git rev-parse B:file)
'
+#
+# criss-cross + modify/modify with very contrived file contents:
+#
+# B D
+# o---o
+# / \ / \
+# A o X ? F
+# \ / \ /
+# o---o
+# C E
+#
+# Commit A: file with contents 'A\n'
+# Commit B: file with contents 'B\n'
+# Commit C: file with contents 'C\n'
+# Commit D: file with contents 'D\n'
+# Commit E: file with contents:
+# <<<<<<< Temporary merge branch 1
+# C
+# =======
+# B
+# >>>>>>> Temporary merge branch 2
+#
+# Now, when we merge commits D & E, does git detect the conflict?
+
+test_expect_success 'setup differently handled merges of content conflict' '
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ echo A >file &&
+ git add file &&
+ test_tick &&
+ git commit -m A &&
+
+ git branch B &&
+ git checkout -b C &&
+ echo C >file &&
+ git add file &&
+ test_tick &&
+ git commit -m C &&
+
+ git checkout B &&
+ echo B >file &&
+ git add file &&
+ test_tick &&
+ git commit -m B &&
+
+ git checkout B^0 &&
+ test_must_fail git merge C &&
+ echo D >file &&
+ git add file &&
+ test_tick &&
+ git commit -m D &&
+ git tag D &&
+
+ git checkout C^0 &&
+ test_must_fail git merge B &&
+ cat <<EOF >file &&
+<<<<<<< Temporary merge branch 1
+C
+=======
+B
+>>>>>>> Temporary merge branch 2
+EOF
+ git add file &&
+ test_tick &&
+ git commit -m E &&
+ git tag E
+'
+
+test_expect_failure 'git detects conflict w/ criss-cross+contrived resolution' '
+ git checkout D^0 &&
+
+ test_must_fail git merge -s recursive E^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 $(git rev-parse :2:file) = $(git rev-parse D:file) &&
+ test $(git rev-parse :3:file) = $(git rev-parse E:file)
+'
+
test_done
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 00/57] Re-roll of en/merge-recursive from pu
From: Elijah Newren @ 2011-08-12 5:19 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
This is a re-roll of en/merge-recursive from pu, fixing half a dozen
bugs (some pre-existing ones, some caused by my first series).
I think the series is now ready for wide consumption and should be
safe for the upcoming release, particularly due to some heavy
additional testing I performed. If others disagree, I'm fine and can
wait out another release cycle. Either way, I am open for suggestions
or ideas on additional testing people would like to see if they have
worries about the robustness of these changes.
*** Heavy additional testing I performed ***
Because it's so hard to rule out regressions with so many changes to a
complicated portion of the code (though hopefully it is less complicated
now), and because we've had multiple problems in the past with the
changes I've been making to merge-recursive, I came up with an idea to
test this series more thoroughly. So, I wrote a script to take every
single merge commit in git.git that had exactly two parents (no octopus
merges) and redid them both with /usr/bin/git and the version of git
from this series. I checked to ensure that the two different versions
of git:
(a) EITHER both failed to merge cleanly OR both merged cleanly
AND
(b) the output of 'git ls-tree -r HEAD' matched between the two
I ran this process with the original version of the series and indeed
found that my original series mis-merged half a dozen or so merges (out
of about 5000).
With this new version of the series, I get no such mis-merges, which
gives me relatively strong confidence that there are no merge
regressions relative to git-1.7.4.4 (the version I happened to have in
/usr/bin).
*** What's fixed ***
Everything that I claimed the original series fixed, is still fixed in
this series.
This series fixes the following bugs from the previous series:
* The mis-merge that Junio reported (which would have been found had I
been more thorough in the new testcases I myself had added -- oops)
* The windows bug identified by Johannes (at least I hope I fixed that
one -- I'll need him to verify)
* The potential incorrect deletion of a path that I reported (while
discussing the patch that Johannes flagged for other reasons)
* A fix to my more detailed conflict markers when renames are involved
(it sometimes swapped filenames between branches)
This series also fixes the following bugs that exist in current git and
had not been fixed by my previous series:
* The incorrectly-deleted file in the created virtual merge base problem
that Junio identified when rename/rename(1to2) conflicts also involved
rename/add-dest conflicts for one or more of the two renames. [That's
a mouthful, isn't it?]
* Sometimes git would erroneously print 'refusing to lose untracked file...'
warning messages.
* There are three more cases where files were needlessly getting rewritten
despite already having the correct contents (though none of these
cases involved content merges).
The fixes to problems in the original series were made as modifications to
the relevant patches. The new fixes were simply added to the end of the
series, in order to facilitate review.
*** What's changed ***
I took the series with Junio's modifications in pu, and made changes as
necessary to address his comments and other problems that were found. I
have left his signed-off-by on patches where no changes were made. Most
patches were unchanged; the full list of changed patches are:
[05] t6042: Add tests for content issues with modify/rename/directory conflicts
Changes: Fleshed out the commit message
[07] t6042: Ensure rename/rename conflicts leave index and workdir in sane state
Changes: Move this patch prior to the previous one (as suggested by
Junio), since it has a simpler rename/rename(1to2) case in
it. (Did not remove Junio's s-o-b, since this was a
trivial test-reorder change.)
[08] t6036: Add differently resolved modify/delete conflict in criss-cross test
Changes: Made wording change suggested by Junio. (Did not remove
Junio's s-o-b, since I took his exact suggestion.)
[10] t6036: tests for criss-cross merges with various directory/file conflicts
Changes: Lots of wording cleanups and making the tests more thorough
[11] t6036: criss-cross w/ rename/rename(1to2)/modify+rename/rename(2to1)/modify
Changes: More thorough testing of result, usage of some of Junio's
wording in a comment
[12] t6036: criss-cross + rename/rename(1to2)/add-source + modify/modify
Changes: More thorough testing of result, improvement on wording describing test
[15] t6022: Add testcase for merging a renamed file with a simple change
Changes: Fix to check actual result is correct; that would have caught the
bug Junio instead found the hard way.
[19] merge-recursive: Consolidate different update_stages functions
Changes: Moved earlier in the series, as per Junio's request; used
to be patch number 27. (Did not remove Junio's s-o-b,
since no significant changes were necessary.)
[23] merge-recursive: Fix sorting order and directory change assumptions
Changes: Mentioned Johannes' squashed-in changes and added his
signed-off-by from his squashed-in patch. (Did not remove
Junio's s-o-b, since he had added his to Johannes' patch.)
[27] string-list: Add API to remove an item from an unsorted list
Changes: This is an entirely new patch, recently submitted to the list by
Johannes.
[28] merge-recursive: Allow make_room_for_path() to remove D/F entries
Changes: (1) Made use of Johannes' new string-list API to remove
items we have already checked in order to avoid trying to
unlink a path more than once, (2) fixed bug with not
checking that basepath was directory
[36] merge-recursive: Provide more info in conflict markers with file renames
Changes: Fix bug where I could get filenames from two branches reversed
[37] merge-recursive: When we detect we can skip an update, actually skip it
Changes: Moved to later in the series (used to be patch #29). Also,
changed the "was_tracked" logic to use information about
what paths were involved in renames to determine whether
the file really 'was tracked' before the merge started.
Also, added another minor tweak to get the expected
"Skipped" messages that t6022.12 expects.
[51-56]
These are entirely new patches providing testcases and fixes for
bugs that exist with current git and were not fixed in my previous
series (but probably should have been).
*** What is still missing ***
Two things:
* Junio had a great suggestion about alternate handling of the index in the
case of rename/rename(2to1) and directory/file conflicts (just rename the
entry in the index to match how we are renaming in the working copy to
some new unique name, in order to allow 'git diff' to provide more useful
information to the user). Just didn't get to it.
* Support for running break detection in diffs, in order to fix the testcase
corrected by Peff in this series. Simply didn't get around to it either.
Neither of these two things represent regressions relative to what is in
current releases of git, however.
*** Full patch list and diffstat ***
Elijah Newren (54):
[01] t6042: Add a testcase where git deletes an untracked file
[02] t6042: Add failing testcase for rename/modify/add-source conflict
[03] t6042: Add a pair of cases where undetected renames cause issues
[04] t6042: Add a testcase where undetected rename causes silent file deletion
[05] t6042: Add tests for content issues with modify/rename/directory conflicts
[06] t6042: Ensure rename/rename conflicts leave index and workdir in sane state
[07] t6042: Add failing testcases for rename/rename/add-{source,dest} conflicts
[08] t6036: Add differently resolved modify/delete conflict in criss-cross test
[09] t6036: criss-cross with weird content can fool git into clean merge
[10] t6036: tests for criss-cross merges with various directory/file conflicts
[11] t6036: criss-cross w/ rename/rename(1to2)/modify+rename/rename(2to1)/modify
[12] t6036: criss-cross + rename/rename(1to2)/add-source + modify/modify
[13] t6022: Remove unnecessary untracked files to make test cleaner
[14] t6022: New tests checking for unnecessary updates of files
[15] t6022: Add testcase for merging a renamed file with a simple change
[16] merge-recursive: Make BUG message more legible by adding a newline
[17] merge-recursive: Correct a comment
[18] merge-recursive: Mark some diff_filespec struct arguments const
[19] merge-recursive: Consolidate different update_stages functions
[20] merge-recursive: Remember to free generated unique path names
[21] merge-recursive: Avoid working directory changes during recursive case
[22] merge-recursive: Fix recursive case with D/F conflict via add/add conflict
[23] merge-recursive: Fix sorting order and directory change assumptions
[24] merge-recursive: Fix code checking for D/F conflicts still being present
[25] merge-recursive: Save D/F conflict filenames instead of unlinking them
[26] merge-recursive: Split was_tracked() out of would_lose_untracked()
[28] merge-recursive: Allow make_room_for_path() to remove D/F entries
[29] merge-recursive: Split update_stages_and_entry; only update stages at end
[30] merge-recursive: Fix deletion of untracked file in rename/delete conflicts
[31] merge-recursive: Make dead code for rename/rename(2to1) conflicts undead
[32] merge-recursive: Add comments about handling rename/add-source cases
[33] merge-recursive: Improve handling of rename target vs. directory addition
[34] merge-recursive: Consolidate process_entry() and process_df_entry()
[35] merge-recursive: Cleanup and consolidation of rename_conflict_info
[36] merge-recursive: Provide more info in conflict markers with file renames
[37] merge-recursive: When we detect we can skip an update, actually skip it
[38] merge-recursive: Fix modify/delete resolution in the recursive case
[39] merge-recursive: Introduce a merge_file convenience function
[40] merge-recursive: Fix rename/rename(1to2) resolution for virtual merge base
[41] merge-recursive: Small cleanups for conflict_rename_rename_1to2
[42] merge-recursive: Defer rename/rename(2to1) handling until process_entry
[43] merge-recursive: Record more data needed for merging with dual renames
[44] merge-recursive: Create function for merging with branchname:file markers
[45] merge-recursive: Consider modifications in rename/rename(2to1) conflicts
[46] merge-recursive: Make modify/delete handling code reusable
[47] merge-recursive: Have conflict_rename_delete reuse modify/delete code
[48] merge-recursive: add handling for rename/rename/add-dest/add-dest
[49] merge-recursive: Fix working copy handling for rename/rename/add/add
[51] t6022: Add testcase for spurious "refusing to lose untracked" messages
[52] merge-recursive: Fix spurious 'refusing to lose untracked file...' messages
[53] t6022: Additional tests checking for unnecessary updates of files
[54] merge-recursive: Avoid unnecessary file rewrites
[55] t6042: Add testcase demonstrating missing file in virtual merge base
[56] merge-recursive: Fix virtual merge base for rename/rename(1to2)/add-dest
Jeff King (1):
[50] t3030: fix accidental success in symlink rename
Johannes Sixt (1):
[27] string-list: Add API to remove an item from an unsorted list
Documentation/technical/api-string-list.txt | 10 +
merge-recursive.c | 1067 +++++++++++++++++----------
merge-recursive.h | 1 +
string-list.c | 9 +
string-list.h | 1 +
t/t3030-merge-recursive.sh | 7 +-
t/t6020-merge-df.sh | 26 +-
t/t6022-merge-rename.sh | 291 +++++++-
t/t6036-recursive-corner-cases.sh | 598 ++++++++++++++-
t/t6042-merge-rename-corner-cases.sh | 625 ++++++++++++++++
10 files changed, 2207 insertions(+), 428 deletions(-)
create mode 100755 t/t6042-merge-rename-corner-cases.sh
--
1.7.6.100.gac5c1
^ permalink raw reply
* [PATCHv2 15/56] t6022: Add testcase for merging a renamed file with a simple change
From: Elijah Newren @ 2011-08-12 5:19 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
This is a testcase that was broken by b2c8c0a (merge-recursive: When we
detect we can skip an update, actually skip it 2011-02-28) and fixed by
6db4105 (Revert "Merge branch 'en/merge-recursive'" 2011-05-19). Include
this testcase to ensure we don't regress it again.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Changes since v1: Check that the result of the merge is actually
correct; had I done that in v1, I would have caught the bug that
Junio found the hard way.
t/t6022-merge-rename.sh | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 11c5c60..6ff4bd2 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -673,4 +673,31 @@ test_expect_failure 'avoid unnecessary update, with D/F conflict' '
test_cmp expect actual # "df" should have stayed intact
'
+test_expect_success 'setup merge of rename + small change' '
+ git reset --hard &&
+ git checkout --orphan rename-plus-small-change &&
+ git rm -rf . &&
+ git clean -fdqx &&
+
+ echo ORIGINAL >file &&
+ git add file &&
+
+ test_tick &&
+ git commit -m Initial &&
+ git checkout -b rename_branch &&
+ git mv file renamed_file &&
+ git commit -m Rename &&
+ git checkout rename-plus-small-change &&
+ echo NEW-VERSION >file &&
+ git commit -a -m Reformat
+'
+
+test_expect_success 'merge rename + small change' '
+ git merge rename_branch &&
+
+ test 1 -eq $(git ls-files -s | wc -l) &&
+ test 0 -eq $(git ls-files -o | wc -l) &&
+ test $(git rev-parse HEAD:renamed_file) = $(git rev-parse HEAD~1:file)
+'
+
test_done
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 07/56] t6042: Add failing testcases for rename/rename/add-{source,dest} conflicts
From: Elijah Newren @ 2011-08-12 5:19 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.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>
---
Changes since v1: moved this patch (and its content) prior to the
patch it used to follow (as suggested by Junio), since it has a
simpler rename/rename(1to2) case in it.
t/t6042-merge-rename-corner-cases.sh | 125 ++++++++++++++++++++++++++++++++++
1 files changed, 125 insertions(+), 0 deletions(-)
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index 371fb39..427fe1c 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -441,4 +441,129 @@ test_expect_success 'merge has correct working tree contents' '
test $(git hash-object c) = $(git rev-parse A:a)
'
+# Testcase setup for rename/rename(1to2)/add-source conflict:
+# Commit A: new file: a
+# Commit B: rename a->b
+# Commit C: rename a->c, add completely different a
+#
+# Merging of B & C should NOT be clean; there's a rename/rename conflict
+
+test_expect_success 'setup rename/rename(1to2)/add-source conflict' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ printf "1\n2\n3\n4\n5\n6\n7\n" >a &&
+ git add a &&
+ git commit -m A &&
+ git tag A &&
+
+ git checkout -b B A &&
+ git mv a b &&
+ git commit -m B &&
+
+ git checkout -b C A &&
+ git mv a c &&
+ echo something completely different >a &&
+ git add a &&
+ git commit -m C
+'
+
+test_expect_failure 'detect conflict with rename/rename(1to2)/add-source merge' '
+ git checkout B^0 &&
+
+ test_must_fail git merge -s recursive C^0 &&
+
+ test 4 -eq $(git ls-files -s | wc -l) &&
+ test 0 -eq $(git ls-files -o | wc -l) &&
+
+ test $(git rev-parse 3:a) = $(git rev-parse C:a) &&
+ test $(git rev-parse 1:a) = $(git rev-parse A:a) &&
+ test $(git rev-parse 2:b) = $(git rev-parse B:b) &&
+ test $(git rev-parse 3:c) = $(git rev-parse C:c) &&
+
+ test -f a &&
+ test -f b &&
+ test -f c
+'
+
+test_expect_success 'setup rename/rename(1to2)/add-source resolvable conflict' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ >a &&
+ git add a &&
+ test_tick &&
+ git commit -m base &&
+ git tag A &&
+
+ git checkout -b B A &&
+ git mv a b &&
+ test_tick &&
+ git commit -m one &&
+
+ git checkout -b C A &&
+ git mv a b &&
+ echo important-info >a &&
+ git add a &&
+ test_tick &&
+ git commit -m two
+'
+
+test_expect_failure 'rename/rename/add-source still tracks new a file' '
+ git checkout C^0 &&
+ git merge -s recursive B^0 &&
+
+ test 2 -eq $(git ls-files -s | wc -l) &&
+ test 0 -eq $(git ls-files -o | wc -l) &&
+
+ test $(git rev-parse HEAD:a) = $(git rev-parse C:a) &&
+ test $(git rev-parse HEAD:b) = $(git rev-parse A:a)
+'
+
+test_expect_success 'setup rename/rename(1to2)/add-dest conflict' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ echo stuff >a &&
+ git add a &&
+ test_tick &&
+ git commit -m base &&
+ git tag A &&
+
+ git checkout -b B A &&
+ git mv a b &&
+ echo precious-data >c &&
+ git add c &&
+ test_tick &&
+ git commit -m one &&
+
+ git checkout -b C A &&
+ git mv a c &&
+ echo important-info >b &&
+ git add b &&
+ test_tick &&
+ git commit -m two
+'
+
+test_expect_failure 'rename/rename/add-dest merge still knows about conflicting file versions' '
+ git checkout C^0 &&
+ test_must_fail git merge -s recursive B^0 &&
+
+ test 5 -eq $(git ls-files -s | wc -l) &&
+ test 2 -eq $(git ls-files -u b | wc -l) &&
+ test 2 -eq $(git ls-files -u c | wc -l) &&
+
+ test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
+ test $(git rev-parse :2:b) = $(git rev-parse C:b) &&
+ test $(git rev-parse :3:b) = $(git rev-parse B:b) &&
+ test $(git rev-parse :2:c) = $(git rev-parse C:c) &&
+ test $(git rev-parse :3:c) = $(git rev-parse B:c)
+'
+
test_done
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 20/56] merge-recursive: Remember to free generated unique path names
From: Elijah Newren @ 2011-08-12 5:19 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 8a5c1a6..b5a8f17 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1063,7 +1063,6 @@ static int process_renames(struct merge_options *o,
renamed: clean merge */
update_file(o, 1, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
} else if (!sha_eq(dst_other.sha1, null_sha1)) {
- const char *new_path;
clean_merge = 0;
try_merge = 1;
output(o, 1, "CONFLICT (rename/add): Rename %s->%s in %s. "
@@ -1092,9 +1091,10 @@ static int process_renames(struct merge_options *o,
ren1_dst);
try_merge = 0;
} else {
- new_path = unique_path(o, ren1_dst, branch2);
+ char *new_path = unique_path(o, ren1_dst, branch2);
output(o, 1, "Adding as %s instead", new_path);
update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
+ free(new_path);
}
} else if ((item = string_list_lookup(renames2Dst, ren1_dst))) {
ren2 = item->util;
@@ -1262,13 +1262,14 @@ static int merge_content(struct merge_options *o,
}
if (df_conflict_remains) {
- const char *new_path;
+ char *new_path;
update_file_flags(o, mfi.sha, mfi.mode, path,
o->call_depth || mfi.clean, 0);
new_path = unique_path(o, path, df_rename_conflict_branch);
mfi.clean = 0;
output(o, 1, "Adding as %s instead", new_path);
update_file_flags(o, mfi.sha, mfi.mode, new_path, 0, 1);
+ free(new_path);
} else {
update_file(o, mfi.clean, mfi.sha, mfi.mode, path);
}
@@ -1424,12 +1425,14 @@ static int process_df_entry(struct merge_options *o,
}
} else if (o_sha && (!a_sha || !b_sha)) {
/* Modify/delete; deleted side may have put a directory in the way */
- const char *new_path = path;
- if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode))
- new_path = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
+ char *renamed = NULL;
+ if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
+ renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
+ }
clean_merge = 0;
- handle_delete_modify(o, path, new_path,
+ handle_delete_modify(o, path, renamed ? renamed : path,
a_sha, a_mode, b_sha, b_mode);
+ free(renamed);
} else if (!o_sha && !!a_sha != !!b_sha) {
/* directory -> (directory, file) or <nothing> -> (directory, file) */
const char *add_branch;
@@ -1452,12 +1455,13 @@ static int process_df_entry(struct merge_options *o,
conf = "directory/file";
}
if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
- const char *new_path = unique_path(o, path, add_branch);
+ char *new_path = unique_path(o, path, add_branch);
clean_merge = 0;
output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
"Adding %s as %s",
conf, path, other_branch, path, new_path);
update_file(o, 0, sha, mode, new_path);
+ free(new_path);
} else {
output(o, 2, "Adding %s", path);
update_file(o, 1, sha, mode, path);
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 24/56] merge-recursive: Fix code checking for D/F conflicts still being present
From: Elijah Newren @ 2011-08-12 5:19 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Previously, we were using lstat() to determine if a directory was still
present after a merge (and thus in the way of adding a file). We should
have been using lstat() only to determine if untracked directories were in
the way (and then only when necessary to check for untracked directories);
we should instead using the index to determine if there is a tracked
directory in the way. Create a new function to do this and use it to
replace the existing checks for directories being in the way.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 49 ++++++++++++++++++++++++++++++++++---------------
1 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 76b895f..9aacf5e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -593,6 +593,30 @@ static void flush_buffer(int fd, const char *buf, unsigned long size)
}
}
+static int dir_in_way(const char *path, int check_working_copy)
+{
+ int pos, pathlen = strlen(path);
+ char *dirpath = xmalloc(pathlen + 2);
+ struct stat st;
+
+ strcpy(dirpath, path);
+ dirpath[pathlen] = '/';
+ dirpath[pathlen+1] = '\0';
+
+ pos = cache_name_pos(dirpath, pathlen+1);
+
+ if (pos < 0)
+ pos = -1 - pos;
+ if (pos < active_nr &&
+ !strncmp(dirpath, active_cache[pos]->name, pathlen+1)) {
+ free(dirpath);
+ return 1;
+ }
+
+ free(dirpath);
+ return check_working_copy && !lstat(path, &st) && S_ISDIR(st.st_mode);
+}
+
static int would_lose_untracked(const char *path)
{
int pos = cache_name_pos(path, strlen(path));
@@ -882,7 +906,6 @@ static void conflict_rename_delete(struct merge_options *o,
{
char *dest_name = pair->two->path;
int df_conflict = 0;
- struct stat st;
output(o, 1, "CONFLICT (rename/delete): Rename %s->%s in %s "
"and deleted in %s",
@@ -892,7 +915,7 @@ static void conflict_rename_delete(struct merge_options *o,
update_stages(dest_name, NULL,
rename_branch == o->branch1 ? pair->two : NULL,
rename_branch == o->branch1 ? NULL : pair->two);
- if (lstat(dest_name, &st) == 0 && S_ISDIR(st.st_mode)) {
+ if (dir_in_way(dest_name, !o->call_depth)) {
dest_name = unique_path(o, dest_name, rename_branch);
df_conflict = 1;
}
@@ -914,13 +937,12 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
const char *ren2_dst = pair2->two->path;
const char *dst_name1 = ren1_dst;
const char *dst_name2 = ren2_dst;
- struct stat st;
- if (lstat(ren1_dst, &st) == 0 && S_ISDIR(st.st_mode)) {
+ if (dir_in_way(ren1_dst, !o->call_depth)) {
dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren1_dst, branch2, dst_name1);
}
- if (lstat(ren2_dst, &st) == 0 && S_ISDIR(st.st_mode)) {
+ if (dir_in_way(ren2_dst, !o->call_depth)) {
dst_name2 = del[delp++] = unique_path(o, ren2_dst, branch2);
output(o, 1, "%s is a directory in %s adding as %s instead",
ren2_dst, branch1, dst_name2);
@@ -1080,7 +1102,7 @@ static int process_renames(struct merge_options *o,
try_merge = 0;
if (sha_eq(src_other.sha1, null_sha1)) {
- if (string_list_has_string(&o->current_directory_set, ren1_dst)) {
+ if (dir_in_way(ren1_dst, 0 /*check_wc*/)) {
ren1->dst_entry->processed = 0;
setup_rename_df_conflict_info(RENAME_DELETE,
ren1->pair,
@@ -1159,7 +1181,7 @@ static int process_renames(struct merge_options *o,
a = &src_other;
}
update_stages_and_entry(ren1_dst, ren1->dst_entry, one, a, b, 1);
- if (string_list_has_string(&o->current_directory_set, ren1_dst)) {
+ if (dir_in_way(ren1_dst, 0 /*check_wc*/)) {
setup_rename_df_conflict_info(RENAME_NORMAL,
ren1->pair,
NULL,
@@ -1264,7 +1286,6 @@ static int merge_content(struct merge_options *o,
const char *reason = "content";
struct merge_file_info mfi;
struct diff_filespec one, a, b;
- struct stat st;
unsigned df_conflict_remains = 0;
if (!o_sha) {
@@ -1281,7 +1302,7 @@ static int merge_content(struct merge_options *o,
mfi = merge_file(o, &one, &a, &b, o->branch1, o->branch2);
if (df_rename_conflict_branch &&
- lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
+ dir_in_way(path, !o->call_depth)) {
df_conflict_remains = 1;
}
@@ -1346,8 +1367,7 @@ static int process_entry(struct merge_options *o,
output(o, 2, "Removing %s", path);
/* do not touch working file if it did not exist */
remove_file(o, 1, path, !a_sha);
- } else if (string_list_has_string(&o->current_directory_set,
- path)) {
+ } else if (dir_in_way(path, 0 /*check_wc*/)) {
entry->processed = 0;
return 1; /* Assume clean until processed */
} else {
@@ -1370,7 +1390,7 @@ static int process_entry(struct merge_options *o,
mode = b_mode;
sha = b_sha;
}
- if (string_list_has_string(&o->current_directory_set, path)) {
+ if (dir_in_way(path, 0 /*check_wc*/)) {
/* Handle D->F conflicts after all subfiles */
entry->processed = 0;
return 1; /* Assume clean until processed */
@@ -1418,7 +1438,6 @@ static int process_df_entry(struct merge_options *o,
unsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);
unsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);
unsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);
- struct stat st;
entry->processed = 1;
if (entry->rename_df_conflict_info) {
@@ -1463,7 +1482,7 @@ static int process_df_entry(struct merge_options *o,
} else if (o_sha && (!a_sha || !b_sha)) {
/* Modify/delete; deleted side may have put a directory in the way */
char *renamed = NULL;
- if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
+ if (dir_in_way(path, !o->call_depth)) {
renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
}
clean_merge = 0;
@@ -1491,7 +1510,7 @@ static int process_df_entry(struct merge_options *o,
sha = b_sha;
conf = "directory/file";
}
- if (lstat(path, &st) == 0 && S_ISDIR(st.st_mode)) {
+ if (dir_in_way(path, !o->call_depth)) {
char *new_path = unique_path(o, path, add_branch);
clean_merge = 0;
output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 42/56] merge-recursive: Defer rename/rename(2to1) handling until process_entry
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
This puts the code for the different types of double rename conflicts
closer together (fewer lines of other code separating the two paths) and
increases similarity between how they are handled.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 104 +++++++++++++++++++++++++++++++---------------------
1 files changed, 62 insertions(+), 42 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 8b65051..77c2c41 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -67,7 +67,8 @@ enum rename_type {
RENAME_NORMAL = 0,
RENAME_DELETE,
RENAME_ONE_FILE_TO_ONE,
- RENAME_ONE_FILE_TO_TWO
+ RENAME_ONE_FILE_TO_TWO,
+ RENAME_TWO_FILES_TO_ONE
};
struct rename_conflict_info {
@@ -1021,32 +1022,40 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
}
static void conflict_rename_rename_2to1(struct merge_options *o,
- struct rename *ren1,
- const char *branch1,
- struct rename *ren2,
- const char *branch2)
+ struct rename_conflict_info *ci)
{
- char *path = ren1->pair->two->path; /* same as ren2->pair->two->path */
- /* Two files were renamed to the same thing. */
+ /* Two files, a & b, were renamed to the same thing, c. */
+ struct diff_filespec *a = ci->pair1->one;
+ struct diff_filespec *b = ci->pair2->one;
+ struct diff_filespec *c1 = ci->pair1->two;
+ struct diff_filespec *c2 = ci->pair2->two;
+ char *path = c1->path; /* == c2->path */
+
+ output(o, 1, "CONFLICT (rename/rename): "
+ "Rename %s->%s in %s. "
+ "Rename %s->%s in %s",
+ a->path, c1->path, ci->branch1,
+ b->path, c2->path, ci->branch2);
+
+ remove_file(o, 1, a->path, would_lose_untracked(a->path));
+ remove_file(o, 1, b->path, would_lose_untracked(b->path));
+
if (o->call_depth) {
struct merge_file_info mfi;
mfi = merge_file(o, path, null_sha1, 0,
- ren1->pair->two->sha1, ren1->pair->two->mode,
- ren2->pair->two->sha1, ren2->pair->two->mode,
- branch1, branch2);
+ c1->sha1, c1->mode,
+ c2->sha1, c2->mode,
+ ci->branch1, ci->branch2);
output(o, 1, "Adding merged %s", path);
update_file(o, 0, mfi.sha, mfi.mode, path);
} else {
- char *new_path1 = unique_path(o, path, branch1);
- char *new_path2 = unique_path(o, path, branch2);
+ char *new_path1 = unique_path(o, path, ci->branch1);
+ char *new_path2 = unique_path(o, path, ci->branch2);
output(o, 1, "Renaming %s to %s and %s to %s instead",
- ren1->pair->one->path, new_path1,
- ren2->pair->one->path, new_path2);
+ a->path, new_path1, b->path, new_path2);
remove_file(o, 0, path, 0);
- update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode,
- new_path1);
- update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode,
- new_path2);
+ update_file(o, 0, c1->sha1, c1->mode, new_path1);
+ update_file(o, 0, c2->sha1, c2->mode, new_path2);
free(new_path2);
free(new_path1);
}
@@ -1077,6 +1086,7 @@ static int process_renames(struct merge_options *o,
struct rename *ren1 = NULL, *ren2 = NULL;
const char *branch1, *branch2;
const char *ren1_src, *ren1_dst;
+ struct string_list_item *lookup;
if (i >= a_renames->nr) {
ren2 = b_renames->items[j++].util;
@@ -1108,30 +1118,30 @@ static int process_renames(struct merge_options *o,
ren1 = tmp;
}
+ if (ren1->processed)
+ continue;
+ ren1->processed = 1;
ren1->dst_entry->processed = 1;
/* BUG: We should only mark src_entry as processed if we
* are not dealing with a rename + add-source case.
*/
ren1->src_entry->processed = 1;
- if (ren1->processed)
- continue;
- ren1->processed = 1;
-
ren1_src = ren1->pair->one->path;
ren1_dst = ren1->pair->two->path;
if (ren2) {
+ /* One file renamed on both sides */
const char *ren2_src = ren2->pair->one->path;
const char *ren2_dst = ren2->pair->two->path;
enum rename_type rename_type;
- /* Renamed in 1 and renamed in 2 */
if (strcmp(ren1_src, ren2_src) != 0)
- die("ren1.src != ren2.src");
+ die("ren1_src != ren2_src");
ren2->dst_entry->processed = 1;
ren2->processed = 1;
if (strcmp(ren1_dst, ren2_dst) != 0) {
rename_type = RENAME_ONE_FILE_TO_TWO;
+ clean_merge = 0;
} else {
rename_type = RENAME_ONE_FILE_TO_ONE;
/* BUG: We should only remove ren1_src in
@@ -1151,9 +1161,32 @@ static int process_renames(struct merge_options *o,
branch2,
ren1->dst_entry,
ren2->dst_entry);
+ } else if ((lookup = string_list_lookup(renames2Dst, ren1_dst))) {
+ /* Two different files renamed to the same thing */
+ char *ren2_dst;
+ ren2 = lookup->util;
+ ren2_dst = ren2->pair->two->path;
+ if (strcmp(ren1_dst, ren2_dst) != 0)
+ die("ren1_dst != ren2_dst");
+
+ clean_merge = 0;
+ ren2->processed = 1;
+ /*
+ * BUG: We should only mark src_entry as processed
+ * if we are not dealing with a rename + add-source
+ * case.
+ */
+ ren2->src_entry->processed = 1;
+
+ setup_rename_conflict_info(RENAME_TWO_FILES_TO_ONE,
+ ren1->pair,
+ ren2->pair,
+ branch1,
+ branch2,
+ ren1->dst_entry,
+ ren2->dst_entry);
} else {
/* Renamed in 1, maybe changed in 2 */
- struct string_list_item *item;
/* we only use sha1 and mode of these */
struct diff_filespec src_other, dst_other;
int try_merge;
@@ -1188,23 +1221,6 @@ static int process_renames(struct merge_options *o,
branch2,
ren1->dst_entry,
NULL);
- } else if ((item = string_list_lookup(renames2Dst, ren1_dst))) {
- char *ren2_src, *ren2_dst;
- ren2 = item->util;
- ren2_src = ren2->pair->one->path;
- ren2_dst = ren2->pair->two->path;
-
- clean_merge = 0;
- ren2->processed = 1;
- remove_file(o, 1, ren2_src,
- renamed_stage == 3 || would_lose_untracked(ren1_src));
-
- output(o, 1, "CONFLICT (rename/rename): "
- "Rename %s->%s in %s. "
- "Rename %s->%s in %s",
- ren1_src, ren1_dst, branch1,
- ren2_src, ren2_dst, branch2);
- conflict_rename_rename_2to1(o, ren1, branch1, ren2, branch2);
} else if ((dst_other.mode == ren1->pair->two->mode) &&
sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {
/* Added file on the other side
@@ -1504,6 +1520,10 @@ static int process_entry(struct merge_options *o,
clean_merge = 0;
conflict_rename_rename_1to2(o, conflict_info);
break;
+ case RENAME_TWO_FILES_TO_ONE:
+ clean_merge = 0;
+ conflict_rename_rename_2to1(o, conflict_info);
+ break;
default:
entry->processed = 0;
break;
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 51/56] t6022: Add testcase for spurious "refusing to lose untracked" messages
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Completely new patch (about a problem that existed previous to v1, but which
I hadn't noticed).
t/t6022-merge-rename.sh | 26 ++++++++++++++++++++++++++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 74dcf20..0fd2b0a 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -767,4 +767,30 @@ test_expect_success 'merge rename into master has correct extended markers' '
test_cmp expected renamed_file
'
+test_expect_success 'setup spurious "refusing to lose untracked" message' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ > irrelevant_file &&
+ printf "1\n2\n3\n4\n5\n6\n7\n8\n" >original_file &&
+ git add irrelevant_file original_file &&
+ git commit -mA &&
+
+ git checkout -b rename &&
+ git mv original_file renamed_file &&
+ git commit -mB &&
+
+ git checkout master &&
+ git rm original_file &&
+ git commit -mC
+'
+
+test_expect_failure 'no spurious "refusing to lose untracked" message' '
+ git checkout master^0 &&
+ test_must_fail git merge rename^0 2>errors.txt &&
+ ! grep "refusing to lose untracked file" errors.txt
+'
+
test_done
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 53/56] t6022: Additional tests checking for unnecessary updates of files
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
I stumbled across a case, this one not involving a content merge, where
git currently rewrites a file unnecessarily. A quick audit uncovered two
additional situations (also not involving content merges) with the same
problem.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Completely new patch (about a problem that existed previous to v1, but which
I hadn't noticed).
t/t6022-merge-rename.sh | 91 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 91 insertions(+), 0 deletions(-)
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 8f75762..c2993fc 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -673,6 +673,97 @@ test_expect_success 'avoid unnecessary update, with D/F conflict' '
test_cmp expect actual # "df" should have stayed intact
'
+test_expect_success 'setup avoid unnecessary update, dir->(file,nothing)' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ >irrelevant &&
+ mkdir df &&
+ >df/file &&
+ git add -A &&
+ git commit -mA &&
+
+ git checkout -b side
+ git rm -rf df &&
+ git commit -mB &&
+
+ git checkout master &&
+ git rm -rf df &&
+ echo bla >df &&
+ git add -A &&
+ git commit -m "Add a newfile"
+'
+
+test_expect_failure 'avoid unnecessary update, dir->(file,nothing)' '
+ git checkout -q master^0 &&
+ test-chmtime =1000000000 df &&
+ test-chmtime -v +0 df >expect &&
+ git merge side &&
+ test-chmtime -v +0 df >actual &&
+ test_cmp expect actual # "df" should have stayed intact
+'
+
+test_expect_success 'setup avoid unnecessary update, modify/delete' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ >irrelevant &&
+ >file &&
+ git add -A &&
+ git commit -mA &&
+
+ git checkout -b side
+ git rm -f file &&
+ git commit -m "Delete file" &&
+
+ git checkout master &&
+ echo bla >file &&
+ git add -A &&
+ git commit -m "Modify file"
+'
+
+test_expect_failure 'avoid unnecessary update, modify/delete' '
+ git checkout -q master^0 &&
+ test-chmtime =1000000000 file &&
+ test-chmtime -v +0 file >expect &&
+ test_must_fail git merge side &&
+ test-chmtime -v +0 file >actual &&
+ test_cmp expect actual # "file" should have stayed intact
+'
+
+test_expect_success 'setup avoid unnecessary update, rename/add-dest' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ printf "1\n2\n3\n4\n5\n6\n7\n8\n" >file &&
+ git add -A &&
+ git commit -mA &&
+
+ git checkout -b side
+ cp file newfile &&
+ git add -A &&
+ git commit -m "Add file copy" &&
+
+ git checkout master &&
+ git mv file newfile &&
+ git commit -m "Rename file"
+'
+
+test_expect_failure 'avoid unnecessary update, rename/add-dest' '
+ git checkout -q master^0 &&
+ test-chmtime =1000000000 newfile &&
+ test-chmtime -v +0 newfile >expect &&
+ git merge side &&
+ test-chmtime -v +0 newfile >actual &&
+ test_cmp expect actual # "file" should have stayed intact
+'
+
test_expect_success 'setup merge of rename + small change' '
git reset --hard &&
git checkout --orphan rename-plus-small-change &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 56/56] merge-recursive: Fix virtual merge base for rename/rename(1to2)/add-dest
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Earlier in this series, the patch "merge-recursive: add handling for
rename/rename/add-dest/add-dest" added code to handle the rename on each
side of history also being involved in a rename/add conflict, but only
did so in the non-recursive case. Add code for the recursive case,
ensuring that the "added" files are not simply deleted.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Completely new patch (about a problem that existed previous to v1, but which
I hadn't noticed).
merge-recursive.c | 23 +++++++++++++++++++++--
t/t6036-recursive-corner-cases.sh | 2 +-
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index f088132..05ba41c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1161,6 +1161,8 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
o->call_depth ? " (left unresolved)" : "");
if (o->call_depth) {
struct merge_file_info mfi;
+ struct diff_filespec other;
+ struct diff_filespec *add;
mfi = merge_file(o, one->path,
one->sha1, one->mode,
a->sha1, a->mode,
@@ -1173,8 +1175,25 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
* unique path, or use that unique path instead of src here.
*/
update_file(o, 0, mfi.sha, mfi.mode, one->path);
- remove_file_from_cache(a->path);
- remove_file_from_cache(b->path);
+
+ /*
+ * Above, we put the merged content at the merge-base's
+ * path. Now we usually need to delete both a->path and
+ * b->path. However, the rename on each side of the merge
+ * could also be involved in a rename/add conflict. In
+ * such cases, we should keep the added file around,
+ * resolving the conflict at that path in its favor.
+ */
+ add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1);
+ if (add)
+ update_file(o, 0, add->sha1, add->mode, a->path);
+ else
+ remove_file_from_cache(a->path);
+ add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1);
+ if (add)
+ update_file(o, 0, add->sha1, add->mode, b->path);
+ else
+ remove_file_from_cache(b->path);
} else {
handle_file(o, a, 2, ci);
handle_file(o, b, 3, ci);
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index e9c7a25..dfee7d1 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -760,7 +760,7 @@ test_expect_success 'setup criss-cross+rename/rename/add-dest + simple modify' '
git tag E
'
-test_expect_failure 'virtual merge base handles rename/rename(1to2)/add-dest' '
+test_expect_success 'virtual merge base handles rename/rename(1to2)/add-dest' '
git checkout D^0 &&
git merge -s recursive E^0 &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 55/56] t6036: criss-cross + rename/rename(1to2)/add-dest + simple modify
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
This is another testcase trying to exercise the virtual merge base
creation in the rename/rename(1to2) code. A testcase is added that we
should be able to merge cleanly, but which requires a virtual merge base
to be created that correctly handles rename/add-dest conflicts within the
rename/rename(1to2) testcase handling.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Completely new patch (about a problem that existed previous to v1, but which
I hadn't noticed).
t/t6036-recursive-corner-cases.sh | 69 +++++++++++++++++++++++++++++++++++++
1 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index d8c6bda..e9c7a25 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -704,4 +704,73 @@ test_expect_failure 'detect rename/rename/add-source for virtual merge-base' '
test "$(cat a)" = "$(printf "1\n2\n3\n4\n5\n6\n7\n8\n")"
'
+#
+# criss-cross with rename/rename(1to2)/add-dest + simple modify:
+#
+# B D
+# o---o
+# / \ / \
+# A o X ? F
+# \ / \ /
+# o---o
+# C E
+#
+# Commit A: new file: a
+# Commit B: rename a->b, add c
+# Commit C: rename a->c
+# Commit D: merge B&C, keeping A:a and B:c
+# Commit E: merge B&C, keeping A:a and slightly modified c from B
+#
+# Merging commits D & E should result in no conflict. The virtual merge
+# base of B & C needs to not delete B:c for that to work, though...
+
+test_expect_success 'setup criss-cross+rename/rename/add-dest + simple modify' '
+ git rm -rf . &&
+ git clean -fdqx &&
+ rm -rf .git &&
+ git init &&
+
+ >a &&
+ git add a &&
+ git commit -m A &&
+ git tag A &&
+
+ git checkout -b B A &&
+ git mv a b &&
+ printf "1\n2\n3\n4\n5\n6\n7\n" >c &&
+ git add c &&
+ git commit -m B &&
+
+ git checkout -b C A &&
+ git mv a c &&
+ git commit -m C &&
+
+ git checkout B^0 &&
+ git merge --no-commit -s ours C^0 &&
+ git mv b a &&
+ git commit -m "D is like B but renames b back to a" &&
+ git tag D &&
+
+ git checkout B^0 &&
+ git merge --no-commit -s ours C^0 &&
+ git mv b a &&
+ echo 8 >>c &&
+ git add c &&
+ git commit -m "E like D but has mod in c" &&
+ git tag E
+'
+
+test_expect_failure 'virtual merge base handles rename/rename(1to2)/add-dest' '
+ git checkout D^0 &&
+
+ git merge -s recursive E^0 &&
+
+ test 2 -eq $(git ls-files -s | wc -l) &&
+ test 0 -eq $(git ls-files -u | wc -l) &&
+ test 0 -eq $(git ls-files -o | wc -l) &&
+
+ test $(git rev-parse HEAD:a) = $(git rev-parse A:a) &&
+ test $(git rev-parse HEAD:c) = $(git rev-parse E:c)
+'
+
test_done
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 52/56] merge-recursive: Fix spurious 'refusing to lose untracked file...' messages
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Calling update_stages() before update_file() can sometimes result in git
thinking the file being updated is untracked (whenever update_stages
moves it to stage 3). Reverse the call order, and add a big comment to
update_stages to hopefully prevent others from making the same mistake.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Completely new patch (about a problem that existed previous to v1, but which
I hadn't noticed).
merge-recursive.c | 34 ++++++++++++++++++++--------------
t/t6022-merge-rename.sh | 2 +-
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 05c8aa0..b2deb53 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -528,6 +528,15 @@ static int update_stages(const char *path, const struct diff_filespec *o,
const struct diff_filespec *a,
const struct diff_filespec *b)
{
+
+ /*
+ * NOTE: It is usually a bad idea to call update_stages on a path
+ * before calling update_file on that same path, since it can
+ * sometimes lead to spurious "refusing to lose untracked file..."
+ * messages from update_file (via make_room_for path via
+ * would_lose_untracked). Instead, reverse the order of the calls
+ * (executing update_file first and then update_stages).
+ */
int clear = 1;
int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK;
if (clear)
@@ -1041,7 +1050,6 @@ static void conflict_rename_delete(struct merge_options *o,
{
const struct diff_filespec *orig = pair->one;
const struct diff_filespec *dest = pair->two;
- const char *path;
const unsigned char *a_sha = NULL;
const unsigned char *b_sha = NULL;
int a_mode = 0;
@@ -1055,22 +1063,21 @@ static void conflict_rename_delete(struct merge_options *o,
b_mode = dest->mode;
}
+ handle_change_delete(o,
+ o->call_depth ? orig->path : dest->path,
+ orig->sha1, orig->mode,
+ a_sha, a_mode,
+ b_sha, b_mode,
+ "rename", "renamed");
+
if (o->call_depth) {
remove_file_from_cache(dest->path);
- path = orig->path;
} else {
- path = dest->path;
update_stages(dest->path, NULL,
rename_branch == o->branch1 ? dest : NULL,
rename_branch == o->branch1 ? NULL : dest);
}
- handle_change_delete(o,
- path,
- orig->sha1, orig->mode,
- a_sha, a_mode,
- b_sha, b_mode,
- "rename", "renamed");
}
static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
@@ -1108,11 +1115,6 @@ static void handle_file(struct merge_options *o,
}
add = filespec_from_entry(&other, dst_entry, stage ^ 1);
- if (stage == 2)
- update_stages(rename->path, NULL, rename, add);
- else
- update_stages(rename->path, NULL, add, rename);
-
if (add) {
char *add_name = unique_path(o, rename->path, other_branch);
update_file(o, 0, add->sha1, add->mode, add_name);
@@ -1127,6 +1129,10 @@ static void handle_file(struct merge_options *o,
}
}
update_file(o, 0, rename->sha1, rename->mode, dst_name);
+ if (stage == 2)
+ update_stages(rename->path, NULL, rename, add);
+ else
+ update_stages(rename->path, NULL, add, rename);
if (dst_name != rename->path)
free(dst_name);
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 0fd2b0a..8f75762 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -787,7 +787,7 @@ test_expect_success 'setup spurious "refusing to lose untracked" message' '
git commit -mC
'
-test_expect_failure 'no spurious "refusing to lose untracked" message' '
+test_expect_success 'no spurious "refusing to lose untracked" message' '
git checkout master^0 &&
test_must_fail git merge rename^0 2>errors.txt &&
! grep "refusing to lose untracked file" errors.txt
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 54/56] merge-recursive: Avoid unnecessary file rewrites
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Often times, a potential conflict at a path is resolved by merge-recursive
by using the content that was already present at that location. In such
cases, we do not want to overwrite the content that is already present, as
that could trigger unnecessary recompilations. One of the patches earlier
in this series ("merge-recursive: When we detect we can skip an update,
actually skip it") fixed the cases that involved content merges, but there
were a few other cases as well.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
Completely new patch (about a problem that existed previous to v1, but which
I hadn't noticed).
merge-recursive.c | 30 ++++++++++++++++++++++++------
t/t6022-merge-rename.sh | 6 +++---
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index b2deb53..f088132 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1038,7 +1038,14 @@ static void handle_change_delete(struct merge_options *o,
change_past, o->branch1, o->branch1, path,
NULL == renamed ? "" : " at ",
NULL == renamed ? "" : renamed);
- update_file(o, 0, a_sha, a_mode, renamed ? renamed : path);
+ if (renamed)
+ update_file(o, 0, a_sha, a_mode, renamed);
+ /*
+ * No need to call update_file() on path when !renamed, since
+ * that would needlessly touch path. We could call
+ * update_file_flags() with update_cache=0 and update_wd=0,
+ * but that's a no-op.
+ */
}
free(renamed);
}
@@ -1398,10 +1405,20 @@ static int process_renames(struct merge_options *o,
NULL);
} else if ((dst_other.mode == ren1->pair->two->mode) &&
sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {
- /* Added file on the other side
- identical to the file being
- renamed: clean merge */
- update_file(o, 1, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);
+ /*
+ * Added file on the other side identical to
+ * the file being renamed: clean merge.
+ * Also, there is no need to overwrite the
+ * file already in the working copy, so call
+ * update_file_flags() instead of
+ * update_file().
+ */
+ update_file_flags(o,
+ ren1->pair->two->sha1,
+ ren1->pair->two->mode,
+ ren1_dst,
+ 1, /* update_cache */
+ 0 /* update_wd */);
} else if (!sha_eq(dst_other.sha1, null_sha1)) {
clean_merge = 0;
try_merge = 1;
@@ -1729,7 +1746,8 @@ static int process_entry(struct merge_options *o,
free(new_path);
} else {
output(o, 2, "Adding %s", path);
- update_file(o, 1, sha, mode, path);
+ /* do not overwrite file if already present */
+ update_file_flags(o, sha, mode, path, 1, !a_sha);
}
} else if (a_sha && b_sha) {
/* Case C: Added in both (check for same permissions) and */
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index c2993fc..9d8584e 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -696,7 +696,7 @@ test_expect_success 'setup avoid unnecessary update, dir->(file,nothing)' '
git commit -m "Add a newfile"
'
-test_expect_failure 'avoid unnecessary update, dir->(file,nothing)' '
+test_expect_success 'avoid unnecessary update, dir->(file,nothing)' '
git checkout -q master^0 &&
test-chmtime =1000000000 df &&
test-chmtime -v +0 df >expect &&
@@ -726,7 +726,7 @@ test_expect_success 'setup avoid unnecessary update, modify/delete' '
git commit -m "Modify file"
'
-test_expect_failure 'avoid unnecessary update, modify/delete' '
+test_expect_success 'avoid unnecessary update, modify/delete' '
git checkout -q master^0 &&
test-chmtime =1000000000 file &&
test-chmtime -v +0 file >expect &&
@@ -755,7 +755,7 @@ test_expect_success 'setup avoid unnecessary update, rename/add-dest' '
git commit -m "Rename file"
'
-test_expect_failure 'avoid unnecessary update, rename/add-dest' '
+test_expect_success 'avoid unnecessary update, rename/add-dest' '
git checkout -q master^0 &&
test-chmtime =1000000000 newfile &&
test-chmtime -v +0 newfile >expect &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 47/56] merge-recursive: Have conflict_rename_delete reuse modify/delete code
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 48 +++++++++++++++++++++++++++++++-----------------
1 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 17fe7c3..c89a5ce 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1039,24 +1039,38 @@ static void conflict_rename_delete(struct merge_options *o,
const char *rename_branch,
const char *other_branch)
{
- char *dest_name = pair->two->path;
- int df_conflict = 0;
+ const struct diff_filespec *orig = pair->one;
+ const struct diff_filespec *dest = pair->two;
+ const char *path;
+ const unsigned char *a_sha = NULL;
+ const unsigned char *b_sha = NULL;
+ int a_mode = 0;
+ int b_mode = 0;
+
+ if (rename_branch == o->branch1) {
+ a_sha = dest->sha1;
+ a_mode = dest->mode;
+ } else {
+ b_sha = dest->sha1;
+ b_mode = dest->mode;
+ }
- output(o, 1, "CONFLICT (rename/delete): Rename %s->%s in %s "
- "and deleted in %s",
- pair->one->path, pair->two->path, rename_branch,
- other_branch);
- if (!o->call_depth)
- update_stages(dest_name, NULL,
- rename_branch == o->branch1 ? pair->two : NULL,
- rename_branch == o->branch1 ? NULL : pair->two);
- if (dir_in_way(dest_name, !o->call_depth)) {
- dest_name = unique_path(o, dest_name, rename_branch);
- df_conflict = 1;
+ if (o->call_depth) {
+ remove_file_from_cache(dest->path);
+ path = orig->path;
+ } else {
+ path = dest->path;
+ update_stages(dest->path, NULL,
+ rename_branch == o->branch1 ? dest : NULL,
+ rename_branch == o->branch1 ? NULL : dest);
}
- update_file(o, 0, pair->two->sha1, pair->two->mode, dest_name);
- if (df_conflict)
- free(dest_name);
+
+ handle_change_delete(o,
+ path,
+ orig->sha1, orig->mode,
+ a_sha, a_mode,
+ b_sha, b_mode,
+ "rename", "renamed");
}
static void conflict_rename_rename_1to2(struct merge_options *o,
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 49/56] merge-recursive: Fix working copy handling for rename/rename/add/add
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
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>
---
No changes since v1.
merge-recursive.c | 73 ++++++++++++++++++++++------------
t/t6042-merge-rename-corner-cases.sh | 11 +++++-
2 files changed, 58 insertions(+), 26 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 8ddd520..05c8aa0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1086,6 +1086,52 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
return target;
}
+static void handle_file(struct merge_options *o,
+ struct diff_filespec *rename,
+ int stage,
+ struct rename_conflict_info *ci)
+{
+ char *dst_name = rename->path;
+ struct stage_data *dst_entry;
+ const char *cur_branch, *other_branch;
+ struct diff_filespec other;
+ struct diff_filespec *add;
+
+ if (stage == 2) {
+ dst_entry = ci->dst_entry1;
+ cur_branch = ci->branch1;
+ other_branch = ci->branch2;
+ } else {
+ dst_entry = ci->dst_entry2;
+ cur_branch = ci->branch2;
+ other_branch = ci->branch1;
+ }
+
+ add = filespec_from_entry(&other, dst_entry, stage ^ 1);
+ if (stage == 2)
+ update_stages(rename->path, NULL, rename, add);
+ else
+ update_stages(rename->path, NULL, add, rename);
+
+ if (add) {
+ char *add_name = unique_path(o, rename->path, other_branch);
+ update_file(o, 0, add->sha1, add->mode, add_name);
+
+ remove_file(o, 0, rename->path, 0);
+ dst_name = unique_path(o, rename->path, cur_branch);
+ } else {
+ if (dir_in_way(rename->path, !o->call_depth)) {
+ dst_name = unique_path(o, rename->path, cur_branch);
+ output(o, 1, "%s is a directory in %s adding as %s instead",
+ rename->path, other_branch, dst_name);
+ }
+ }
+ update_file(o, 0, rename->sha1, rename->mode, dst_name);
+
+ if (dst_name != rename->path)
+ free(dst_name);
+}
+
static void conflict_rename_rename_1to2(struct merge_options *o,
struct rename_conflict_info *ci)
{
@@ -1093,10 +1139,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
struct diff_filespec *one = ci->pair1->one;
struct diff_filespec *a = ci->pair1->two;
struct diff_filespec *b = ci->pair2->two;
- const char *dst_name_a = a->path;
- const char *dst_name_b = b->path;
- char *del[2];
- int delp = 0;
output(o, 1, "CONFLICT (rename/rename): "
"Rename \"%s\"->\"%s\" in branch \"%s\" "
@@ -1104,16 +1146,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
one->path, a->path, ci->branch1,
one->path, b->path, ci->branch2,
o->call_depth ? " (left unresolved)" : "");
- if (dir_in_way(a->path, !o->call_depth)) {
- dst_name_a = del[delp++] = unique_path(o, a->path, ci->branch1);
- output(o, 1, "%s is a directory in %s adding as %s instead",
- a->path, ci->branch2, dst_name_a);
- }
- if (dir_in_way(b->path, !o->call_depth)) {
- dst_name_b = del[delp++] = unique_path(o, b->path, ci->branch2);
- output(o, 1, "%s is a directory in %s adding as %s instead",
- b->path, ci->branch1, dst_name_b);
- }
if (o->call_depth) {
struct merge_file_info mfi;
mfi = merge_file(o, one->path,
@@ -1131,18 +1163,9 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
remove_file_from_cache(a->path);
remove_file_from_cache(b->path);
} else {
- struct diff_filespec other;
- update_stages(a->path, NULL,
- a, filespec_from_entry(&other, ci->dst_entry1, 3));
-
- update_stages(b->path, NULL,
- filespec_from_entry(&other, ci->dst_entry2, 2), b);
-
- update_file(o, 0, a->sha1, a->mode, dst_name_a);
- update_file(o, 0, b->sha1, b->mode, dst_name_b);
+ handle_file(o, a, 2, ci);
+ handle_file(o, b, 3, ci);
}
- while (delp--)
- free(del[delp]);
}
static void conflict_rename_rename_2to1(struct merge_options *o,
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index 6875919..32591f9 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -558,12 +558,21 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting
test 5 -eq $(git ls-files -s | wc -l) &&
test 2 -eq $(git ls-files -u b | wc -l) &&
test 2 -eq $(git ls-files -u c | wc -l) &&
+ test 4 -eq $(git ls-files -o | wc -l) &&
test $(git rev-parse :1:a) = $(git rev-parse A:a) &&
test $(git rev-parse :2:b) = $(git rev-parse C:b) &&
test $(git rev-parse :3:b) = $(git rev-parse B:b) &&
test $(git rev-parse :2:c) = $(git rev-parse C:c) &&
- test $(git rev-parse :3:c) = $(git rev-parse B:c)
+ test $(git rev-parse :3:c) = $(git rev-parse B:c) &&
+
+ test $(git hash-object c~HEAD) = $(git rev-parse C:c) &&
+ test $(git hash-object c~B\^0) = $(git rev-parse B:c) &&
+ test $(git hash-object b~HEAD) = $(git rev-parse C:b) &&
+ test $(git hash-object b~B\^0) = $(git rev-parse B:b) &&
+
+ test ! -f b &&
+ test ! -f c
'
test_done
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 50/56] t3030: fix accidental success in symlink rename
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Jeff King, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
From: Jeff King <peff@peff.net>
In this test, we have merge two branches. On one branch, we
renamed "a" to "e". On the other, we renamed "a" to "e" and
then added a symlink pointing at "a" pointing to "e".
The results for the test indicate that the merge should
succeed, but also that "a" should no longer exist. Since
both sides renamed "a" to the same destination, we will end
up comparing those destinations for content.
But what about what's left? One side (the rename only),
replaced "a" with nothing. The other side replaced it with a
symlink. The common base must also be nothing, because any
"a" before this was meaningless (it was totally unrelated
content that ended up getting renamed).
The only sensible resolution is to keep the symlink. The
rename-only side didn't touch the content versus the common
base, and the other side added content. The 3-way merge
dictates that we take the side with a change.
And this gives the overall merge an intuitive result. One
side made one change (a rename), and the other side made two
changes: an identical rename, and an addition (that just
happened to be at the same spot). The end result should
contain both changes.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
This patch was not part of v1, but Junio pulled it out of a separate
series and combined it.
t/t3030-merge-recursive.sh | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 0c02d56..55ef189 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -267,7 +267,8 @@ test_expect_success 'setup 8' '
ln -s e a &&
git add a e &&
test_tick &&
- git commit -m "rename a->e, symlink a->e"
+ git commit -m "rename a->e, symlink a->e" &&
+ oln=`printf e | git hash-object --stdin`
fi
'
@@ -630,16 +631,18 @@ test_expect_success 'merge-recursive copy vs. rename' '
if test_have_prereq SYMLINKS
then
- test_expect_success 'merge-recursive rename vs. rename/symlink' '
+ test_expect_failure 'merge-recursive rename vs. rename/symlink' '
git checkout -f rename &&
git merge rename-ln &&
( git ls-tree -r HEAD ; git ls-files -s ) >actual &&
(
+ echo "120000 blob $oln a"
echo "100644 blob $o0 b"
echo "100644 blob $o0 c"
echo "100644 blob $o0 d/e"
echo "100644 blob $o0 e"
+ echo "120000 $oln 0 a"
echo "100644 $o0 0 b"
echo "100644 $o0 0 c"
echo "100644 $o0 0 d/e"
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 33/56] merge-recursive: Improve handling of rename target vs. directory addition
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.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>
---
No changes since v1.
merge-recursive.c | 27 ++++++++++++++++++++++-----
t/t6022-merge-rename.sh | 4 ++--
t/t6042-merge-rename-corner-cases.sh | 2 +-
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 5f28905..f1160d5 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1381,19 +1381,34 @@ static int merge_content(struct merge_options *o,
reason = "submodule";
output(o, 1, "CONFLICT (%s): Merge conflict in %s",
reason, path);
- if (involved_in_rename)
+ if (involved_in_rename && !df_conflict_remains)
update_stages(path, &one, &a, &b);
}
if (df_conflict_remains) {
char *new_path;
- update_file_flags(o, mfi.sha, mfi.mode, path,
- o->call_depth || mfi.clean, 0);
+ if (o->call_depth) {
+ remove_file_from_cache(path);
+ } else {
+ if (!mfi.clean)
+ update_stages(path, &one, &a, &b);
+ else {
+ int file_from_stage2 = was_tracked(path);
+ struct diff_filespec merged;
+ hashcpy(merged.sha1, mfi.sha);
+ merged.mode = mfi.mode;
+
+ update_stages(path, NULL,
+ file_from_stage2 ? &merged : NULL,
+ file_from_stage2 ? NULL : &merged);
+ }
+
+ }
new_path = unique_path(o, path, df_rename_conflict_branch);
- mfi.clean = 0;
output(o, 1, "Adding as %s instead", new_path);
- update_file_flags(o, mfi.sha, mfi.mode, new_path, 0, 1);
+ update_file(o, 0, mfi.sha, mfi.mode, new_path);
free(new_path);
+ mfi.clean = 0;
} else {
update_file(o, mfi.clean, mfi.sha, mfi.mode, path);
}
@@ -1582,6 +1597,8 @@ static int process_df_entry(struct merge_options *o,
output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
"Adding %s as %s",
conf, path, other_branch, path, new_path);
+ if (o->call_depth)
+ remove_file_from_cache(path);
update_file(o, 0, sha, mode, new_path);
if (o->call_depth)
remove_file_from_cache(path);
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index 6ff4bd2..fcc1d4c 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -307,7 +307,7 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
grep "Auto-merging dir" output &&
grep "Adding as dir~HEAD instead" output &&
- test 2 -eq "$(git ls-files -u | wc -l)" &&
+ test 3 -eq "$(git ls-files -u | wc -l)" &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
test_must_fail git diff --quiet &&
@@ -329,7 +329,7 @@ test_expect_success 'Same as previous, but merged other way' '
grep "Auto-merging dir" output &&
grep "Adding as dir~renamed-file-has-no-conflicts instead" output &&
- test 2 -eq "$(git ls-files -u | wc -l)" &&
+ test 3 -eq "$(git ls-files -u | wc -l)" &&
test 2 -eq "$(git ls-files -u dir/file-in-the-way | wc -l)" &&
test_must_fail git diff --quiet &&
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index 668ec6d..968055d 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -234,7 +234,7 @@ test_expect_success 'setup content merge + rename/directory conflict' '
git commit -m left
'
-test_expect_failure 'rename/directory conflict + clean content merge' '
+test_expect_success 'rename/directory conflict + clean content merge' '
git reset --hard &&
git reset --hard &&
git clean -fdqx &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 48/56] merge-recursive: add handling for rename/rename/add-dest/add-dest
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.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>
---
No changes since v1.
merge-recursive.c | 21 +++++++++++++++++++--
t/t6042-merge-rename-corner-cases.sh | 2 +-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c89a5ce..8ddd520 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1073,6 +1073,19 @@ static void conflict_rename_delete(struct merge_options *o,
"rename", "renamed");
}
+static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,
+ struct stage_data *entry,
+ int stage)
+{
+ unsigned char *sha = entry->stages[stage].sha;
+ unsigned mode = entry->stages[stage].mode;
+ if (mode == 0 || is_null_sha1(sha))
+ return NULL;
+ hashcpy(target->sha1, sha);
+ target->mode = mode;
+ return target;
+}
+
static void conflict_rename_rename_1to2(struct merge_options *o,
struct rename_conflict_info *ci)
{
@@ -1118,8 +1131,12 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
remove_file_from_cache(a->path);
remove_file_from_cache(b->path);
} else {
- update_stages(a->path, NULL, a, NULL);
- update_stages(b->path, NULL, NULL, b);
+ struct diff_filespec other;
+ update_stages(a->path, NULL,
+ a, filespec_from_entry(&other, ci->dst_entry1, 3));
+
+ update_stages(b->path, NULL,
+ filespec_from_entry(&other, ci->dst_entry2, 2), b);
update_file(o, 0, a->sha1, a->mode, dst_name_a);
update_file(o, 0, b->sha1, b->mode, dst_name_b);
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index 3be5059..6875919 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -551,7 +551,7 @@ test_expect_success 'setup rename/rename(1to2)/add-dest conflict' '
git commit -m two
'
-test_expect_failure 'rename/rename/add-dest merge still knows about conflicting file versions' '
+test_expect_success 'rename/rename/add-dest merge still knows about conflicting file versions' '
git checkout C^0 &&
test_must_fail git merge -s recursive B^0 &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 45/56] merge-recursive: Consider modifications in rename/rename(2to1) conflicts
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.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>
---
No changes since v1.
merge-recursive.c | 30 ++++++++++++++++++--------
t/t6036-recursive-corner-cases.sh | 38 +++++++++-------------------------
t/t6042-merge-rename-corner-cases.sh | 2 +-
3 files changed, 32 insertions(+), 38 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 390abea..c305857 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1083,6 +1083,8 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
struct diff_filespec *c1 = ci->pair1->two;
struct diff_filespec *c2 = ci->pair2->two;
char *path = c1->path; /* == c2->path */
+ struct merge_file_info mfi_c1;
+ struct merge_file_info mfi_c2;
output(o, 1, "CONFLICT (rename/rename): "
"Rename %s->%s in %s. "
@@ -1093,22 +1095,32 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
remove_file(o, 1, a->path, would_lose_untracked(a->path));
remove_file(o, 1, b->path, would_lose_untracked(b->path));
+ mfi_c1 = merge_file_special_markers(o, a, c1, &ci->ren1_other,
+ o->branch1, c1->path,
+ o->branch2, ci->ren1_other.path);
+ mfi_c2 = merge_file_special_markers(o, b, &ci->ren2_other, c2,
+ o->branch1, ci->ren2_other.path,
+ o->branch2, c2->path);
+
if (o->call_depth) {
- struct merge_file_info mfi;
- mfi = merge_file(o, path, null_sha1, 0,
- c1->sha1, c1->mode,
- c2->sha1, c2->mode,
- ci->branch1, ci->branch2);
- output(o, 1, "Adding merged %s", path);
- update_file(o, 0, mfi.sha, mfi.mode, path);
+ /*
+ * If mfi_c1.clean && mfi_c2.clean, then it might make
+ * sense to do a two-way merge of those results. But, I
+ * think in all cases, it makes sense to have the virtual
+ * merge base just undo the renames; they can be detected
+ * again later for the non-recursive merge.
+ */
+ remove_file(o, 0, path, 0);
+ update_file(o, 0, mfi_c1.sha, mfi_c1.mode, a->path);
+ update_file(o, 0, mfi_c2.sha, mfi_c2.mode, b->path);
} else {
char *new_path1 = unique_path(o, path, ci->branch1);
char *new_path2 = unique_path(o, path, ci->branch2);
output(o, 1, "Renaming %s to %s and %s to %s instead",
a->path, new_path1, b->path, new_path2);
remove_file(o, 0, path, 0);
- update_file(o, 0, c1->sha1, c1->mode, new_path1);
- update_file(o, 0, c2->sha1, c2->mode, new_path2);
+ update_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1);
+ update_file(o, 0, mfi_c2.sha, mfi_c2.mode, new_path2);
free(new_path2);
free(new_path1);
}
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index 5a7af0c..d8c6bda 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -57,23 +57,15 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' '
test_must_fail git merge -s recursive R2^0 &&
- test 5 = $(git ls-files -s | wc -l) &&
- test 3 = $(git ls-files -u | wc -l) &&
- test 0 = $(git ls-files -o | wc -l) &&
+ test 2 = $(git ls-files -s | wc -l) &&
+ test 2 = $(git ls-files -u | wc -l) &&
+ test 2 = $(git ls-files -o | wc -l) &&
- test $(git rev-parse :0:one) = $(git rev-parse L2:one) &&
- test $(git rev-parse :0:two) = $(git rev-parse R2:two) &&
test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
- cp one merged &&
- >empty &&
- test_must_fail git merge-file \
- -L "Temporary merge branch 1" \
- -L "" \
- -L "Temporary merge branch 2" \
- merged empty two &&
- test $(git rev-parse :1:three) = $(git hash-object merged)
+ test $(git rev-parse L2:three) = $(git hash-object three~HEAD) &&
+ test $(git rev-parse R2:three) = $(git hash-object three~R2^0)
'
#
@@ -132,25 +124,15 @@ test_expect_success 'merge criss-cross + rename merges with basic modification'
test_must_fail git merge -s recursive R2^0 &&
- test 5 = $(git ls-files -s | wc -l) &&
- test 3 = $(git ls-files -u | wc -l) &&
- test 0 = $(git ls-files -o | wc -l) &&
+ test 2 = $(git ls-files -s | wc -l) &&
+ test 2 = $(git ls-files -u | wc -l) &&
+ test 2 = $(git ls-files -o | wc -l) &&
- test $(git rev-parse :0:one) = $(git rev-parse L2:one) &&
- test $(git rev-parse :0:two) = $(git rev-parse R2:two) &&
test $(git rev-parse :2:three) = $(git rev-parse L2:three) &&
test $(git rev-parse :3:three) = $(git rev-parse R2:three) &&
- head -n 10 two >merged &&
- cp one merge-me &&
- >empty &&
- test_must_fail git merge-file \
- -L "Temporary merge branch 1" \
- -L "" \
- -L "Temporary merge branch 2" \
- merge-me empty merged &&
-
- test $(git rev-parse :1:three) = $(git hash-object merge-me)
+ test $(git rev-parse L2:three) = $(git hash-object three~HEAD) &&
+ test $(git rev-parse R2:three) = $(git hash-object three~R2^0)
'
#
diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh
index bfc3179..3be5059 100755
--- a/t/t6042-merge-rename-corner-cases.sh
+++ b/t/t6042-merge-rename-corner-cases.sh
@@ -376,7 +376,7 @@ test_expect_success 'setup rename/rename (2to1) + modify/modify' '
git commit -m C
'
-test_expect_failure 'handle rename/rename (2to1) conflict correctly' '
+test_expect_success 'handle rename/rename (2to1) conflict correctly' '
git checkout B^0 &&
test_must_fail git merge -s recursive C^0 >out &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 46/56] merge-recursive: Make modify/delete handling code reusable
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
modify/delete and rename/delete share a lot of similarities; we'd like all
the criss-cross and D/F conflict handling specializations to be shared
between the two.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 82 +++++++++++++++++++++++++++-------------------
t/t6022-merge-rename.sh | 4 +-
2 files changed, 50 insertions(+), 36 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index c305857..17fe7c3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -994,6 +994,46 @@ static struct merge_file_info merge_file(struct merge_options *o,
return merge_file_1(o, &one, &a, &b, branch1, branch2);
}
+static void handle_change_delete(struct merge_options *o,
+ const char *path,
+ const unsigned char *o_sha, int o_mode,
+ const unsigned char *a_sha, int a_mode,
+ const unsigned char *b_sha, int b_mode,
+ const char *change, const char *change_past)
+{
+ char *renamed = NULL;
+ if (dir_in_way(path, !o->call_depth)) {
+ renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
+ }
+
+ if (o->call_depth) {
+ /*
+ * We cannot arbitrarily accept either a_sha or b_sha as
+ * correct; since there is no true "middle point" between
+ * them, simply reuse the base version for virtual merge base.
+ */
+ remove_file_from_cache(path);
+ update_file(o, 0, o_sha, o_mode, renamed ? renamed : path);
+ } else if (!a_sha) {
+ output(o, 1, "CONFLICT (%s/delete): %s deleted in %s "
+ "and %s in %s. Version %s of %s left in tree%s%s.",
+ change, path, o->branch1,
+ change_past, o->branch2, o->branch2, path,
+ NULL == renamed ? "" : " at ",
+ NULL == renamed ? "" : renamed);
+ update_file(o, 0, b_sha, b_mode, renamed ? renamed : path);
+ } else {
+ output(o, 1, "CONFLICT (%s/delete): %s deleted in %s "
+ "and %s in %s. Version %s of %s left in tree%s%s.",
+ change, path, o->branch2,
+ change_past, o->branch1, o->branch1, path,
+ NULL == renamed ? "" : " at ",
+ NULL == renamed ? "" : renamed);
+ update_file(o, 0, a_sha, a_mode, renamed ? renamed : path);
+ }
+ free(renamed);
+}
+
static void conflict_rename_delete(struct merge_options *o,
struct diff_filepair *pair,
const char *rename_branch,
@@ -1411,44 +1451,18 @@ error_return:
return ret;
}
-static void handle_delete_modify(struct merge_options *o,
+static void handle_modify_delete(struct merge_options *o,
const char *path,
unsigned char *o_sha, int o_mode,
unsigned char *a_sha, int a_mode,
unsigned char *b_sha, int b_mode)
{
- char *renamed = NULL;
- if (dir_in_way(path, !o->call_depth)) {
- renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
- }
-
- if (o->call_depth) {
- /*
- * We cannot arbitrarily accept either a_sha or b_sha as
- * correct; since there is no true "middle point" between
- * them, simply reuse the base version for virtual merge base.
- */
- remove_file_from_cache(path);
- update_file(o, 0, o_sha, o_mode, renamed ? renamed : path);
- } else if (!a_sha) {
- output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "
- "and modified in %s. Version %s of %s left in tree%s%s.",
- path, o->branch1,
- o->branch2, o->branch2, path,
- NULL == renamed ? "" : " at ",
- NULL == renamed ? "" : renamed);
- update_file(o, 0, b_sha, b_mode, renamed ? renamed : path);
- } else {
- output(o, 1, "CONFLICT (delete/modify): %s deleted in %s "
- "and modified in %s. Version %s of %s left in tree%s%s.",
- path, o->branch2,
- o->branch1, o->branch1, path,
- NULL == renamed ? "" : " at ",
- NULL == renamed ? "" : renamed);
- update_file(o, 0, a_sha, a_mode, renamed ? renamed : path);
- }
- free(renamed);
-
+ handle_change_delete(o,
+ path,
+ o_sha, o_mode,
+ a_sha, a_mode,
+ b_sha, b_mode,
+ "modify", "modified");
}
static int merge_content(struct merge_options *o,
@@ -1614,7 +1628,7 @@ static int process_entry(struct merge_options *o,
} else {
/* Modify/delete; deleted side may have put a directory in the way */
clean_merge = 0;
- handle_delete_modify(o, path, o_sha, o_mode,
+ handle_modify_delete(o, path, o_sha, o_mode,
a_sha, a_mode, b_sha, b_mode);
}
} else if ((!o_sha && a_sha && !b_sha) ||
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index d96d3c5..74dcf20 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -303,7 +303,7 @@ test_expect_success 'Rename+D/F conflict; renamed file merges but dir in way' '
git checkout -q renamed-file-has-no-conflicts^0 &&
test_must_fail git merge --strategy=recursive dir-in-way >output &&
- grep "CONFLICT (delete/modify): dir/file-in-the-way" output &&
+ grep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
grep "Auto-merging dir" output &&
grep "Adding as dir~HEAD instead" output &&
@@ -325,7 +325,7 @@ test_expect_success 'Same as previous, but merged other way' '
test_must_fail git merge --strategy=recursive renamed-file-has-no-conflicts >output 2>errors &&
! grep "error: refusing to lose untracked file at" errors &&
- grep "CONFLICT (delete/modify): dir/file-in-the-way" output &&
+ grep "CONFLICT (modify/delete): dir/file-in-the-way" output &&
grep "Auto-merging dir" output &&
grep "Adding as dir~renamed-file-has-no-conflicts instead" output &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 43/56] merge-recursive: Record more data needed for merging with dual renames
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
When two different files are renamed to one, we need to be able to do
three-way merges for both of those files. To do that, we need to record
the sha1sum of the (possibly modified) file on the unrenamed side. Modify
setup_rename_conflict_info() to take this extra information and record it
when the rename_type is RENAME_TWO_FILES_TO_ONE.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 42 +++++++++++++++++++++++++++++++++++++++---
1 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 77c2c41..63f3aa0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -79,6 +79,8 @@ struct rename_conflict_info {
const char *branch2;
struct stage_data *dst_entry1;
struct stage_data *dst_entry2;
+ struct diff_filespec ren1_other;
+ struct diff_filespec ren2_other;
};
/*
@@ -100,7 +102,10 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,
const char *branch1,
const char *branch2,
struct stage_data *dst_entry1,
- struct stage_data *dst_entry2)
+ struct stage_data *dst_entry2,
+ struct merge_options *o,
+ struct stage_data *src_entry1,
+ struct stage_data *src_entry2)
{
struct rename_conflict_info *ci = xcalloc(1, sizeof(struct rename_conflict_info));
ci->rename_type = rename_type;
@@ -118,6 +123,24 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,
ci->pair2 = pair2;
dst_entry2->rename_conflict_info = ci;
}
+
+ if (rename_type == RENAME_TWO_FILES_TO_ONE) {
+ /*
+ * For each rename, there could have been
+ * modifications on the side of history where that
+ * file was not renamed.
+ */
+ int ostage1 = o->branch1 == branch1 ? 3 : 2;
+ int ostage2 = ostage1 ^ 1;
+
+ ci->ren1_other.path = pair1->one->path;
+ hashcpy(ci->ren1_other.sha1, src_entry1->stages[ostage1].sha);
+ ci->ren1_other.mode = src_entry1->stages[ostage1].mode;
+
+ ci->ren2_other.path = pair2->one->path;
+ hashcpy(ci->ren2_other.sha1, src_entry2->stages[ostage2].sha);
+ ci->ren2_other.mode = src_entry2->stages[ostage2].mode;
+ }
}
static int show(struct merge_options *o, int v)
@@ -1160,7 +1183,10 @@ static int process_renames(struct merge_options *o,
branch1,
branch2,
ren1->dst_entry,
- ren2->dst_entry);
+ ren2->dst_entry,
+ o,
+ NULL,
+ NULL);
} else if ((lookup = string_list_lookup(renames2Dst, ren1_dst))) {
/* Two different files renamed to the same thing */
char *ren2_dst;
@@ -1184,7 +1210,11 @@ static int process_renames(struct merge_options *o,
branch1,
branch2,
ren1->dst_entry,
- ren2->dst_entry);
+ ren2->dst_entry,
+ o,
+ ren1->src_entry,
+ ren2->src_entry);
+
} else {
/* Renamed in 1, maybe changed in 2 */
/* we only use sha1 and mode of these */
@@ -1220,6 +1250,9 @@ static int process_renames(struct merge_options *o,
branch1,
branch2,
ren1->dst_entry,
+ NULL,
+ o,
+ NULL,
NULL);
} else if ((dst_other.mode == ren1->pair->two->mode) &&
sha_eq(dst_other.sha1, ren1->pair->two->sha1)) {
@@ -1271,6 +1304,9 @@ static int process_renames(struct merge_options *o,
branch1,
NULL,
ren1->dst_entry,
+ NULL,
+ o,
+ NULL,
NULL);
}
}
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 44/56] merge-recursive: Create function for merging with branchname:file markers
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
We want to be able to reuse the code to do a three-way file content merge
and have the conflict markers use both branchname and filename. Split it
out into a separate function.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 42 +++++++++++++++++++++++++++++++++---------
1 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 63f3aa0..390abea 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -944,6 +944,36 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
return result;
}
+static struct merge_file_info
+merge_file_special_markers(struct merge_options *o,
+ const struct diff_filespec *one,
+ const struct diff_filespec *a,
+ const struct diff_filespec *b,
+ const char *branch1,
+ const char *filename1,
+ const char *branch2,
+ const char *filename2)
+{
+ char *side1 = NULL;
+ char *side2 = NULL;
+ struct merge_file_info mfi;
+
+ if (filename1) {
+ side1 = xmalloc(strlen(branch1) + strlen(filename1) + 2);
+ sprintf(side1, "%s:%s", branch1, filename1);
+ }
+ if (filename2) {
+ side2 = xmalloc(strlen(branch2) + strlen(filename2) + 2);
+ sprintf(side2, "%s:%s", branch2, filename2);
+ }
+
+ mfi = merge_file_1(o, one, a, b,
+ side1 ? side1 : branch1, side2 ? side2 : branch2);
+ free(side1);
+ free(side2);
+ return mfi;
+}
+
static struct merge_file_info merge_file(struct merge_options *o,
const char *path,
const unsigned char *o_sha, int o_mode,
@@ -1417,7 +1447,6 @@ static int merge_content(struct merge_options *o,
struct rename_conflict_info *rename_conflict_info)
{
const char *reason = "content";
- char *side1 = NULL, *side2 = NULL;
const char *path1 = NULL, *path2 = NULL;
struct merge_file_info mfi;
struct diff_filespec one, a, b;
@@ -1447,18 +1476,13 @@ static int merge_content(struct merge_options *o,
path2 = (rename_conflict_info->pair2 ||
o->branch2 == rename_conflict_info->branch1) ?
pair1->two->path : pair1->one->path;
- side1 = xmalloc(strlen(o->branch1) + strlen(path1) + 2);
- side2 = xmalloc(strlen(o->branch2) + strlen(path2) + 2);
- sprintf(side1, "%s:%s", o->branch1, path1);
- sprintf(side2, "%s:%s", o->branch2, path2);
if (dir_in_way(path, !o->call_depth))
df_conflict_remains = 1;
}
- mfi = merge_file_1(o, &one, &a, &b,
- side1 ? side1 : o->branch1, side2 ? side2 : o->branch2);
- free(side1);
- free(side2);
+ mfi = merge_file_special_markers(o, &one, &a, &b,
+ o->branch1, path1,
+ o->branch2, path2);
if (mfi.clean && !df_conflict_remains &&
sha_eq(mfi.sha, a_sha) && mfi.mode == a_mode) {
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 39/56] merge-recursive: Introduce a merge_file convenience function
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
merge_file previously required diff_filespec arguments, but all callers
only had sha1s and modes. Rename merge_file to merge_file_1 and introduce
a new merge_file convenience function which takes the sha1s and modes and
creates the temporary diff_filespec variables needed to call merge_file_1.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 72 +++++++++++++++++++++++++++-------------------------
1 files changed, 37 insertions(+), 35 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index a0dc0bf..76a451c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -846,12 +846,12 @@ static int merge_3way(struct merge_options *o,
return merge_status;
}
-static struct merge_file_info merge_file(struct merge_options *o,
- const struct diff_filespec *one,
- const struct diff_filespec *a,
- const struct diff_filespec *b,
- const char *branch1,
- const char *branch2)
+static struct merge_file_info merge_file_1(struct merge_options *o,
+ const struct diff_filespec *one,
+ const struct diff_filespec *a,
+ const struct diff_filespec *b,
+ const char *branch1,
+ const char *branch2)
{
struct merge_file_info result;
result.merge = 0;
@@ -920,6 +920,26 @@ static struct merge_file_info merge_file(struct merge_options *o,
return result;
}
+static struct merge_file_info merge_file(struct merge_options *o,
+ const char *path,
+ const unsigned char *o_sha, int o_mode,
+ const unsigned char *a_sha, int a_mode,
+ const unsigned char *b_sha, int b_mode,
+ const char *branch1,
+ const char *branch2)
+{
+ struct diff_filespec one, a, b;
+
+ one.path = a.path = b.path = (char *)path;
+ hashcpy(one.sha1, o_sha);
+ one.mode = o_mode;
+ hashcpy(a.sha1, a_sha);
+ a.mode = a_mode;
+ hashcpy(b.sha1, b_sha);
+ b.mode = b_mode;
+ return merge_file_1(o, &one, &a, &b, branch1, branch2);
+}
+
static void conflict_rename_delete(struct merge_options *o,
struct diff_filepair *pair,
const char *rename_branch,
@@ -1017,16 +1037,10 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
/* Two files were renamed to the same thing. */
if (o->call_depth) {
struct merge_file_info mfi;
- struct diff_filespec one, a, b;
-
- one.path = a.path = b.path = path;
- hashcpy(one.sha1, null_sha1);
- one.mode = 0;
- hashcpy(a.sha1, ren1->pair->two->sha1);
- a.mode = ren1->pair->two->mode;
- hashcpy(b.sha1, ren2->pair->two->sha1);
- b.mode = ren2->pair->two->mode;
- mfi = merge_file(o, &one, &a, &b, branch1, branch2);
+ mfi = merge_file(o, path, null_sha1, 0,
+ ren1->pair->two->sha1, ren1->pair->two->mode,
+ ren2->pair->two->sha1, ren2->pair->two->mode,
+ branch1, branch2);
output(o, 1, "Adding merged %s", path);
update_file(o, 0, mfi.sha, mfi.mode, path);
} else {
@@ -1213,24 +1227,12 @@ static int process_renames(struct merge_options *o,
ren1_dst, branch2);
if (o->call_depth) {
struct merge_file_info mfi;
- struct diff_filespec one, a, b;
-
- one.path = a.path = b.path =
- (char *)ren1_dst;
- hashcpy(one.sha1, null_sha1);
- one.mode = 0;
- hashcpy(a.sha1, ren1->pair->two->sha1);
- a.mode = ren1->pair->two->mode;
- hashcpy(b.sha1, dst_other.sha1);
- b.mode = dst_other.mode;
- mfi = merge_file(o, &one, &a, &b,
- branch1,
- branch2);
+ mfi = merge_file(o, ren1_dst, null_sha1, 0,
+ ren1->pair->two->sha1, ren1->pair->two->mode,
+ dst_other.sha1, dst_other.mode,
+ branch1, branch2);
output(o, 1, "Adding merged %s", ren1_dst);
- update_file(o, 0,
- mfi.sha,
- mfi.mode,
- ren1_dst);
+ update_file(o, 0, mfi.sha, mfi.mode, ren1_dst);
try_merge = 0;
} else {
char *new_path = unique_path(o, ren1_dst, branch2);
@@ -1408,8 +1410,8 @@ static int merge_content(struct merge_options *o,
if (dir_in_way(path, !o->call_depth))
df_conflict_remains = 1;
}
- mfi = merge_file(o, &one, &a, &b,
- side1 ? side1 : o->branch1, side2 ? side2 : o->branch2);
+ mfi = merge_file_1(o, &one, &a, &b,
+ side1 ? side1 : o->branch1, side2 ? side2 : o->branch2);
free(side1);
free(side2);
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 40/56] merge-recursive: Fix rename/rename(1to2) resolution for virtual merge base
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
When renaming one file to two files, we really should be doing a content
merge. Also, in the recursive case, undoing the renames and recording the
merged file in the index with the source of the rename (while deleting
both destinations) allows the renames to be re-detected in the
non-recursive merge and will result in fewer spurious conflicts.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 30 +++++++++++++-----------------
t/t6036-recursive-corner-cases.sh | 2 +-
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 76a451c..58a6ce3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -986,17 +986,6 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
src, pair1->two->path, branch1,
src, pair2->two->path, branch2,
o->call_depth ? " (left unresolved)" : "");
- if (o->call_depth) {
- /*
- * FIXME: Why remove file from cache, and then
- * immediately readd it? Why not just overwrite using
- * update_file only? Also...this is buggy for
- * rename/add-source situations...
- */
- remove_file_from_cache(src);
- update_file(o, 0, pair1->one->sha1, pair1->one->mode, src);
- }
-
if (dir_in_way(ren1_dst, !o->call_depth)) {
dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1);
output(o, 1, "%s is a directory in %s adding as %s instead",
@@ -1008,14 +997,21 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
ren2_dst, branch1, dst_name2);
}
if (o->call_depth) {
- remove_file_from_cache(dst_name1);
- remove_file_from_cache(dst_name2);
+ struct merge_file_info mfi;
+ mfi = merge_file(o, src,
+ pair1->one->sha1, pair1->one->mode,
+ pair1->two->sha1, pair1->two->mode,
+ pair2->two->sha1, pair2->two->mode,
+ branch1, branch2);
/*
- * Uncomment to leave the conflicting names in the resulting tree
- *
- * update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1);
- * update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2);
+ * FIXME: For rename/add-source conflicts (if we could detect
+ * such), this is wrong. We should instead find a unique
+ * pathname and then either rename the add-source file to that
+ * unique path, or use that unique path instead of src here.
*/
+ update_file(o, 0, mfi.sha, mfi.mode, src);
+ remove_file_from_cache(ren1_dst);
+ remove_file_from_cache(ren2_dst);
} else {
update_stages(ren1_dst, NULL, pair1->two, NULL);
update_stages(ren2_dst, NULL, NULL, pair2->two);
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
index 314fdae..5a7af0c 100755
--- a/t/t6036-recursive-corner-cases.sh
+++ b/t/t6036-recursive-corner-cases.sh
@@ -633,7 +633,7 @@ test_expect_success 'setup rename/rename(1to2)/modify followed by what looks lik
git tag E
'
-test_expect_failure 'handle rename/rename(1to2)/modify followed by what looks like rename/rename(2to1)/modify' '
+test_expect_success 'handle rename/rename(1to2)/modify followed by what looks like rename/rename(2to1)/modify' '
git checkout D^0 &&
git merge -s recursive E^0 &&
--
1.7.6.100.gac5c1
^ permalink raw reply related
* [PATCHv2 41/56] merge-recursive: Small cleanups for conflict_rename_rename_1to2
From: Elijah Newren @ 2011-08-12 5:20 UTC (permalink / raw)
To: gitster; +Cc: git, Jim Foucar, Elijah Newren
In-Reply-To: <1313126429-17368-1-git-send-email-newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
No changes since v1.
merge-recursive.c | 60 +++++++++++++++++++++++-----------------------------
1 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 58a6ce3..8b65051 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -966,58 +966,55 @@ static void conflict_rename_delete(struct merge_options *o,
}
static void conflict_rename_rename_1to2(struct merge_options *o,
- struct diff_filepair *pair1,
- const char *branch1,
- struct diff_filepair *pair2,
- const char *branch2)
+ struct rename_conflict_info *ci)
{
/* One file was renamed in both branches, but to different names. */
+ struct diff_filespec *one = ci->pair1->one;
+ struct diff_filespec *a = ci->pair1->two;
+ struct diff_filespec *b = ci->pair2->two;
+ const char *dst_name_a = a->path;
+ const char *dst_name_b = b->path;
char *del[2];
int delp = 0;
- const char *src = pair1->one->path;
- const char *ren1_dst = pair1->two->path;
- const char *ren2_dst = pair2->two->path;
- const char *dst_name1 = ren1_dst;
- const char *dst_name2 = ren2_dst;
output(o, 1, "CONFLICT (rename/rename): "
"Rename \"%s\"->\"%s\" in branch \"%s\" "
"rename \"%s\"->\"%s\" in \"%s\"%s",
- src, pair1->two->path, branch1,
- src, pair2->two->path, branch2,
+ one->path, a->path, ci->branch1,
+ one->path, b->path, ci->branch2,
o->call_depth ? " (left unresolved)" : "");
- if (dir_in_way(ren1_dst, !o->call_depth)) {
- dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1);
+ if (dir_in_way(a->path, !o->call_depth)) {
+ dst_name_a = del[delp++] = unique_path(o, a->path, ci->branch1);
output(o, 1, "%s is a directory in %s adding as %s instead",
- ren1_dst, branch2, dst_name1);
+ a->path, ci->branch2, dst_name_a);
}
- if (dir_in_way(ren2_dst, !o->call_depth)) {
- dst_name2 = del[delp++] = unique_path(o, ren2_dst, branch2);
+ if (dir_in_way(b->path, !o->call_depth)) {
+ dst_name_b = del[delp++] = unique_path(o, b->path, ci->branch2);
output(o, 1, "%s is a directory in %s adding as %s instead",
- ren2_dst, branch1, dst_name2);
+ b->path, ci->branch1, dst_name_b);
}
if (o->call_depth) {
struct merge_file_info mfi;
- mfi = merge_file(o, src,
- pair1->one->sha1, pair1->one->mode,
- pair1->two->sha1, pair1->two->mode,
- pair2->two->sha1, pair2->two->mode,
- branch1, branch2);
+ mfi = merge_file(o, one->path,
+ one->sha1, one->mode,
+ a->sha1, a->mode,
+ b->sha1, b->mode,
+ ci->branch1, ci->branch2);
/*
* FIXME: For rename/add-source conflicts (if we could detect
* such), this is wrong. We should instead find a unique
* pathname and then either rename the add-source file to that
* unique path, or use that unique path instead of src here.
*/
- update_file(o, 0, mfi.sha, mfi.mode, src);
- remove_file_from_cache(ren1_dst);
- remove_file_from_cache(ren2_dst);
+ update_file(o, 0, mfi.sha, mfi.mode, one->path);
+ remove_file_from_cache(a->path);
+ remove_file_from_cache(b->path);
} else {
- update_stages(ren1_dst, NULL, pair1->two, NULL);
- update_stages(ren2_dst, NULL, NULL, pair2->two);
+ update_stages(a->path, NULL, a, NULL);
+ update_stages(b->path, NULL, NULL, b);
- update_file(o, 0, pair1->two->sha1, pair1->two->mode, dst_name1);
- update_file(o, 0, pair2->two->sha1, pair2->two->mode, dst_name2);
+ update_file(o, 0, a->sha1, a->mode, dst_name_a);
+ update_file(o, 0, b->sha1, b->mode, dst_name_b);
}
while (delp--)
free(del[delp]);
@@ -1505,10 +1502,7 @@ static int process_entry(struct merge_options *o,
break;
case RENAME_ONE_FILE_TO_TWO:
clean_merge = 0;
- conflict_rename_rename_1to2(o, conflict_info->pair1,
- conflict_info->branch1,
- conflict_info->pair2,
- conflict_info->branch2);
+ conflict_rename_rename_1to2(o, conflict_info);
break;
default:
entry->processed = 0;
--
1.7.6.100.gac5c1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox