Rev 2603: Merge uses Tree._iter_changes (Aaron Bentley) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Jul 12 03:12:39 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2603
revision-id: pqm at pqm.ubuntu.com-20070712021235-0a3tqhdt9nxk0x9y
parent: pqm at pqm.ubuntu.com-20070711221443-ntcgqkwy42mislml
parent: abentley at panoramicfeedback.com-20070710181401-lfw19orwavwyk603
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2007-07-12 03:12:35 +0100
message:
  Merge uses Tree._iter_changes (Aaron Bentley)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/merge.py                merge.py-20050513021216-953b65a438527106
  bzrlib/tests/test_merge_core.py test_merge_core.py-20050824132511-eb99b23a0eec641b
  bzrlib/transform.py            transform.py-20060105172343-dd99e54394d91687
  doc/developers/performance.dot performance.dot-20070527173558-rqaqxn1al7vzgcto-3
    ------------------------------------------------------------
    revno: 2590.2.10
    merged: abentley at panoramicfeedback.com-20070710181401-lfw19orwavwyk603
    parent: abentley at panoramicfeedback.com-20070710175519-qfdqkmx0zankbgfo
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: changes-merge
    timestamp: Tue 2007-07-10 14:14:01 -0400
    message:
      Updates from review
    ------------------------------------------------------------
    revno: 2590.2.9
    merged: abentley at panoramicfeedback.com-20070710175519-qfdqkmx0zankbgfo
    parent: abentley at panoramicfeedback.com-20070706172134-kz3k30d2pu61qngi
    parent: pqm at pqm.ubuntu.com-20070710021221-8o98e4q8vcpaarnk
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: changes-merge
    timestamp: Tue 2007-07-10 13:55:19 -0400
    message:
      Merge from bzr.dev
    ------------------------------------------------------------
    revno: 2590.2.8
    merged: abentley at panoramicfeedback.com-20070706172134-kz3k30d2pu61qngi
    parent: aaron.bentley at utoronto.ca-20070706013111-gykms7zh122pm0z8
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: changes-merge
    timestamp: Fri 2007-07-06 13:21:34 -0400
    message:
      Restore conflict handling changes
    ------------------------------------------------------------
    revno: 2590.2.7
    merged: aaron.bentley at utoronto.ca-20070706013111-gykms7zh122pm0z8
    parent: aaron.bentley at utoronto.ca-20070706005922-7j1b5ed8spn9ml7d
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: changes-merge
    timestamp: Thu 2007-07-05 21:31:11 -0400
    message:
      Misc cleanup
    ------------------------------------------------------------
    revno: 2590.2.6
    merged: aaron.bentley at utoronto.ca-20070706005922-7j1b5ed8spn9ml7d
    parent: aaron.bentley at utoronto.ca-20070706005858-covn9uur45ir2mz9
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: changes-merge
    timestamp: Thu 2007-07-05 20:59:22 -0400
    message:
      Update docs to note that iter_changes-merge is done
    ------------------------------------------------------------
    revno: 2590.2.5
    merged: aaron.bentley at utoronto.ca-20070706005858-covn9uur45ir2mz9
    parent: abentley at panoramicfeedback.com-20070705161911-35s2ct3lnb8mx3wq
    committer: Aaron Bentley <aaron.bentley at utoronto.ca>
    branch nick: changes-merge
    timestamp: Thu 2007-07-05 20:58:58 -0400
    message:
      Allow selected files to be specified instead of selected ids
    ------------------------------------------------------------
    revno: 2590.2.4
    merged: abentley at panoramicfeedback.com-20070705161911-35s2ct3lnb8mx3wq
    parent: abentley at panoramicfeedback.com-20070705153934-ap3nxs9fa9u8yvfr
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: changes-merge
    timestamp: Thu 2007-07-05 12:19:11 -0400
    message:
      Move entry generation to a helper
    ------------------------------------------------------------
    revno: 2590.2.3
    merged: abentley at panoramicfeedback.com-20070705153934-ap3nxs9fa9u8yvfr
    parent: abentley at panoramicfeedback.com-20070705151112-207czq9spmtd485e
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: changes-merge
    timestamp: Thu 2007-07-05 11:39:34 -0400
    message:
      Merge the execute bit based on iter_changes
    ------------------------------------------------------------
    revno: 2590.2.2
    merged: abentley at panoramicfeedback.com-20070705151112-207czq9spmtd485e
    parent: abentley at panoramicfeedback.com-20070705144545-fnl4i5s4vdi8wyh0
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: changes-merge
    timestamp: Thu 2007-07-05 11:11:12 -0400
    message:
      Do most name merging from iter_changes output
    ------------------------------------------------------------
    revno: 2590.2.1
    merged: abentley at panoramicfeedback.com-20070705144545-fnl4i5s4vdi8wyh0
    parent: pqm at pqm.ubuntu.com-20070705054119-z89j99ne0vvywkgu
    committer: Aaron Bentley <abentley at panoramicfeedback.com>
    branch nick: changes-merge
    timestamp: Thu 2007-07-05 10:45:45 -0400
    message:
      Start work on merging names based on iter_changes
=== modified file 'NEWS'
--- a/NEWS	2007-07-11 04:54:43 +0000
+++ b/NEWS	2007-07-12 02:12:35 +0000
@@ -17,7 +17,10 @@
 
   INTERNALS:
 
-    * None yet ...
+    * merge now uses iter_changes to calculate changes, which makes room for
+      future performance increases.  It is also more consistent with other
+      operations that perform comparisons, and reduces reliance on
+      Tree.inventory.  (Aaron Bentley)
 
   TESTING:
 

=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py	2007-06-21 03:29:39 +0000
+++ b/bzrlib/merge.py	2007-07-10 18:14:01 +0000
@@ -45,8 +45,8 @@
 from bzrlib.textfile import check_text_lines
 from bzrlib.trace import mutter, warning, note
 from bzrlib.transform import (TreeTransform, resolve_conflicts, cook_conflicts,
-                              FinalPaths, create_by_entry, unique_add,
-                              ROOT_PARENT)
+                              conflict_pass, FinalPaths, create_by_entry,
+                              unique_add, ROOT_PARENT)
 from bzrlib.versionedfile import WeaveMerge
 from bzrlib import ui
 
@@ -114,6 +114,7 @@
         self.ignore_zero = False
         self.backup_files = False
         self.interesting_ids = None
+        self.interesting_files = None
         self.show_base = False
         self.reprocess = False
         self._pb = pb
@@ -168,11 +169,7 @@
             self.this_rev_id = self.this_basis
 
     def set_interesting_files(self, file_list):
-        try:
-            self._set_interesting_files(file_list)
-        except NotVersionedError, e:
-            raise BzrCommandError("%s is not a source file in any"
-                                      " tree." % e.path)
+        self.interesting_files = file_list
 
     def _set_interesting_files(self, file_list):
         """Set the list of interesting ids from a list of files."""
@@ -294,6 +291,7 @@
         kwargs = {'working_tree':self.this_tree, 'this_tree': self.this_tree,
                   'other_tree': self.other_tree,
                   'interesting_ids': self.interesting_ids,
+                  'interesting_files': self.interesting_files,
                   'pp': self.pp}
         if self.merge_type.requires_base:
             kwargs['base_tree'] = self.base_tree
@@ -423,12 +421,39 @@
     supports_reprocess = True
     supports_show_base = True
     history_based = False
+    winner_idx = {"this": 2, "other": 1, "conflict": 1}
 
     def __init__(self, working_tree, this_tree, base_tree, other_tree, 
                  interesting_ids=None, reprocess=False, show_base=False,
-                 pb=DummyProgress(), pp=None, change_reporter=None):
-        """Initialize the merger object and perform the merge."""
+                 pb=DummyProgress(), pp=None, change_reporter=None,
+                 interesting_files=None):
+        """Initialize the merger object and perform the merge.
+
+        :param working_tree: The working tree to apply the merge to
+        :param this_tree: The local tree in the merge operation
+        :param base_tree: The common tree in the merge operation
+        :param other_tree: The other other tree to merge changes from
+        :param interesting_ids: The file_ids of files that should be
+            participate in the merge.  May not be combined with
+            interesting_files.
+        :param: reprocess If True, perform conflict-reduction processing.
+        :param show_base: If True, show the base revision in text conflicts.
+            (incompatible with reprocess)
+        :param pb: A Progress bar
+        :param pp: A ProgressPhase object
+        :param change_reporter: An object that should report changes made
+        :param interesting_files: The tree-relative paths of files that should
+            participate in the merge.  If these paths refer to directories,
+            the contents of those directories will also be included.  May not
+            be combined with interesting_ids.  If neither interesting_files nor
+            interesting_ids is specified, all files may participate in the
+            merge.
+        """
         object.__init__(self)
+        if interesting_files is not None:
+            assert interesting_ids is None
+        self.interesting_ids = interesting_ids
+        self.interesting_files = interesting_files
         self.this_tree = working_tree
         self.this_tree.lock_tree_write()
         self.base_tree = base_tree
@@ -445,28 +470,30 @@
         if self.pp is None:
             self.pp = ProgressPhase("Merge phase", 3, self.pb)
 
-        if interesting_ids is not None:
-            all_ids = interesting_ids
-        else:
-            all_ids = set(base_tree)
-            all_ids.update(other_tree)
         self.tt = TreeTransform(working_tree, self.pb)
         try:
             self.pp.next_phase()
+            entries = self._entries3()
             child_pb = ui.ui_factory.nested_progress_bar()
             try:
-                for num, file_id in enumerate(all_ids):
-                    child_pb.update('Preparing file merge', num, len(all_ids))
-                    self.merge_names(file_id)
-                    file_status = self.merge_contents(file_id)
-                    self.merge_executable(file_id, file_status)
+                for num, (file_id, changed, parents3, names3,
+                          executable3) in enumerate(entries):
+                    child_pb.update('Preparing file merge', num, len(entries))
+                    self._merge_names(file_id, parents3, names3)
+                    if changed:
+                        file_status = self.merge_contents(file_id)
+                    else:
+                        file_status = 'unmodified'
+                    self._merge_executable(file_id,
+                        executable3, file_status)
             finally:
                 child_pb.finished()
             self.fix_root()
             self.pp.next_phase()
             child_pb = ui.ui_factory.nested_progress_bar()
             try:
-                fs_conflicts = resolve_conflicts(self.tt, child_pb)
+                fs_conflicts = resolve_conflicts(self.tt, child_pb,
+                    lambda t, c: conflict_pass(t, c, self.other_tree))
             finally:
                 child_pb.finished()
             if change_reporter is not None:
@@ -489,6 +516,39 @@
             self.this_tree.unlock()
             self.pb.clear()
 
+    def _entries3(self):
+        """Gather data about files modified between three trees.
+
+        Return a list of tuples of file_id, changed, parents3, names3,
+        executable3.  changed is a boolean indicating whether the file contents
+        or kind were changed.  parents3 is a tuple of parent ids for base,
+        other and this.  names3 is a tuple of names for base, other and this.
+        executable3 is a tuple of execute-bit values for base, other and this.
+        """
+        result = []
+        iterator = self.other_tree._iter_changes(self.base_tree,
+                include_unchanged=True, specific_files=self.interesting_files,
+                extra_trees=[self.this_tree])
+        for (file_id, paths, changed, versioned, parents, names, kind,
+             executable) in iterator:
+            if (self.interesting_ids is not None and
+                file_id not in self.interesting_ids):
+                continue
+            if file_id in self.this_tree.inventory:
+                entry = self.this_tree.inventory[file_id]
+                this_name = entry.name
+                this_parent = entry.parent_id
+                this_executable = entry.executable
+            else:
+                this_name = None
+                this_parent = None
+                this_executable = None
+            parents3 = parents + (this_parent,)
+            names3 = names + (this_name,)
+            executable3 = executable + (this_executable,)
+            result.append((file_id, changed, parents3, names3, executable3))
+        return result
+
     def fix_root(self):
         try:
             self.tt.final_kind(self.tt.root)
@@ -566,6 +626,20 @@
         return tree.kind(file_id)
 
     @staticmethod
+    def _three_way(base, other, this):
+        #if base == other, either they all agree, or only THIS has changed.
+        if base == other:
+            return 'this'
+        elif this not in (base, other):
+            return 'conflict'
+        # "Ambiguous clean merge" -- both sides have made the same change.
+        elif this == other:
+            return "this"
+        # this == base: only other has changed.
+        else:
+            return "other"
+
+    @staticmethod
     def scalar_three_way(this_tree, base_tree, other_tree, file_id, key):
         """Do a three-way test on a scalar.
         Return "this", "other" or "conflict", depending whether a value wins.
@@ -586,7 +660,6 @@
             return "other"
 
     def merge_names(self, file_id):
-        """Perform a merge on file_id names and parents"""
         def get_entry(tree):
             if file_id in tree.inventory:
                 return tree.inventory[file_id]
@@ -595,12 +668,27 @@
         this_entry = get_entry(self.this_tree)
         other_entry = get_entry(self.other_tree)
         base_entry = get_entry(self.base_tree)
-        name_winner = self.scalar_three_way(this_entry, base_entry, 
-                                            other_entry, file_id, self.name)
-        parent_id_winner = self.scalar_three_way(this_entry, base_entry, 
-                                                 other_entry, file_id, 
-                                                 self.parent)
-        if this_entry is None:
+        entries = (base_entry, other_entry, this_entry)
+        names = []
+        parents = []
+        for entry in entries:
+            if entry is None:
+                names.append(None)
+                parents.append(None)
+            else:
+                names.append(entry.name)
+                parents.append(entry.parent_id)
+        return self._merge_names(file_id, parents, names)
+
+    def _merge_names(self, file_id, parents, names):
+        """Perform a merge on file_id names and parents"""
+        base_name, other_name, this_name = names
+        base_parent, other_parent, this_parent = parents
+
+        name_winner = self._three_way(*names)
+
+        parent_id_winner = self._three_way(*parents)
+        if this_name is None:
             if name_winner == "this":
                 name_winner = "other"
             if parent_id_winner == "this":
@@ -610,25 +698,21 @@
         if name_winner == "conflict":
             trans_id = self.tt.trans_id_file_id(file_id)
             self._raw_conflicts.append(('name conflict', trans_id, 
-                                        self.name(this_entry, file_id), 
-                                        self.name(other_entry, file_id)))
+                                        this_name, other_name))
         if parent_id_winner == "conflict":
             trans_id = self.tt.trans_id_file_id(file_id)
             self._raw_conflicts.append(('parent conflict', trans_id, 
-                                        self.parent(this_entry, file_id), 
-                                        self.parent(other_entry, file_id)))
-        if other_entry is None:
+                                        this_parent, other_parent))
+        if other_name is None:
             # it doesn't matter whether the result was 'other' or 
             # 'conflict'-- if there's no 'other', we leave it alone.
             return
         # if we get here, name_winner and parent_winner are set to safe values.
-        winner_entry = {"this": this_entry, "other": other_entry, 
-                        "conflict": other_entry}
         trans_id = self.tt.trans_id_file_id(file_id)
-        parent_id = winner_entry[parent_id_winner].parent_id
+        parent_id = parents[self.winner_idx[parent_id_winner]]
         if parent_id is not None:
             parent_trans_id = self.tt.trans_id_file_id(parent_id)
-            self.tt.adjust_path(winner_entry[name_winner].name, 
+            self.tt.adjust_path(names[self.winner_idx[name_winner]],
                                 parent_trans_id, trans_id)
 
     def merge_contents(self, file_id):
@@ -794,6 +878,13 @@
 
     def merge_executable(self, file_id, file_status):
         """Perform a merge on the execute bit."""
+        executable = [self.executable(t, file_id) for t in (self.base_tree,
+                      self.other_tree, self.this_tree)]
+        self._merge_executable(file_id, executable, file_status)
+
+    def _merge_executable(self, file_id, executable, file_status):
+        """Perform a merge on the execute bit."""
+        base_executable, other_executable, this_executable = executable
         if file_status == "deleted":
             return
         trans_id = self.tt.trans_id_file_id(file_id)
@@ -802,9 +893,7 @@
                 return
         except NoSuchFile:
             return
-        winner = self.scalar_three_way(self.this_tree, self.base_tree, 
-                                       self.other_tree, file_id, 
-                                       self.executable)
+        winner = self._three_way(*executable)
         if winner == "conflict":
         # There must be a None in here, if we have a conflict, but we
         # need executability since file status was not deleted.
@@ -814,18 +903,18 @@
                 winner = "other"
         if winner == "this":
             if file_status == "modified":
-                executability = self.this_tree.is_executable(file_id)
+                executability = this_executable
                 if executability is not None:
                     trans_id = self.tt.trans_id_file_id(file_id)
                     self.tt.set_executability(executability, trans_id)
         else:
             assert winner == "other"
             if file_id in self.other_tree:
-                executability = self.other_tree.is_executable(file_id)
+                executability = other_executable
             elif file_id in self.this_tree:
-                executability = self.this_tree.is_executable(file_id)
+                executability = this_executable
             elif file_id in self.base_tree:
-                executability = self.base_tree.is_executable(file_id)
+                executability = base_executable
             if executability is not None:
                 trans_id = self.tt.trans_id_file_id(file_id)
                 self.tt.set_executability(executability, trans_id)
@@ -897,7 +986,8 @@
 
     def __init__(self, working_tree, this_tree, base_tree, other_tree, 
                  interesting_ids=None, pb=DummyProgress(), pp=None,
-                 reprocess=False, change_reporter=None):
+                 reprocess=False, change_reporter=None,
+                 interesting_files=None):
         self.this_revision_tree = self._get_revision_tree(this_tree)
         self.other_revision_tree = self._get_revision_tree(other_tree)
         super(WeaveMerger, self).__init__(working_tree, this_tree, 
@@ -1031,7 +1121,7 @@
     if interesting_files:
         assert not interesting_ids, ('Only supply interesting_ids'
                                      ' or interesting_files')
-        merger._set_interesting_files(interesting_files)
+        merger.interesting_files = interesting_files
     merger.show_base = show_base
     merger.reprocess = reprocess
     merger.other_rev_id = other_rev_id

=== modified file 'bzrlib/tests/test_merge_core.py'
--- a/bzrlib/tests/test_merge_core.py	2007-07-04 08:46:22 +0000
+++ b/bzrlib/tests/test_merge_core.py	2007-07-06 00:58:58 +0000
@@ -416,6 +416,14 @@
         self.assertEqual(conflicts, []) 
         builder.cleanup()
 
+    def test_merge_one_renamed(self):
+        builder = MergeBuilder(getcwd())
+        builder.add_file('1', builder.tree_root, 'name1', 'text1a', False)
+        builder.change_name('1', this='name2')
+        builder.change_contents('1', other='text2')
+        builder.merge(interesting_files=['name2'])
+        self.assertEqual('text2', builder.this.get_file('1').read())
+        builder.cleanup()
 
 class FunctionalMergeTest(TestCaseWithTransport):
 

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2007-06-06 04:10:17 +0000
+++ b/bzrlib/transform.py	2007-07-06 17:21:34 +0000
@@ -1645,8 +1645,13 @@
         pb.clear()
 
 
-def conflict_pass(tt, conflicts):
-    """Resolve some classes of conflicts."""
+def conflict_pass(tt, conflicts, path_tree=None):
+    """Resolve some classes of conflicts.
+
+    :param tt: The transform to resolve conflicts in
+    :param conflicts: The conflicts to resolve
+    :param path_tree: A Tree to get supplemental paths from
+    """
     new_conflicts = set()
     for c_type, conflict in ((c[0], c) for c in conflicts):
         if c_type == 'duplicate id':
@@ -1683,6 +1688,13 @@
             except KeyError:
                 tt.create_directory(trans_id)
                 new_conflicts.add((c_type, 'Created directory', trans_id))
+                try:
+                    tt.final_name(trans_id)
+                except NoFinalPath:
+                    file_id = tt.final_file_id(trans_id)
+                    entry = path_tree.inventory[file_id]
+                    parent_trans_id = tt.trans_id_file_id(entry.parent_id)
+                    tt.adjust_path(entry.name, parent_trans_id, trans_id)
         elif c_type == 'unversioned parent':
             tt.version_file(tt.inactive_file_id(conflict[1]), conflict[1])
             new_conflicts.add((c_type, 'Versioned directory', conflict[1]))

=== modified file 'doc/developers/performance.dot'
--- a/doc/developers/performance.dot	2007-07-02 05:18:12 +0000
+++ b/doc/developers/performance.dot	2007-07-06 00:59:22 +0000
@@ -15,6 +15,7 @@
   status_analysis[label="Work required analysis for status"];
   uncommit_analysis[label="Work required analysis for uncommit"];
   wt_disk_order[label="Working Tree disk ordering\n6-8 weeks"];
+  iter_merge[label="iter_changes based merge\n2 days"];
 
   /* uncompleted node list - add new tasks here */
   node[color="blue"];
@@ -62,7 +63,6 @@
   repo_disk_order[label="Repository disk ordering\n1 month"];
   pack_repository[label="Pack based repository format"];
   graph_api[label="Network-efficient revision-graph API\n3 week"];
-  iter_merge[label="iter_changes based merge\n2 days"];
   validators[label="Build new validators for revisions and trees."];
 
   /* under discussion/optional */




More information about the bazaar-commits mailing list