Launchpad itself

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

  1. bug471148-ui
  2. Merge into db-devel
Proposed by Jelmer Vernooij
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merge reported by: Jelmer Vernooij
Merged at revision: not available
Proposed branch: lp:~jelmer/launchpad/bug471148-ui
Merge into: lp:launchpad/db-devel
Diff against target: 328 lines (+145/-14) (has conflicts)
9 files modified
database/schema/patch-2207-35-0.sql (+1/-1)
database/schema/security.cfg (+2/-2)
lib/lp/soyuz/browser/archive.py (+3/-2)
lib/lp/soyuz/configure.zcml (+4/-0)
lib/lp/soyuz/interfaces/archive.py (+3/-0)
lib/lp/soyuz/model/archive.py (+35/-0)
lib/lp/soyuz/tests/test_archive.py (+53/-0)
lib/lp/soyuz/tests/test_processor.py (+2/-0)
lib/lp/soyuz/tests/test_publishing.py (+42/-9)
Text conflict in lib/lp/soyuz/configure.zcml
To merge this branch: bzr merge lp:~jelmer/launchpad/bug471148-ui
Reviewer Review Type Date Requested Status
Michael Nelson (community) ui Approve
Abel Deuring (community) code Approve
Review via email: mp+20208@code.launchpad.net

Commit message

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

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

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.

Revision history for this message
Abel Deuring (adeuring) wrote :

(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)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== renamed file 'database/schema/patch-9999-24-0.sql' => 'database/schema/patch-2207-35-0.sql'
2--- database/schema/patch-9999-24-0.sql 2010-02-26 15:19:18 +0000
3+++ database/schema/patch-2207-35-0.sql 2010-02-26 15:19:22 +0000
4@@ -5,4 +5,4 @@
5
6 ALTER TABLE processorfamily ADD COLUMN restricted boolean DEFAULT FALSE NOT NULL;
7
8-INSERT INTO LaunchpadDatabaseRevision VALUES (9999, 24, 0);
9+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 35, 0);
10
11=== modified file 'database/schema/security.cfg'
12--- database/schema/security.cfg 2010-02-22 12:44:29 +0000
13+++ database/schema/security.cfg 2010-02-26 15:19:22 +0000
14@@ -125,7 +125,7 @@
15 public.archive = SELECT, INSERT, UPDATE
16 public.archiveauthtoken = SELECT, INSERT, UPDATE
17 public.archivesubscriber = SELECT, INSERT, UPDATE
18-public.archivearch = SELECT, INSERT, UPDATE
19+public.archivearch = SELECT, INSERT, UPDATE, DELETE
20 public.archivedependency = SELECT, INSERT, DELETE
21 public.archivepermission = SELECT, INSERT, UPDATE, DELETE
22 public.authtoken = SELECT, INSERT, UPDATE, DELETE
23@@ -916,7 +916,7 @@
24 # certain processes, such as the librarian tables. This group is deprecated -
25 # access should be explicitly granted to users.
26 public.archive = SELECT, INSERT, UPDATE
27-public.archivearch = SELECT, INSERT, UPDATE
28+public.archivearch = SELECT, INSERT, UPDATE, DELETE
29 public.binarypackagerelease = SELECT, INSERT, UPDATE
30 public.binarypackagefile = SELECT, INSERT, UPDATE
31 public.binarypackagefilepublishing = SELECT, INSERT, UPDATE
32
33=== modified file 'lib/lp/soyuz/browser/archive.py'
34--- lib/lp/soyuz/browser/archive.py 2010-02-25 16:49:16 +0000
35+++ lib/lp/soyuz/browser/archive.py 2010-02-26 15:19:22 +0000
36@@ -46,6 +46,7 @@
37 from canonical.launchpad import _
38 from canonical.launchpad.helpers import english_list
39 from canonical.lazr.utils import smartquote
40+from canonical.widgets import CheckBoxMatrixWidget
41 from lp.services.browser_helpers import get_user_agent_distroseries
42 from lp.soyuz.browser.build import BuildRecordsView
43 from lp.soyuz.browser.sourceslist import (
44@@ -1790,7 +1791,7 @@
45
46 field_names = ['enabled', 'private', 'require_virtualized',
47 'buildd_secret', 'authorized_size', 'relative_build_score',
48- 'external_dependencies']
49+ 'external_dependencies', 'arm_builds_allowed']
50
51 custom_widget('external_dependencies', TextAreaWidget, height=3)
52
53@@ -1817,7 +1818,7 @@
54 'Do not specify for non-private archives')
55
56 # Check the external_dependencies field.
57- ext_deps = data.get('external_dependencies')
58+ ext_deps = data.get('external_dependencies')
59 if ext_deps is not None:
60 errors = self.validate_external_dependencies(ext_deps)
61 if len(errors) != 0:
62
63=== modified file 'lib/lp/soyuz/configure.zcml'
64--- lib/lp/soyuz/configure.zcml 2010-02-25 22:41:23 +0000
65+++ lib/lp/soyuz/configure.zcml 2010-02-26 15:19:22 +0000
66@@ -400,7 +400,11 @@
67 <require
68 permission="launchpad.Edit"
69 interface="lp.soyuz.interfaces.archive.IArchiveEdit"
70+<<<<<<< TREE
71 set_attributes="description displayname publish"/>
72+=======
73+ set_attributes="description displayname arm_builds_allowed"/>
74+>>>>>>> MERGE-SOURCE
75 <require
76 permission="launchpad.Commercial"
77 set_attributes="authorized_size buildd_secret
78
79=== modified file 'lib/lp/soyuz/interfaces/archive.py'
80--- lib/lp/soyuz/interfaces/archive.py 2010-02-25 16:49:16 +0000
81+++ lib/lp/soyuz/interfaces/archive.py 2010-02-26 15:19:22 +0000
82@@ -1090,6 +1090,9 @@
83 def disable():
84 """Disable the archive."""
85
86+ arm_builds_allowed = Bool(
87+ title=_("Builds on ARM are allowed for this archive."))
88+
89
90 class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView):
91 """Main Archive interface."""
92
93=== modified file 'lib/lp/soyuz/model/archive.py'
94--- lib/lp/soyuz/model/archive.py 2010-02-18 17:05:50 +0000
95+++ lib/lp/soyuz/model/archive.py 2010-02-26 15:19:22 +0000
96@@ -59,6 +59,7 @@
97 IDistributionArchive, InvalidComponent, IPPA, MAIN_ARCHIVE_PURPOSES,
98 NoSuchPPA, PocketNotFound, VersionRequiresName, default_name_by_purpose)
99 from lp.soyuz.interfaces.archiveauthtoken import IArchiveAuthTokenSet
100+from lp.soyuz.interfaces.archivearch import IArchiveArchSet
101 from lp.soyuz.interfaces.archivepermission import (
102 ArchivePermissionType, IArchivePermissionSet)
103 from lp.soyuz.interfaces.archivesubscriber import (
104@@ -75,6 +76,7 @@
105 from lp.registry.interfaces.role import IHasOwner
106 from lp.soyuz.interfaces.queue import PackageUploadStatus
107 from lp.soyuz.interfaces.packagecopyrequest import IPackageCopyRequestSet
108+from lp.soyuz.interfaces.processor import IProcessorFamilySet
109 from lp.soyuz.interfaces.publishing import (
110 active_publishing_status, PackagePublishingStatus, IPublishingSet)
111 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
112@@ -182,6 +184,39 @@
113 external_dependencies = StringCol(
114 dbName='external_dependencies', notNull=False, default=None)
115
116+ def _get_arm_builds_enabled(self):
117+ """Check whether ARM builds are allowed for this archive."""
118+ archive_arch_set = getUtility(IArchiveArchSet)
119+ restricted_families = archive_arch_set.getRestrictedfamilies(self)
120+ arm = getUtility(IProcessorFamilySet).getByName('arm')
121+ for (family, archive_arch) in restricted_families:
122+ if family == arm:
123+ return (archive_arch is not None)
124+ # ARM doesn't exist or isn't restricted. Either way, there is no
125+ # need for an explicit association.
126+ return False
127+
128+ def _set_arm_builds_enabled(self, value):
129+ """Set whether ARM builds are enabled for this archive."""
130+ archive_arch_set = getUtility(IArchiveArchSet)
131+ restricted_families = archive_arch_set.getRestrictedfamilies(self)
132+ arm = getUtility(IProcessorFamilySet).getByName('arm')
133+ for (family, archive_arch) in restricted_families:
134+ if family == arm:
135+ if value:
136+ if archive_arch is not None:
137+ # ARM builds are already enabled
138+ return
139+ else:
140+ archive_arch_set.new(self, family)
141+ else:
142+ if archive_arch is not None:
143+ Store.of(self).remove(archive_arch)
144+ else:
145+ pass # ARM builds are already disabled
146+ arm_builds_allowed = property(_get_arm_builds_enabled,
147+ _set_arm_builds_enabled)
148+
149 def _init(self, *args, **kw):
150 """Provide the right interface for URL traversal."""
151 SQLBase._init(self, *args, **kw)
152
153=== modified file 'lib/lp/soyuz/tests/test_archive.py'
154--- lib/lp/soyuz/tests/test_archive.py 2010-02-08 16:33:12 +0000
155+++ lib/lp/soyuz/tests/test_archive.py 2010-02-26 15:19:22 +0000
156@@ -19,8 +19,10 @@
157 from lp.registry.interfaces.person import IPersonSet
158 from lp.services.job.interfaces.job import JobStatus
159 from lp.soyuz.interfaces.archive import IArchiveSet, ArchivePurpose
160+from lp.soyuz.interfaces.archivearch import IArchiveArchSet
161 from lp.soyuz.interfaces.binarypackagerelease import BinaryPackageFormat
162 from lp.soyuz.interfaces.build import BuildStatus
163+from lp.soyuz.interfaces.processor import IProcessorFamilySet
164 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
165 from lp.soyuz.model.build import Build
166 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
167@@ -558,5 +560,56 @@
168 self.assertEqual(1, len(pubs))
169 self.assertEqual('0.5.11~ppa1', pubs[0].source_package_version)
170
171+
172+class TestARMBuildsAllowed(TestCaseWithFactory):
173+ """Ensure that ARM builds can be allowed and disallowed correctly."""
174+
175+ layer = LaunchpadZopelessLayer
176+
177+ def setUp(self):
178+ """Setup an archive with relevant publications."""
179+ super(TestARMBuildsAllowed, self).setUp()
180+ self.publisher = SoyuzTestPublisher()
181+ self.publisher.prepareBreezyAutotest()
182+ self.archive = self.factory.makeArchive()
183+ self.archive_arch_set = getUtility(IArchiveArchSet)
184+ self.arm = getUtility(IProcessorFamilySet).getByName('arm')
185+
186+ def test_default(self):
187+ """By default, ARM builds are not allowed."""
188+ self.assertEquals(0,
189+ self.archive_arch_set.getByArchive(self.archive, self.arm).count())
190+ self.assertFalse(self.archive.arm_builds_allowed)
191+
192+ def test_get_uses_archivearch(self):
193+ """Adding an entry to ArchiveArch for ARM and an archive will
194+ enable arm_builds_allowed for that archive."""
195+ self.assertFalse(self.archive.arm_builds_allowed)
196+ self.archive_arch_set.new(self.archive, self.arm)
197+ self.assertTrue(self.archive.arm_builds_allowed)
198+
199+ def test_get_uses_arm_only(self):
200+ """Adding an entry to ArchiveArch for something other than ARM
201+ does not enable arm_builds_allowed for that archive."""
202+ self.assertFalse(self.archive.arm_builds_allowed)
203+ self.archive_arch_set.new(self.archive,
204+ getUtility(IProcessorFamilySet).getByName('amd64'))
205+ self.assertFalse(self.archive.arm_builds_allowed)
206+
207+ def test_set(self):
208+ """The property remembers its value correctly and sets ArchiveArch."""
209+ self.archive.arm_builds_allowed = True
210+ allowed_restricted_families = self.archive_arch_set.getByArchive(
211+ self.archive, self.arm)
212+ self.assertEquals(1, allowed_restricted_families.count())
213+ self.assertEquals(self.arm,
214+ allowed_restricted_families[0].processorfamily)
215+ self.assertTrue(self.archive.arm_builds_allowed)
216+ self.archive.arm_builds_allowed = False
217+ self.assertEquals(0,
218+ self.archive_arch_set.getByArchive(self.archive, self.arm).count())
219+ self.assertFalse(self.archive.arm_builds_allowed)
220+
221+
222 def test_suite():
223 return unittest.TestLoader().loadTestsFromName(__name__)
224
225=== modified file 'lib/lp/soyuz/tests/test_processor.py'
226--- lib/lp/soyuz/tests/test_processor.py 2010-02-26 15:19:18 +0000
227+++ lib/lp/soyuz/tests/test_processor.py 2010-02-26 15:19:22 +0000
228@@ -18,11 +18,13 @@
229 layer = LaunchpadZopelessLayer
230
231 def test_create(self):
232+ """Test adding a new ProcessorFamily."""
233 family = getUtility(IProcessorFamilySet).new("avr", "Atmel AVR",
234 "The Modified Harvard architecture 8-bit RISC processors.")
235 self.assertProvides(family, IProcessorFamily)
236
237 def test_add_processor(self):
238+ """Test adding a new Processor to a ProcessorFamily."""
239 family = getUtility(IProcessorFamilySet).new("avr", "Atmel AVR",
240 "The Modified Harvard architecture 8-bit RISC processors.")
241 proc = family.addProcessor("avr2001", "The 2001 AVR", "Fast as light.")
242
243=== modified file 'lib/lp/soyuz/tests/test_publishing.py'
244--- lib/lp/soyuz/tests/test_publishing.py 2010-02-26 15:19:18 +0000
245+++ lib/lp/soyuz/tests/test_publishing.py 2010-02-26 15:19:22 +0000
246@@ -883,8 +883,19 @@
247 self.sparc_distroarch = self.factory.makeDistroArchSeries(
248 architecturetag='sparc', processorfamily=self.sparc_family,
249 distroseries=self.distroseries, supports_virtualized=True)
250+ self.distroseries.nominatedarchindep = self.sparc_distroarch
251 self.addFakeChroots(self.distroseries)
252
253+ def getPubSource(self, architecturehintlist):
254+ """Return a mock source package publishing record for the archive
255+ and architecture used in this testcase.
256+
257+ :param architecturehintlist: Architecture hint list (e.g. "i386 amd64")
258+ """
259+ return super(BuildRecordCreationTests, self).getPubSource(
260+ archive=self.archive, distroseries=self.distroseries,
261+ architecturehintlist=architecturehintlist)
262+
263 def test__getAllowedArchitectures_restricted(self):
264 """Test _getAllowedArchitectures doesn't return unrestricted
265 archs.
266@@ -893,8 +904,7 @@
267 be used.
268 """
269 available_archs = [self.sparc_distroarch, self.avr_distroarch]
270- pubrec = self.getPubSource(distroseries=self.distroseries,
271- archive=self.archive, architecturehintlist='any')
272+ pubrec = self.getPubSource(architecturehintlist='any')
273 self.assertEquals([self.sparc_distroarch],
274 pubrec._getAllowedArchitectures(available_archs))
275
276@@ -906,22 +916,45 @@
277 """
278 available_archs = [self.sparc_distroarch, self.avr_distroarch]
279 getUtility(IArchiveArchSet).new(self.archive, self.avr_family)
280- pubrec = self.getPubSource(distroseries=self.distroseries,
281- archive=self.archive, architecturehintlist='any')
282+ pubrec = self.getPubSource(architecturehintlist='any')
283 self.assertEquals([self.sparc_distroarch, self.avr_distroarch],
284 pubrec._getAllowedArchitectures(available_archs))
285
286- def test_createMissingBuilds_restricts(self):
287- pubrec = self.getPubSource(distroseries=self.distroseries,
288- archive=self.archive, architecturehintlist='any')
289+ def test_createMissingBuilds_restricts_any(self):
290+ """createMissingBuilds() should limit builds targeted at 'any'
291+ architecture to those allowed for the archive.
292+ """
293+ pubrec = self.getPubSource(architecturehintlist='any')
294+ builds = pubrec.createMissingBuilds()
295+ self.assertEquals(1, len(builds))
296+ self.assertEquals(self.sparc_distroarch, builds[0].distroarchseries)
297+
298+ def test_createMissingBuilds_restricts_explicitlist(self):
299+ """createMissingBuilds() should limit builds targeted at a
300+ variety of architectures architecture to those allowed for the archive.
301+ """
302+ pubrec = self.getPubSource(architecturehintlist='sparc i386 avr')
303+ builds = pubrec.createMissingBuilds()
304+ self.assertEquals(1, len(builds))
305+ self.assertEquals(self.sparc_distroarch, builds[0].distroarchseries)
306+
307+ def test_createMissingBuilds_restricts_all(self):
308+ """createMissingBuilds() should limit builds targeted at 'all'
309+ architectures to the nominated independent architecture,
310+ if that is allowed for the archive.
311+ """
312+ pubrec = self.getPubSource(architecturehintlist='all')
313 builds = pubrec.createMissingBuilds()
314 self.assertEquals(1, len(builds))
315 self.assertEquals(self.sparc_distroarch, builds[0].distroarchseries)
316
317 def test_createMissingBuilds_restrict_override(self):
318+ """createMissingBuilds() should limit builds targeted at 'any'
319+ architecture to architectures that are unrestricted or
320+ explicitly associated with the archive.
321+ """
322 getUtility(IArchiveArchSet).new(self.archive, self.avr_family)
323- pubrec = self.getPubSource(distroseries=self.distroseries,
324- archive=self.archive, architecturehintlist='any')
325+ pubrec = self.getPubSource(architecturehintlist='any')
326 builds = pubrec.createMissingBuilds()
327 self.assertEquals(2, len(builds))
328 self.assertEquals(self.avr_distroarch, builds[0].distroarchseries)

Subscribers

People subscribed via source and target branches

to status/vote changes: