launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00176
[Merge] lp:~rockstar/launchpad/unsupported-series-recipe-api into lp:launchpad/devel
Paul Hummer has proposed merging lp:~rockstar/launchpad/unsupported-series-recipe-api into lp:launchpad/devel with lp:~rockstar/launchpad/create-recipe-error-messages as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#607125 Forbid creation of recipe builds for obsolete series through the API
https://bugs.launchpad.net/bugs/607125
This branch fixes bug #607125 - Yesterday, we had a user request builds for distroserieses that we don't have chroots for anymore using the API. At least, we SUSPECT that it was through the API, since the UI prevents this.
Going on the theory that it was through the API and that it's not possible to do through the UI, I took the code that helps the UI show the correct set of distroserieses and moved it to lp.code.model.sourcepackagerecipe. I also moved the vocabularies into the (already existing) lp.code.vocabularies.sourcepackagerecipe.
After everything was moved around, I made it so that ISourcePackageRecipe.requestBuild checked the distroseries to make sure it was valid. I ran the tests there, and found a few breakages. While fixing these breakages, I found the unfortunate case where I needed to use sample data on Ubuntu Hardy and Warty because makeArchive automatically makes the Archive use Ubuntu. After talking to jml about it, I decided to make a specific function for this need so that when we entirely get rid of sampledata, it's a little plainer what's going on.
Once the tests were passing again, I wrote some a webservice test to stop that vector.
If you see random oddness in my branch, it's because I took the liberty of cleaning up some lint I found in 'make lint.' Make lint is also a little brain-dead in thinking that you can't have a single item tuple have a comma without whitespace. I didn't fix those, because I thought that rule was stupid... I'm happy to fix it if you're that hung up on it.
--
https://code.launchpad.net/~rockstar/launchpad/unsupported-series-recipe-api/+merge/30397
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rockstar/launchpad/unsupported-series-recipe-api into lp:launchpad/devel.
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2010-07-14 10:09:35 +0000
+++ lib/lp/code/browser/configure.zcml 2010-07-20 12:55:27 +0000
@@ -1201,14 +1201,14 @@
</facet>
<securedutility
name="BuildableDistroSeries"
- component="lp.code.browser.sourcepackagerecipe.buildable_distroseries_vocabulary"
+ component="lp.code.vocabularies.sourcepackagerecipe.buildable_distroseries_vocabulary"
provides="zope.schema.interfaces.IVocabularyFactory"
>
<allow interface="zope.schema.interfaces.IVocabularyFactory"/>
</securedutility>
<securedutility
name="TargetPPAs"
- component="lp.code.browser.sourcepackagerecipe.target_ppas_vocabulary"
+ component="lp.code.vocabularies.sourcepackagerecipe.target_ppas_vocabulary"
provides="zope.schema.interfaces.IVocabularyFactory"
>
<allow interface="zope.schema.interfaces.IVocabularyFactory"/>
=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2010-07-19 10:31:16 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2010-07-20 12:55:27 +0000
@@ -24,18 +24,14 @@
from zope.event import notify
from zope.interface import implements, Interface
from zope.schema import Choice, List, Text
-from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
from canonical.database.constants import UTC_NOW
from canonical.launchpad.browser.launchpad import Hierarchy
-from canonical.launchpad.interfaces import ILaunchBag
from canonical.launchpad.webapp import (
action, canonical_url, ContextMenu, custom_widget,
enabled_with_permission, LaunchpadEditFormView, LaunchpadFormView,
LaunchpadView, Link, Navigation, NavigationMenu, stepthrough, structured)
-from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.breadcrumb import Breadcrumb
-from canonical.launchpad.webapp.sorting import sorted_dotted_numbers
from canonical.widgets.itemswidgets import LabeledMultiCheckBoxWidget
from lp.code.errors import ForbiddenInstruction
from lp.code.errors import BuildAlreadyPending
@@ -44,10 +40,6 @@
ISourcePackageRecipe, ISourcePackageRecipeSource, MINIMAL_RECIPE_TEXT)
from lp.code.interfaces.sourcepackagerecipebuild import (
ISourcePackageRecipeBuildSource)
-from lp.soyuz.browser.archive import make_archive_vocabulary
-from lp.soyuz.interfaces.archive import (
- IArchiveSet)
-from lp.registry.interfaces.distroseries import IDistroSeriesSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
RECIPE_BETA_MESSAGE = structured(
@@ -148,8 +140,8 @@
"""Default view of a SourcePackageRecipe."""
def initialize(self):
- # XXX: rockstar: This should be removed when source package recipes are
- # put into production. spec=sourcepackagerecipes
+ # XXX: rockstar: This should be removed when source package recipes
+ # are put into production. spec=sourcepackagerecipes
super(SourcePackageRecipeView, self).initialize()
self.request.response.addWarningNotification(RECIPE_BETA_MESSAGE)
@@ -177,28 +169,6 @@
return builds
-def buildable_distroseries_vocabulary(context):
- """Return a vocabulary of buildable distroseries."""
- ppas = getUtility(IArchiveSet).getPPAsForUser(getUtility(ILaunchBag).user)
- supported_distros = [ppa.distribution for ppa in ppas]
- dsset = getUtility(IDistroSeriesSet).search()
- terms = sorted_dotted_numbers(
- [SimpleTerm(distro, distro.id, distro.displayname)
- for distro in dsset if (
- distro.active and distro.distribution in supported_distros)],
- key=lambda term: term.value.version)
- terms.reverse()
- return SimpleVocabulary(terms)
-
-
-def target_ppas_vocabulary(context):
- """Return a vocabulary of ppas that the current user can target."""
- ppas = getUtility(IArchiveSet).getPPAsForUser(getUtility(ILaunchBag).user)
- return make_archive_vocabulary(
- ppa for ppa in ppas
- if check_permission('launchpad.Append', ppa))
-
-
class SourcePackageRecipeRequestBuildsView(LaunchpadFormView):
"""A view for requesting builds of a SourcePackageRecipe."""
@@ -261,7 +231,6 @@
self.next_url = self.cancel_url
-
class ISourcePackageAddEditSchema(Interface):
"""Schema for adding or editing a recipe."""
@@ -281,7 +250,6 @@
description=u'The text of the recipe.')
-
class RecipeTextValidatorMixin:
"""Class to validate that the Source Package Recipe text is valid."""
@@ -310,8 +278,8 @@
custom_widget('distros', LabeledMultiCheckBoxWidget)
def initialize(self):
- # XXX: rockstar: This should be removed when source package recipes are
- # put into production. spec=sourcepackagerecipes
+ # XXX: rockstar: This should be removed when source package recipes
+ # are put into production. spec=sourcepackagerecipes
super(SourcePackageRecipeAddView, self).initialize()
self.request.response.addWarningNotification(RECIPE_BETA_MESSAGE)
=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py 2010-06-17 20:45:58 +0000
+++ lib/lp/code/errors.py 2010-07-20 12:55:27 +0000
@@ -8,6 +8,7 @@
'BadBranchMergeProposalSearchContext',
'BadStateTransition',
'BuildAlreadyPending',
+ 'BuildNotAllowedForDistro',
'BranchMergeProposalExists',
'CodeImportAlreadyRequested',
'CodeImportAlreadyRunning',
@@ -144,3 +145,14 @@
RecipeBuildException.__init__(
self, recipe, distroseries,
'An identical build of this recipe is already pending.')
+
+
+class BuildNotAllowedForDistro(RecipeBuildException):
+ """A build was requested against an unsupported distroseries."""
+
+ webservice_error(400)
+
+ def __init__(self, recipe, distroseries):
+ RecipeBuildException.__init__(
+ self, recipe, distroseries,
+ 'A build against this distro is not allowed.')
=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py 2010-07-19 15:32:21 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py 2010-07-20 12:55:27 +0000
@@ -25,7 +25,8 @@
from canonical.launchpad.interfaces.lpstorm import IMasterStore, IStore
from lp.buildmaster.interfaces.buildbase import BuildStatus
-from lp.code.errors import BuildAlreadyPending, TooManyBuilds
+from lp.code.errors import (BuildAlreadyPending, BuildNotAllowedForDistro,
+ TooManyBuilds)
from lp.code.interfaces.sourcepackagerecipe import (
ISourcePackageRecipe, ISourcePackageRecipeSource,
ISourcePackageRecipeData)
@@ -33,11 +34,24 @@
ISourcePackageRecipeBuildSource)
from lp.code.model.sourcepackagerecipebuild import SourcePackageRecipeBuild
from lp.code.model.sourcepackagerecipedata import SourcePackageRecipeData
+from lp.registry.interfaces.distroseries import IDistroSeriesSet
from lp.registry.model.distroseries import DistroSeries
-from lp.soyuz.interfaces.archive import ArchivePurpose
+from lp.soyuz.interfaces.archive import ArchivePurpose, IArchiveSet
from lp.soyuz.interfaces.component import IComponentSet
+def get_buildable_distroseries_set(user):
+ ppas = getUtility(IArchiveSet).getPPAsForUser(user)
+ supported_distros = [ppa.distribution for ppa in ppas]
+ distros = getUtility(IDistroSeriesSet).search()
+
+ buildables = []
+ for distro in distros:
+ if distro.active and distro.distribution in supported_distros:
+ buildables.append(distro)
+ return buildables
+
+
class NonPPABuildRequest(Exception):
"""A build was requested to a non-PPA and this is currently
unsupported."""
@@ -188,6 +202,13 @@
if archive.purpose != ArchivePurpose.PPA:
raise NonPPABuildRequest
component = getUtility(IComponentSet)["multiverse"]
+
+ from lp.code.vocabularies.sourcepackagerecipe import (
+ get_buildable_distroseries_set)
+ buildable_distros = get_buildable_distroseries_set(archive.owner)
+ if distroseries not in buildable_distros:
+ raise BuildNotAllowedForDistro(self, distroseries)
+
reject_reason = archive.checkUpload(
requester, self.distroseries, None, component, pocket)
if reject_reason is not None:
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-06-18 19:36:55 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-07-20 12:55:27 +0000
@@ -45,7 +45,6 @@
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.job.interfaces.job import (
IJob, JobStatus)
-from lp.soyuz.model.processor import ProcessorFamily
from lp.testing import (
ANONYMOUS, launchpadlib_for, login, login_person, person_logged_in,
TestCaseWithFactory, ws_object)
@@ -167,7 +166,6 @@
branch2 = self.factory.makeAnyBranch()
builder_recipe2 = self.factory.makeRecipe(branch2)
login_person(sp_recipe.owner.teamowner)
- #import pdb; pdb.set_trace()
sp_recipe.builder_recipe = builder_recipe2
self.assertEquals([branch2], list(sp_recipe.getReferencedBranches()))
@@ -297,6 +295,7 @@
name=u'myrecipe', owner=requester)
series = list(recipe.distroseries)[0]
archive = self.factory.makeArchive(owner=requester)
+
def request_build():
build = recipe.requestBuild(archive, requester, series,
PackagePublishingPocket.RELEASE)
@@ -322,10 +321,7 @@
self.factory.makeArchive(owner=recipe.owner), recipe.owner,
series, PackagePublishingPocket.RELEASE)
# Varying distroseries allows build.
- new_distroseries = self.factory.makeDistroSeries()
- new_distroseries.nominatedarchindep = new_distroseries.newArch(
- 'i386', ProcessorFamily.get(1), False, recipe.owner,
- supports_virtualized=True)
+ new_distroseries = archive.distribution.getSeries('hoary')
recipe.requestBuild(archive, recipe.owner,
new_distroseries, PackagePublishingPocket.RELEASE)
# Changing status of old build allows new build.
@@ -430,6 +426,7 @@
build.buildduration = timedelta(minutes=10)
self.assertEqual(
timedelta(minutes=10), recipe.getMedianBuildDuration())
+
def addBuild(minutes):
build = removeSecurityProxy(
self.factory.makeSourcePackageRecipeBuild(recipe=recipe))
@@ -691,11 +688,7 @@
"""Build requests can be performed."""
person = self.factory.makePerson()
archive = self.factory.makeArchive(owner=person)
- distroseries = self.factory.makeDistroSeries()
- distroseries_i386 = distroseries.newArch(
- 'i386', ProcessorFamily.get(1), False, person,
- supports_virtualized=True)
- distroseries.nominatedarchindep = distroseries_i386
+ distroseries = self.factory.makeSourcePackageRecipeDistroseries()
recipe, user, launchpad = self.makeRecipe(person)
distroseries = ws_object(launchpad, distroseries)
@@ -708,11 +701,7 @@
"""Build requests are rejected if already pending."""
person = self.factory.makePerson()
archive = self.factory.makeArchive(owner=person)
- distroseries = self.factory.makeDistroSeries()
- distroseries_i386 = distroseries.newArch(
- 'i386', ProcessorFamily.get(1), False, person,
- supports_virtualized=True)
- distroseries.nominatedarchindep = distroseries_i386
+ distroseries = self.factory.makeSourcePackageRecipeDistroseries()
recipe, user, launchpad = self.makeRecipe(person)
distroseries = ws_object(launchpad, distroseries)
@@ -729,11 +718,7 @@
"""Build requests are rejected if they exceed quota."""
person = self.factory.makePerson()
archives = [self.factory.makeArchive(owner=person) for x in range(6)]
- distroseries = self.factory.makeDistroSeries()
- distroseries_i386 = distroseries.newArch(
- 'i386', ProcessorFamily.get(1), False, person,
- supports_virtualized=True)
- distroseries.nominatedarchindep = distroseries_i386
+ distroseries = self.factory.makeSourcePackageRecipeDistroseries()
recipe, user, launchpad = self.makeRecipe(person)
distroseries = ws_object(launchpad, distroseries)
@@ -749,6 +734,21 @@
pocket=PackagePublishingPocket.RELEASE.title)
self.assertIn('TooManyBuilds', str(e))
+ def test_requestBuildRejectUnsupportedDistroSeries(self):
+ """Build requests are rejected if they exceed quota."""
+ person = self.factory.makePerson()
+ archives = [self.factory.makeArchive(owner=person) for x in range(6)]
+ distroseries = self.factory.makeDistroSeries()
+
+ recipe, user, launchpad = self.makeRecipe(person)
+ distroseries = ws_object(launchpad, distroseries)
+ archive = ws_object(launchpad, archives[-1])
+
+ e = self.assertRaises(Exception, recipe.requestBuild,
+ archive=archive, distroseries=distroseries,
+ pocket=PackagePublishingPocket.RELEASE.title)
+ self.assertIn('BuildNotAllowedForDistro', str(e))
+
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
=== removed file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py.moved'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py.moved 2010-03-26 19:29:56 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py.moved 1970-01-01 00:00:00 +0000
@@ -1,40 +0,0 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-
-__metaclass__ = type
-
-
-import unittest
-
-from canonical.testing import DatabaseFunctionalLayer
-from lp.testing import login_person, TestCaseWithFactory
-
-
-class TestSourcePackageRecipe(TestCaseWithFactory):
-
- layer = DatabaseFunctionalLayer
-
- def test_distroseries(self):
- """Test that the distroseries behaves as a set."""
- recipe = self.factory.makeSourcePackageRecipe()
- distroseries = self.factory.makeDistroSeries()
- (old_distroseries,) = recipe.distroseries
- recipe.distroseries.add(distroseries)
- self.assertEqual(
- set([distroseries, old_distroseries]), set(recipe.distroseries))
- recipe.distroseries.remove(distroseries)
- self.assertEqual([old_distroseries], list(recipe.distroseries))
- recipe.distroseries.clear()
- self.assertEqual([], list(recipe.distroseries))
-
- def test_build_daily(self):
- """Test that build_daily behaves as a bool."""
- recipe = self.factory.makeSourcePackageRecipe()
- self.assertFalse(recipe.build_daily)
- login_person(recipe.owner)
- recipe.build_daily = True
- self.assertTrue(recipe.build_daily)
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-07-14 08:42:01 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-07-20 12:55:27 +0000
@@ -249,10 +249,8 @@
recipe.requestBuild(
recipe.daily_build_archive, recipe.owner, first_distroseries,
PackagePublishingPocket.RELEASE)
- second_distroseries = self.factory.makeDistroSeries()
- second_distroseries.nominatedarchindep = second_distroseries.newArch(
- 'i386', ProcessorFamily.get(1), False, self.factory.makePerson(),
- supports_virtualized=True)
+ second_distroseries = \
+ self.factory.makeSourcePackageRecipeDistroseries("hoary")
recipe.distroseries.add(second_distroseries)
builds = SourcePackageRecipeBuild.makeDailyBuilds()
self.assertEqual(
@@ -274,6 +272,7 @@
recipe=recipe, distroseries=series)
self.factory.makeSourcePackageRecipeBuild(
requester=requester, distroseries=series)
+
def get_recent():
Store.of(build).flush()
return SourcePackageRecipeBuild.getRecentBuilds(
@@ -332,11 +331,11 @@
body, footer = message.get_payload(decode=True).split('\n-- \n')
self.assertEqual(
'Build person/recipe into ppa for distroseries: Successfully'
- ' built.\n', body
- )
+ ' built.\n', body)
def test_handleStatusNotifies(self):
""""handleStatus causes notification, even if OK."""
+
def prepare_build():
queue_record = self.factory.makeSourcePackageRecipeBuildJob()
build = queue_record.specific_job.build
@@ -345,6 +344,7 @@
slave = WaitingSlave('BuildStatus.OK')
queue_record.builder.setSlaveForTesting(slave)
return build
+
def assertNotifyOnce(status, build):
build.handleStatus(status, None, {'filemap': {}})
self.assertEqual(1, len(pop_notifications()))
=== added file 'lib/lp/code/vocabularies/sourcepackagerecipe.py'
--- lib/lp/code/vocabularies/sourcepackagerecipe.py 1970-01-01 00:00:00 +0000
+++ lib/lp/code/vocabularies/sourcepackagerecipe.py 2010-07-20 12:55:27 +0000
@@ -0,0 +1,32 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Source Package Recipe vocabularies used in the lp/code modules."""
+from zope.component import getUtility
+from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
+
+from canonical.launchpad.interfaces import ILaunchBag
+from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.sorting import sorted_dotted_numbers
+from lp.code.model.sourcepackagerecipe import get_buildable_distroseries_set
+from lp.soyuz.browser.archive import make_archive_vocabulary
+from lp.soyuz.interfaces.archive import IArchiveSet
+
+
+def buildable_distroseries_vocabulary(context):
+ """Return a vocabulary of buildable distroseries."""
+ distros = get_buildable_distroseries_set(getUtility(ILaunchBag).user)
+ terms = sorted_dotted_numbers(
+ [SimpleTerm(distro, distro.id, distro.displayname)
+ for distro in distros],
+ key=lambda term: term.value.version)
+ terms.reverse()
+ return SimpleVocabulary(terms)
+
+
+def target_ppas_vocabulary(context):
+ """Return a vocabulary of ppas that the current user can target."""
+ ppas = getUtility(IArchiveSet).getPPAsForUser(getUtility(ILaunchBag).user)
+ return make_archive_vocabulary(
+ ppa for ppa in ppas
+ if check_permission('launchpad.Append', ppa))
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2010-07-17 18:59:08 +0000
+++ lib/lp/testing/factory.py 2010-07-20 12:55:27 +0000
@@ -145,7 +145,7 @@
from lp.soyuz.interfaces.section import ISectionSet
from lp.soyuz.model.binarypackagename import BinaryPackageName
from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
-from lp.soyuz.model.processor import ProcessorFamily, ProcessorFamilySet
+from lp.soyuz.model.processor import ProcessorFamilySet
from lp.soyuz.model.publishing import (
BinaryPackagePublishingHistory, SourcePackagePublishingHistory)
from lp.testing import (
@@ -1812,6 +1812,15 @@
parser = RecipeParser(self.makeRecipeText(*branches))
return parser.parse()
+ def makeSourcePackageRecipeDistroseries(self, name="warty"):
+ """Return a supported Distroseries to use with Source Package Recipes.
+
+ Ew. This uses sampledata currently, which is the ONLY reason this
+ method exists: it gives us a migration path away from sampledata.
+ """
+ ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
+ return ubuntu.getSeries(name)
+
def makeSourcePackageRecipe(self, registrant=None, owner=None,
distroseries=None, name=None,
description=None, branches=(),
@@ -1823,10 +1832,7 @@
if owner is None:
owner = self.makePerson()
if distroseries is None:
- distroseries = self.makeDistroSeries()
- distroseries.nominatedarchindep = distroseries.newArch(
- 'i386', ProcessorFamily.get(1), False, owner,
- supports_virtualized=True)
+ distroseries = self.makeSourcePackageRecipeDistroseries()
if name is None:
name = self.getUniqueString().decode('utf8')
@@ -1917,6 +1923,7 @@
ddd57463774cae9b50e70cd51221281b 185913 ed_0.2.orig.tar.gz
f9e1e5f13725f581919e9bfd62272a05 8506 ed_0.2-20.diff.gz
"""))
+
class Changes:
architectures = ['source']
logger = QuietFakeLogger()
