Merge lp:~jelmer/bzr-builddeb/obj into lp:~bzr-builddeb-hackers/bzr-builddeb/trunk-old

[Jelmer Vernooij]

[Jelmer Vernooij (jelmer)]

The attached patch adds a new class UpstreamSource (for lack of a
better name) that represents a source of upstream tarballs.
UpstreamSources currently only can retrieve specific version tarballs,
but the idea is that they'll in the future also be able to (where
possible/applicable):

 * retrieve the last upstream version string
 * retrieve the last upstream version tarball
 * return the revid matching a particular upstream version
 * list the available upstream versions

It also makes bzr-builddeb support retrieval of upstream snapshot
versions again ("1.0~bzr13").

Cheers,

Jelmer

[James Westby (james-w)]

Merged,

Thanks

review: Approve

=== modified file 'cmds.py'
--- cmds.py	2009-04-15 11:50:06 +0000
+++ cmds.py	2009-04-16 10:42:12 +0000
@@ -74,7 +74,11 @@
         MergeModeDistiller,
         NativeSourceDistiller,
         )
-from bzrlib.plugins.builddeb.upstream import UpstreamProvider
+from bzrlib.plugins.builddeb.upstream import (
+        UpstreamProvider,
+        UpstreamBranchSource,
+        get_upstream_sources,
+        )
 from bzrlib.plugins.builddeb.util import (find_changelog,
         lookup_distribution,
         suite_to_distribution,
@@ -362,10 +366,11 @@
                     self._get_upstream_branch(merge, export_upstream,
                             export_upstream_revision, config)
 
-            upstream_provider = UpstreamProvider(tree, branch,
+            upstream_provider = UpstreamProvider(
                     changelog.package, changelog.version.upstream_version,
-                    orig_dir, larstiq=larstiq, upstream_branch=upstream_branch,
-                    upstream_revision=upstream_revision, allow_split=split)
+                    orig_dir, get_upstream_sources(tree, branch, 
+                    larstiq=larstiq, upstream_branch=upstream_branch,
+                    upstream_revision=upstream_revision, allow_split=split))
 
             if merge:
                 distiller_cls = MergeModeDistiller
@@ -379,8 +384,8 @@
                     is_working_tree=working_tree)
 
             build_source_dir = os.path.join(build_dir,
-                    changelog.package + "-"
-                    + changelog.version.upstream_version)
+                    "%s-%s" % (changelog.package,
+                               changelog.version.upstream_version))
 
             builder = DebBuild(distiller, build_source_dir, build_cmd,
                     use_existing=use_existing)
@@ -565,11 +570,10 @@
             tarball_filename = os.path.join(orig_dir, dest_name)
 
             if upstream_branch and no_tarball:
-                info("Exporting the upstream branch to create the tarball")
-                rev_tree = upstream_branch.repository.revision_tree(
+                upstream = UpstreamBranchSource(upstream_branch, 
                         upstream_revision)
-                export(rev_tree, tarball_filename, format='tgz',
-                        root="%s-%s" % (package, version.upstream_version))
+                upstream.get_specific_version(package, version.upstream_version,
+                        orig_dir)
             else:
                 try:
                     repack_tarball(location, dest_name, target_dir=orig_dir)
@@ -764,9 +768,9 @@
         orig_dir = config.orig_dir
         if orig_dir is None:
             orig_dir = default_orig_dir
-        upstream_provider = UpstreamProvider(t, t.branch,
-                changelog.package, changelog.version.upstream_version,
-                orig_dir, larstiq=larstiq)
+        upstream_provider = UpstreamProvider(changelog.package, 
+                changelog.version.upstream_version, orig_dir, 
+                get_upstream_sources(t, t.branch, larstiq=larstiq))
 
         distiller = MergeModeDistiller(t, upstream_provider,
                 larstiq=larstiq)
 
=== modified file 'errors.py'
--- errors.py	2009-03-02 22:36:36 +0000
+++ errors.py	2009-04-16 09:30:49 +0000
@@ -153,6 +153,14 @@
     _fmt = "You did not specify a package version."
 
 
+class PackageVersionNotPresent(BzrError):
+    _fmt = "%(package)s %(version)s was not found in %(upstream)s."
+
+    def __init__(self, package, version, upstream):
+        BzrError.__init__(self, package=package, version=version, 
+                          upstream=upstream)
+
+
 class UnsupportedRepackFormat(BzrError):
     _fmt = ('Either the file extension of "%(location)s" indicates that '
             'it is a format unsupported for repacking or it is a '
 
=== modified file 'import_dsc.py'
--- import_dsc.py	2009-03-11 07:23:20 +0000
+++ import_dsc.py	2009-04-15 23:17:50 +0000
@@ -57,6 +57,7 @@
         )
 from bzrlib.export import export
 from bzrlib.osutils import file_iterator, isdir, basename, splitpath
+from bzrlib.revisionspec import RevisionSpec
 from bzrlib.revision import NULL_REVISION
 from bzrlib.trace import warning, info, mutter
 from bzrlib.transform import TreeTransform, cook_conflicts, resolve_conflicts
@@ -73,7 +74,7 @@
                 UpstreamAlreadyImported,
                 UpstreamBranchAlreadyMerged,
                 )
-from bzrlib.plugins.builddeb.util import get_commit_info_from_changelog
+from bzrlib.plugins.builddeb.util import get_commit_info_from_changelog, get_snapshot_revision
 
 
 files_to_ignore = set(['.cvsignore', '.arch-inventory', '.bzrignore',
@@ -1534,16 +1535,23 @@
                 "Should use self.upstream_branch if set"
         tempdir = tempfile.mkdtemp(dir=os.path.join(self.tree.basedir, '..'))
         try:
+            previous_upstream_revision = get_snapshot_revision(previous_version.upstream_version)
             if previous_version is not None:
-                if not self.has_upstream_version_in_packaging_branch(
+                if self.has_upstream_version_in_packaging_branch(
                         previous_version):
+                    upstream_tip = self._revid_of_upstream_version_from_branch(
+                            previous_version)
+                    self._extract_upstream_tree(upstream_tip, tempdir)
+                elif (upstream_branch is not None and 
+                      previous_upstream_revision is not None):
+                    upstream_tip = RevisionSpec.from_string(previous_upstream_revision).as_revision_id(upstream_branch)
+                    assert isinstance(upstream_tip, str)
+                    self._extract_upstream_tree(upstream_tip, tempdir)
+                else:
                     raise BzrCommandError("Unable to find the tag for the "
                             "previous upstream version, %s, in the branch: "
                             "%s" % (previous_version,
                                 self.upstream_tag_name(previous_version)))
-                upstream_tip = self._revid_of_upstream_version_from_branch(
-                        previous_version)
-                self._extract_upstream_tree(upstream_tip, tempdir)
             else:
                 self._create_empty_upstream_tree(tempdir)
             if self.has_upstream_version_in_packaging_branch(version):
 
=== modified file 'tests/test_upstream.py'
--- tests/test_upstream.py	2009-03-08 23:18:01 +0000
+++ tests/test_upstream.py	2009-04-16 10:42:12 +0000
@@ -24,14 +24,21 @@
 
 from debian_bundle.changelog import Version
 
-from bzrlib.tests import TestCaseWithTransport
+from bzrlib.tests import (
+        TestCase,
+        TestCaseWithTransport,
+        )
 from bzrlib.plugins.builddeb.errors import (
         MissingUpstreamTarball,
+        PackageVersionNotPresent,
         )
 from bzrlib.plugins.builddeb.upstream import (
+        AptSource,
+        PristineTarSource,
+        StackedUpstreamSource,
         UpstreamProvider,
-        get_apt_command_for_source,
-        provide_with_apt,
+        UpstreamSource,
+        UScanSource,
         )
 from bzrlib.plugins.builddeb.util import (
         get_parent_dir,
@@ -39,156 +46,6 @@
         )
 
 
-class MockProvider(object):
-
-    def create_target(self, path):
-        parent_dir = get_parent_dir(path)
-        if parent_dir != '' and not os.path.exists(parent_dir):
-            os.makedirs(parent_dir)
-        f = open(path, "wb")
-        try:
-            f.write('')
-        finally:
-            f.close()
-
-    def tarball_name(self, package, upstream_version):
-        return tarball_name(package, upstream_version)
-
-
-class MockAptProvider(MockProvider):
-
-    def __init__(self, find=False):
-        self.find = find
-        self.called_times = 0
-        self.package = None
-        self.upstream_version = None
-        self.target_dir = None
-
-    def provide(self, package, upstream_version, target_dir):
-        self.called_times += 1
-        self.package = package
-        self.upstream_version = upstream_version
-        self.target_dir = target_dir
-        if self.find:
-            self.create_target(os.path.join(target_dir,
-                    self.tarball_name(package, upstream_version)))
-        return self.find
-
-
-class MockUscanProvider(MockProvider):
-
-    def __init__(self, find=False):
-        self.find = find
-        self.called_times = 0
-        self.package = None
-        self.upstream_version = None
-        self.watch_file_contents = None
-        self.target_dir = None
-
-    def provide(self, package, upstream_version, watch_file, target_dir):
-        self.called_times += 1
-        self.package = package
-        self.upstream_version = upstream_version
-        f = open(watch_file, "rb")
-        try:
-            self.watch_file_contents = f.read()
-        finally:
-            f.close()
-        self.target_dir = target_dir
-        if self.find:
-            self.create_target(os.path.join(target_dir,
-                    self.tarball_name(package, upstream_version)))
-        return self.find
-
-
-class MockPristineProvider(MockProvider):
-
-    def __init__(self, find=False):
-        self.find = find
-        self.called_times = 0
-        self.tree = None
-        self.branch = None
-        self.package = None
-        self.version = None
-        self.target_filename = None
-
-    def provide(self, tree, branch, package, version, target_filename):
-        self.called_times += 1
-        self.tree = tree
-        self.branch = branch
-        self.package = package
-        self.version = version
-        self.target_filename = target_filename
-        if self.find:
-            self.create_target(target_filename)
-        return self.find
-
-
-class MockOrigSourceProvider(MockProvider):
-
-    def __init__(self, find=False):
-        self.find = find
-        self.called_times = 0
-        self.source_dir = None
-        self.fetch_dir = None
-        self.desired_tarball_name = None
-        self.target_dir = None
-
-    def provide(self, source_dir, fetch_dir, desired_tarball_name,
-            target_dir):
-        self.called_times += 1
-        self.source_dir = source_dir
-        self.fetch_dir = fetch_dir
-        self.desired_tarball_name = desired_tarball_name
-        self.target_dir = target_dir
-        if self.find:
-            self.create_target(os.path.join(target_dir, desired_tarball_name))
-        return self.find
-
-
-class MockOtherBranchProvider(MockProvider):
-
-    def __init__(self, find=False):
-        self.find = find
-        self.called_times = 0
-        self.upstream_branch = None
-        self.upstream_revision = None
-        self.target_filename = None
-        self.tarball_base = None
-
-    def provide(self, upstream_branch, upstream_revision, target_filename,
-            tarball_base):
-        self.called_times += 1
-        self.upstream_branch = upstream_branch
-        self.upstream_revision = upstream_revision
-        self.target_filename = target_filename
-        self.tarball_base = tarball_base
-        if self.find:
-            self.create_target(target_filename)
-        return self.find
-
-
-class MockSplitProvider(MockProvider):
-
-    def __init__(self, find=False):
-        self.find = find
-        self.called_times = 0
-        self.tree = None
-        self.package = None
-        self.upstream_version = None
-        self.target_filename = None
-
-    def provide(self, tree, package, upstream_version, target_filename):
-        self.called_times += 1
-        self.tree = tree
-        self.package = package
-        self.upstream_version = upstream_version
-        self.target_filename = target_filename
-        if self.find:
-            self.create_target(self.target_filename)
-        return self.find
-
-
 class MockSources(object):
 
     def __init__(self, versions):
@@ -245,210 +102,21 @@
         return self.work
 
 
-class UpstreamProviderTests(TestCaseWithTransport):
-
-    def setUp(self):
-        super(UpstreamProviderTests, self).setUp()
-        self.tree = self.make_branch_and_tree(".")
-        self.branch = self.tree.branch
-        self.package = "package"
-        self.version = Version("0.1-1")
-        self.upstream_version = self.version.upstream_version
-        self.desired_tarball_name = tarball_name(self.package,
-                self.upstream_version)
-        self.tarball_base = "%s-%s" % (self.package, self.upstream_version)
-        self.store_dir = "store"
-        self.provider = UpstreamProvider(self.tree, self.branch,
-                self.package, self.version, self.store_dir)
-        self.providers = {}
-        self.providers["apt"]= MockAptProvider()
-        self.provider._apt_provider = self.providers["apt"].provide
-        self.providers["uscan"] = MockUscanProvider()
-        self.provider._uscan_provider = self.providers["uscan"].provide
-        self.providers["pristine"] = MockPristineProvider()
-        self.provider._pristine_provider = self.providers["pristine"].provide
-        self.providers["orig"] = MockOrigSourceProvider()
-        self.provider._orig_source_provider = self.providers["orig"].provide
-        self.providers["upstream"] = MockOtherBranchProvider()
-        self.provider._upstream_branch_provider = \
-                                self.providers["upstream"].provide
-        self.providers["split"] = MockSplitProvider()
-        self.provider._split_provider = self.providers["split"].provide
-        self.target_dir = "target"
-        self.target_filename = os.path.join(self.target_dir,
-                self.desired_tarball_name)
-        self.store_filename = os.path.join(self.store_dir,
-                tarball_name(self.package, self.version.upstream_version))
-
-    def assertProvidersCalled(self, providers):
-        for provider_name, provider in self.providers.items():
-            if provider_name in providers:
-                self.assertCalledCorrectly(provider_name)
-            else:
-                self.assertEqual(provider.called_times, 0,
-                        "%s wasn't expected to be called" % provider_name)
-
-    def call_provider(self):
-        self.assertEqual(self.provider.provide(self.target_dir),
-                self.target_filename)
-
-    def test_already_in_target(self):
-        os.makedirs(self.target_dir)
-        f = open(self.target_filename, "wb")
-        f.close()
-        self.call_provider()
-        self.failUnlessExists(self.target_filename)
-        # Should this be copied across?
-        self.failIfExists(self.store_filename)
-        self.assertProvidersCalled({})
-
-    def test_already_in_store(self):
-        os.makedirs(self.store_dir)
-        f = open(self.store_filename, "wb")
-        f.close()
-        self.call_provider()
-        self.failUnlessExists(self.target_filename)
-        self.failUnlessExists(self.store_filename)
-        self.assertProvidersCalled({})
-
-    def assertCalledCorrectly(self, provider_name):
-        provider = self.providers[provider_name]
-        for attr_name in provider.__dict__:
-            if attr_name in ("find", "provide", "source_dir"):
-                continue
-            if attr_name == "called_times":
-                self.assertEqual(provider.called_times, 1,
-                        "%s was not called" % provider_name)
-                continue
-            if attr_name == "target_filename":
-                self.assertEqual(provider.target_filename,
-                        self.store_filename)
-                continue
-            if attr_name == "target_dir":
-                self.assertEqual(provider.target_dir,
-                        self.store_dir)
-                continue
-            if attr_name == "fetch_dir":
-                self.assertEqual(provider.fetch_dir,
-                        os.path.dirname(provider.source_dir))
-                continue
-            attr = getattr(provider, attr_name)
-            correct_attr = getattr(self, attr_name)
-            self.assertEqual(correct_attr, attr,
-                    "%s doesn't match\nexpected: %s\ngot: %s"
-                    % (attr_name, correct_attr, attr))
-
-    def assertSuccesfulCall(self, provider, other_providers):
-        self.providers[provider].find = True
-        self.call_provider()
-        self.failUnlessExists(self.target_filename)
-        self.failUnlessExists(self.store_filename)
-        self.assertProvidersCalled([provider] + other_providers)
-
-    def test_from_pristine_tar(self):
-        self.assertSuccesfulCall("pristine", [])
-
-    def test_from_apt(self):
-        self.assertSuccesfulCall("apt", ["pristine"])
-
-    def test_from_uscan(self):
-        self.build_tree(["watch", "debian/", "debian/watch"])
-        self.tree.add(["watch", "debian/", "debian/watch"])
-        self.watch_file_contents = "contents of debian/watch\n"
-        self.assertSuccesfulCall("uscan", ["pristine", "apt"])
-
-    def test_uscan_not_called_if_not_watch(self):
-        self.build_tree(["watch"])
-        self.tree.add(["watch"])
-        self.assertRaises(MissingUpstreamTarball, self.provider.provide,
-                self.target_dir)
-        self.failIfExists(self.target_filename)
-        self.failIfExists(self.store_filename)
-        self.assertProvidersCalled(["pristine", "apt"])
-
-    def test_uscan_in_larstiq(self):
-        self.build_tree(["watch", "debian/", "debian/watch"])
-        self.tree.add(["watch", "debian/", "debian/watch"])
-        self.watch_file_contents = "contents of watch\n"
-        self.provider.larstiq = True
-        self.assertSuccesfulCall("uscan", ["pristine", "apt"])
-
-    def test_from_get_orig_source(self):
-        self.build_tree(["rules", "debian/", "debian/rules"])
-        self.tree.add(["rules", "debian/", "debian/rules"])
-        self.watch_file_contents = "contents of debian/rules\n"
-        self.assertSuccesfulCall("orig", ["pristine", "apt"])
-
-    def test_get_orig_source_not_called_if_no_rules(self):
-        self.build_tree(["rules"])
-        self.tree.add(["rules"])
-        self.assertRaises(MissingUpstreamTarball, self.provider.provide,
-                self.target_dir)
-        self.failIfExists(self.target_filename)
-        self.failIfExists(self.store_filename)
-        self.assertProvidersCalled(["pristine", "apt"])
-
-    def test_get_orig_source_in_larstiq(self):
-        self.build_tree(["rules", "debian/", "debian/rules"])
-        self.tree.add(["rules", "debian/", "debian/rules"])
-        self.watch_file_contents = "contents of rules\n"
-        self.provider.larstiq = True
-        self.assertSuccesfulCall("orig", ["pristine", "apt"])
-
-    def test_from_upstream_branch(self):
-        upstream_tree = self.make_branch_and_tree("upstream")
-        self.build_tree(["upstream/foo"])
-        upstream_tree.add(["foo"])
-        self.upstream_branch = upstream_tree.branch
-        self.upstream_revision = upstream_tree.commit("upstream one")
-        self.provider.upstream_revision = self.upstream_revision
-        self.provider.upstream_branch = self.upstream_branch
-        self.assertSuccesfulCall("upstream", ["pristine", "apt"])
-
-    def test_from_split(self):
-        self.provider.allow_split = True
-        self.assertSuccesfulCall("split", ["pristine", "apt"])
-
-    def test_upstream_before_orig_source(self):
-        upstream_tree = self.make_branch_and_tree("upstream")
-        self.build_tree(["upstream/foo"])
-        upstream_tree.add(["foo"])
-        self.upstream_branch = upstream_tree.branch
-        self.upstream_revision = upstream_tree.commit("upstream one")
-        self.provider.upstream_revision = self.upstream_revision
-        self.provider.upstream_branch = self.upstream_branch
-        self.build_tree(["rules", "debian/", "debian/rules"])
-        self.tree.add(["rules", "debian/", "debian/rules"])
-        self.watch_file_contents = "contents of debian/rules\n"
-        self.assertSuccesfulCall("upstream", ["pristine", "apt"])
-
-    def test_get_orig_source_before_uscan(self):
-        self.build_tree(["rules", "debian/", "debian/rules"])
-        self.tree.add(["rules", "debian/", "debian/rules"])
-        self.watch_file_contents = "contents of debian/rules\n"
-        self.build_tree(["watch", "debian/watch"])
-        self.tree.add(["watch", "debian/watch"])
-        self.watch_file_contents = "contents of debian/watch\n"
-        self.assertSuccesfulCall("orig", ["pristine", "apt"])
-
-    def test_uscan_before_split(self):
-        self.build_tree(["watch", "debian/", "debian/watch"])
-        self.tree.add(["watch", "debian/", "debian/watch"])
-        self.watch_file_contents = "contents of debian/watch\n"
-        self.provider.allow_split = True
-        self.assertSuccesfulCall("uscan", ["pristine", "apt"])
+class AptSourceTests(TestCase):
 
     def test_get_apt_command_for_source(self):
         self.assertEqual("apt-get source -y --only-source --tar-only "
                 "apackage=someversion",
-                get_apt_command_for_source("apackage", "someversion"))
+                AptSource()._get_command("apackage", "someversion"))
 
     def test_apt_provider_no_package(self):
         caller = MockAptCaller()
         sources = MockSources([])
         apt_pkg = MockAptPkg(sources)
-        self.assertEqual(False, provide_with_apt("apackage", "0.2",
-                    "target", _apt_pkg=apt_pkg, _apt_caller=caller))
+        src = AptSource()
+        src._run_apt_source = caller.call
+        self.assertRaises(PackageVersionNotPresent, src.get_specific_version,
+            "apackage", "0.2", "target", _apt_pkg=apt_pkg)
         self.assertEqual(1, apt_pkg.init_called_times)
         self.assertEqual(1, apt_pkg.get_pkg_source_records_called_times)
         self.assertEqual(1, sources.restart_called_times)
@@ -460,8 +128,10 @@
         caller = MockAptCaller()
         sources = MockSources(["0.1-1"])
         apt_pkg = MockAptPkg(sources)
-        self.assertEqual(False, provide_with_apt("apackage", "0.2",
-                    "target", _apt_pkg=apt_pkg, _apt_caller=caller))
+        src = AptSource()
+        src._run_apt_source = caller.call
+        self.assertRaises(PackageVersionNotPresent, src.get_specific_version,
+            "apackage", "0.2", "target", _apt_pkg=apt_pkg)
         self.assertEqual(1, apt_pkg.init_called_times)
         self.assertEqual(1, apt_pkg.get_pkg_source_records_called_times)
         self.assertEqual(1, sources.restart_called_times)
@@ -473,8 +143,10 @@
         caller = MockAptCaller(work=True)
         sources = MockSources(["0.1-1", "0.2-1"])
         apt_pkg = MockAptPkg(sources)
-        self.assertEqual(True, provide_with_apt("apackage", "0.2",
-                    "target", _apt_pkg=apt_pkg, _apt_caller=caller.call))
+        src = AptSource()
+        src._run_apt_source = caller.call
+        src.get_specific_version("apackage", "0.2", "target", 
+            _apt_pkg=apt_pkg)
         self.assertEqual(1, apt_pkg.init_called_times)
         self.assertEqual(1, apt_pkg.get_pkg_source_records_called_times)
         self.assertEqual(1, sources.restart_called_times)
@@ -490,8 +162,11 @@
         caller = MockAptCaller()
         sources = MockSources(["0.1-1", "0.2-1"])
         apt_pkg = MockAptPkg(sources)
-        self.assertEqual(False, provide_with_apt("apackage", "0.2",
-                    "target", _apt_pkg=apt_pkg, _apt_caller=caller.call))
+        src = AptSource()
+        src._run_apt_source = caller.call
+        self.assertRaises(PackageVersionNotPresent, src.get_specific_version,
+            "apackage", "0.2", "target", 
+            _apt_pkg=apt_pkg)
         self.assertEqual(1, apt_pkg.init_called_times)
         self.assertEqual(1, apt_pkg.get_pkg_source_records_called_times)
         self.assertEqual(1, sources.restart_called_times)
@@ -502,3 +177,71 @@
         self.assertEqual("apackage", caller.package)
         self.assertEqual("0.2-1", caller.version_str)
         self.assertEqual("target", caller.target_dir)
+
+
+class RecordingSource(object):
+
+    def __init__(self, succeed):
+        self._succeed = succeed
+        self._specific_versions = []
+
+    def get_specific_version(self, package, version, target_dir):
+        self._specific_versions.append((package, version, target_dir))
+        if not self._succeed:
+            raise PackageVersionNotPresent(package, version, self)
+
+    def __repr__(self):
+        return "%s()" % self.__class__.__name__
+
+
+class StackedUpstreamSourceTests(TestCase):
+
+    def test_first_wins(self):
+        a = RecordingSource(False)
+        b = RecordingSource(True)
+        c = RecordingSource(False)
+        stack = StackedUpstreamSource([a, b, c])
+        stack.get_specific_version("mypkg", "1.0", "bla")
+        self.assertEquals([("mypkg", "1.0", "bla")], b._specific_versions)
+        self.assertEquals([("mypkg", "1.0", "bla")], a._specific_versions)
+        self.assertEquals([], c._specific_versions)
+
+    def test_repr(self):
+        self.assertEquals("StackedUpstreamSource([])", 
+                repr(StackedUpstreamSource([])))
+        self.assertEquals("StackedUpstreamSource([RecordingSource()])", 
+                repr(StackedUpstreamSource([RecordingSource(False)])))
+
+    def test_none(self):
+        a = RecordingSource(False)
+        b = RecordingSource(False)
+        stack = StackedUpstreamSource([a, b])
+        self.assertRaises(PackageVersionNotPresent, 
+                stack.get_specific_version, "pkg", "1.0", "bla")
+        self.assertEquals([("pkg", "1.0", "bla")], b._specific_versions)
+        self.assertEquals([("pkg", "1.0", "bla")], a._specific_versions)
+
+
+class UScanSourceTests(TestCaseWithTransport):
+
+    def setUp(self):
+        super(UScanSourceTests, self).setUp()
+        self.tree = self.make_branch_and_tree('.')
+
+    def test_export_watchfile_none(self):
+        src = UScanSource(self.tree, False)
+        self.assertEquals(None, src._export_watchfile())
+
+    def test_export_watchfile_larstiq(self):
+        src = UScanSource(self.tree, True)
+        self.build_tree(['watch'])
+        self.assertEquals(None, src._export_watchfile())
+        self.tree.add(['watch'])
+        self.assertTrue(src._export_watchfile() is not None)
+
+    def test_export_watchfile(self):
+        src = UScanSource(self.tree, False)
+        self.build_tree(['debian/', 'debian/watch'])
+        self.assertEquals(None, src._export_watchfile())
+        self.tree.smart_add(['debian/watch'])
+        self.assertTrue(src._export_watchfile() is not None)
 
=== modified file 'upstream.py'
--- upstream.py	2009-03-07 23:54:46 +0000
+++ upstream.py	2009-04-16 10:06:01 +0000
@@ -1,5 +1,6 @@
 #    upstream.py -- Providers of upstream source
 #    Copyright (C) 2009 Canonical Ltd.
+#    Copyright (C) 2009 Jelmer Vernooij <[e-mail]>
 #
 #    This file is part of bzr-builddeb.
 #
@@ -26,119 +27,313 @@
 from debian_bundle.changelog import Version
 
 from bzrlib.export import export
+from bzrlib.revisionspec import RevisionSpec
 from bzrlib.trace import info
 
-from bzrlib.plugins.builddeb.errors import MissingUpstreamTarball
+from bzrlib.plugins.builddeb.errors import (
+    MissingUpstreamTarball,
+    PackageVersionNotPresent,
+    )
 from bzrlib.plugins.builddeb.import_dsc import DistributionBranch
 from bzrlib.plugins.builddeb.repack_tarball import repack_tarball
-
-
-def get_apt_command_for_source(package, version_str):
-    return 'apt-get source -y --only-source --tar-only %s=%s' % \
-        (package, version_str)
-
-
-def call_apt_for_source(package, version_str, target_dir):
-    command = get_apt_command_for_source(package, version_str)
-    proc = subprocess.Popen(command, shell=True, cwd=target_dir)
-    proc.wait()
-    if proc.returncode != 0:
-        return False
-    return True
-
-
-def provide_with_apt(package, upstream_version, target_dir, _apt_pkg=None,
-        _apt_caller=None):
-    if _apt_pkg is None:
-        import apt_pkg
-    else:
-        apt_pkg = _apt_pkg
-    if _apt_caller is None:
-        _apt_caller = call_apt_for_source
-    apt_pkg.init()
-    sources = apt_pkg.GetPkgSrcRecords()
-    sources.Restart()
-    info("Using apt to look for the upstream tarball.")
-    while sources.Lookup(package):
-        if upstream_version \
-            == Version(sources.Version).upstream_version:
-            if _apt_caller(package, sources.Version, target_dir):
-                return True
-            break
-    info("apt could not find the needed tarball.")
-    return False
-
-
-def provide_with_uscan(package, upstream_version, watch_file, target_dir):
-    info("Using uscan to look for the upstream tarball.")
-    r = os.system("uscan --upstream-version %s --force-download --rename "
-                  "--package %s --watchfile %s --check-dirname-level 0 " 
-                  "--download --repack --destdir %s" %
-                  (upstream_version, package, watch_file, target_dir))
-    if r != 0:
-        info("uscan could not find the needed tarball.")
-        return False
-    return True
-
-
-def provide_with_pristine_tar(tree, branch, package, version,
-        target_filename):
-    db = DistributionBranch(branch, None, tree=tree)
-    if not db.has_upstream_version_in_packaging_branch(version):
-        return False
-    revid = db._revid_of_upstream_version_from_branch(version)
-    if not db.has_pristine_tar_delta(revid):
-        return False
-    info("Using pristine-tar to reconstruct the needed tarball.")
-    db.reconstruct_pristine_tar(revid, package, version, target_filename)
-    return True
-
-
-def provide_with_get_orig_source(source_dir, fetch_dir, desired_tarball_name,
-        target_dir):
-    info("Trying to use get-orig-source to retrieve needed tarball.")
-    command = ["/usr/bin/make", "-f", "debian/rules", "get-orig-source"]
-    proc = subprocess.Popen(command, cwd=source_dir)
-    ret = proc.wait()
-    if ret != 0:
-        info("Trying to run get-orig-source rule failed")
-        return False
-    fetched_tarball = os.path.join(fetch_dir, desired_tarball_name)
-    if not os.path.exists(fetched_tarball):
-        info("get-orig-source did not create %s"
-                % desired_tarball_name)
-        return False
-    repack_tarball(fetched_tarball, desired_tarball_name,
-                   target_dir=target_dir)
-    return True
-
-
-def provide_from_other_branch(branch, revision, target_filename,
-        tarball_base):
-    branch.lock_read()
-    try:
-        rev_tree = branch.repository.revision_tree(revision)
-        export(rev_tree, target_filename, 'tgz', tarball_base)
-        return True
-    finally:
-        branch.unlock()
-
-
-def provide_by_split(tree, package, upstream_version, target_filename):
-    tmpdir = tempfile.mkdtemp(prefix="builddeb-get-orig-source-")
-    try:
-        export_dir = os.path.join(tmpdir,
-                "%s-%s" % (package, upstream_version))
-        export(tree, export_dir, format="dir")
-        shutil.rmtree(os.path.join(export_dir, "debian"))
-        tar = tarfile.open(target_filename, "w:gz")
-        try:
-            tar.add(export_dir, "%s-%s" % (package, upstream_version))
-        finally:
-            tar.close()
-        return True
-    finally:
-        shutil.rmtree(tmpdir)
+from bzrlib.plugins.builddeb.util import (
+    get_snapshot_revision,
+    tarball_name,
+    )
+
+
+class UpstreamSource(object):
+    """A source for upstream versions (uscan, get-orig-source, etc)."""
+
+    def get_latest_version(self, package, target_dir):
+        """Fetch the source tarball for the latest available version.
+
+        :param package: Name of the package
+        :param target_dir: Directory in which to store the tarball
+        """
+        raise NotImplemented(self.get_latest_version)
+
+    def get_specific_version(self, package, version, target_dir):
+        """Fetch the source tarball for a particular version.
+
+        :param package: Name of the package
+        :param version: Version string of the version to fetch
+        :param target_dir: Directory in which to store the tarball
+        """
+        raise NotImplemented(self.get_specific_version)
+
+    def _tarball_path(self, package, version, target_dir):
+        return os.path.join(target_dir, tarball_name(package, version))
+
+
+class PristineTarSource(UpstreamSource):
+    """Source that uses the pristine-tar revisions in the packaging branch."""
+
+    def __init__(self, tree, branch):
+        self.branch = branch
+        self.tree = tree
+
+    def get_specific_version(self, package, upstream_version, target_dir):
+        # FIXME: Make provide_with_pristine_tar take a upstream 
+        # version string; it doesn't need the debian-specific data
+        version = Version("%s-1" % upstream_version)
+        target_filename = self._tarball_path(package, upstream_version, 
+                                             target_dir)
+        db = DistributionBranch(self.branch, None, tree=self.tree)
+        if not db.has_upstream_version_in_packaging_branch(version):
+            raise PackageVersionNotPresent(package, upstream_version, self)
+        revid = db._revid_of_upstream_version_from_branch(version)
+        if not db.has_pristine_tar_delta(revid):
+            raise PackageVersionNotPresent(package, upstream_version, self)
+        info("Using pristine-tar to reconstruct the needed tarball.")
+        db.reconstruct_pristine_tar(revid, package, version, target_filename)
+
+
+class AptSource(UpstreamSource):
+    """Upstream source that uses apt-source."""
+
+    def get_specific_version(self, package, upstream_version, target_dir, 
+            _apt_pkg=None):
+        if _apt_pkg is None:
+            import apt_pkg
+        else:
+            apt_pkg = _apt_pkg
+        apt_pkg.init()
+        sources = apt_pkg.GetPkgSrcRecords()
+        sources.Restart()
+        info("Using apt to look for the upstream tarball.")
+        while sources.Lookup(package):
+            if upstream_version \
+                == Version(sources.Version).upstream_version:
+                if self._run_apt_source(package, sources.Version, target_dir):
+                    return
+                break
+        info("apt could not find the needed tarball.")
+        raise PackageVersionNotPresent(package, upstream_version, self)
+
+    def _get_command(self, package, version_str):
+        return 'apt-get source -y --only-source --tar-only %s=%s' % \
+            (package, version_str)
+
+    def _run_apt_source(self, package, version_str, target_dir):
+        command = self._get_command(package, version_str)
+        proc = subprocess.Popen(command, shell=True, cwd=target_dir)
+        proc.wait()
+        if proc.returncode != 0:
+            return False
+        return True
+
+
+class UpstreamBranchSource(UpstreamSource):
+    """Upstream source that uses the upstream branch."""
+
+    def __init__(self, upstream_branch, upstream_revision=None, 
+                 fallback_revspec=None):
+        self.upstream_branch = upstream_branch
+        self.upstream_revision = upstream_revision
+        self.fallback_revspec = fallback_revspec
+
+    def _get_revision_id(self, version):
+        if self.upstream_revision is not None:
+            # Explicit revision id to use set
+            return self.upstream_revision
+        revspec = get_snapshot_revision(version)
+        if revspec is None:
+            revspec = self.fallback_revspec
+        if revspec is not None:
+            return RevisionSpec.from_string(
+                revspec).as_revision_id(self.upstream_branch)
+        return self.upstream_branch.last_revision()
+
+    def get_specific_version(self, package, version, target_dir):
+        self.upstream_branch.lock_read()
+        try:
+            revid = self._get_revision_id(version)
+            info("Exporting upstream branch revision %s to create the tarball",
+                 revid)
+            target_filename = self._tarball_path(package, version, target_dir)
+            tarball_base = "%s-%s" % (package, version)
+            rev_tree = self.upstream_branch.repository.revision_tree(revid)
+            export(rev_tree, target_filename, 'tgz', tarball_base)
+        finally:
+            self.upstream_branch.unlock()
+
+
+class GetOrigSourceSource(UpstreamSource):
+    """Upstream source that uses the get-orig-source rule in debian/rules."""
+
+    def __init__(self, tree, larstiq):
+        self.tree = tree
+        self.larstiq = larstiq
+
+    def _get_orig_source(self, source_dir, fetch_dir, desired_tarball_name, 
+                        target_dir):
+        info("Trying to use get-orig-source to retrieve needed tarball.")
+        command = ["/usr/bin/make", "-f", "debian/rules", "get-orig-source"]
+        proc = subprocess.Popen(command, cwd=source_dir)
+        ret = proc.wait()
+        if ret != 0:
+            info("Trying to run get-orig-source rule failed")
+            return False
+        fetched_tarball = os.path.join(fetch_dir, desired_tarball_name)
+        if not os.path.exists(fetched_tarball):
+            info("get-orig-source did not create %s", desired_tarball_name)
+            return False
+        repack_tarball(fetched_tarball, desired_tarball_name, 
+                       target_dir=target_dir)
+        return True
+
+    def get_specific_version(self, package, version, target_dir):
+        if self.larstiq:
+            rules_name = 'rules'
+        else:
+            rules_name = 'debian/rules'
+        rules_id = self.tree.path2id(rules_name)
+        if rules_id is not None:
+            desired_tarball_name = tarball_name(package, version)
+            tmpdir = tempfile.mkdtemp(prefix="builddeb-get-orig-source-")
+            try:
+                base_export_dir = os.path.join(tmpdir, "export")
+                export_dir = base_export_dir
+                if self.larstiq:
+                    os.mkdir(export_dir)
+                    export_dir = os.path.join(export_dir, "debian")
+                export(self.tree, export_dir, format="dir")
+                if not self._get_orig_source(base_export_dir, tmpdir,
+                        desired_tarball_name, target_dir):
+                    raise PackageVersionNotPresent(package, version, self)
+                return
+            finally:
+                shutil.rmtree(tmpdir)
+        info("No debian/rules file to try and use for a get-orig-source rule")
+        raise PackageVersionNotPresent(package, version, self)
+
+
+class UScanSource(UpstreamSource):
+    """Upstream source that uses uscan."""
+
+    def __init__(self, tree, larstiq):
+        self.tree = tree
+        self.larstiq = larstiq
+
+    def _uscan(self, package, upstream_version, watch_file, target_dir):
+        info("Using uscan to look for the upstream tarball.")
+        r = os.system("uscan --upstream-version %s --force-download --rename "
+                      "--package %s --watchfile %s --check-dirname-level 0 " 
+                      "--download --repack --destdir %s" %
+                      (upstream_version, package, watch_file, target_dir))
+        if r != 0:
+            info("uscan could not find the needed tarball.")
+            return False
+        return True
+
+    def _export_watchfile(self):
+        if self.larstiq:
+            watchfile = 'watch'
+        else:
+            watchfile = 'debian/watch'
+        watch_id = self.tree.path2id(watchfile)
+        if watch_id is None:
+            info("No watch file to use to retrieve upstream tarball.")
+            return None
+        (tmp, tempfilename) = tempfile.mkstemp()
+        try:
+            tmp = os.fdopen(tmp, 'wb')
+            watch = self.tree.get_file_text(watch_id)
+            tmp.write(watch)
+        finally:
+            tmp.close()
+        return tempfilename
+
+    def get_specific_version(self, package, version, target_dir):
+        tempfilename = self._export_watchfile()
+        if tempfilename is None:
+            raise PackageVersionNotPresent(package, version, self)
+        try:
+            if not self._uscan(package, version, tempfilename, 
+                    target_dir):
+                raise PackageVersionNotPresent(package, version, self)
+        finally:
+          os.unlink(tempfilename)
+
+
+class SelfSplitSource(UpstreamSource):
+
+    def __init__(self, tree):
+        self.tree = tree
+
+    def _split(self, package, upstream_version, target_filename):
+        tmpdir = tempfile.mkdtemp(prefix="builddeb-get-orig-source-")
+        try:
+            export_dir = os.path.join(tmpdir,
+                    "%s-%s" % (package, upstream_version))
+            export(self.tree, export_dir, format="dir")
+            shutil.rmtree(os.path.join(export_dir, "debian"))
+            tar = tarfile.open(target_filename, "w:gz")
+            try:
+                tar.add(export_dir, "%s-%s" % (package, upstream_version))
+            finally:
+                tar.close()
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def get_specific_version(self, package, version, target_dir):
+        info("Using the current branch without the 'debian' directory "
+                "to create the tarball")
+        self._split(package, version, 
+                    self._tarball_path(package, version, target_dir))
+
+
+class StackedUpstreamSource(UpstreamSource):
+    """An upstream source that checks a list of other upstream sources.
+    
+    The first source that can provide a tarball, wins. 
+    """
+
+    def __init__(self, sources):
+        self._sources = sources
+
+    def __repr__(self):
+        return "%s(%r)" % (self.__class__.__name__, self._sources)
+
+    def get_specific_version(self, package, version, target_dir):
+        for source in self._sources:
+            try:
+                return source.get_specific_version(package, version, target_dir)
+            except PackageVersionNotPresent:
+                pass
+        raise PackageVersionNotPresent(package, version, self)
+
+
+def get_upstream_sources(tree, branch, larstiq=False, upstream_branch=None,
+                         upstream_revision=None, allow_split=False):
+    """Get the list of upstream sources to retrieve upstream versions from.
+
+    :param tree: The tree that is being built from.
+    :param branch: The branch that is being built from.
+    :param larstiq: Whether the tree versions the root of ./debian.
+    :param upstream_branch: An upstream branch that can be exported
+        if needed.
+    :param upstream_revision: The revision to use of the upstream branch
+        if it is used.
+    :param allow_split: Whether the provider can provide the tarball
+        by exporting the branch and removing the "debian" dir.
+
+    """
+    sources = [
+        PristineTarSource(tree, branch), 
+        AptSource(),
+        ]
+    if upstream_branch is not None:
+        sources.append(
+            UpstreamBranchSource(upstream_branch, upstream_revision))
+    sources.extend([
+        GetOrigSourceSource(tree, larstiq), 
+        UScanSource(tree, larstiq),
+        ])
+    if allow_split:
+        sources.append(SelfSplitSource(tree))
+    return sources
 
 
 class UpstreamProvider(object):
@@ -148,40 +343,17 @@
     instance using pristine-tar, or using apt.
     """
 
-    def __init__(self, tree, branch, package, version, store_dir,
-            larstiq=False, upstream_branch=None, upstream_revision=None,
-            allow_split=False):
+    def __init__(self, package, version, store_dir, sources):
         """Create an UpstreamProvider.
 
-        :param tree: The tree that is being built from.
-        :param branch: The branch that is being built from.
         :param package: the name of the source package that is being built.
         :param version: the Version of the package that is being built.
         :param store_dir: A directory to cache the tarballs.
-        :param larstiq: Whether the tree versions the root of ./debian.
-        :param upstream_branch: An upstream branch that can be exported
-            if needed.
-        :param upstream_revision: The revision to use of the upstream branch
-            if it is used.
-        :param allow_split: Whether the provider can provide the tarball
-            by exporting the branch and removing the "debian" dir.
         """
-        self.tree = tree
-        self.branch = branch
         self.package = package
         self.version = Version(version)
         self.store_dir = store_dir
-        self.larstiq = larstiq
-        self.upstream_branch = upstream_branch
-        self.upstream_revision = upstream_revision
-        self.allow_split = allow_split
-        # for testing
-        self._apt_provider = provide_with_apt
-        self._uscan_provider = provide_with_uscan
-        self._pristine_provider = provide_with_pristine_tar
-        self._orig_source_provider = provide_with_get_orig_source
-        self._upstream_branch_provider = provide_from_other_branch
-        self._split_provider = provide_by_split
+        self.source = StackedUpstreamSource(sources)
 
     def provide(self, target_dir):
         """Provide the upstream tarball any way possible.
@@ -214,24 +386,16 @@
         if not self.already_exists_in_store():
             if not os.path.exists(self.store_dir):
                 os.makedirs(self.store_dir)
-            if self.provide_with_pristine_tar(self.store_dir):
-                pass
-            elif self.provide_with_apt(self.store_dir):
-                pass
-            elif self.provide_from_upstream_branch(self.store_dir):
-                pass
-            elif self.provide_with_get_orig_source(self.store_dir):
-                pass
-            elif self.provide_with_uscan(self.store_dir):
-                pass
-            elif self.provide_from_self_by_split(self.store_dir):
-                pass
+            try:
+                self.source.get_specific_version(self.package, 
+                    self.version.upstream_version, target_dir)
+            except PackageVersionNotPresent:
+                raise MissingUpstreamTarball(self._tarball_name())
         else:
              info("Using the upstream tarball that is present in "
                      "%s" % self.store_dir)
-        if self.provide_from_store_dir(target_dir):
-            return os.path.join(target_dir, self._tarball_name())
-        raise MissingUpstreamTarball(self._tarball_name())
+        assert self.provide_from_store_dir(target_dir)
+        return os.path.join(target_dir, self._tarball_name())
 
     def already_exists_in_target(self, target_dir):
         return os.path.exists(os.path.join(target_dir, self._tarball_name()))
@@ -247,83 +411,8 @@
             return True
         return False
 
-    def provide_with_apt(self, target_dir):
-        return self._apt_provider(self.package, self.version.upstream_version,
-                target_dir)
-
-    def provide_with_uscan(self, target_dir):
-        if self.larstiq:
-            watchfile = 'watch'
-        else:
-            watchfile = 'debian/watch'
-        watch_id = self.tree.path2id(watchfile)
-        if watch_id is None:
-            info("No watch file to use to retrieve upstream tarball.")
-            return False
-        (tmp, tempfilename) = tempfile.mkstemp()
-        try:
-            tmp = os.fdopen(tmp, 'wb')
-            watch = self.tree.get_file_text(watch_id)
-            tmp.write(watch)
-        finally:
-            tmp.close()
-        try:
-            return self._uscan_provider(self.package,
-                    self.version.upstream_version, tempfilename, target_dir)
-        finally:
-          os.unlink(tempfilename)
-
-    def provide_with_pristine_tar(self, target_dir):
-        target_filename = os.path.join(target_dir, self._tarball_name())
-        return self._pristine_provider(self.tree, self.branch, self.package,
-                self.version, target_filename)
-
-    def provide_with_get_orig_source(self, target_dir):
-        if self.larstiq:
-            rules_name = 'rules'
-        else:
-            rules_name = 'debian/rules'
-        rules_id = self.tree.path2id(rules_name)
-        if rules_id is not None:
-            desired_tarball_name = self._tarball_name()
-            tmpdir = tempfile.mkdtemp(prefix="builddeb-get-orig-source-")
-            try:
-                base_export_dir = os.path.join(tmpdir, "export")
-                export_dir = base_export_dir
-                if self.larstiq:
-                    os.mkdir(export_dir)
-                    export_dir = os.path.join(export_dir, "debian")
-                export(self.tree, export_dir, format="dir")
-                return self._orig_source_provider(base_export_dir, tmpdir,
-                        desired_tarball_name, target_dir)
-            finally:
-                shutil.rmtree(tmpdir)
-        info("No debian/rules file to try and use for a get-orig-source "
-             "rule")
-        return False
-
     def _tarball_name(self):
-        return "%s_%s.orig.tar.gz" % (self.package,
-                self.version.upstream_version)
-
-    def provide_from_upstream_branch(self, target_dir):
-        if self.upstream_branch is None:
-            return False
-        assert self.upstream_revision is not None
-        info("Exporting the upstream branch to create the tarball")
-        target_filename = os.path.join(target_dir, self._tarball_name())
-        tarball_base = "%s-%s" % (self.package, self.version.upstream_version)
-        return self._upstream_branch_provider(self.upstream_branch,
-                self.upstream_revision, target_filename, tarball_base)
-
-    def provide_from_self_by_split(self, target_dir):
-        if not self.allow_split:
-            return False
-        info("Using the current branch without the 'debian' directory "
-                "to create the tarball")
-        target_filename = os.path.join(target_dir, self._tarball_name())
-        return self._split_provider(self.tree, self.package,
-                self.version.upstream_version, target_filename)
+        return tarball_name(self.package, self.version.upstream_version)
 
 
 class _MissingUpstreamProvider(UpstreamProvider):
@@ -347,6 +436,7 @@
         f.write("I am a tarball, honest\n")
         f.close()
 
+
 class _SimpleUpstreamProvider(UpstreamProvider):
     """For tests"""