Merge lp:~jelmer/bzr-builddeb/multi-tarball-pt7 into lp:bzr-builddeb

[Jelmer Vernooij]

More work towards supporting multiple upstream tarballs.

This mainly simplifies some code, and makes us look directly at
pristine_upstream_source rather than through helper methods.

[James Westby (james-w)]

review: Approve

=== modified file 'import_dsc.py'
--- import_dsc.py	2011-06-28 08:41:44 +0000
+++ import_dsc.py	2011-07-17 22:29:31 +0000
@@ -277,25 +277,25 @@
         return str(version)
 
     def _has_version(self, branch, tag_name, md5=None):
-        if branch.tags.has_tag(tag_name):
-            revid = branch.tags.lookup_tag(tag_name)
-            branch.lock_read()
-            try:
-                graph = branch.repository.get_graph()
-                if not graph.is_ancestor(revid, branch.last_revision()):
-                    return False
-            finally:
-                branch.unlock()
-            if md5 is None:
-                return True
-            rev = branch.repository.get_revision(revid)
-            try:
-                return rev.properties['deb-md5'] == md5
-            except KeyError:
-                warning("tag %s present in branch, but there is no "
-                    "associated 'deb-md5' property" % tag_name)
-                pass
-        return False
+        if not branch.tags.has_tag(tag_name):
+            return False
+        revid = branch.tags.lookup_tag(tag_name)
+        branch.lock_read()
+        try:
+            graph = branch.repository.get_graph()
+            if not graph.is_ancestor(revid, branch.last_revision()):
+                return False
+        finally:
+            branch.unlock()
+        if md5 is None:
+            return True
+        rev = branch.repository.get_revision(revid)
+        try:
+            return rev.properties['deb-md5'] == md5
+        except KeyError:
+            warning("tag %s present in branch, but there is no "
+                "associated 'deb-md5' property" % tag_name)
+            return False
 
     def has_version(self, version, md5=None):
         """Whether this branch contains the package version specified.
@@ -409,18 +409,6 @@
             return self.branch.tags.lookup_tag(ubuntu_tag_name)
         return self.branch.tags.lookup_tag(tag_name)
 
-    def revid_of_upstream_version(self, package, version, tarballs=None):
-        """Returns the revision id corresponding to the upstream version.
-
-        :param version: the Version object to extract the upstream version
-            from to retrieve the revid of. The upstream version must be
-            present in the upstream branch.
-        :return: the revision id corresponding to the upstream portion
-            of the version
-        """
-        return self.pristine_upstream_source.version_as_revision(package, version,
-            tarballs)
-
     def tag_version(self, version, revid=None):
         """Tags the branch's last revision with the given version.
 
@@ -506,6 +494,21 @@
         except KeyError:
             return False
 
+    def can_pull_from_branch(self, branch, version, md5):
+        if not branch.has_version(version, md5=md5):
+            return False
+
+        # Check that they haven't diverged
+        branch.branch.lock_read()
+        try:
+            graph = branch.branch.repository.get_graph(
+                    self.branch.repository)
+            return graph.is_ancestor(self.branch.last_revision(),
+                    branch.revid_of_version(version))
+        finally:
+            branch.branch.unlock()
+
+
     def branch_to_pull_version_from(self, version, md5):
         """Checks whether this upload is a pull from a lesser branch.
 
@@ -531,29 +534,11 @@
         self.branch.lock_read()
         try:
             for branch in reversed(self.get_lesser_branches()):
-                if branch.has_version(version, md5=md5):
-                    # Check that they haven't diverged
-                    branch.branch.lock_read()
-                    try:
-                        graph = branch.branch.repository.get_graph(
-                                self.branch.repository)
-                        if graph.is_ancestor(self.branch.last_revision(),
-                                branch.revid_of_version(version)):
-                            return branch
-                    finally:
-                        branch.branch.unlock()
+                if self.can_pull_from_branch(branch, version, md5):
+                    return branch
             for branch in self.get_greater_branches():
-                if branch.has_version(version, md5=md5):
-                    # Check that they haven't diverged
-                    branch.branch.lock_read()
-                    try:
-                        graph = branch.branch.repository.get_graph(
-                                self.branch.repository)
-                        if graph.is_ancestor(self.branch.last_revision(),
-                                branch.revid_of_version(version)):
-                            return branch
-                    finally:
-                        branch.branch.unlock()
+                if self.can_pull_from_branch(branch, version, md5):
+                    return branch
             return None
         finally:
             self.branch.unlock()
@@ -581,7 +566,7 @@
                 graph = other_up_branch.repository.get_graph(
                         up_branch.repository)
                 return graph.is_ancestor(up_branch.last_revision(),
-                        branch.revid_of_upstream_version(package, version))
+                        branch.pristine_upstream_source.version_as_revision(package, version))
             finally:
                 other_up_branch.unlock()
         finally:
@@ -695,7 +680,7 @@
         :param version: the upstream version string
         """
         assert isinstance(version, str)
-        pull_revision = pull_branch.revid_of_upstream_version(package, version)
+        pull_revision = pull_branch.pristine_upstream_source.version_as_revision(package, version)
         mutter("Pulling upstream part of %s from revision %s" % \
                 (version, pull_revision))
         assert self.pristine_upstream_tree is not None, \
@@ -753,7 +738,7 @@
                     "present in the upstream branch")
 
     def get_parents_with_upstream(self, package, version, versions,
-            force_upstream_parent=False):
+            tarballs, force_upstream_parent=False):
         """Get the list of parents including any upstream parents.
 
         Further to get_parents this method includes any upstream parents
@@ -765,7 +750,7 @@
         If force_upstream_parent is True then the upstream parent will
         be included, even if another parent is already using that
         upstream. This is for use in cases where the .orig.tar.gz
-        is different in two ditributions.
+        is different in two distributions.
 
         :param version: the Version that we are currently importing.
         :param versions: the list of Versions that are ancestors of
@@ -789,8 +774,8 @@
                     break
         real_parents = [p[2] for p in parents]
         if need_upstream_parent:
-            parent_revid = self.revid_of_upstream_version(package,
-                version.upstream_version)
+            parent_revid = self.pristine_upstream_source.version_as_revision(package,
+                version.upstream_version, tarballs)
             if len(parents) > 0:
                 real_parents.insert(1, parent_revid)
             else:
@@ -1069,7 +1054,7 @@
                 pull_branch = pull_parents[1][0]
                 pull_version = pull_parents[1][1]
             if not pull_branch.is_version_native(pull_version):
-                pull_revid = pull_branch.revid_of_upstream_version(
+                pull_revid = pull_branch.pristine_upstream_source.version_as_revision(
                     package, pull_version.upstream_version)
                 mutter("Initialising upstream from %s, version %s",
                     str(pull_branch), str(pull_version))
@@ -1142,7 +1127,7 @@
             else:
                 mutter("We already have the needed upstream part")
             parents = self.get_parents_with_upstream(package, version, versions,
-                    force_upstream_parent=imported_upstream)
+                    upstream_tarballs, force_upstream_parent=imported_upstream)
             # Now we have the list of parents we need to import the .diff.gz
             self.import_debian(debian_part, version, parents, md5,
                     timestamp=timestamp, file_ids_from=file_ids_from)
 
=== modified file 'tests/test_import_dsc.py'
--- tests/test_import_dsc.py	2011-06-28 08:41:44 +0000
+++ tests/test_import_dsc.py	2011-07-17 22:29:31 +0000
@@ -203,14 +203,14 @@
         db.tag_version(version)
         self.assertEqual(db.revid_of_version(version), revid)
 
-    def test_revid_of_upstream_version(self):
+    def test_upstream_version_as_revid(self):
         db = self.db1
         tree = self.up_tree1
         version = "0.1"
         revid = tree.commit("one")
         db.tag_upstream_version(version)
         self.assertEqual(
-            db.revid_of_upstream_version("package", version), revid)
+            db.pristine_upstream_source.version_as_revision("package", version), revid)
 
     def test_contained_versions(self):
         db = self.db1
@@ -401,13 +401,13 @@
         up_revid = self.up_tree1.commit("one")
         db.tag_upstream_version(version1.upstream_version)
         self.assertEqual(
-            db.get_parents_with_upstream("package", version1, [version1]),
+            db.get_parents_with_upstream("package", version1, [version1], None),
             [up_revid])
         db = self.db2
         self.up_tree2.pull(self.up_tree1.branch)
         db.tag_upstream_version(version1.upstream_version)
         self.assertEqual(
-            db.get_parents_with_upstream("package", version1, [version1]),
+            db.get_parents_with_upstream("package", version1, [version1], None),
             [up_revid])
 
     def test_get_parents_with_upstream_second_version(self):
@@ -420,7 +420,7 @@
         db.tag_upstream_version(version1.upstream_version)
         # No upstream parent
         self.assertEqual(db.get_parents_with_upstream(
-            "package", version2, [version2, version1]), [revid1])
+            "package", version2, [version2, version1], None), [revid1])
 
     def test_get_parents_with_upstream_merge_from_lesser(self):
         version1 = Version("0.1-1")
@@ -437,7 +437,7 @@
         versions = [version3, version1, version2]
         # No upstream parent
         self.assertEqual(self.db2.get_parents_with_upstream(
-            "package", version3, versions), [revid2, revid1])
+            "package", version3, versions, None), [revid2, revid1])
 
     def test_get_parents_with_upstream_merge_from_greater(self):
         version1 = Version("0.1-1")
@@ -454,7 +454,7 @@
         versions = [version3, version2, version1]
         # No upstream parent
         self.assertEqual(self.db1.get_parents_with_upstream(
-            "package", version3, versions), [revid1, revid2])
+            "package", version3, versions, None), [revid1, revid2])
 
     def test_get_parents_with_upstream_new_upstream_import(self):
         version1 = Version("0.1-1")
@@ -470,7 +470,7 @@
         versions = [version2, version1]
         # Upstream parent as it is new upstream version
         self.assertEqual(self.db2.get_parents_with_upstream(
-            "package", version2, versions), [revid1, up_revid2])
+            "package", version2, versions, None), [revid1, up_revid2])
 
     def test_get_parents_merge_new_upstream_from_lesser(self):
         version1 = Version("0.1-1")
@@ -494,7 +494,7 @@
         versions = [version4, version3, version2, version1]
         # no upstream parent as the lesser branch has already merged it
         self.assertEqual(self.db2.get_parents_with_upstream(
-            "package", version4, versions), [revid2, revid3])
+            "package", version4, versions, None), [revid2, revid3])
 
     def test_get_parents_with_upstream_force_upstream(self):
         version1 = Version("0.1-1")
@@ -509,7 +509,7 @@
         # a previous test checked that this wouldn't give an
         # upstream parent, but we are requiring one.
         self.assertEqual(self.db2.get_parents_with_upstream(
-            "package", version2, versions, force_upstream_parent=True),
+            "package", version2, versions, None, force_upstream_parent=True),
             [revid1, up_revid2])
 
     def test_get_parents_with_upstream_sync_when_diverged(self):
@@ -530,7 +530,7 @@
         # This is a sync but we are diverged so we should get two
         # parents
         self.assertEqual(self.db2.get_parents_with_upstream(
-            "package", version3, versions), [revid2, revid3])
+            "package", version3, versions, None), [revid2, revid3])
 
     def test_get_parents_with_upstream_sync_new_upstream(self):
         version1 = Version("0.1-1")
@@ -553,7 +553,7 @@
         # parents. There should be no upstream as the synced
         # version will already have it.
         self.assertEqual(self.db2.get_parents_with_upstream(
-            "package", version3, versions), [revid2, revid3])
+            "package", version3, versions, None), [revid2, revid3])
 
     def test_get_parents_with_upstream_sync_new_upstream_force(self):
         version1 = Version("0.1-1")
@@ -580,7 +580,7 @@
         # when we fake-sync, but we are forcing one here.
         #TODO: should the upstream parent be second or third?
         self.assertEqual(self.db2.get_parents_with_upstream(
-            "package", version3, versions, force_upstream_parent=True),
+            "package", version3, versions, None, force_upstream_parent=True),
                 [revid2, up_revid3, revid3])
 
     def test_branch_to_pull_version_from(self):
@@ -700,7 +700,7 @@
         self.assertEqual(self.tree2.branch.last_revision(), revid)
         self.assertEqual(self.up_tree2.branch.last_revision(), up_revid)
         self.assertEqual(self.db2.revid_of_version(version), revid)
-        self.assertEqual(self.db2.revid_of_upstream_version(
+        self.assertEqual(self.db2.pristine_upstream_source.version_as_revision(
             "package", version.upstream_version), up_revid)
 
     def test_pull_from_lesser_branch_with_upstream(self):
@@ -716,7 +716,7 @@
         self.assertEqual(self.tree2.branch.last_revision(), revid)
         self.assertEqual(self.up_tree2.branch.last_revision(), up_revid)
         self.assertEqual(self.db2.revid_of_version(version), revid)
-        self.assertEqual(self.db2.revid_of_upstream_version(
+        self.assertEqual(self.db2.pristine_upstream_source.version_as_revision(
             "package", version.upstream_version), up_revid)
 
     def test_pull_upstream_from_branch(self):
@@ -727,7 +727,7 @@
         self.assertNotEqual(self.up_tree2.branch.last_revision(), up_revid)
         self.db2.pull_upstream_from_branch(self.db1, "package", version)
         self.assertEqual(self.up_tree2.branch.last_revision(), up_revid)
-        self.assertEqual(self.db2.revid_of_upstream_version("package", version),
+        self.assertEqual(self.db2.pristine_upstream_source.version_as_revision("package", version),
                 up_revid)
 
     def check_changes(self, changes, added=[], removed=[], modified=[],
@@ -784,7 +784,7 @@
         branch = tree.branch
         rh = branch.revision_history()
         self.assertEqual(len(rh), 1)
-        self.assertEqual(self.db1.revid_of_upstream_version(
+        self.assertEqual(self.db1.pristine_upstream_source.version_as_revision(
             "package", version.upstream_version), rh[0])
         rev = branch.repository.get_revision(rh[0])
         self.assertEqual(rev.message,
@@ -823,7 +823,7 @@
         rh = branch.revision_history()
         self.assertEqual(len(rh), 2)
         self.assertEqual(
-            self.db1.revid_of_upstream_version("package", version2.upstream_version), rh[1])
+            self.db1.pristine_upstream_source.version_as_revision("package", version2.upstream_version), rh[1])
         rev = branch.repository.get_revision(rh[1])
         self.assertEqual(rev.message,
                 "Import upstream version %s" % str(version2.upstream_version))
@@ -854,7 +854,7 @@
         branch = tree.branch
         rh = branch.revision_history()
         self.assertEqual(len(rh), 1)
-        self.assertEqual(self.db1.revid_of_upstream_version(
+        self.assertEqual(self.db1.pristine_upstream_source.version_as_revision(
             "package", version.upstream_version), rh[0])
         rev = branch.repository.get_revision(rh[0])
         self.assertEqual(rev.message,
@@ -882,7 +882,7 @@
         branch = tree.branch
         rh = branch.revision_history()
         self.assertEqual(len(rh), 1)
-        self.assertEqual(self.db1.revid_of_upstream_version(
+        self.assertEqual(self.db1.pristine_upstream_source.version_as_revision(
             "package", version.upstream_version), rh[0])
         rev = branch.repository.get_revision(rh[0])
         self.assertEqual(rev.message,
@@ -916,7 +916,7 @@
         branch = tree.branch
         rh = branch.revision_history()
         self.assertEqual(len(rh), 1)
-        self.assertEqual(self.db1.revid_of_upstream_version(
+        self.assertEqual(self.db1.pristine_upstream_source.version_as_revision(
             "package", version.upstream_version), rh[0])
         rev = branch.repository.get_revision(rh[0])
         self.assertEqual(rev.message,
@@ -1356,7 +1356,7 @@
         self.assertEqual(self.db1.revid_of_version(version2), rh1[2])
         self.assertEqual(self.db1.revid_of_version(version3), rh1[3])
         self.assertEqual(
-            self.db1.revid_of_upstream_version("package", version1.upstream_version),
+            self.db1.pristine_upstream_source.version_as_revision("package", version1.upstream_version),
             up_rh1[0])
         self.tree1.lock_read()
         self.addCleanup(self.tree1.unlock)
@@ -1409,10 +1409,10 @@
         self.assertEqual(self.db1.revid_of_version(version2), rh1[2])
         self.assertEqual(self.db1.revid_of_version(version3), rh1[3])
         self.assertEqual(
-            self.db1.revid_of_upstream_version("package", version1.upstream_version),
+            self.db1.pristine_upstream_source.version_as_revision("package", version1.upstream_version),
             up_rh1[0])
         self.assertEqual(
-            self.db1.revid_of_upstream_version("package", version3.upstream_version),
+            self.db1.pristine_upstream_source.version_as_revision("package", version3.upstream_version),
             up_rh1[1])
         self.tree1.lock_read()
         self.addCleanup(self.tree1.unlock)
@@ -1464,10 +1464,10 @@
         self.assertEqual(self.db1.revid_of_version(version2), rh1[2])
         self.assertEqual(self.db1.revid_of_version(version3), rh1[3])
         self.assertEqual(
-            self.db1.revid_of_upstream_version("package", version1.upstream_version),
+            self.db1.pristine_upstream_source.version_as_revision("package", version1.upstream_version),
             up_rh1[0])
         self.assertEqual(
-            self.db1.revid_of_upstream_version("package", version3.upstream_version),
+            self.db1.pristine_upstream_source.version_as_revision("package", version3.upstream_version),
             up_rh1[1])
         self.tree1.lock_read()
         self.addCleanup(self.tree1.unlock)
 
=== modified file 'upstream/pristinetar.py'
--- upstream/pristinetar.py	2011-06-28 08:41:44 +0000
+++ upstream/pristinetar.py	2011-07-17 22:29:31 +0000
@@ -217,10 +217,7 @@
     def fetch_tarballs(self, package, version, target_dir):
         return [self.fetch_component_tarball(package, version, None, target_dir)]
 
-    def _has_version_component(self, tag_name, md5=None):
-        if not self.branch.tags.has_tag(tag_name):
-            return False
-        revid = self.branch.tags.lookup_tag(tag_name)
+    def _has_revision(self, revid, md5=None):
         self.branch.lock_read()
         try:
             graph = self.branch.repository.get_graph()
@@ -234,8 +231,9 @@
         try:
             return rev.properties['deb-md5'] == md5
         except KeyError:
-            warning("tag %s present in branch, but there is no "
-                "associated 'deb-md5' property" % tag_name)
+            warning("tag present in branch, but there is no "
+                "associated 'deb-md5' property in associated "
+                "revision %s", revid)
             return True
 
     def version_as_revision(self, package, version, tarballs=None):
@@ -244,13 +242,19 @@
         elif len(tarballs) > 1:
             raise MultipleUpstreamTarballsNotSupported()
         else:
-            return self.version_component_as_revision(package, version, tarballs[0][1])
+            return self.version_component_as_revision(package, version, tarballs[0][1],
+                tarballs[0][2])
 
-    def version_component_as_revision(self, package, version, component, tarballs=None):
+    def version_component_as_revision(self, package, version, component, md5=None):
         assert isinstance(version, str)
         for tag_name in self.possible_tag_names(version, component=component):
-            if self._has_version_component(tag_name, tarballs):
-                return self.branch.tags.lookup_tag(tag_name)
+            try:
+                revid = self.branch.tags.lookup_tag(tag_name)
+            except NoSuchTag:
+                continue
+            else:
+                if self._has_revision(revid, md5=md5):
+                    return revid
         tag_name = self.tag_name(version, component=component)
         try:
             return self.branch.tags.lookup_tag(tag_name)
@@ -269,8 +273,13 @@
     def has_version_component(self, package, version, component, md5=None):
         assert isinstance(version, str), str(type(version))
         for tag_name in self.possible_tag_names(version, component=component):
-            if self._has_version_component(tag_name, md5=md5):
-                return True
+            try:
+                revid = self.branch.tags.lookup_tag(tag_name)
+            except NoSuchTag:
+                continue
+            else:
+                if self._has_revision(revid, md5=md5):
+                    return True
         return False
 
     def possible_tag_names(self, version, component):