Merge lp:~jelmer/bzr-builddeb/get-packaged-orig-source into lp:bzr-builddeb

[Jelmer Vernooij]

Deprecate the use of 'debian/rules get-orig-source' to retrieve the upstream tarball for the currently packaged upstream source, as this conflicts with the behaviour for get-orig-source mandated by debian policy (section 4.9).

Instead, support a new debian/rules target 'get-packaged-orig-source'. bzr-builddeb will now warn when it retrieves the upstream source using 'debian/rules get-orig-source'. I'm not entirely happy about the name of the new rule, better suggestions welcome. :-)

We should probably also rename the 'bzr get-orig-source' command while we're changing the rules name to prevent any confusion - we haven't done a release with the current name yet, so I think renaming it is reasonable enough.

[Martin Packman (gz)]

Refactoring looks good, removal of the duplication of gather_orig_files is nice.

review: Approve (code)

[James Westby (james-w)]

review: Approve

=== modified file 'cmds.py'
--- cmds.py	2011-11-26 15:01:25 +0000
+++ cmds.py	2011-11-30 17:07:30 +0000
@@ -88,7 +88,7 @@
         )
 from bzrlib.plugins.builddeb.upstream import (
         AptSource,
-        GetOrigSourceSource,
+        DebianRulesSource,
         SelfSplitSource,
         TarfileSource,
         UScanSource,
@@ -396,7 +396,7 @@
             elif not native and config.upstream_branch is not None:
                 upstream_sources.append(LazyUpstreamBranchSource(config.upstream_branch))
             upstream_sources.extend([
-                GetOrigSourceSource(tree, top_level),
+                DebianRulesSource(tree, top_level),
                 UScanSource(tree, top_level),
                 ])
             if build_type == BUILD_TYPE_SPLIT:
@@ -479,7 +479,7 @@
                 changelog.version.upstream_version, orig_dir,
                 [PristineTarSource(tree, tree.branch),
                  AptSource(),
-                 GetOrigSourceSource(tree, larstiq),
+                 DebianRulesSource(tree, larstiq),
                  UScanSource(tree, larstiq) ])
 
         result = upstream_provider.provide(orig_dir)
@@ -1084,7 +1084,7 @@
                 changelog.version.upstream_version, orig_dir,
                 [PristineTarSource(t, t.branch),
                  AptSource(),
-                 GetOrigSourceSource(t, top_level),
+                 DebianRulesSource(t, top_level),
                  UScanSource(t, top_level) ])
 
         distiller = MergeModeDistiller(t, upstream_provider,

=== modified file 'debian/NEWS'
--- debian/NEWS	2011-02-06 13:25:43 +0000
+++ debian/NEWS	2011-11-30 17:07:30 +0000
@@ -1,3 +1,15 @@
+bzr-builddeb (2.7.10) precise; urgency=low
+
+  bzr-builddeb now warns when using the get-orig-source target in debian/rules
+  to fetch the currently package upstream tarball. get-packaged-orig-source
+  should be provided instead.
+
+  Debian policy (section 4.9) dictates that get-orig-source should fetch the
+  latest upstream tarball rather than the upstream tarball for the upstream
+  version that is currently packaged.
+
+ -- Jelmer Vernooij <[e-mail]>  Wed, 30 Nov 2011 17:13:49 +0100
+
 bzr-builddeb (2.5.1) unstable; urgency=low
 
   The 'deb:' directory service has been renamed to 'apt:' to avoid confusion.

=== modified file 'debian/changelog'
--- debian/changelog	2011-11-29 14:39:05 +0000
+++ debian/changelog	2011-11-30 17:07:30 +0000
@@ -25,8 +25,11 @@
     unless 'commit-message-from-changelog' is explicitly set to True. LP: #812749
   * Support running dep3-patch against remote repositories, and with
     open-ended revision ranges. LP: #893608
+  * bzr-builddeb now prefers the 'get-packaged-orig-source' rule to
+    retrieve the packaged upstream source, and warns about
+    'get-orig-source'. LP: #409862
 
- -- Jelmer Vernooij <[e-mail]>  Sat, 26 Nov 2011 16:01:16 +0100
+ -- Jelmer Vernooij <[e-mail]>  Wed, 30 Nov 2011 17:13:38 +0100
 
 bzr-builddeb (2.7.9) unstable; urgency=low
 

=== modified file 'tests/blackbox/test_get_tar.py'
--- tests/blackbox/test_get_tar.py	2011-10-03 10:49:13 +0000
+++ tests/blackbox/test_get_tar.py	2011-11-30 17:07:30 +0000
@@ -18,9 +18,6 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
 #
 
-import os
-import tarfile
-
 try:
     from debian.changelog import (Changelog,
                                   Version,

=== modified file 'upstream/__init__.py'
--- upstream/__init__.py	2011-11-08 10:10:05 +0000
+++ upstream/__init__.py	2011-11-30 17:07:30 +0000
@@ -64,7 +64,7 @@
 
 
 class UpstreamSource(object):
-    """A source for upstream versions (uscan, get-orig-source, etc)."""
+    """A source for upstream versions (uscan, debian/rules, etc)."""
 
     def get_latest_version(self, package, current_version):
         """Check what the latest upstream version is.
@@ -171,59 +171,77 @@
         return True
 
 
-class GetOrigSourceSource(UpstreamSource):
-    """Upstream source that uses the get-orig-source rule in debian/rules."""
+class DebianRulesSource(UpstreamSource):
+    """Upstream source that uses rules in debian/rules."""
 
     def __init__(self, tree, top_level):
         self.tree = tree
         self.top_level = top_level
 
-    def _get_orig_source(self, source_dir, prefix, target_dir):
-        note("Trying to use get-orig-source to retrieve needed tarball.")
-        command = ["make", "-f", "debian/rules", "get-orig-source"]
+    def _get_rule_source(self, source_dir, rule, make_vars=None):
+        command = ["make", "-f", "debian/rules", rule]
+        if make_vars is not None:
+            command.append(["%s=%s" % item for item in make_vars.iteritems()])
+
         proc = subprocess.Popen(command, cwd=source_dir)
         ret = proc.wait()
         if ret != 0:
-            note("Trying to run get-orig-source rule failed")
+            note("Trying to run %s rule failed" % rule)
+            return False
+        return True
+
+    def _get_current_source(self, source_dir, package, version, target_dir):
+        rule = "get-packaged-orig-source"
+        note("Trying to use %s to retrieve needed tarball." % rule)
+        if not self._get_rule_source(source_dir, rule, {
+                "ORIG_VERSION": version,
+                "ORIG_PACKAGE": package}):
+            rule = "get-orig-source"
+            note("Trying to use %s to retrieve needed tarball (deprecated)." % rule)
+            if not self._get_rule_source(source_dir, rule):
+                return None
+        filenames = gather_orig_files(package, version, source_dir)
+        if not filenames:
+            note("%s did not create file for %s version %s", rule, package, version)
             return None
-        filenames = []
-        for filename in os.listdir(source_dir):
-            if not filename.startswith(prefix):
-                continue
-            fetched_tarball = os.path.join(source_dir, filename)
-            if os.path.exists(fetched_tarball):
-                repack_tarball(fetched_tarball, filename,
-                               target_dir=target_dir)
-                filenames.append(os.path.join(target_dir, filename))
-        if filenames:
-            return filenames
-        note("get-orig-source did not create file with prefix %s", prefix)
-        return None
+        if rule == "get-orig-source":
+            warning("Using get-orig-source to retrieve the packaged orig tarball "
+                    "is deprecated (see debian policy section 4.9). Provide the "
+                    "'get-packaged-orig-source' target instead.")
+        ret = []
+        for filename in filenames:
+            repack_tarball(
+                filename, os.path.basename(filename), target_dir=target_dir)
+            ret.append(os.path.join(target_dir, os.path.basename(filename)))
+        return ret
 
-    def fetch_tarballs(self, package, version, target_dir, components=None):
+    def _get_rules_id(self):
         if self.top_level:
             rules_name = 'rules'
         else:
             rules_name = 'debian/rules'
-        rules_id = self.tree.path2id(rules_name)
-        if rules_id is not None:
-            tmpdir = tempfile.mkdtemp(prefix="builddeb-get-orig-source-")
-            try:
-                base_export_dir = os.path.join(tmpdir, "export")
-                export_dir = base_export_dir
-                if self.top_level:
-                    os.mkdir(export_dir)
-                    export_dir = os.path.join(export_dir, "debian")
-                export(self.tree, export_dir, format="dir")
-                tarball_paths = self._get_orig_source(base_export_dir,
-                        "%s_%s.orig" % (package, version), target_dir)
-                if tarball_paths is None:
-                    raise PackageVersionNotPresent(package, version, self)
-                return tarball_paths
-            finally:
-                shutil.rmtree(tmpdir)
-        note("No debian/rules file to try and use for a get-orig-source rule")
-        raise PackageVersionNotPresent(package, version, self)
+        return self.tree.path2id(rules_name)
+
+    def fetch_tarballs(self, package, version, target_dir, components=None):
+        rules_id = self._get_rules_id()
+        if rules_id is None:
+            note("No debian/rules file to use to retrieve upstream tarball.")
+            raise PackageVersionNotPresent(package, version, self)
+        tmpdir = tempfile.mkdtemp(prefix="builddeb-get-source-")
+        try:
+            base_export_dir = os.path.join(tmpdir, "export")
+            export_dir = base_export_dir
+            if self.top_level:
+                os.mkdir(export_dir)
+                export_dir = os.path.join(export_dir, "debian")
+            export(self.tree, export_dir, format="dir")
+            tarball_paths = self._get_current_source(base_export_dir,
+                    package, version, target_dir)
+            if tarball_paths is None:
+                raise PackageVersionNotPresent(package, version, self)
+            return tarball_paths
+        finally:
+            shutil.rmtree(tmpdir)
 
 
 class UScanSource(UpstreamSource):
@@ -418,7 +436,7 @@
              retrieved.
            - Else if uscan knows about that version it will be downloaded
              and repacked as needed.
-           - Else a call will be made to get-orig-source to try and retrieve
+           - Else a call will be made to debian/rules to try and retrieve
              the tarball.
 
         If the tarball can't be found at all then MissingUpstreamTarball
@@ -561,7 +579,7 @@
             warning("More than one release file for release %s of package %s"
                     "found on Launchpad. Using the first.", version, package)
         hosted_file = release_files[0]
-        tmpdir = tempfile.mkdtemp(prefix="builddeb-get-orig-source-")
+        tmpdir = tempfile.mkdtemp(prefix="builddeb-launchpad-source-")
         try:
             inf = hosted_file.open()
             try: