Merge lp:~jelmer/bzr-builddeb/722217-avoid-o-history into lp:bzr-builddeb

[Jelmer Vernooij]

Avoid constructing the full upstream branch history when merging from an upstream branch. This can be quite expensive for large upstream branches, and is unnecessary.

[James Westby (james-w)]

review: Approve

=== modified file 'cmds.py'
--- cmds.py	2011-02-11 14:29:07 +0000
+++ cmds.py	2011-02-21 16:22:07 +0000
@@ -323,7 +323,7 @@
                         upstream_revision = upstream_branch.last_revision()
                     else:
                         upstream_revision = upstream_source.version_as_revision(
-                            version)
+                            None, str(version))
                 finally:
                     upstream_branch.unlock()
         return (upstream_branch, upstream_revision)

=== modified file 'tests/test_upstream.py'
--- tests/test_upstream.py	2011-02-11 14:21:19 +0000
+++ tests/test_upstream.py	2011-02-21 16:22:07 +0000
@@ -368,7 +368,7 @@
     """
 
     def get_suffix(self, version_string, revid):
-        revno = self.revhistory.index(revid)+1
+        revno = len(self.revhistory) - self.revhistory.index(revid)
         if "bzr" in version_string:
             return "%sbzr%d" % (version_string.split("bzr")[0], revno)
         return "%s+bzr%d" % (version_string, revno)
@@ -390,18 +390,20 @@
           _upstream_branch_version(self.revhistory, {"somerevid": ["1.3"]}, "bla", "1.2", self.get_suffix))
 
     def test_refresh_snapshot_pre(self):
-      self.revhistory = ["oldrevid", "somerevid"]
-      self.assertEquals(Version("1.3~bzr2"),
-          _upstream_branch_version(self.revhistory, {}, "bla", "1.3~bzr1", self.get_suffix))
+      self.revhistory = ["somerevid", "oldrevid"]
+      self.assertEquals("1.3~bzr2",
+          _upstream_branch_version(self.revhistory, {}, "bla", "1.3~bzr1",
+              self.get_suffix))
 
     def test_refresh_snapshot_post(self):
-      self.revhistory = ["oldrevid", "somerevid"]
-      self.assertEquals(Version("1.3+bzr2"),
-          _upstream_branch_version(self.revhistory, {}, "bla", "1.3+bzr1", self.get_suffix))
+      self.revhistory = ["somerevid", "oldrevid"]
+      self.assertEquals("1.3+bzr2",
+          _upstream_branch_version(self.revhistory, {}, "bla", "1.3+bzr1",
+              self.get_suffix))
 
     def test_new_tag_refresh_snapshot(self):
-      self.revhistory = ["oldrevid", "somerevid", "newrevid"]
-      self.assertEquals(Version("1.3+bzr3"),
+      self.revhistory = ["newrevid", "somerevid", "oldrevid"]
+      self.assertEquals("1.3+bzr3",
             _upstream_branch_version(self.revhistory,
                 {"somerevid": ["1.3"]}, "bla", "1.2+bzr1", self.get_suffix))
 

=== modified file 'upstream/branch.py'
--- upstream/branch.py	2011-01-29 01:03:18 +0000
+++ upstream/branch.py	2011-02-21 16:22:07 +0000
@@ -58,7 +58,7 @@
     version is used and combined with the bzr revision number
     (usually <version>+bzr<revno>).
 
-    :param revhistory: Branch revision history.
+    :param revhistory: Reverse branch revision history.
     :param reverse_tag_dict: Reverse tag dictionary (revid -> list of tags)
     :param package: Name of package.
     :param previous_version: Previous upstream version in debian changelog.
@@ -68,7 +68,7 @@
     if revhistory == []:
         # No new version to merge
         return previous_version
-    for r in reversed(revhistory):
+    for r in revhistory:
         if r in reverse_tag_dict:
             # If there is a newer version tagged in branch,
             # convert to upstream version
@@ -77,11 +77,11 @@
                 upstream_version = upstream_tag_to_version(tag,
                                                    package=package)
                 if upstream_version is not None:
-                    if r != revhistory[-1]:
+                    if r != revhistory[0]:
                         upstream_version = add_rev(
-                          upstream_version, revhistory[-1])
+                          str(upstream_version), revhistory[0])
                     return upstream_version
-    return add_rev(previous_version, revhistory[-1])
+    return add_rev(str(previous_version), revhistory[0])
 
 
 def extract_svn_revno(rev):
@@ -170,22 +170,19 @@
     :param previous_version: The previous upstream version string
     :return: Upstream version string for `upstream_revision`.
     """
-    dotted_revno = upstream_branch.revision_id_to_dotted_revno(upstream_revision)
-    if len(dotted_revno) > 1:
-        revno = -2
-    else:
-        revno = dotted_revno[0]
-    revhistory = upstream_branch.revision_history()
+    graph = upstream_branch.repository.get_graph()
     previous_revision = get_snapshot_revision(previous_version)
     if previous_revision is not None:
         previous_revspec = RevisionSpec.from_string(previous_revision)
-        previous_revno, _ = previous_revspec.in_history(upstream_branch)
+        previous_revno, previous_revid = previous_revspec.in_history(upstream_branch)
         # Trim revision history - we don't care about any revisions
         # before the revision of the previous version
+        stop_revids = [previous_revid]
     else:
         previous_revno = 0
-    revhistory = revhistory[previous_revno:revno+1]
-    return _upstream_branch_version(revhistory,
+        stop_revids = None
+    revhistory = graph.iter_lefthand_ancestry(upstream_revision, stop_revids)
+    return _upstream_branch_version(list(revhistory),
             upstream_branch.tags.get_reverse_tag_dict(), package,
             previous_version,
             lambda version, revision: upstream_version_add_revision(upstream_branch, version, revision))
@@ -241,8 +238,12 @@
             self.upstream_branch.last_revision())
 
     def get_version(self, package, current_version, revision):
-        return upstream_branch_version(self.upstream_branch,
-            revision, package, current_version)
+        self.upstream_branch.lock_read()
+        try:
+            return upstream_branch_version(self.upstream_branch,
+                revision, package, current_version)
+        finally:
+            self.upstream_branch.unlock()
 
     def fetch_tarball(self, package, version, target_dir):
         self.upstream_branch.lock_read()