Rev 4889: (jam) Teach bzr merge --weave/--lca how to create a .BASE file (bug in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Dec 10 23:57:41 GMT 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4889 [merge]
revision-id: pqm at pqm.ubuntu.com-20091210235739-zwgaqsbb2mk57gyv
parent: pqm at pqm.ubuntu.com-20091210215626-qzr3coh4bgh71lb6
parent: john at arbash-meinel.com-20091210231135-1w7w19m19u3j386c
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2009-12-10 23:57:39 +0000
message:
(jam) Teach bzr merge --weave/--lca how to create a .BASE file (bug
#40412)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/merge.py merge.py-20050513021216-953b65a438527106
bzrlib/tests/blackbox/test_merge.py test_merge.py-20060323225809-9bc0459c19917f41
bzrlib/tests/blackbox/test_remerge.py test_remerge.py-20060629162739-o9m3s6143v8jnr2y-1
bzrlib/tests/test_merge.py testmerge.py-20050905070950-c1b5aa49ff911024
bzrlib/tests/test_merge_core.py test_merge_core.py-20050824132511-eb99b23a0eec641b
bzrlib/versionedfile.py versionedfile.py-20060222045106-5039c71ee3b65490
=== modified file 'NEWS'
--- a/NEWS 2009-12-10 01:30:32 +0000
+++ b/NEWS 2009-12-10 21:49:20 +0000
@@ -49,16 +49,21 @@
* ``bzr log -n0 -rN`` should not return revisions beyond its merged revisions.
(#325618, #484109, Marius Kruger)
+* ``bzr merge --weave`` and ``--lca`` will now create ``.BASE`` files for
+ files with conflicts (similar to ``--merge3``). The contents of the file
+ is a synthesis of all bases used for the merge.
+ (John Arbash Meinel, #40412)
+
* ``bzr mv --quiet`` really is quiet now. (Gordon Tyler, #271790)
* ``bzr serve`` is more clear about the risk of supplying --allow-writes.
(Robert Collins, #84659)
+* ``bzr serve --quiet`` really is quiet now. (Gordon Tyler, #252834)
+
* Fix bug with redirected URLs over authenticated HTTP.
(Glen Mailer, Neil Martinsen-Burrell, Vincent Ladeuil, #395714)
-* ``bzr serve --quiet`` really is quiet now. (Gordon Tyler, #252834)
-
* Interactive merge doesn't leave branch locks behind. (Aaron Bentley)
* Lots of bugfixes for the test suite on Windows. We should once again
=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py 2009-12-03 02:24:54 +0000
+++ b/bzrlib/merge.py 2009-12-10 17:16:19 +0000
@@ -1407,70 +1407,62 @@
supports_reverse_cherrypick = False
history_based = True
- def _merged_lines(self, file_id):
- """Generate the merged lines.
- There is no distinction between lines that are meant to contain <<<<<<<
- and conflicts.
- """
- if self.cherrypick:
- base = self.base_tree
- else:
- base = None
- plan = self.this_tree.plan_file_merge(file_id, self.other_tree,
+ def _generate_merge_plan(self, file_id, base):
+ return self.this_tree.plan_file_merge(file_id, self.other_tree,
base=base)
+
+ def _merged_lines(self, file_id):
+ """Generate the merged lines.
+ There is no distinction between lines that are meant to contain <<<<<<<
+ and conflicts.
+ """
+ if self.cherrypick:
+ base = self.base_tree
+ else:
+ base = None
+ plan = self._generate_merge_plan(file_id, base)
if 'merge' in debug.debug_flags:
plan = list(plan)
trans_id = self.tt.trans_id_file_id(file_id)
name = self.tt.final_name(trans_id) + '.plan'
- contents = ('%10s|%s' % l for l in plan)
+ contents = ('%11s|%s' % l for l in plan)
self.tt.new_file(name, self.tt.final_parent(trans_id), contents)
textmerge = versionedfile.PlanWeaveMerge(plan, '<<<<<<< TREE\n',
'>>>>>>> MERGE-SOURCE\n')
- return textmerge.merge_lines(self.reprocess)
+ lines, conflicts = textmerge.merge_lines(self.reprocess)
+ if conflicts:
+ base_lines = textmerge.base_from_plan()
+ else:
+ base_lines = None
+ return lines, base_lines
def text_merge(self, file_id, trans_id):
"""Perform a (weave) text merge for a given file and file-id.
If conflicts are encountered, .THIS and .OTHER files will be emitted,
and a conflict will be noted.
"""
- lines, conflicts = self._merged_lines(file_id)
+ lines, base_lines = self._merged_lines(file_id)
lines = list(lines)
# Note we're checking whether the OUTPUT is binary in this case,
# because we don't want to get into weave merge guts.
textfile.check_text_lines(lines)
self.tt.create_file(lines, trans_id)
- if conflicts:
+ if base_lines is not None:
+ # Conflict
self._raw_conflicts.append(('text conflict', trans_id))
name = self.tt.final_name(trans_id)
parent_id = self.tt.final_parent(trans_id)
file_group = self._dump_conflicts(name, parent_id, file_id,
- no_base=True)
+ no_base=False,
+ base_lines=base_lines)
file_group.append(trans_id)
class LCAMerger(WeaveMerger):
- def _merged_lines(self, file_id):
- """Generate the merged lines.
- There is no distinction between lines that are meant to contain <<<<<<<
- and conflicts.
- """
- if self.cherrypick:
- base = self.base_tree
- else:
- base = None
- plan = self.this_tree.plan_file_lca_merge(file_id, self.other_tree,
+ def _generate_merge_plan(self, file_id, base):
+ return self.this_tree.plan_file_lca_merge(file_id, self.other_tree,
base=base)
- if 'merge' in debug.debug_flags:
- plan = list(plan)
- trans_id = self.tt.trans_id_file_id(file_id)
- name = self.tt.final_name(trans_id) + '.plan'
- contents = ('%10s|%s' % l for l in plan)
- self.tt.new_file(name, self.tt.final_parent(trans_id), contents)
- textmerge = versionedfile.PlanWeaveMerge(plan, '<<<<<<< TREE\n',
- '>>>>>>> MERGE-SOURCE\n')
- return textmerge.merge_lines(self.reprocess)
-
class Diff3Merger(Merge3Merger):
"""Three-way merger using external diff3 for text merging"""
=== modified file 'bzrlib/tests/blackbox/test_merge.py'
--- a/bzrlib/tests/blackbox/test_merge.py 2009-12-07 21:46:28 +0000
+++ b/bzrlib/tests/blackbox/test_merge.py 2009-12-10 17:16:19 +0000
@@ -211,6 +211,22 @@
self.failUnlessExists('sub/a.txt.OTHER')
self.failUnlessExists('sub/a.txt.BASE')
+ def test_conflict_leaves_base_this_other_files(self):
+ tree, other = self.create_conflicting_branches()
+ self.run_bzr('merge ../other', working_dir='tree',
+ retcode=1)
+ self.assertFileEqual('a\nb\nc\n', 'tree/fname.BASE')
+ self.assertFileEqual('a\nB\nD\n', 'tree/fname.OTHER')
+ self.assertFileEqual('a\nB\nC\n', 'tree/fname.THIS')
+
+ def test_weave_conflict_leaves_base_this_other_files(self):
+ tree, other = self.create_conflicting_branches()
+ self.run_bzr('merge ../other --weave', working_dir='tree',
+ retcode=1)
+ self.assertFileEqual('a\nb\nc\n', 'tree/fname.BASE')
+ self.assertFileEqual('a\nB\nD\n', 'tree/fname.OTHER')
+ self.assertFileEqual('a\nB\nC\n', 'tree/fname.THIS')
+
def test_merge_remember(self):
"""Merge changes from one branch to another, test submit location."""
tree_a = self.make_branch_and_tree('branch_a')
=== modified file 'bzrlib/tests/blackbox/test_remerge.py'
--- a/bzrlib/tests/blackbox/test_remerge.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/blackbox/test_remerge.py 2009-12-10 23:11:35 +0000
@@ -85,7 +85,7 @@
'remerge --merge-type weave', retcode=1)
self.failUnlessExists('hello.OTHER')
- self.failIfExists('hello.BASE')
+ self.failUnless('hello.BASE')
self.assertFalse('|||||||' in conflict_text)
self.assertFalse('hi world' in conflict_text)
=== modified file 'bzrlib/tests/test_merge.py'
--- a/bzrlib/tests/test_merge.py 2009-09-19 00:32:14 +0000
+++ b/bzrlib/tests/test_merge.py 2009-12-10 17:16:19 +0000
@@ -38,6 +38,7 @@
from bzrlib.workingtree import WorkingTree
from bzrlib.transform import TreeTransform
+
class TestMerge(TestCaseWithTransport):
"""Test appending more than one revision"""
@@ -524,6 +525,12 @@
self.add_uncommitted_version(('root', 'C:'), [('root', 'A')], 'fabg')
return _PlanMerge('B:', 'C:', self.plan_merge_vf, ('root',))
+ def test_base_from_plan(self):
+ self.setup_plan_merge()
+ plan = self.plan_merge_vf.plan_merge('B', 'C')
+ pwm = versionedfile.PlanWeaveMerge(plan)
+ self.assertEqual(['a\n', 'b\n', 'c\n'], pwm.base_from_plan())
+
def test_unique_lines(self):
plan = self.setup_plan_merge()
self.assertEqual(plan._unique_lines(
@@ -827,6 +834,111 @@
('unchanged', 'f\n'),
('unchanged', 'g\n')],
list(plan))
+ plan = self.plan_merge_vf.plan_lca_merge('F', 'G')
+ # This is one of the main differences between plan_merge and
+ # plan_lca_merge. plan_lca_merge generates a conflict for 'x => z',
+ # because 'x' was not present in one of the bases. However, in this
+ # case it is spurious because 'x' does not exist in the global base A.
+ self.assertEqual([
+ ('unchanged', 'h\n'),
+ ('unchanged', 'a\n'),
+ ('conflicted-a', 'x\n'),
+ ('new-b', 'z\n'),
+ ('unchanged', 'c\n'),
+ ('unchanged', 'd\n'),
+ ('unchanged', 'y\n'),
+ ('unchanged', 'f\n'),
+ ('unchanged', 'g\n')],
+ list(plan))
+
+ def test_criss_cross_flip_flop(self):
+ # This is specificly trying to trigger problems when using limited
+ # ancestry and weaves. The ancestry graph looks like:
+ # XX unused ancestor, should not show up in the weave
+ # |
+ # A Unique LCA
+ # / \
+ # B C B & C both introduce a new line
+ # |\ /|
+ # | X |
+ # |/ \|
+ # D E B & C are both merged, so both are common ancestors
+ # In the process of merging, both sides order the new
+ # lines differently
+ #
+ self.add_rev('root', 'XX', [], 'qrs')
+ self.add_rev('root', 'A', ['XX'], 'abcdef')
+ self.add_rev('root', 'B', ['A'], 'abcdgef')
+ self.add_rev('root', 'C', ['A'], 'abcdhef')
+ self.add_rev('root', 'D', ['B', 'C'], 'abcdghef')
+ self.add_rev('root', 'E', ['C', 'B'], 'abcdhgef')
+ plan = list(self.plan_merge_vf.plan_merge('D', 'E'))
+ self.assertEqual([
+ ('unchanged', 'a\n'),
+ ('unchanged', 'b\n'),
+ ('unchanged', 'c\n'),
+ ('unchanged', 'd\n'),
+ ('new-b', 'h\n'),
+ ('unchanged', 'g\n'),
+ ('killed-b', 'h\n'),
+ ('unchanged', 'e\n'),
+ ('unchanged', 'f\n'),
+ ], plan)
+ pwm = versionedfile.PlanWeaveMerge(plan)
+ self.assertEqualDiff('\n'.join('abcdghef') + '\n',
+ ''.join(pwm.base_from_plan()))
+ # Reversing the order reverses the merge plan, and final order of 'hg'
+ # => 'gh'
+ plan = list(self.plan_merge_vf.plan_merge('E', 'D'))
+ self.assertEqual([
+ ('unchanged', 'a\n'),
+ ('unchanged', 'b\n'),
+ ('unchanged', 'c\n'),
+ ('unchanged', 'd\n'),
+ ('new-b', 'g\n'),
+ ('unchanged', 'h\n'),
+ ('killed-b', 'g\n'),
+ ('unchanged', 'e\n'),
+ ('unchanged', 'f\n'),
+ ], plan)
+ pwm = versionedfile.PlanWeaveMerge(plan)
+ self.assertEqualDiff('\n'.join('abcdhgef') + '\n',
+ ''.join(pwm.base_from_plan()))
+ # This is where lca differs, in that it (fairly correctly) determines
+ # that there is a conflict because both sides resolved the merge
+ # differently
+ plan = list(self.plan_merge_vf.plan_lca_merge('D', 'E'))
+ self.assertEqual([
+ ('unchanged', 'a\n'),
+ ('unchanged', 'b\n'),
+ ('unchanged', 'c\n'),
+ ('unchanged', 'd\n'),
+ ('conflicted-b', 'h\n'),
+ ('unchanged', 'g\n'),
+ ('conflicted-a', 'h\n'),
+ ('unchanged', 'e\n'),
+ ('unchanged', 'f\n'),
+ ], plan)
+ pwm = versionedfile.PlanWeaveMerge(plan)
+ self.assertEqualDiff('\n'.join('abcdgef') + '\n',
+ ''.join(pwm.base_from_plan()))
+ # Reversing it changes what line is doubled, but still gives a
+ # double-conflict
+ plan = list(self.plan_merge_vf.plan_lca_merge('E', 'D'))
+ self.assertEqual([
+ ('unchanged', 'a\n'),
+ ('unchanged', 'b\n'),
+ ('unchanged', 'c\n'),
+ ('unchanged', 'd\n'),
+ ('conflicted-b', 'g\n'),
+ ('unchanged', 'h\n'),
+ ('conflicted-a', 'g\n'),
+ ('unchanged', 'e\n'),
+ ('unchanged', 'f\n'),
+ ], plan)
+ pwm = versionedfile.PlanWeaveMerge(plan)
+ self.assertEqualDiff('\n'.join('abcdhef') + '\n',
+ ''.join(pwm.base_from_plan()))
def assertRemoveExternalReferences(self, filtered_parent_map,
child_map, tails, parent_map):
=== modified file 'bzrlib/tests/test_merge_core.py'
--- a/bzrlib/tests/test_merge_core.py 2009-08-20 04:09:58 +0000
+++ b/bzrlib/tests/test_merge_core.py 2009-12-08 21:04:07 +0000
@@ -434,6 +434,7 @@
self.assertEqual('text2', builder.this.get_file('1').read())
builder.cleanup()
+
class FunctionalMergeTest(TestCaseWithTransport):
def test_trivial_star_merge(self):
@@ -467,30 +468,61 @@
self.assertEqual("Mary\n", open("original/file2", "rt").read())
def test_conflicts(self):
- os.mkdir('a')
wta = self.make_branch_and_tree('a')
- a = wta.branch
- file('a/file', 'wb').write('contents\n')
+ self.build_tree_contents([('a/file', 'contents\n')])
wta.add('file')
wta.commit('base revision', allow_pointless=False)
- d_b = a.bzrdir.clone('b')
- b = d_b.open_branch()
- file('a/file', 'wb').write('other contents\n')
+ d_b = wta.branch.bzrdir.clone('b')
+ self.build_tree_contents([('a/file', 'other contents\n')])
wta.commit('other revision', allow_pointless=False)
- file('b/file', 'wb').write('this contents contents\n')
+ self.build_tree_contents([('b/file', 'this contents contents\n')])
wtb = d_b.open_workingtree()
wtb.commit('this revision', allow_pointless=False)
self.assertEqual(1, wtb.merge_from_branch(wta.branch))
- self.assert_(os.path.lexists('b/file.THIS'))
- self.assert_(os.path.lexists('b/file.BASE'))
- self.assert_(os.path.lexists('b/file.OTHER'))
+ self.failUnlessExists('b/file.THIS')
+ self.failUnlessExists('b/file.BASE')
+ self.failUnlessExists('b/file.OTHER')
wtb.revert()
self.assertEqual(1, wtb.merge_from_branch(wta.branch,
merge_type=WeaveMerger))
- self.assert_(os.path.lexists('b/file'))
- self.assert_(os.path.lexists('b/file.THIS'))
- self.assert_(not os.path.lexists('b/file.BASE'))
- self.assert_(os.path.lexists('b/file.OTHER'))
+ self.failUnlessExists('b/file')
+ self.failUnlessExists('b/file.THIS')
+ self.failUnlessExists('b/file.BASE')
+ self.failUnlessExists('b/file.OTHER')
+
+ def test_weave_conflicts_not_in_base(self):
+ builder = self.make_branch_builder('source')
+ builder.start_series()
+ # See bug #494197
+ # A base revision (before criss-cross)
+ # |\
+ # B C B does nothing, C adds 'foo'
+ # |X|
+ # D E D and E modify foo in incompatible ways
+ #
+ # Merging will conflict, with C as a clean base text. However, the
+ # current code uses A as the global base and 'foo' doesn't exist there.
+ # It isn't trivial to create foo.BASE because it tries to look up
+ # attributes like 'executable' in A.
+ builder.build_snapshot('A-id', None, [
+ ('add', ('', 'TREE_ROOT', 'directory', None))])
+ builder.build_snapshot('B-id', ['A-id'], [])
+ builder.build_snapshot('C-id', ['A-id'], [
+ ('add', ('foo', 'foo-id', 'file', 'orig\ncontents\n'))])
+ builder.build_snapshot('D-id', ['B-id', 'C-id'], [
+ ('add', ('foo', 'foo-id', 'file', 'orig\ncontents\nand D\n'))])
+ builder.build_snapshot('E-id', ['C-id', 'B-id'], [
+ ('modify', ('foo-id', 'orig\ncontents\nand E\n'))])
+ builder.finish_series()
+ tree = builder.get_branch().create_checkout('tree', lightweight=True)
+ self.assertEqual(1, tree.merge_from_branch(tree.branch,
+ to_revision='D-id',
+ merge_type=WeaveMerger))
+ self.failUnlessExists('tree/foo.THIS')
+ self.failUnlessExists('tree/foo.OTHER')
+ self.expectFailure('fail to create .BASE in some criss-cross merges',
+ self.failUnlessExists, 'tree/foo.BASE')
+ self.failUnlessExists('tree/foo.BASE')
def test_merge_unrelated(self):
"""Sucessfully merges unrelated branches with no common names"""
=== modified file 'bzrlib/versionedfile.py'
--- a/bzrlib/versionedfile.py 2009-10-19 15:06:58 +0000
+++ b/bzrlib/versionedfile.py 2009-12-10 17:16:19 +0000
@@ -1426,7 +1426,7 @@
def __init__(self, plan, a_marker=TextMerge.A_MARKER,
b_marker=TextMerge.B_MARKER):
TextMerge.__init__(self, a_marker, b_marker)
- self.plan = plan
+ self.plan = list(plan)
def _merge_struct(self):
lines_a = []
@@ -1490,6 +1490,60 @@
for struct in outstanding_struct():
yield struct
+ def base_from_plan(self):
+ """Construct a BASE file from the plan text."""
+ base_lines = []
+ for state, line in self.plan:
+ if state in ('killed-a', 'killed-b', 'killed-both', 'unchanged'):
+ # If unchanged, then this line is straight from base. If a or b
+ # or both killed the line, then it *used* to be in base.
+ base_lines.append(line)
+ else:
+ if state not in ('killed-base', 'irrelevant',
+ 'ghost-a', 'ghost-b',
+ 'new-a', 'new-b',
+ 'conflicted-a', 'conflicted-b'):
+ # killed-base, irrelevant means it doesn't apply
+ # ghost-a/ghost-b are harder to say for sure, but they
+ # aren't in the 'inc_c' which means they aren't in the
+ # shared base of a & b. So we don't include them. And
+ # obviously if the line is newly inserted, it isn't in base
+
+ # If 'conflicted-a' or b, then it is new vs one base, but
+ # old versus another base. However, if we make it present
+ # in the base, it will be deleted from the target, and it
+ # seems better to get a line doubled in the merge result,
+ # rather than have it deleted entirely.
+ # Example, each node is the 'text' at that point:
+ # MN
+ # / \
+ # MaN MbN
+ # | X |
+ # MabN MbaN
+ # \ /
+ # ???
+ # There was a criss-cross conflict merge. Both sides
+ # include the other, but put themselves first.
+ # Weave marks this as a 'clean' merge, picking OTHER over
+ # THIS. (Though the details depend on order inserted into
+ # weave, etc.)
+ # LCA generates a plan:
+ # [('unchanged', M),
+ # ('conflicted-b', b),
+ # ('unchanged', a),
+ # ('conflicted-a', b),
+ # ('unchanged', N)]
+ # If you mark 'conflicted-*' as part of BASE, then a 3-way
+ # merge tool will cleanly generate "MaN" (as BASE vs THIS
+ # removes one 'b', and BASE vs OTHER removes the other)
+ # If you include neither, 3-way creates a clean "MbabN" as
+ # THIS adds one 'b', and OTHER does too.
+ # It seems that having the line 2 times is better than
+ # having it omitted. (Easier to manually delete than notice
+ # it needs to be added.)
+ raise AssertionError('Unknown state: %s' % (state,))
+ return base_lines
+
class WeaveMerge(PlanWeaveMerge):
"""Weave merge that takes a VersionedFile and two versions as its input."""
More information about the bazaar-commits
mailing list