Rev 3949: Fix an edge case with deleted files and criss-cross merges. in lp:///~jameinel/bzr/1.12-lca-deleted
John Arbash Meinel
john at arbash-meinel.com
Tue Jan 20 18:13:56 GMT 2009
At lp:///~jameinel/bzr/1.12-lca-deleted
------------------------------------------------------------
revno: 3949
revision-id: john at arbash-meinel.com-20090120181350-wzl4x01ns3rly9y5
parent: pqm at pqm.ubuntu.com-20090120044335-pwr2rshr1yu6vzti
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 1.12-lca-deleted
timestamp: Tue 2009-01-20 12:13:50 -0600
message:
Fix an edge case with deleted files and criss-cross merges.
We wanted to be careful that if you had a criss-cross merge and both sides
resolved the conflict in a different way, that you got a conflict.
However, the original test was actually testing when one LCA dominated the
other, which is a case where we shouldn't conflict.
Test cases updated appropriately.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2009-01-18 01:48:15 +0000
+++ b/NEWS 2009-01-20 18:13:50 +0000
@@ -12,6 +12,14 @@
* Progress bars now show the rate of activity for some sftp
operations, and they are drawn different. (Martin Pool, #172741)
+ BUG FIXES:
+
+ * There was a bug in how we handled resolving when a file is deleted
+ in one branch, and modified in the other. If there was a criss-cross
+ merge, we would cause the deletion to conflict a second time.
+ (Vincent Ladeuil, John Arbash Meinel)
+
+
NOT RELEASED YET
----------------
=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py 2009-01-14 12:27:02 +0000
+++ b/bzrlib/merge.py 2009-01-20 18:13:50 +0000
@@ -794,16 +794,12 @@
content_changed = True
if kind_winner == 'this':
# No kind change in OTHER, see if there are *any* changes
- if other_ie.kind == None:
- # No content and 'this' wins the kind, so skip this?
- # continue
- pass
- elif other_ie.kind == 'directory':
+ if other_ie.kind == 'directory':
if parent_id_winner == 'this' and name_winner == 'this':
# No change for this directory in OTHER, skip
continue
content_changed = False
- elif other_ie.kind == 'file':
+ elif other_ie.kind is None or other_ie.kind == 'file':
def get_sha1(ie, tree):
if ie.kind != 'file':
return None
=== modified file 'bzrlib/tests/test_merge.py'
--- a/bzrlib/tests/test_merge.py 2008-09-05 03:11:40 +0000
+++ b/bzrlib/tests/test_merge.py 2009-01-20 18:13:50 +0000
@@ -1410,7 +1410,12 @@
# B C B nothing, C deletes foo
# |X|
# D E D restores foo (same as B), E leaves it deleted
- # We should emit an entry for this
+ # Analysis:
+ # A => B, no changes
+ # A => C, delete foo (C should supersede B)
+ # C => D, restore foo
+ # C => E, no changes
+ # D would then win 'cleanly' and no record would be given
builder = self.get_builder()
builder.build_snapshot('A-id', None,
[('add', (u'', 'a-root-id', 'directory', None)),
@@ -1424,6 +1429,37 @@
entries = list(merge_obj._entries_lca())
root_id = 'a-root-id'
+ self.assertEqual([], entries)
+
+ def test_not_in_other_mod_in_lca1_not_in_lca2(self):
+ # A base, introduces 'foo'
+ # |\
+ # B C B changes 'foo', C deletes foo
+ # |X|
+ # D E D restores foo (same as B), E leaves it deleted (as C)
+ # Analysis:
+ # A => B, modified foo
+ # A => C, delete foo, C does not supersede B
+ # B => D, no changes
+ # C => D, resolve in favor of B
+ # B => E, resolve in favor of E
+ # C => E, no changes
+ # In this case, we have a conflict of how the changes were resolved. E
+ # picked C and D picked B, so we should issue a conflict
+ builder = self.get_builder()
+ builder.build_snapshot('A-id', None,
+ [('add', (u'', 'a-root-id', 'directory', None)),
+ ('add', (u'foo', 'foo-id', 'file', 'content\n'))])
+ builder.build_snapshot('B-id', ['A-id'], [
+ ('modify', ('foo-id', 'new-content\n'))])
+ builder.build_snapshot('C-id', ['A-id'],
+ [('unversion', 'foo-id')])
+ builder.build_snapshot('E-id', ['C-id', 'B-id'], [])
+ builder.build_snapshot('D-id', ['B-id', 'C-id'], [])
+ merge_obj = self.make_merge_obj(builder, 'E-id')
+
+ entries = list(merge_obj._entries_lca())
+ root_id = 'a-root-id'
self.assertEqual([('foo-id', True,
((root_id, [root_id, None]), None, root_id),
((u'foo', [u'foo', None]), None, 'foo'),
@@ -1431,6 +1467,21 @@
], entries)
def test_only_in_one_lca(self):
+ # A add only root
+ # |\
+ # B C B nothing, C add file
+ # |X|
+ # D E D still has nothing, E removes file
+ # Analysis:
+ # B => D, no change
+ # C => D, removed the file
+ # B => E, no change
+ # C => E, removed the file
+ # Thus D & E have identical changes, and this is a no-op
+ # Alternatively:
+ # A => B, no change
+ # A => C, add file, thus C supersedes B
+ # w/ C=BASE, D=THIS, E=OTHER we have 'happy convergence'
builder = self.get_builder()
builder.build_snapshot('A-id', None,
[('add', (u'', 'a-root-id', 'directory', None))])
@@ -1444,11 +1495,7 @@
entries = list(merge_obj._entries_lca())
root_id = 'a-root-id'
- self.assertEqual([('a-id', True,
- ((None, [None, root_id]), None, None),
- ((None, [None, u'a']), None, None),
- ((None, [None, False]), None, None)),
- ], entries)
+ self.assertEqual([], entries)
def test_only_in_other(self):
builder = self.get_builder()
More information about the bazaar-commits
mailing list