Skip to content

Commit 883dcfc

Browse files
committed
Merge branch 'en/ort-recursive-d-f-conflict-fix' into next
The ort merge machinery was hit an assertion failure in a history with criss-cross merges renamed a directory and a non-directory, which has been corrected. * en/ort-recursive-d-f-conflict-fix: merge-ort: fix corner case recursive submodule/directory conflict handling
2 parents d1d712e + 979ee83 commit 883dcfc

File tree

2 files changed

+120
-1
lines changed

2 files changed

+120
-1
lines changed

merge-ort.c

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1502,11 +1502,44 @@ static void resolve_trivial_directory_merge(struct conflict_info *ci, int side)
15021502
VERIFY_CI(ci);
15031503
assert((side == 1 && ci->match_mask == 5) ||
15041504
(side == 2 && ci->match_mask == 3));
1505+
1506+
/*
1507+
* Since ci->stages[0] matches ci->stages[3-side], resolve merge in
1508+
* favor of ci->stages[side].
1509+
*/
15051510
oidcpy(&ci->merged.result.oid, &ci->stages[side].oid);
15061511
ci->merged.result.mode = ci->stages[side].mode;
15071512
ci->merged.is_null = is_null_oid(&ci->stages[side].oid);
1513+
1514+
/*
1515+
* Because we resolved in favor of "side", we are no longer
1516+
* considering the paths which matched (i.e. had the same hash) any
1517+
* more. Strip the matching paths from both dirmask & filemask.
1518+
* Another consequence of merging in favor of side is that we can no
1519+
* longer have a directory/file conflict either..but there's a slight
1520+
* nuance we consider before clearing it.
1521+
*
1522+
* In most cases, resolving in favor of the other side means there's
1523+
* no conflict at all, but if we had a directory/file conflict to
1524+
* start, and the directory is resolved away, the remaining file could
1525+
* still be part of a rename. If the remaining file is part of a
1526+
* rename, then it may also be part of a rename conflict (e.g.
1527+
* rename/delete or rename/rename(1to2)), so we can't
1528+
* mark it as a clean merge if we started with a directory/file
1529+
* conflict and still have a file left.
1530+
*
1531+
* In contrast, if we started with a directory/file conflict and
1532+
* still have a directory left, no file under that directory can be
1533+
* part of a rename, otherwise we would have had to recurse into the
1534+
* directory and would have never ended up within
1535+
* resolve_trivial_directory_merge() for that directory.
1536+
*/
1537+
ci->dirmask &= (~ci->match_mask);
1538+
ci->filemask &= (~ci->match_mask);
1539+
assert(!ci->filemask || !ci->dirmask);
15081540
ci->match_mask = 0;
1509-
ci->merged.clean = 1; /* (ci->filemask == 0); */
1541+
ci->merged.clean = !ci->df_conflict || ci->dirmask;
1542+
ci->df_conflict = 0;
15101543
}
15111544

15121545
static int handle_deferred_entries(struct merge_options *opt,

t/t6422-merge-rename-corner-cases.sh

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,4 +1439,90 @@ test_expect_success 'rename/rename(1to2) with a binary file' '
14391439
)
14401440
'
14411441

1442+
# Testcase preliminary submodule/directory conflict and submodule rename
1443+
# Commit O: <empty, or additional irrelevant stuff>
1444+
# Commit A1: introduce "folder" (as a tree)
1445+
# Commit B1: introduce "folder" (as a submodule)
1446+
# Commit A2: merge B1 into A1, but keep folder as a tree
1447+
# Commit B2: merge A1 into B1, but keep folder as a submodule
1448+
# Merge A2 & B2
1449+
test_setup_submodule_directory_preliminary_conflict () {
1450+
git init submodule_directory_preliminary_conflict &&
1451+
(
1452+
cd submodule_directory_preliminary_conflict &&
1453+
1454+
# Trying to do the A2 and B2 merges above is slightly more
1455+
# challenging with a local submodule (because checking out
1456+
# another commit has the submodule in the way). Instead,
1457+
# first create the commits with the wrong parents but right
1458+
# trees, in the order A1, A2, B1, B2...
1459+
#
1460+
# Then go back and create new A2 & B2 with the correct
1461+
# parents and the same trees.
1462+
1463+
git commit --allow-empty -m orig &&
1464+
1465+
git branch A &&
1466+
git branch B &&
1467+
1468+
git checkout B &&
1469+
mkdir folder &&
1470+
echo A>folder/A &&
1471+
echo B>folder/B &&
1472+
echo C>folder/C &&
1473+
echo D>folder/D &&
1474+
echo E>folder/E &&
1475+
git add folder &&
1476+
git commit -m B1 &&
1477+
1478+
git commit --allow-empty -m B2 &&
1479+
1480+
git checkout A &&
1481+
git init folder &&
1482+
(
1483+
cd folder &&
1484+
>Z &&
1485+
>Y &&
1486+
git add Z Y &&
1487+
git commit -m "original submodule commit"
1488+
) &&
1489+
git add folder &&
1490+
git commit -m A1 &&
1491+
1492+
git commit --allow-empty -m A2 &&
1493+
1494+
NewA2=$(git commit-tree -p A^ -p B^ -m "Merge B into A" A^{tree}) &&
1495+
NewB2=$(git commit-tree -p B^ -p A^ -m "Merge A into B" B^{tree}) &&
1496+
git update-ref refs/heads/A $NewA2 &&
1497+
git update-ref refs/heads/B $NewB2
1498+
)
1499+
}
1500+
1501+
test_expect_success 'submodule/directory preliminary conflict' '
1502+
test_setup_submodule_directory_preliminary_conflict &&
1503+
(
1504+
cd submodule_directory_preliminary_conflict &&
1505+
1506+
git checkout A^0 &&
1507+
1508+
test_expect_code 1 git merge B^0 &&
1509+
1510+
# Make sure the index has the right number of entries
1511+
git ls-files -s >actual &&
1512+
test_line_count = 2 actual &&
1513+
1514+
# The "folder" as directory should have been resolved away
1515+
# as part of the merge. The "folder" as submodule got
1516+
# renamed to "folder~Temporary merge branch 2" in the
1517+
# virtual merge base, resulting in a
1518+
# "folder~Temporary merge branch 2" -> "folder"
1519+
# rename in the outermerge for the submodule, which then
1520+
# becomes part of a rename/delete conflict (because "folder"
1521+
# as a submodule was deleted in A2).
1522+
submod=$(git rev-parse A:folder) &&
1523+
printf "160000 $submod 1\tfolder\n160000 $submod 2\tfolder\n" >expect &&
1524+
test_cmp expect actual
1525+
)
1526+
'
1527+
14421528
test_done

0 commit comments

Comments
 (0)