Merge lp:~wgrant/launchpad/per-archive-build-debug-symbols into lp:launchpad/db-devel

[William Grant]

Allow debug package creation to be toggled on a per-archive basis.

As part of PPA debug symbol support (bug #156575), we need to be able to switch debug package building on for just some archives. launchpad-buildd and pkg-create-dbgsym already have support for this, so we just need to pass 'build_debug_symbols' to the slave when we want them.

Eventually we probably want to allow PPA users to configure this themselves, but I think it's wise to restrict it to admins for now until we work out expiration policies some other stuff. So it's launchpad.Commercial, like the other similar attributes.

[Robert Collins (lifeless)]

Please add a column comment, other feedback given on IRC. With that change, bine by me - stub can allocate a db patch id for you.

review: Approve

[Stuart Bishop (stub)]

You forgot the NOT NULL clause in the ALTER TABLE statement, and a comment in comments.sql. Add these and you are good to go.

patch-2207-77-0.sql

review: Approve (db)

[Michael Hudson-Doyle (mwhudson)]

Code changes look fine to me.

review: Approve

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-08-04 00:03:20 +0000
+++ database/schema/comments.sql	2010-08-05 02:36:53 +0000
@@ -1992,6 +1992,7 @@
 COMMENT ON COLUMN Archive.relative_build_score IS 'A delta to the build score that is applied to all builds in this archive.';
 COMMENT ON COLUMN Archive.external_dependencies IS 'Newline-separated list of repositories to be used to retrieve any external build dependencies when building packages in this archive, in the format: deb http[s]://[user:pass@]<host>[/path] %(series)s[-pocket] [components]  The series variable is replaced with the series name of the context build.  This column is specifically and only intended for OEM migration to Launchpad and should be re-examined in October 2010 to see if it is still relevant.';
 COMMENT ON COLUMN Archive.commercial IS 'Whether this archive is a commercial Archive and should appear in the Software Center.';
+COMMENT ON COLUMN Archive.build_debug_symbols IS 'Whether builds for this archive should create debug symbol packages.';
 
 -- ArchiveAuthToken
 

=== added file 'database/schema/patch-2207-77-0.sql'
--- database/schema/patch-2207-77-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2207-77-0.sql	2010-08-05 02:36:53 +0000
@@ -0,0 +1,7 @@
+-- Copyright 2009 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+ALTER TABLE Archive
+    ADD COLUMN build_debug_symbols boolean NOT NULL DEFAULT false;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 77, 0);

=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
--- lib/lp/buildmaster/tests/test_manager.py	2010-07-28 09:56:24 +0000
+++ lib/lp/buildmaster/tests/test_manager.py	2010-08-05 02:36:53 +0000
@@ -648,6 +648,7 @@
                 'archive_purpose': 'PRIMARY',
                 'archives':
                 ['deb http://ftpmaster.internal/ubuntu hoary main'],
+                'build_debug_symbols': False,
                 'ogrecomponent': 'main',
                 'suite': u'hoary'}))],
             slave.calls, "Job was not properly dispatched.")
@@ -886,7 +887,7 @@
         polled a total of poll_repeat+1 times. The default values create a
         total poll time of 11 seconds. The BuilddManager logs
         "scanning cycles" every 5 seconds so these settings should see an
-        increase if the process is logging to this file. 
+        increase if the process is logging to this file.
     """
     last_size = None
     for poll in range(poll_repeat+1):

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2010-08-04 16:13:16 +0000
+++ lib/lp/soyuz/browser/archive.py	2010-08-05 02:36:53 +0000
@@ -919,7 +919,7 @@
                 'status': status_names[current_status.title],
                 'status_class': current_status.title,
                 'duration': duration,
-                'builds': builds
+                'builds': builds,
                 })
 
         return latest_updates_list
@@ -1871,8 +1871,8 @@
 class ArchiveAdminView(BaseArchiveEditView):
 
     field_names = ['enabled', 'private', 'commercial', 'require_virtualized',
-                   'buildd_secret', 'authorized_size', 'relative_build_score',
-                   'external_dependencies']
+                   'build_debug_symbols', 'buildd_secret', 'authorized_size',
+                   'relative_build_score', 'external_dependencies']
 
     custom_widget('external_dependencies', TextAreaWidget, height=3)
 
@@ -1917,7 +1917,7 @@
                 'Do not specify for non-private archives')
 
         # Check the external_dependencies field.
-        ext_deps =  data.get('external_dependencies')
+        ext_deps = data.get('external_dependencies')
         if ext_deps is not None:
             errors = self.validate_external_dependencies(ext_deps)
             if len(errors) != 0:

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2010-06-30 17:35:36 +0000
+++ lib/lp/soyuz/configure.zcml	2010-08-05 02:36:53 +0000
@@ -405,9 +405,9 @@
             set_attributes="description displayname publish status"/>
         <require
             permission="launchpad.Commercial"
-            set_attributes="authorized_size buildd_secret
-                            enabled_restricted_families
-                            commercial external_dependencies private
+            set_attributes="authorized_size build_debug_symbols buildd_secret
+                            commercial enabled_restricted_families
+                            external_dependencies private
                             require_virtualized relative_build_score "/>
         <require
             permission="launchpad.Admin"

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2010-08-03 08:49:19 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2010-08-05 02:36:53 +0000
@@ -265,6 +265,11 @@
             description=_("Whether this archive requires its packages to be "
                           "built on a virtual builder."), readonly=False))
 
+    build_debug_symbols = Bool(
+        title=_("Build debug symbols"), required=False,
+        description=_("Whether builds for this archive should create debug "
+                      "symbol packages."))
+
     authorized_size = Int(
         title=_("Authorized PPA size "), required=False,
         max=2 ** 31 - 1,
@@ -518,7 +523,7 @@
         """
 
     def checkUploadToPocket(distroseries, pocket):
-        """Check if uploading to a particular pocket in an archive is possible.
+        """Check if an upload to a particular archive and pocket is possible.
 
         :param distroseries: A `IDistroSeries`
         :param pocket: A `PackagePublishingPocket`
@@ -542,7 +547,7 @@
             vocabulary=DBEnumeratedType,
             required=True),
         strict_component=Bool(
-            title=_("Strict component"), required=False)
+            title=_("Strict component"), required=False),
         )
     @export_operation_as("checkUpload")
     @export_read_operation()
@@ -1136,6 +1141,7 @@
         :return: A new IArchiveAuthToken
         """
 
+
 class IArchiveAppend(Interface):
     """Archive interface for operations restricted by append privilege."""
 
@@ -1432,7 +1438,6 @@
         Only public and published sources are considered.
         """
 
-
     def new(purpose, owner, name=None, displayname=None, distribution=None,
             description=None, enabled=True, require_virtualized=True):
         """Create a new archive.
@@ -1446,8 +1451,9 @@
             given it uses the names defined in
             `IArchiveSet._getDefaultArchiveNameForPurpose`;
         :param displayname: optional text that will be used as a reference
-            to this archive in the UI. If not provided a default text (
-            including the archive name and the owner displayname will be used.)
+            to this archive in the UI. If not provided a default text
+            (including the archive name and the owner displayname)  will be
+            used.
         :param distribution: optional `IDistribution` to which the archive
             will be attached;
         :param description: optional text to be set as the archive
@@ -1661,7 +1667,6 @@
         """)
 
 
-
 default_name_by_purpose = {
     ArchivePurpose.PRIMARY: 'primary',
     ArchivePurpose.PPA: 'ppa',

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2010-08-04 09:39:04 +0000
+++ lib/lp/soyuz/model/archive.py	2010-08-05 02:36:53 +0000
@@ -173,6 +173,9 @@
     require_virtualized = BoolCol(
         dbName='require_virtualized', notNull=True, default=True)
 
+    build_debug_symbols = BoolCol(
+        dbName='build_debug_symbols', notNull=True, default=False)
+
     authorized_size = IntCol(
         dbName='authorized_size', notNull=False, default=2048)
 

=== modified file 'lib/lp/soyuz/model/binarypackagebuildbehavior.py'
--- lib/lp/soyuz/model/binarypackagebuildbehavior.py	2010-05-20 13:24:01 +0000
+++ lib/lp/soyuz/model/binarypackagebuildbehavior.py	2010-08-05 02:36:53 +0000
@@ -174,7 +174,7 @@
                          "(%s, %s)" % (
                             self._builder.url, file_name, url, sha1))
             self._builder.slave.sendFileToSlave(
-                sha1, url,  "buildd", archive.buildd_secret)
+                sha1, url, "buildd", archive.buildd_secret)
 
     def _extraBuildArgs(self, build):
         """
@@ -210,8 +210,7 @@
 
         args['archives'] = get_sources_list_for_building(build,
             build.distro_arch_series, build.source_package_release.name)
-
-        # Let the build slave know whether this is a build in a private
-        # archive.
         args['archive_private'] = build.archive.private
+        args['build_debug_symbols'] = build.archive.build_debug_symbols
+
         return args

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2010-08-03 19:10:42 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2010-08-05 02:36:53 +0000
@@ -36,7 +36,8 @@
 from lp.soyuz.model.binarypackagerelease import (
     BinaryPackageReleaseDownloadCount)
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
-from lp.testing import login, login_person, TestCaseWithFactory
+from lp.testing import ANONYMOUS, login, login_person, TestCaseWithFactory
+from lp.testing.sampledata import COMMERCIAL_ADMIN_EMAIL
 
 
 class TestGetPublicationsInArchive(TestCaseWithFactory):
@@ -1078,11 +1079,42 @@
             Unauthorized, self.setCommercial, self.archive, True)
 
         # Commercial admins can change it.
-        login("[e-mail]")
+        login(COMMERCIAL_ADMIN_EMAIL)
         self.setCommercial(self.archive, True)
         self.assertTrue(self.archive.commercial)
 
 
+class TestBuildDebugSymbols(TestCaseWithFactory):
+    """Tests relating to the build_debug_symbols flag."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBuildDebugSymbols, self).setUp()
+        self.archive = self.factory.makeArchive()
+
+    def setBuildDebugSymbols(self, archive, build_debug_symbols):
+        """Helper function."""
+        archive.build_debug_symbols = build_debug_symbols
+
+    def test_build_debug_symbols_is_public(self):
+        # Anyone can see the attribute.
+        login(ANONYMOUS)
+        self.assertFalse(self.archive.build_debug_symbols)
+
+    def test_owner_cannot_set_build_debug_symbols(self):
+        # The archive owner cannot set it.
+        login_person(self.archive.owner)
+        self.assertRaises(
+            Unauthorized, self.setBuildDebugSymbols, self.archive, True)
+
+    def test_commercial_admin_can_set_build_debug_symbols(self):
+        # A commercial admin can set it.
+        login(COMMERCIAL_ADMIN_EMAIL)
+        self.setBuildDebugSymbols(self.archive, True)
+        self.assertTrue(self.archive.build_debug_symbols)
+
+
 class TestFindDepCandidates(TestCaseWithFactory):
     """Tests for Archive.findDepCandidates."""
 

=== modified file 'lib/lp/testing/sampledata.py'
--- lib/lp/testing/sampledata.py	2010-08-02 15:36:19 +0000
+++ lib/lp/testing/sampledata.py	2010-08-05 02:36:53 +0000
@@ -11,6 +11,7 @@
 __all__ = [
     'BUILDD_ADMIN_USERNAME',
     'CHROOT_LIBRARYFILEALIAS',
+    'COMMERCIAL_ADMIN_EMAIL',
     'HOARY_DISTROSERIES_NAME',
     'I386_ARCHITECTURE_NAME',
     'LAUNCHPAD_DBUSER_NAME',
@@ -40,6 +41,7 @@
 LAUNCHPAD_DBUSER_NAME = 'launchpad'
 MAIN_COMPONENT_NAME = 'main'
 NO_PRIVILEGE_EMAIL = '[e-mail]'
+COMMERCIAL_ADMIN_EMAIL = '[e-mail]'
 # A user that is an admin of ubuntu-team, which has upload rights
 # to Ubuntu.
 UBUNTU_DEVELOPER_ADMIN_NAME = 'name16'