Bazaar

Merge lp:~spiv/bzr/merge-into-merger into lp:bzr/2.2

  1. merge-into-merger
  2. Merge into 2.2
Proposed by Andrew Bennetts
Status: Merged
Approved by: Andrew Bennetts
Approved revision: no longer in the source branch.
Merged at revision: 5066
Proposed branch: lp:~spiv/bzr/merge-into-merger
Merge into: lp:bzr/2.2
Diff against target: 527 lines (+422/-9)
3 files modified
NEWS (+3/-0)
bzrlib/merge.py (+177/-7)
bzrlib/tests/test_merge.py (+242/-2)
To merge this branch: bzr merge lp:~spiv/bzr/merge-into-merger
Reviewer Review Type Date Requested Status
bzr-core Pending
Review via email: mp+31247@code.launchpad.net

Commit message

Add MergeIntoMerger, which can merge all or part of a tree from an unrelated branch into a new subdirectory.

Description of the change

This addresses all the review points from Robert's review and John's initial look. It's targetted to 2.2 because it makes no API breaks (it just adds a new API), so we may as well get it out there sooner rather than later.

As Robert already voted Approve on the last round, I intend to submit this to PQM straight away; this merge proposal is just to increase visibility of the change and make it simpler for me to submit to PQM :)

To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-07-22 03:28:54 +0000
3+++ NEWS 2010-07-29 06:52:45 +0000
4@@ -56,6 +56,9 @@
5 Improvements
6 ************
7
8+* Add ``bzrlib.merge.MergeIntoMerger``, which can merge part or all of a
9+ tree, and works with unrelated branches. (Andrew Bennetts)
10+
11 Documentation
12 *************
13
14
15=== modified file 'bzrlib/merge.py'
16--- bzrlib/merge.py 2010-06-04 03:09:35 +0000
17+++ bzrlib/merge.py 2010-07-29 06:52:45 +0000
18@@ -22,7 +22,7 @@
19 branch as _mod_branch,
20 conflicts as _mod_conflicts,
21 debug,
22- errors,
23+ generate_ids,
24 graph as _mod_graph,
25 merge3,
26 osutils,
27@@ -35,11 +35,13 @@
28 tsort,
29 ui,
30 versionedfile,
31+ workingtree,
32 )
33 from bzrlib.cleanup import OperationWithCleanups
34 """)
35 from bzrlib import (
36 decorators,
37+ errors,
38 hooks,
39 )
40 from bzrlib.symbol_versioning import (
41@@ -426,7 +428,6 @@
42 return self._cached_trees[revision_id]
43
44 def _get_tree(self, treespec, possible_transports=None):
45- from bzrlib import workingtree
46 location, revno = treespec
47 if revno is None:
48 tree = workingtree.WorkingTree.open_containing(location)[0]
49@@ -861,6 +862,13 @@
50 finally:
51 child_pb.finished()
52 self.fix_root()
53+ self._finish_computing_transform()
54+
55+ def _finish_computing_transform(self):
56+ """Finalize the transform and report the changes.
57+
58+ This is the second half of _compute_transform.
59+ """
60 child_pb = ui.ui_factory.nested_progress_bar()
61 try:
62 fs_conflicts = transform.resolve_conflicts(self.tt, child_pb,
63@@ -1076,7 +1084,6 @@
64 ))
65 return result
66
67-
68 def fix_root(self):
69 try:
70 self.tt.final_kind(self.tt.root)
71@@ -1755,6 +1762,168 @@
72 osutils.rmtree(temp_dir)
73
74
75+class PathNotInTree(errors.BzrError):
76+
77+ _fmt = """Merge-into failed because %(tree)s does not contain %(path)s."""
78+
79+ def __init__(self, path, tree):
80+ errors.BzrError.__init__(self, path=path, tree=tree)
81+
82+
83+class MergeIntoMerger(Merger):
84+ """Merger that understands other_tree will be merged into a subdir.
85+
86+ This also changes the Merger api so that it uses real Branch, revision_id,
87+ and RevisonTree objects, rather than using revision specs.
88+ """
89+
90+ def __init__(self, this_tree, other_branch, other_tree, target_subdir,
91+ source_subpath, other_rev_id=None):
92+ """Create a new MergeIntoMerger object.
93+
94+ source_subpath in other_tree will be effectively copied to
95+ target_subdir in this_tree.
96+
97+ :param this_tree: The tree that we will be merging into.
98+ :param other_branch: The Branch we will be merging from.
99+ :param other_tree: The RevisionTree object we want to merge.
100+ :param target_subdir: The relative path where we want to merge
101+ other_tree into this_tree
102+ :param source_subpath: The relative path specifying the subtree of
103+ other_tree to merge into this_tree.
104+ """
105+ # It is assumed that we are merging a tree that is not in our current
106+ # ancestry, which means we are using the "EmptyTree" as our basis.
107+ null_ancestor_tree = this_tree.branch.repository.revision_tree(
108+ _mod_revision.NULL_REVISION)
109+ super(MergeIntoMerger, self).__init__(
110+ this_branch=this_tree.branch,
111+ this_tree=this_tree,
112+ other_tree=other_tree,
113+ base_tree=null_ancestor_tree,
114+ )
115+ self._target_subdir = target_subdir
116+ self._source_subpath = source_subpath
117+ self.other_branch = other_branch
118+ if other_rev_id is None:
119+ other_rev_id = other_tree.get_revision_id()
120+ self.other_rev_id = self.other_basis = other_rev_id
121+ self.base_is_ancestor = True
122+ self.backup_files = True
123+ self.merge_type = Merge3Merger
124+ self.show_base = False
125+ self.reprocess = False
126+ self.interesting_ids = None
127+ self.merge_type = _MergeTypeParameterizer(MergeIntoMergeType,
128+ target_subdir=self._target_subdir,
129+ source_subpath=self._source_subpath)
130+ if self._source_subpath != '':
131+ # If this isn't a partial merge make sure the revisions will be
132+ # present.
133+ self._maybe_fetch(self.other_branch, self.this_branch,
134+ self.other_basis)
135+
136+ def set_pending(self):
137+ if self._source_subpath != '':
138+ return
139+ Merger.set_pending(self)
140+
141+
142+class _MergeTypeParameterizer(object):
143+ """Wrap a merge-type class to provide extra parameters.
144+
145+ This is hack used by MergeIntoMerger to pass some extra parameters to its
146+ merge_type. Merger.do_merge() sets up its own set of parameters to pass to
147+ the 'merge_type' member. It is difficult override do_merge without
148+ re-writing the whole thing, so instead we create a wrapper which will pass
149+ the extra parameters.
150+ """
151+
152+ def __init__(self, merge_type, **kwargs):
153+ self._extra_kwargs = kwargs
154+ self._merge_type = merge_type
155+
156+ def __call__(self, *args, **kwargs):
157+ kwargs.update(self._extra_kwargs)
158+ return self._merge_type(*args, **kwargs)
159+
160+ def __getattr__(self, name):
161+ return getattr(self._merge_type, name)
162+
163+
164+class MergeIntoMergeType(Merge3Merger):
165+ """Merger that incorporates a tree (or part of a tree) into another."""
166+
167+ def __init__(self, *args, **kwargs):
168+ """Initialize the merger object.
169+
170+ :param args: See Merge3Merger.__init__'s args.
171+ :param kwargs: See Merge3Merger.__init__'s keyword args, except for
172+ source_subpath and target_subdir.
173+ :keyword source_subpath: The relative path specifying the subtree of
174+ other_tree to merge into this_tree.
175+ :keyword target_subdir: The relative path where we want to merge
176+ other_tree into this_tree
177+ """
178+ # All of the interesting work happens during Merge3Merger.__init__(),
179+ # so we have have to hack in to get our extra parameters set.
180+ self._source_subpath = kwargs.pop('source_subpath')
181+ self._target_subdir = kwargs.pop('target_subdir')
182+ super(MergeIntoMergeType, self).__init__(*args, **kwargs)
183+
184+ def _compute_transform(self):
185+ child_pb = ui.ui_factory.nested_progress_bar()
186+ try:
187+ entries = self._entries_to_incorporate()
188+ entries = list(entries)
189+ for num, (entry, parent_id) in enumerate(entries):
190+ child_pb.update('Preparing file merge', num, len(entries))
191+ parent_trans_id = self.tt.trans_id_file_id(parent_id)
192+ trans_id = transform.new_by_entry(self.tt, entry,
193+ parent_trans_id, self.other_tree)
194+ finally:
195+ child_pb.finished()
196+ self._finish_computing_transform()
197+
198+ def _entries_to_incorporate(self):
199+ """Yields pairs of (inventory_entry, new_parent)."""
200+ other_inv = self.other_tree.inventory
201+ subdir_id = other_inv.path2id(self._source_subpath)
202+ if subdir_id is None:
203+ # XXX: The error would be clearer if it gave the URL of the source
204+ # branch, but we don't have a reference to that here.
205+ raise PathNotInTree(self._source_subpath, "Source tree")
206+ subdir = other_inv[subdir_id]
207+ parent_in_target = osutils.dirname(self._target_subdir)
208+ target_id = self.this_tree.inventory.path2id(parent_in_target)
209+ if target_id is None:
210+ raise PathNotInTree(self._target_subdir, "Target tree")
211+ name_in_target = osutils.basename(self._target_subdir)
212+ merge_into_root = subdir.copy()
213+ merge_into_root.name = name_in_target
214+ if merge_into_root.file_id in self.this_tree.inventory:
215+ # Give the root a new file-id.
216+ # This can happen fairly easily if the directory we are
217+ # incorporating is the root, and both trees have 'TREE_ROOT' as
218+ # their root_id. Users will expect this to Just Work, so we
219+ # change the file-id here.
220+ # Non-root file-ids could potentially conflict too. That's really
221+ # an edge case, so we don't do anything special for those. We let
222+ # them cause conflicts.
223+ merge_into_root.file_id = generate_ids.gen_file_id(name_in_target)
224+ yield (merge_into_root, target_id)
225+ if subdir.kind != 'directory':
226+ # No children, so we are done.
227+ return
228+ for ignored_path, entry in other_inv.iter_entries_by_dir(subdir_id):
229+ parent_id = entry.parent_id
230+ if parent_id == subdir.file_id:
231+ # The root's parent ID has changed, so make sure children of
232+ # the root refer to the new ID.
233+ parent_id = merge_into_root.file_id
234+ yield (entry, parent_id)
235+
236+
237 def merge_inner(this_branch, other_tree, base_tree, ignore_zero=False,
238 backup_files=False,
239 merge_type=Merge3Merger,
240@@ -1768,10 +1937,11 @@
241 change_reporter=None):
242 """Primary interface for merging.
243
244- typical use is probably
245- 'merge_inner(branch, branch.get_revision_tree(other_revision),
246- branch.get_revision_tree(base_revision))'
247- """
248+ Typical use is probably::
249+
250+ merge_inner(branch, branch.get_revision_tree(other_revision),
251+ branch.get_revision_tree(base_revision))
252+ """
253 if this_tree is None:
254 raise errors.BzrError("bzrlib.merge.merge_inner requires a this_tree "
255 "parameter as of bzrlib version 0.8.")
256
257=== modified file 'bzrlib/tests/test_merge.py'
258--- bzrlib/tests/test_merge.py 2010-02-23 07:43:11 +0000
259+++ bzrlib/tests/test_merge.py 2010-07-29 06:52:45 +0000
260@@ -18,13 +18,16 @@
261 from StringIO import StringIO
262
263 from bzrlib import (
264+ branch as _mod_branch,
265+ cleanup,
266 conflicts,
267 errors,
268+ inventory,
269 knit,
270 memorytree,
271 merge as _mod_merge,
272 option,
273- progress,
274+ revision as _mod_revision,
275 tests,
276 transform,
277 versionedfile,
278@@ -32,7 +35,7 @@
279 from bzrlib.conflicts import ConflictList, TextConflict
280 from bzrlib.errors import UnrelatedBranches, NoCommits
281 from bzrlib.merge import transform_tree, merge_inner, _PlanMerge
282-from bzrlib.osutils import pathjoin, file_kind
283+from bzrlib.osutils import basename, pathjoin, file_kind
284 from bzrlib.tests import (
285 TestCaseWithMemoryTransport,
286 TestCaseWithTransport,
287@@ -2917,3 +2920,240 @@
288 conflicts = builder.merge()
289 # The hook should not call the merge_text() method
290 self.assertEqual([], self.calls)
291+
292+
293+class TestMergeIntoBase(tests.TestCaseWithTransport):
294+
295+ def setup_simple_branch(self, relpath, shape=None, root_id=None):
296+ """One commit, containing tree specified by optional shape.
297+
298+ Default is empty tree (just root entry).
299+ """
300+ if root_id is None:
301+ root_id = '%s-root-id' % (relpath,)
302+ wt = self.make_branch_and_tree(relpath)
303+ wt.set_root_id(root_id)
304+ if shape is not None:
305+ adjusted_shape = [relpath + '/' + elem for elem in shape]
306+ self.build_tree(adjusted_shape)
307+ ids = ['%s-%s-id' % (relpath, basename(elem.rstrip('/')))
308+ for elem in shape]
309+ wt.add(shape, ids=ids)
310+ rev_id = 'r1-%s' % (relpath,)
311+ wt.commit("Initial commit of %s" % (relpath,), rev_id=rev_id)
312+ self.assertEqual(root_id, wt.path2id(''))
313+ return wt
314+
315+ def setup_two_branches(self, custom_root_ids=True):
316+ """Setup 2 branches, one will be a library, the other a project."""
317+ if custom_root_ids:
318+ root_id = None
319+ else:
320+ root_id = inventory.ROOT_ID
321+ project_wt = self.setup_simple_branch(
322+ 'project', ['README', 'dir/', 'dir/file.c'],
323+ root_id)
324+ lib_wt = self.setup_simple_branch(
325+ 'lib1', ['README', 'Makefile', 'foo.c'], root_id)
326+
327+ return project_wt, lib_wt
328+
329+ def do_merge_into(self, location, merge_as):
330+ """Helper for using MergeIntoMerger.
331+
332+ :param location: location of directory to merge from, either the
333+ location of a branch or of a path inside a branch.
334+ :param merge_as: the path in a tree to add the new directory as.
335+ :returns: the conflicts from 'do_merge'.
336+ """
337+ operation = cleanup.OperationWithCleanups(self._merge_into)
338+ return operation.run(location, merge_as)
339+
340+ def _merge_into(self, op, location, merge_as):
341+ # Open and lock the various tree and branch objects
342+ wt, subdir_relpath = WorkingTree.open_containing(merge_as)
343+ op.add_cleanup(wt.lock_write().unlock)
344+ branch_to_merge, subdir_to_merge = _mod_branch.Branch.open_containing(
345+ location)
346+ op.add_cleanup(branch_to_merge.lock_read().unlock)
347+ other_tree = branch_to_merge.basis_tree()
348+ op.add_cleanup(other_tree.lock_read().unlock)
349+ # Perform the merge
350+ merger = _mod_merge.MergeIntoMerger(this_tree=wt, other_tree=other_tree,
351+ other_branch=branch_to_merge, target_subdir=subdir_relpath,
352+ source_subpath=subdir_to_merge)
353+ merger.set_base_revision(_mod_revision.NULL_REVISION, branch_to_merge)
354+ conflicts = merger.do_merge()
355+ merger.set_pending()
356+ return conflicts
357+
358+ def assertTreeEntriesEqual(self, expected_entries, tree):
359+ """Assert that 'tree' contains the expected inventory entries.
360+
361+ :param expected_entries: sequence of (path, file-id) pairs.
362+ """
363+ files = [(path, ie.file_id) for path, ie in tree.iter_entries_by_dir()]
364+ self.assertEqual(expected_entries, files)
365+
366+
367+class TestMergeInto(TestMergeIntoBase):
368+
369+ def test_newdir_with_unique_roots(self):
370+ """Merge a branch with a unique root into a new directory."""
371+ project_wt, lib_wt = self.setup_two_branches()
372+ self.do_merge_into('lib1', 'project/lib1')
373+ project_wt.lock_read()
374+ self.addCleanup(project_wt.unlock)
375+ # The r1-lib1 revision should be merged into this one
376+ self.assertEqual(['r1-project', 'r1-lib1'], project_wt.get_parent_ids())
377+ self.assertTreeEntriesEqual(
378+ [('', 'project-root-id'),
379+ ('README', 'project-README-id'),
380+ ('dir', 'project-dir-id'),
381+ ('lib1', 'lib1-root-id'),
382+ ('dir/file.c', 'project-file.c-id'),
383+ ('lib1/Makefile', 'lib1-Makefile-id'),
384+ ('lib1/README', 'lib1-README-id'),
385+ ('lib1/foo.c', 'lib1-foo.c-id'),
386+ ], project_wt)
387+
388+ def test_subdir(self):
389+ """Merge a branch into a subdirectory of an existing directory."""
390+ project_wt, lib_wt = self.setup_two_branches()
391+ self.do_merge_into('lib1', 'project/dir/lib1')
392+ project_wt.lock_read()
393+ self.addCleanup(project_wt.unlock)
394+ # The r1-lib1 revision should be merged into this one
395+ self.assertEqual(['r1-project', 'r1-lib1'], project_wt.get_parent_ids())
396+ self.assertTreeEntriesEqual(
397+ [('', 'project-root-id'),
398+ ('README', 'project-README-id'),
399+ ('dir', 'project-dir-id'),
400+ ('dir/file.c', 'project-file.c-id'),
401+ ('dir/lib1', 'lib1-root-id'),
402+ ('dir/lib1/Makefile', 'lib1-Makefile-id'),
403+ ('dir/lib1/README', 'lib1-README-id'),
404+ ('dir/lib1/foo.c', 'lib1-foo.c-id'),
405+ ], project_wt)
406+
407+ def test_newdir_with_repeat_roots(self):
408+ """If the file-id of the dir to be merged already exists a new ID will
409+ be allocated to let the merge happen.
410+ """
411+ project_wt, lib_wt = self.setup_two_branches(custom_root_ids=False)
412+ root_id = project_wt.path2id('')
413+ self.do_merge_into('lib1', 'project/lib1')
414+ project_wt.lock_read()
415+ self.addCleanup(project_wt.unlock)
416+ # The r1-lib1 revision should be merged into this one
417+ self.assertEqual(['r1-project', 'r1-lib1'], project_wt.get_parent_ids())
418+ new_lib1_id = project_wt.path2id('lib1')
419+ self.assertNotEqual(None, new_lib1_id)
420+ self.assertTreeEntriesEqual(
421+ [('', root_id),
422+ ('README', 'project-README-id'),
423+ ('dir', 'project-dir-id'),
424+ ('lib1', new_lib1_id),
425+ ('dir/file.c', 'project-file.c-id'),
426+ ('lib1/Makefile', 'lib1-Makefile-id'),
427+ ('lib1/README', 'lib1-README-id'),
428+ ('lib1/foo.c', 'lib1-foo.c-id'),
429+ ], project_wt)
430+
431+ def test_name_conflict(self):
432+ """When the target directory name already exists a conflict is
433+ generated and the original directory is renamed to foo.moved.
434+ """
435+ dest_wt = self.setup_simple_branch('dest', ['dir/', 'dir/file.txt'])
436+ src_wt = self.setup_simple_branch('src', ['README'])
437+ conflicts = self.do_merge_into('src', 'dest/dir')
438+ self.assertEqual(1, conflicts)
439+ dest_wt.lock_read()
440+ self.addCleanup(dest_wt.unlock)
441+ # The r1-lib1 revision should be merged into this one
442+ self.assertEqual(['r1-dest', 'r1-src'], dest_wt.get_parent_ids())
443+ self.assertTreeEntriesEqual(
444+ [('', 'dest-root-id'),
445+ ('dir', 'src-root-id'),
446+ ('dir.moved', 'dest-dir-id'),
447+ ('dir/README', 'src-README-id'),
448+ ('dir.moved/file.txt', 'dest-file.txt-id'),
449+ ], dest_wt)
450+
451+ def test_file_id_conflict(self):
452+ """A conflict is generated if the merge-into adds a file (or other
453+ inventory entry) with a file-id that already exists in the target tree.
454+ """
455+ dest_wt = self.setup_simple_branch('dest', ['file.txt'])
456+ # Make a second tree with a file-id that will clash with file.txt in
457+ # dest.
458+ src_wt = self.make_branch_and_tree('src')
459+ self.build_tree(['src/README'])
460+ src_wt.add(['README'], ids=['dest-file.txt-id'])
461+ src_wt.commit("Rev 1 of src.", rev_id='r1-src')
462+ conflicts = self.do_merge_into('src', 'dest/dir')
463+ # This is an edge case that shouldn't happen to users very often. So
464+ # we don't care really about the exact presentation of the conflict,
465+ # just that there is one.
466+ self.assertEqual(1, conflicts)
467+
468+ def test_only_subdir(self):
469+ """When the location points to just part of a tree, merge just that
470+ subtree.
471+ """
472+ dest_wt = self.setup_simple_branch('dest')
473+ src_wt = self.setup_simple_branch(
474+ 'src', ['hello.txt', 'dir/', 'dir/foo.c'])
475+ conflicts = self.do_merge_into('src/dir', 'dest/dir')
476+ dest_wt.lock_read()
477+ self.addCleanup(dest_wt.unlock)
478+ # The r1-lib1 revision should NOT be merged into this one (this is a
479+ # partial merge).
480+ self.assertEqual(['r1-dest'], dest_wt.get_parent_ids())
481+ self.assertTreeEntriesEqual(
482+ [('', 'dest-root-id'),
483+ ('dir', 'src-dir-id'),
484+ ('dir/foo.c', 'src-foo.c-id'),
485+ ], dest_wt)
486+
487+ def test_only_file(self):
488+ """An edge case: merge just one file, not a whole dir."""
489+ dest_wt = self.setup_simple_branch('dest')
490+ two_file_wt = self.setup_simple_branch(
491+ 'two-file', ['file1.txt', 'file2.txt'])
492+ conflicts = self.do_merge_into('two-file/file1.txt', 'dest/file1.txt')
493+ dest_wt.lock_read()
494+ self.addCleanup(dest_wt.unlock)
495+ # The r1-lib1 revision should NOT be merged into this one
496+ self.assertEqual(['r1-dest'], dest_wt.get_parent_ids())
497+ self.assertTreeEntriesEqual(
498+ [('', 'dest-root-id'), ('file1.txt', 'two-file-file1.txt-id')],
499+ dest_wt)
500+
501+ def test_no_such_source_path(self):
502+ """PathNotInTree is raised if the specified path in the source tree
503+ does not exist.
504+ """
505+ dest_wt = self.setup_simple_branch('dest')
506+ two_file_wt = self.setup_simple_branch('src', ['dir/'])
507+ self.assertRaises(_mod_merge.PathNotInTree, self.do_merge_into,
508+ 'src/no-such-dir', 'dest/foo')
509+ dest_wt.lock_read()
510+ self.addCleanup(dest_wt.unlock)
511+ # The dest tree is unmodified.
512+ self.assertEqual(['r1-dest'], dest_wt.get_parent_ids())
513+ self.assertTreeEntriesEqual([('', 'dest-root-id')], dest_wt)
514+
515+ def test_no_such_target_path(self):
516+ """PathNotInTree is also raised if the specified path in the target
517+ tree does not exist.
518+ """
519+ dest_wt = self.setup_simple_branch('dest')
520+ two_file_wt = self.setup_simple_branch('src', ['file.txt'])
521+ self.assertRaises(_mod_merge.PathNotInTree, self.do_merge_into,
522+ 'src', 'dest/no-such-dir/foo')
523+ dest_wt.lock_read()
524+ self.addCleanup(dest_wt.unlock)
525+ # The dest tree is unmodified.
526+ self.assertEqual(['r1-dest'], dest_wt.get_parent_ids())
527+ self.assertTreeEntriesEqual([('', 'dest-root-id')], dest_wt)

Subscribers

People subscribed via source and target branches