Merge lp:~julian-edwards/launchpad/ddeb-override-lockstep into lp:launchpad

[Julian Edwards]

[r=danilo][bug=747558][incr] At upload time, override DDEB files so they are the same as their DEB counterparts. This ensures that domination will work.

= Summary =
At upload time, override DDEB files so they are the same as their DEB counterparts. This ensures that domination will work.

== Pre-implementation notes ==
See comments on bug 747558

== Implementation details ==
Add a new method to the upload processor, "_overrideDDEBSs", that ensures that
any DDEBs in the upload are overridden to have the same
component/section/priority as their counterpart DEBS. This method is then called
from process().

There are new unit tests for both of these changes. There's also some lint
fixed.

== Tests ==
bin/test -cvv test_nascentupload TestOverrideDDEBs
bin/test -cvv test_test_uploadprocessor test_ddeb_upload_overrides

== Demo and Q/A ==
Upload some packages that create DDEBs in a PPA and check their overrides.

= Launchpad lint =

Ignore this bogus lint.

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/archiveuploader/nascentupload.py
  lib/lp/archiveuploader/tests/test_nascentupload.py
  lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug_1.0-1_i386.changes
  lib/lp/archiveuploader/tests/test_uploadprocessor.py

./lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug_1.0-1_i386.changes
      11: Line has trailing whitespace.
      13: Line has trailing whitespace.
      17: Line has trailing whitespace.
      20: Line has trailing whitespace.
      21: Line exceeds 78 characters.
      22: Line exceeds 78 characters.
      23: Line has trailing whitespace.
      25: Line exceeds 78 characters.

[Данило Шеган (danilo)]

The branch looks great.

I wonder if "overriding" component, section and priority means that ddebs will end up in the PPA as well? If they will, will it affect default allocated space to PPAs (i.e. some might hit the limit sooner, or might start hitting the limit once this is rolled out)?

review: Approve

[Данило Шеган (danilo)]

Btw, it would be nice to exclude .changes files from the linter (I hit that myself in my branch). Just saying...

=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py	2011-09-29 04:12:59 +0000
+++ lib/lp/archiveuploader/nascentupload.py	2011-10-20 12:15:28 +0000
@@ -178,6 +178,7 @@
             # before doing component verifications because the component
             # actually comes from overrides for packages that are not NEW.
             self.find_and_apply_overrides()
+            self._overrideDDEBSs()
 
         # Override archive location if necessary.
         self.overrideArchive()
@@ -365,9 +366,23 @@
                 "Orphaned debug packages: %s" % ', '.join(
                     '%s %s (%s)' % d for d in unmatched_ddebs))
 
+    def _overrideDDEBSs(self):
+        """Make sure that any DDEBs in the upload have the same overrides
+        as their counterpart DEBs.  This method needs to be called *after*
+        _matchDDEBS.
+
+        This is required so that domination can supersede both files in
+        lockstep.
+        """
+        for uploaded_file in self.changes.files:
+            if isinstance(uploaded_file, DdebBinaryUploadFile):
+                if uploaded_file.deb_file is not None:
+                    self._overrideBinaryFile(uploaded_file,
+                                             uploaded_file.deb_file)
     #
     # Helpers for warnings and rejections
     #
+
     def run_and_check_error(self, callable):
         """Run the given callable and process errors and warnings.
 
@@ -697,7 +712,9 @@
             override.binarypackagerelease.title,
             override.distroarchseries.architecturetag,
             override.pocket.name))
+        self._overrideBinaryFile(uploaded_file, override)
 
+    def _overrideBinaryFile(self, uploaded_file, override):
         uploaded_file.component_name = override.component.name
         uploaded_file.section_name = override.section.name
         # Both, changesfiles and nascentuploadfile local maps, reffer to
 
=== modified file 'lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug-bin-dbgsym_1.0-1_i386.ddeb'
 Binary files lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug-bin-dbgsym_1.0-1_i386.ddeb	2009-10-11 05:25:10 +0000 and lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug-bin-dbgsym_1.0-1_i386.ddeb	2011-10-20 12:15:28 +0000 differ
=== modified file 'lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug_1.0-1_i386.changes'
--- lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug_1.0-1_i386.changes	2009-10-11 05:25:10 +0000
+++ lib/lp/archiveuploader/tests/data/suite/debug_1.0-1/debug_1.0-1_i386.changes	2011-10-20 12:15:28 +0000
@@ -16,10 +16,10 @@
    * Testing debug binaries
 Checksums-Sha1: 
  91556113ad38eb35d2fe03d27ae646e0ed487a3d 654 debug-bin_1.0-1_i386.deb
- d120be63a4535e31ec08657cc4d3018cc8b62d54 668 debug-bin-dbgsym_1.0-1_i386.ddeb
+ 0dbf774a1c46ba6c115790d69466a8ccde9de2e4 648 debug-bin-dbgsym_1.0-1_i386.ddeb
 Checksums-Sha256: 
  5ca0ce3d3bfde3cc698d9ad4b027663abecb958bea641d29ac3bf8333fd3ebad 654 debug-bin_1.0-1_i386.deb
- 9c0013badb1bb78fa1ccbf1508398cbf51904f7242d8931f4994c1aa18519b63 668 debug-bin-dbgsym_1.0-1_i386.ddeb
+ 232cec57f6e35995d8715633ba1302d35a0ff9bb69d27399aa18f18b69c5acc0 648 debug-bin-dbgsym_1.0-1_i386.ddeb
 Files: 
  48b4eed5980c754cc74a06ffc4372f64 654 devel optional debug-bin_1.0-1_i386.deb
- ac5c44025ffc026485b22722e0f13804 668 devel optional debug-bin-dbgsym_1.0-1_i386.ddeb
+ 2491341743bc6466f83765eb67d42f0b 648 devel extra debug-bin-dbgsym_1.0-1_i386.ddeb
 
=== modified file 'lib/lp/archiveuploader/tests/test_nascentupload.py'
--- lib/lp/archiveuploader/tests/test_nascentupload.py	2010-12-22 01:08:48 +0000
+++ lib/lp/archiveuploader/tests/test_nascentupload.py	2011-10-20 12:15:28 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 from testtools import TestCase
+from testtools.matchers import MatchesStructure
 
 from canonical.testing.layers import LaunchpadZopelessLayer
 from lp.archiveuploader.changesfile import determine_file_class_and_name
@@ -32,13 +33,15 @@
         self.changes = FakeChangesFile()
         self.upload = NascentUpload(self.changes, None, DevNullLogger())
 
-    def addFile(self, filename):
+    def addFile(self, filename, comp_and_section='main/devel',
+                priority='extra'):
         """Add a file of the right type to the upload."""
         package, cls = determine_file_class_and_name(filename)
         file = cls(
-            filename, None, 100, 'devel', 'extra', package, '666',
+            filename, None, 100, comp_and_section, priority, package, '666',
             self.changes, None, self.upload.logger)
         self.changes.files.append(file)
+        return file
 
     def assertMatchDDEBErrors(self, error_list):
         self.assertEquals(
@@ -83,3 +86,20 @@
         self.addFile('libblah-dbgsym_1.0_amd64.ddeb')
         self.assertMatchDDEBErrors(
             ['Orphaned debug packages: libblah-dbgsym 666 (amd64)'])
+
+
+class TestOverrideDDEBs(TestMatchDDEBs):
+
+    def test_DDEBsGetOverrideFromDEBs(self):
+        # Test the basic case ensuring that DDEB files always match the
+        # DEB's overrides.
+        deb = self.addFile("foo_1.0_i386.deb", "main/devel", "extra")
+        ddeb = self.addFile(
+            "foo-dbgsym_1.0_i386.ddeb", "universe/web",  "low")
+        self.assertMatchDDEBErrors([])
+        self.upload._overrideDDEBSs()
+
+        self.assertThat(
+            ddeb,
+            MatchesStructure.fromExample(
+                deb, "component_name", "section_name", "priority_name"))
 
=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py	2011-09-29 06:59:03 +0000
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py	2011-10-20 12:15:28 +0000
@@ -32,8 +32,15 @@
 from canonical.launchpad.webapp.errorlog import ErrorReportingUtility
 from canonical.testing.layers import LaunchpadZopelessLayer
 from lp.app.errors import NotFoundError
+from lp.archiveuploader.nascentupload import NascentUpload
+from lp.archiveuploader.nascentuploadfile import DdebBinaryUploadFile
+from lp.archiveuploader.tests import (
+    datadir,
+    getPolicy,
+    )
 from lp.archiveuploader.uploadpolicy import (
     AbstractUploadPolicy,
+    ArchiveUploadType,
     findPolicyByName,
     IArchiveUploadPolicy,
     )
@@ -57,7 +64,10 @@
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.services.features.testing import FeatureFixture
-from lp.services.log.logger import BufferLogger
+from lp.services.log.logger import (
+    BufferLogger,
+    DevNullLogger,
+    )
 from lp.services.mail import stub
 from lp.soyuz.enums import (
     ArchivePermissionType,
@@ -174,7 +184,7 @@
         if builds is None:
             builds = self.options.builds
 
-        def getPolicy(distro, build):
+        def getUploadPolicy(distro, build):
             self.options.distro = distro.name
             policy = findPolicyByName(self.options.context)
             if builds:
@@ -186,8 +196,8 @@
 
         upload_processor = UploadProcessor(
             self.options.base_fsroot, self.options.dryrun,
-            self.options.nomails, builds, self.options.keep, getPolicy, txn,
-            self.log)
+            self.options.nomails, builds, self.options.keep, getUploadPolicy,
+            txn, self.log)
         self.switchToUploader()
         return upload_processor
 
@@ -1228,7 +1238,7 @@
         # Housekeeping so the next test won't fail.
         shutil.rmtree(upload_dir)
 
-    def disabled_per_bug_825486_testPartnerUploadToNonReleaseOrProposedPocket(self):
+    def disabled_testPartnerUploadToNonReleaseOrProposedPocket(self):
         # XXX: bug 825486 robertcollins 2011-08-13 this test is broken.
         """Test partner upload pockets.
 
@@ -1927,6 +1937,29 @@
         self.assertEqual(len(stub.test_emails), 0)
         self.assertNoNewOops(last_oops)
 
+    def test_ddeb_upload_overrides(self):
+        # DDEBs should always be overridden to the same values as their
+        # counterpart DEB's.
+        policy = getPolicy(
+            name="sync", distro="ubuntu", distroseries="hoary")
+        policy.accepted_type = ArchiveUploadType.BINARY_ONLY
+        uploader = NascentUpload.from_changesfile_path(
+            datadir("suite/debug_1.0-1/debug_1.0-1_i386.changes"),
+            policy, DevNullLogger())
+        uploader.process()
+
+        # The package data on disk that we just uploaded has a different
+        # priority setting between the deb and the ddeb. We can now assert
+        # that the process() method above overrode the ddeb.
+        for uploaded_file in uploader.changes.files:
+            if isinstance(uploaded_file, DdebBinaryUploadFile):
+                ddeb = uploaded_file
+            else:
+                deb = uploaded_file
+
+        self.assertEqual("optional", ddeb.priority_name)
+        self.assertEqual(deb.priority_name, ddeb.priority_name)
+
 
 class TestBuildUploadProcessor(TestUploadProcessorBase):
     """Test that processing build uploads works."""