Merge lp:~jelmer/launchpad/bug471148-ui into lp:launchpad/db-devel

[Jelmer Vernooij]

Add a button in the archive admin form to allow or disallow builds on ARM.

[Jelmer Vernooij (jelmer)]

Add a button in the archive admin form to allow or disallow builds on ARM. This uses the previously added infrastructure (lp:~jelmer/launchpad/bug471148, lp:~al-maisan/launchpad/restricted-ui) that provide generic support for restricting architectures to certain PPA's.

Because of time constraints we have only added a checkbox for ARM in the UI for now, since there is an immediate need for that.

[Abel Deuring (adeuring)]

(14:46:44) adeuring: jelmer: could you add doc strings to _get_arm_builds_enabled() and _get_arm_builds_disabled()?
(14:47:36) adeuring: jelmer: sorry... i mean _set_arm_builds_enabled() instead of the "disabled"
(14:47:54) jelmer: adeuring: ok
(14:48:34) abentley [~<email address hidden>] hat den Raum betreten.
(14:49:15) adeuring: jelmer: there is an "elif" in set_arms_builds_enabled() without a final "else". Could you add such an "else: pass", together with a comment why nothing needs to be done in this case?
(14:51:04) adeuring: jelmer: and (sorry about all these nitpick for one method...): The comment "a link is required but it is present" is a bit enigmatic, at least for me. What about sonething like "Arms builds are already enabled"?
(14:51:41) jelmer: adeuring: Sure, that's more sensible indeed.

review: Approve (code)

[Michael Nelson (michael.nelson)]

So just an update of the label text for now (other tiny improvements possible and outlined below, but can be made later).

16:43 < jelmer> noodles: Could you perhaps do a quick UI review?
16:44 < noodles> yeah, I'm just trying to get the 7th branch in my pipe through too.
16:44 < jelmer> We've just added a checkbox
16:44 < noodles> Sure.
16:44 < jelmer> http://code.launchpad.net/~jelmer/launchpad/bug471148-ui is the code
16:44 < edbot> Bug 471148 is private
16:44 < jelmer> I'm uploading a screenshot
16:44 < noodles> Great.
16:45 < noodles> Have you got an MP ready? (You can make it a "work in progress" one).
16:46 < jelmer> https://code.edge.launchpad.net/~jelmer/launchpad/bug471148-ui/+merge/20208 is the mp
16:46 < edbot> Bug 471148 is private
16:46 < jelmer> abel has already reviewed the code
16:50 < noodles> jelmer: you guys are aware of the conflict right?
16:51 < noodles> oh, it's not in the diff...
16:51 < noodles> ah, yes it is :)
16:52 < noodles> Just a conflict with bigjools' configure changes from yesterday.
16:52 < jelmer> ah
16:52 < jelmer> I'll have a look
16:52 < jelmer> thanks
16:53 < bigjools> noodles: where are you seeing a conflict notified?
16:53 < noodles> bigjools: on jelmer's MP
16:53 < bigjools> oh - lol :)
16:53 < bigjools> thought it was db_lp
16:58 < jelmer> noodles: Muharem is uploading the screenshot, my launchpad instance is b0rked for some reason
16:59 < noodles> jelmer: I've merged it locally, so I'll run it here.
16:59 < noodles> But the screenshot will still be handy.
17:00 < al-maisan> noodles: here you go: https://devpad.canonical.com/~muharem/bug471148-2.png
17:00 < edbot> Bug 471148 is private
17:00 < noodles> Ta
17:03 < bigjools> fixed, thanks :)
17:04 < noodles> al-maisan, jelmer, wording-wise, I think "Allow ARM builds for this archive" (note: without a period) would be more consistent?
17:05 < al-maisan> hm .. hm .. OK
17:06 < noodles> If you've got other thoughts, just shout them out, I'm just comparing to the other checkboxes on that form (and looking for other examples).
17:07 * jelmer fix0rs
17:07 < noodles> And oh, would you mind cleaning up the capitalization on that form, there's lots of Headline Case for all the labels, that should be
                 sentence case (https://dev.launchpad.net/UserInterfaceWording)
17:07 < noodles> eg
17:07 < noodles> Require Virtualized Builder, Buildd Secret etc.
17:08 < noodles> (they probably have never been updated since we had the UIwording guidelines).
17:09 < noodles> But I'll understand if you don't want to risk test breakages etc. this late on Fri. afternoon.
17:09 < bigjools> hehe jelmer changed his gravatar again :)
17:09 < jelmer> bigjools: I just uploaded a bunch of images and gravatar makes it very easy to switch :-)
17:09 < bigjools> you need a floaty head!
17:09 < jelmer> noodles: Yeah, it'd be nice if we could just land this for now
17:09 < noodles> so ui=me just with the updated label for your checkbox.
17:09 < noodles> Yep.
17:10 < jelmer> noodles: I've made the wording change on the checkbox label
17:10 < noodles> Great.
17:10 < jelmer> noodles: Thanks!
17:10 < noodles> NP.

review: Approve (ui)

=== renamed file 'database/schema/patch-9999-24-0.sql' => 'database/schema/patch-2207-35-0.sql'
--- database/schema/patch-9999-24-0.sql	2010-02-26 15:19:18 +0000
+++ database/schema/patch-2207-35-0.sql	2010-02-26 15:19:22 +0000
@@ -5,4 +5,4 @@
 
 ALTER TABLE processorfamily ADD COLUMN restricted boolean DEFAULT FALSE NOT NULL;
 
-INSERT INTO LaunchpadDatabaseRevision VALUES (9999, 24, 0);
+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 35, 0);

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-02-22 12:44:29 +0000
+++ database/schema/security.cfg	2010-02-26 15:19:22 +0000
@@ -125,7 +125,7 @@
 public.archive                          = SELECT, INSERT, UPDATE
 public.archiveauthtoken                 = SELECT, INSERT, UPDATE
 public.archivesubscriber                = SELECT, INSERT, UPDATE
-public.archivearch                      = SELECT, INSERT, UPDATE
+public.archivearch                      = SELECT, INSERT, UPDATE, DELETE
 public.archivedependency                = SELECT, INSERT, DELETE
 public.archivepermission                = SELECT, INSERT, UPDATE, DELETE
 public.authtoken                        = SELECT, INSERT, UPDATE, DELETE
@@ -916,7 +916,7 @@
 # certain processes, such as the librarian tables. This group is deprecated -
 # access should be explicitly granted to users.
 public.archive                          = SELECT, INSERT, UPDATE
-public.archivearch                      = SELECT, INSERT, UPDATE
+public.archivearch                      = SELECT, INSERT, UPDATE, DELETE
 public.binarypackagerelease             = SELECT, INSERT, UPDATE
 public.binarypackagefile                = SELECT, INSERT, UPDATE
 public.binarypackagefilepublishing      = SELECT, INSERT, UPDATE

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2010-02-25 16:49:16 +0000
+++ lib/lp/soyuz/browser/archive.py	2010-02-26 15:19:22 +0000
@@ -46,6 +46,7 @@
 from canonical.launchpad import _
 from canonical.launchpad.helpers import english_list
 from canonical.lazr.utils import smartquote
+from canonical.widgets import CheckBoxMatrixWidget
 from lp.services.browser_helpers import get_user_agent_distroseries
 from lp.soyuz.browser.build import BuildRecordsView
 from lp.soyuz.browser.sourceslist import (
@@ -1790,7 +1791,7 @@
 
     field_names = ['enabled', 'private', 'require_virtualized',
                    'buildd_secret', 'authorized_size', 'relative_build_score',
-                   'external_dependencies']
+                   'external_dependencies', 'arm_builds_allowed']
 
     custom_widget('external_dependencies', TextAreaWidget, height=3)
 
@@ -1817,7 +1818,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-02-25 22:41:23 +0000
+++ lib/lp/soyuz/configure.zcml	2010-02-26 15:19:22 +0000
@@ -400,7 +400,11 @@
         <require
             permission="launchpad.Edit"
             interface="lp.soyuz.interfaces.archive.IArchiveEdit"
+<<<<<<< TREE
             set_attributes="description displayname publish"/>
+=======
+            set_attributes="description displayname arm_builds_allowed"/>
+>>>>>>> MERGE-SOURCE
         <require
             permission="launchpad.Commercial"
             set_attributes="authorized_size buildd_secret

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2010-02-25 16:49:16 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2010-02-26 15:19:22 +0000
@@ -1090,6 +1090,9 @@
     def disable():
         """Disable the archive."""
 
+    arm_builds_allowed = Bool(
+        title=_("Builds on ARM are allowed for this archive."))
+
 
 class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView):
     """Main Archive interface."""

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2010-02-18 17:05:50 +0000
+++ lib/lp/soyuz/model/archive.py	2010-02-26 15:19:22 +0000
@@ -59,6 +59,7 @@
     IDistributionArchive, InvalidComponent, IPPA, MAIN_ARCHIVE_PURPOSES,
     NoSuchPPA, PocketNotFound, VersionRequiresName, default_name_by_purpose)
 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
+from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.archivepermission import (
     ArchivePermissionType, IArchivePermissionSet)
 from lp.soyuz.interfaces.archivesubscriber import (
@@ -75,6 +76,7 @@
 from lp.registry.interfaces.role import IHasOwner
 from lp.soyuz.interfaces.queue import PackageUploadStatus
 from lp.soyuz.interfaces.packagecopyrequest import IPackageCopyRequestSet
+from lp.soyuz.interfaces.processor import IProcessorFamilySet
 from lp.soyuz.interfaces.publishing import (
     active_publishing_status, PackagePublishingStatus, IPublishingSet)
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
@@ -182,6 +184,39 @@
     external_dependencies = StringCol(
         dbName='external_dependencies', notNull=False, default=None)
 
+    def _get_arm_builds_enabled(self):
+        """Check whether ARM builds are allowed for this archive."""
+        archive_arch_set = getUtility(IArchiveArchSet)
+        restricted_families = archive_arch_set.getRestrictedfamilies(self)
+        arm = getUtility(IProcessorFamilySet).getByName('arm')
+        for (family, archive_arch) in restricted_families:
+            if family == arm:
+                return (archive_arch is not None)
+        # ARM doesn't exist or isn't restricted. Either way, there is no 
+        # need for an explicit association.
+        return False
+
+    def _set_arm_builds_enabled(self, value):
+        """Set whether ARM builds are enabled for this archive."""
+        archive_arch_set = getUtility(IArchiveArchSet)
+        restricted_families = archive_arch_set.getRestrictedfamilies(self)
+        arm = getUtility(IProcessorFamilySet).getByName('arm')
+        for (family, archive_arch) in restricted_families:
+            if family == arm:
+                if value:
+                    if archive_arch is not None:
+                        # ARM builds are already enabled
+                        return
+                    else:
+                        archive_arch_set.new(self, family)
+                else:
+                    if archive_arch is not None:
+                        Store.of(self).remove(archive_arch)
+                    else:
+                        pass # ARM builds are already disabled
+    arm_builds_allowed = property(_get_arm_builds_enabled,
+        _set_arm_builds_enabled)
+
     def _init(self, *args, **kw):
         """Provide the right interface for URL traversal."""
         SQLBase._init(self, *args, **kw)

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2010-02-08 16:33:12 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2010-02-26 15:19:22 +0000
@@ -19,8 +19,10 @@
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.job.interfaces.job import JobStatus
 from lp.soyuz.interfaces.archive import IArchiveSet, ArchivePurpose
+from lp.soyuz.interfaces.archivearch import IArchiveArchSet
 from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
 from lp.soyuz.interfaces.build import BuildStatus
+from lp.soyuz.interfaces.processor import IProcessorFamilySet
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 from lp.soyuz.model.build import Build
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
@@ -558,5 +560,56 @@
         self.assertEqual(1, len(pubs))
         self.assertEqual('0.5.11~ppa1', pubs[0].source_package_version)
 
+
+class TestARMBuildsAllowed(TestCaseWithFactory):
+    """Ensure that ARM builds can be allowed and disallowed correctly."""
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        """Setup an archive with relevant publications."""
+        super(TestARMBuildsAllowed, self).setUp()
+        self.publisher = SoyuzTestPublisher()
+        self.publisher.prepareBreezyAutotest()
+        self.archive = self.factory.makeArchive()
+        self.archive_arch_set = getUtility(IArchiveArchSet)
+        self.arm = getUtility(IProcessorFamilySet).getByName('arm')
+
+    def test_default(self):
+        """By default, ARM builds are not allowed."""
+        self.assertEquals(0, 
+            self.archive_arch_set.getByArchive(self.archive, self.arm).count())
+        self.assertFalse(self.archive.arm_builds_allowed)
+
+    def test_get_uses_archivearch(self):
+        """Adding an entry to ArchiveArch for ARM and an archive will 
+        enable arm_builds_allowed for that archive."""
+        self.assertFalse(self.archive.arm_builds_allowed)
+        self.archive_arch_set.new(self.archive, self.arm)
+        self.assertTrue(self.archive.arm_builds_allowed)
+
+    def test_get_uses_arm_only(self):
+        """Adding an entry to ArchiveArch for something other than ARM 
+        does not enable arm_builds_allowed for that archive."""
+        self.assertFalse(self.archive.arm_builds_allowed)
+        self.archive_arch_set.new(self.archive, 
+            getUtility(IProcessorFamilySet).getByName('amd64'))
+        self.assertFalse(self.archive.arm_builds_allowed)
+
+    def test_set(self):
+        """The property remembers its value correctly and sets ArchiveArch."""
+        self.archive.arm_builds_allowed = True
+        allowed_restricted_families = self.archive_arch_set.getByArchive(
+            self.archive, self.arm)
+        self.assertEquals(1, allowed_restricted_families.count())
+        self.assertEquals(self.arm,
+            allowed_restricted_families[0].processorfamily)
+        self.assertTrue(self.archive.arm_builds_allowed)
+        self.archive.arm_builds_allowed = False
+        self.assertEquals(0, 
+            self.archive_arch_set.getByArchive(self.archive, self.arm).count())
+        self.assertFalse(self.archive.arm_builds_allowed)
+
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/soyuz/tests/test_processor.py'
--- lib/lp/soyuz/tests/test_processor.py	2010-02-26 15:19:18 +0000
+++ lib/lp/soyuz/tests/test_processor.py	2010-02-26 15:19:22 +0000
@@ -18,11 +18,13 @@
     layer = LaunchpadZopelessLayer
 
     def test_create(self):
+        """Test adding a new ProcessorFamily."""
         family = getUtility(IProcessorFamilySet).new("avr", "Atmel AVR",
             "The Modified Harvard architecture 8-bit RISC processors.")
         self.assertProvides(family, IProcessorFamily)
 
     def test_add_processor(self):
+        """Test adding a new Processor to a ProcessorFamily."""
         family = getUtility(IProcessorFamilySet).new("avr", "Atmel AVR",
             "The Modified Harvard architecture 8-bit RISC processors.")
         proc = family.addProcessor("avr2001", "The 2001 AVR", "Fast as light.")

=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
--- lib/lp/soyuz/tests/test_publishing.py	2010-02-26 15:19:18 +0000
+++ lib/lp/soyuz/tests/test_publishing.py	2010-02-26 15:19:22 +0000
@@ -883,8 +883,19 @@
         self.sparc_distroarch = self.factory.makeDistroArchSeries(
             architecturetag='sparc', processorfamily=self.sparc_family,
             distroseries=self.distroseries, supports_virtualized=True)
+        self.distroseries.nominatedarchindep = self.sparc_distroarch
         self.addFakeChroots(self.distroseries)
 
+    def getPubSource(self, architecturehintlist):
+        """Return a mock source package publishing record for the archive 
+        and architecture used in this testcase.
+
+        :param architecturehintlist: Architecture hint list (e.g. "i386 amd64")
+        """
+        return super(BuildRecordCreationTests, self).getPubSource(
+            archive=self.archive, distroseries=self.distroseries,
+            architecturehintlist=architecturehintlist)
+
     def test__getAllowedArchitectures_restricted(self):
         """Test _getAllowedArchitectures doesn't return unrestricted
         archs.
@@ -893,8 +904,7 @@
         be used.
         """
         available_archs = [self.sparc_distroarch, self.avr_distroarch]
-        pubrec = self.getPubSource(distroseries=self.distroseries,
-            archive=self.archive, architecturehintlist='any')
+        pubrec = self.getPubSource(architecturehintlist='any')
         self.assertEquals([self.sparc_distroarch],
             pubrec._getAllowedArchitectures(available_archs))
 
@@ -906,22 +916,45 @@
         """
         available_archs = [self.sparc_distroarch, self.avr_distroarch]
         getUtility(IArchiveArchSet).new(self.archive, self.avr_family)
-        pubrec = self.getPubSource(distroseries=self.distroseries,
-            archive=self.archive, architecturehintlist='any')
+        pubrec = self.getPubSource(architecturehintlist='any')
         self.assertEquals([self.sparc_distroarch, self.avr_distroarch], 
             pubrec._getAllowedArchitectures(available_archs))
 
-    def test_createMissingBuilds_restricts(self):
-        pubrec = self.getPubSource(distroseries=self.distroseries,
-            archive=self.archive, architecturehintlist='any')
+    def test_createMissingBuilds_restricts_any(self):
+        """createMissingBuilds() should limit builds targeted at 'any'
+        architecture to those allowed for the archive.
+        """
+        pubrec = self.getPubSource(architecturehintlist='any')
+        builds = pubrec.createMissingBuilds()
+        self.assertEquals(1, len(builds))
+        self.assertEquals(self.sparc_distroarch, builds[0].distroarchseries)
+
+    def test_createMissingBuilds_restricts_explicitlist(self):
+        """createMissingBuilds() should limit builds targeted at a 
+        variety of architectures architecture to those allowed for the archive.
+        """
+        pubrec = self.getPubSource(architecturehintlist='sparc i386 avr')
+        builds = pubrec.createMissingBuilds()
+        self.assertEquals(1, len(builds))
+        self.assertEquals(self.sparc_distroarch, builds[0].distroarchseries)
+
+    def test_createMissingBuilds_restricts_all(self):
+        """createMissingBuilds() should limit builds targeted at 'all'
+        architectures to the nominated independent architecture, 
+        if that is allowed for the archive.
+        """
+        pubrec = self.getPubSource(architecturehintlist='all')
         builds = pubrec.createMissingBuilds()
         self.assertEquals(1, len(builds))
         self.assertEquals(self.sparc_distroarch, builds[0].distroarchseries)
 
     def test_createMissingBuilds_restrict_override(self):
+        """createMissingBuilds() should limit builds targeted at 'any'
+        architecture to architectures that are unrestricted or 
+        explicitly associated with the archive.
+        """
         getUtility(IArchiveArchSet).new(self.archive, self.avr_family)
-        pubrec = self.getPubSource(distroseries=self.distroseries,
-            archive=self.archive, architecturehintlist='any')
+        pubrec = self.getPubSource(architecturehintlist='any')
         builds = pubrec.createMissingBuilds()
         self.assertEquals(2, len(builds))
         self.assertEquals(self.avr_distroarch, builds[0].distroarchseries)