launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/gina-user-defined-fields into lp:launchpad.

Commit message:
Make gina import user-defined fields.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1456264 in Launchpad itself: "gina doesn't import user_defined_fields"
  https://bugs.launchpad.net/launchpad/+bug/1456264

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/gina-user-defined-fields/+merge/259602

Make gina import user-defined fields.

Ideally this would use much more of archiveuploader, but for now I at least arranged to use the sets of known fields encoded there.  We have to take account of Packages/Sources files having a few extra fields.

The name of the new set_field method doesn't fit LP style, but I was fitting in with the rest of the file in question.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/gina-user-defined-fields into lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/gina/handlers.py'
--- lib/lp/soyuz/scripts/gina/handlers.py	2014-10-31 06:37:14 +0000
+++ lib/lp/soyuz/scripts/gina/handlers.py	2015-05-20 11:49:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2014 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Gina db handlers.
@@ -606,6 +606,9 @@
         sectionID = self.distro_handler.ensureSection(src.section).id
         maintainer_line = "%s <%s>" % (displayname, emailaddress)
         name = self.ensureSourcePackageName(src.package)
+        kwargs = {}
+        if src._user_defined:
+            kwargs["user_defined_fields"] = src._user_defined
         spr = SourcePackageRelease(
             section=sectionID,
             creator=maintainer.id,
@@ -630,7 +633,8 @@
             dsc_maintainer_rfc822=maintainer_line,
             dsc_standards_version=src.standards_version,
             dsc_binaries=", ".join(src.binaries),
-            upload_archive=distroseries.main_archive)
+            upload_archive=distroseries.main_archive,
+            **kwargs)
         log.info('Source Package Release %s (%s) created' %
                  (name.name, src.version))
 
@@ -787,6 +791,9 @@
         build = self.ensureBuild(bin, srcpkg, distroarchseries, archtag)
 
         # Create the binarypackage entry on lp db.
+        kwargs = {}
+        if bin._user_defined:
+            kwargs["user_defined_fields"] = bin._user_defined
         binpkg = BinaryPackageRelease(
             binarypackagename=bin_name.id,
             component=componentID,
@@ -810,7 +817,7 @@
             essential=bin.essential,
             installedsize=bin.installed_size,
             architecturespecific=architecturespecific,
-            )
+            **kwargs)
         log.info('Binary Package Release %s (%s) created' %
                  (bin_name.name, bin.version))
 

=== modified file 'lib/lp/soyuz/scripts/gina/packages.py'
--- lib/lp/soyuz/scripts/gina/packages.py	2013-05-22 09:51:08 +0000
+++ lib/lp/soyuz/scripts/gina/packages.py	2015-05-20 11:49:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Package information classes.
@@ -31,6 +31,8 @@
 from lp.app.validators.version import valid_debian_version
 from lp.archivepublisher.diskpool import poolify
 from lp.archiveuploader.changesfile import ChangesFile
+from lp.archiveuploader.dscfile import DSCFile
+from lp.archiveuploader.nascentuploadfile import BaseBinaryUploadFile
 from lp.archiveuploader.utils import (
     DpkgSourceError,
     extract_dpkg_source,
@@ -210,6 +212,7 @@
     archive_root = None
     package = None
     _required = None
+    _user_defined = None
     version = None
 
     # Component is something of a special case. It is set up in
@@ -251,6 +254,20 @@
         self.date_uploaded = UTC_NOW
         return True
 
+    def set_field(self, key, value):
+        """Record an arbitrary control field."""
+        lowkey = key.lower()
+        # _known_fields contains the fields that archiveuploader recognises
+        # from a raw .dsc or .*deb; _required contains a few extra fields
+        # that are added to Sources and Packages index files.  If a field is
+        # in neither, it counts as user-defined.
+        if lowkey in self._known_fields or lowkey in self._required:
+            setattr(self, lowkey.replace("-", "_"), value)
+        else:
+            if self._user_defined is None:
+                self._user_defined = []
+            self._user_defined.append([key, value])
+
     def do_package(self, distro_name, archive_root):
         """To be provided by derived class."""
         raise NotImplementedError
@@ -286,6 +303,8 @@
         'component',
         ]
 
+    _known_fields = set(k.lower() for k in DSCFile.known_fields)
+
     def __init__(self, **args):
         for k, v in args.items():
             if k == 'Binary':
@@ -318,7 +337,7 @@
                 for f in files:
                     self.files.append(stripseq(f.split(" ")))
             else:
-                setattr(self, k.lower().replace("-", "_"), v)
+                self.set_field(k, v)
 
         if self.section is None:
             self.section = 'misc'
@@ -405,6 +424,8 @@
         'priority',
         ]
 
+    _known_fields = set(k.lower() for k in BaseBinaryUploadFile.known_fields)
+
     # Set in __init__
     source = None
     source_version = None
@@ -452,7 +473,7 @@
                     raise MissingRequiredArguments("Installed-Size is "
                         "not a valid integer: %r" % v)
             else:
-                setattr(self, k.lower().replace("-", "_"), v)
+                self.set_field(k, v)
 
         if self.source:
             # We need to handle cases like "Source: myspell

=== modified file 'lib/lp/soyuz/scripts/tests/test_gina.py'
--- lib/lp/soyuz/scripts/tests/test_gina.py	2013-10-18 04:33:58 +0000
+++ lib/lp/soyuz/scripts/tests/test_gina.py	2015-05-20 11:49:53 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from doctest import DocTestSuite
@@ -12,9 +12,11 @@
 from lp.archiveuploader.tagfiles import parse_tagfile
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
+from lp.services.database.constants import UTC_NOW
 from lp.services.features.testing import FeatureFixture
 from lp.services.log.logger import DevNullLogger
 from lp.services.tarfile_helpers import LaunchpadWriteTarFile
+from lp.services.osutils import write_file
 from lp.soyuz.enums import PackagePublishingStatus
 from lp.soyuz.scripts.gina import ExecutionError
 from lp.soyuz.scripts.gina.archive import (
@@ -24,8 +26,10 @@
 from lp.soyuz.scripts.gina.dominate import dominate_imported_source_packages
 import lp.soyuz.scripts.gina.handlers
 from lp.soyuz.scripts.gina.handlers import (
+    BinaryPackageHandler,
     BinaryPackagePublisher,
     ImporterHandler,
+    SourcePackageHandler,
     SourcePackagePublisher,
     )
 from lp.soyuz.scripts.gina.packages import (
@@ -223,6 +227,49 @@
         sp_data.do_package("debian", archive_root)
 
 
+class TestSourcePackageHandler(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def test_user_defined_fields(self):
+        series = self.factory.makeDistroSeries()
+        archive_root = self.useTempDir()
+        sphandler = SourcePackageHandler(
+            series.distribution.name, archive_root,
+            PackagePublishingPocket.RELEASE, None)
+        dsc_contents = {
+            "Format": "3.0 (quilt)",
+            "Source": "foo",
+            "Binary": "foo",
+            "Architecture": "all arm64",
+            "Version": "1.0-1",
+            "Maintainer": "Foo Bar <foo@xxxxxxxxxxxxx>",
+            "Files": "xxx 000 foo_1.0-1.dsc",
+            "Build-Indep-Architecture": "amd64",
+            "Directory": "pool/main/f/foo",
+            "Package": "foo",
+            "Component": "main",
+            "Section": "misc",
+            }
+        sp_data = SourcePackageData(**dsc_contents)
+        self.assertEqual(
+            [["Build-Indep-Architecture", "amd64"]], sp_data._user_defined)
+        sp_data.archive_root = archive_root
+        sp_data.dsc = ""
+        sp_data.copyright = ""
+        sp_data.urgency = "low"
+        sp_data.changelog = None
+        sp_data.changelog_entry = None
+        sp_data.date_uploaded = UTC_NOW
+        # We don't need a real .dsc here.
+        write_file(
+            os.path.join(archive_root, "pool/main/f/foo/foo_1.0-1.dsc"), "x")
+        spr = sphandler.createSourcePackageRelease(sp_data, series)
+        self.assertIsNotNone(spr)
+        self.assertEqual(
+            [["Build-Indep-Architecture", "amd64"]], spr.user_defined_fields)
+
+
 class TestSourcePackagePublisher(TestCaseWithFactory):
 
     layer = ZopelessDatabaseLayer
@@ -244,6 +291,48 @@
         self.assertEqual(PackagePublishingStatus.PUBLISHED, spph.status)
 
 
+class TestBinaryPackageHandler(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def test_user_defined_fields(self):
+        das = self.factory.makeDistroArchSeries()
+        archive_root = self.useTempDir()
+        sphandler = SourcePackageHandler(
+            das.distroseries.distribution.name, archive_root,
+            PackagePublishingPocket.RELEASE, None)
+        bphandler = BinaryPackageHandler(
+            sphandler, archive_root, PackagePublishingPocket.RELEASE)
+        spr = self.factory.makeSourcePackageRelease(
+            distroseries=das.distroseries)
+        deb_contents = {
+            "Package": "foo",
+            "Installed-Size": "0",
+            "Maintainer": "Foo Bar <foo@xxxxxxxxxxxxx>",
+            "Section": "misc",
+            "Architecture": "amd64",
+            "Version": "1.0-1",
+            "Filename": "pool/main/f/foo/foo_1.0-1_amd64.deb",
+            "Component": "main",
+            "Size": "0",
+            "MD5sum": "0" * 32,
+            "Description": "",
+            "Summary": "",
+            "Priority": "extra",
+            "Python-Version": "2.7",
+            }
+        bp_data = BinaryPackageData(**deb_contents)
+        self.assertEqual([["Python-Version", "2.7"]], bp_data._user_defined)
+        bp_data.archive_root = archive_root
+        # We don't need a real .deb here.
+        write_file(
+            os.path.join(archive_root, "pool/main/f/foo/foo_1.0-1_amd64.deb"),
+            "x")
+        bpr = bphandler.createBinaryPackage(bp_data, spr, das, "amd64")
+        self.assertIsNotNone(bpr)
+        self.assertEqual([["Python-Version", "2.7"]], bpr.user_defined_fields)
+
+
 class TestBinaryPackagePublisher(TestCaseWithFactory):
 
     layer = ZopelessDatabaseLayer