launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17283
[Merge] lp:~wgrant/launchpad/createPPA-distro into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/createPPA-distro into lp:launchpad.
Commit message:
Person.createPPA now takes a distribution, defaulting to Ubuntu if not specified. It'll refuse to operate on a distribution that doesn't have supports_ppas set.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/createPPA-distro/+merge/229182
Person.createPPA now takes a distribution, defaulting to Ubuntu if not specified. It'll refuse to operate on a distribution that doesn't have supports_ppas set.
--
https://code.launchpad.net/~wgrant/launchpad/createPPA-distro/+merge/229182
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/createPPA-distro into lp:launchpad.
=== modified file 'lib/lp/_schema_circular_imports.py'
--- lib/lp/_schema_circular_imports.py 2014-07-03 04:59:00 +0000
+++ lib/lp/_schema_circular_imports.py 2014-08-01 09:33:16 +0000
@@ -325,6 +325,8 @@
patch_plain_parameter_type(
IPersonLimitedView, 'getPPAByName', 'distribution', IDistribution)
patch_entry_return_type(IPersonLimitedView, 'getPPAByName', IArchive)
+patch_plain_parameter_type(
+ IPersonEditRestricted, 'createPPA', 'distribution', IDistribution)
patch_entry_return_type(IPersonEditRestricted, 'createPPA', IArchive)
IHasBuildRecords['getBuildRecords'].queryTaggedValue(
=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2014-04-02 22:43:37 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2014-08-01 09:33:16 +0000
@@ -79,6 +79,7 @@
TextLineEditorWidget,
)
from lp.app.browser.tales import format_link
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.validators.name import name_validator
from lp.app.widgets.itemswidgets import (
LabeledMultiCheckBoxWidget,
@@ -817,7 +818,8 @@
owner = data['owner']
if data['use_ppa'] == CREATE_NEW:
ppa_name = data.get('ppa_name', None)
- ppa = owner.createPPA(ppa_name)
+ ppa = owner.createPPA(
+ getUtility(ILaunchpadCelebrities).ubuntu, ppa_name)
else:
ppa = data['daily_build_archive']
try:
@@ -849,7 +851,8 @@
self.setFieldError(
'ppa_name', 'You need to specify a name for the PPA.')
else:
- error = validate_ppa(owner, ppa_name)
+ error = validate_ppa(
+ owner, getUtility(ILaunchpadCelebrities).ubuntu, ppa_name)
if error is not None:
self.setFieldError('ppa_name', error)
=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2014-07-09 03:08:57 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2014-08-01 09:33:16 +0000
@@ -687,7 +687,7 @@
browser.getControl('Create Recipe').click()
self.assertEqual(
get_feedback_messages(browser.contents)[1],
- html_escape("You already have a PPA named 'foo'."))
+ html_escape("You already have a PPA for Ubuntu named 'foo'."))
def test_create_new_ppa_missing_name(self):
# If a new PPA is being created, and the user has not specified a
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2014-07-03 04:59:00 +0000
+++ lib/lp/registry/interfaces/person.py 2014-08-01 09:33:16 +0000
@@ -1740,6 +1740,7 @@
"""
@operation_parameters(
+ distribution=Reference(schema=Interface, required=False),
name=TextLine(required=True, constraint=name_validator),
displayname=TextLine(required=False),
description=TextLine(required=False),
@@ -1748,12 +1749,13 @@
)
@export_factory_operation(Interface, []) # Really IArchive.
@operation_for_version("beta")
- def createPPA(name=None, displayname=None, description=None,
- private=False, suppress_subscription_notifications=False):
+ def createPPA(distribution=None, name=None, displayname=None,
+ description=None, private=False,
+ suppress_subscription_notifications=False):
"""Create a PPA.
- :param name: A string with the name of the new PPA to create. If
- not specified, defaults to 'ppa'.
+ :param distribution: The distribution that this archive is for.
+ :param name: The name of the new PPA to create.
:param displayname: The displayname for the new PPA.
:param description: The description for the new PPA.
:param private: Whether or not to create a private PPA. Defaults to
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2014-07-03 04:50:57 +0000
+++ lib/lp/registry/model/person.py 2014-08-01 09:33:16 +0000
@@ -3072,19 +3072,20 @@
raise NoSuchPPA(name)
return ppa
- def createPPA(self, name=None, displayname=None, description=None,
- private=False, suppress_subscription_notifications=False):
+ def createPPA(self, distribution=None, name=None, displayname=None,
+ description=None, private=False,
+ suppress_subscription_notifications=False):
"""See `IPerson`."""
- errors = validate_ppa(self, name, private)
+ if distribution is None:
+ distribution = getUtility(ILaunchpadCelebrities).ubuntu
+ if name is None:
+ name = "ppa"
+ errors = validate_ppa(self, distribution, name, private)
if errors:
raise PPACreationError(errors)
- # XXX cprov 2009-03-27 bug=188564: We currently only create PPAs
- # for Ubuntu distribution. PPA creation should be revisited when we
- # start supporting other distribution (debian, mainly).
- ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
return getUtility(IArchiveSet).new(
owner=self, purpose=ArchivePurpose.PPA,
- distribution=ubuntu, name=name, displayname=displayname,
+ distribution=distribution, name=name, displayname=displayname,
description=description, private=private,
suppress_subscription_notifications=(
suppress_subscription_notifications))
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2014-07-24 07:56:06 +0000
+++ lib/lp/soyuz/browser/archive.py 2014-08-01 09:33:16 +0000
@@ -1928,7 +1928,8 @@
'name for the new PPA and resubmit the form.')
errors = validate_ppa(
- self.context, proposed_name, private=self.is_private_team)
+ self.context, self.ubuntu, proposed_name,
+ private=self.is_private_team)
if errors is not None:
self.addError(errors)
@@ -1947,7 +1948,8 @@
displayname = data['displayname']
description = data['description']
ppa = self.context.createPPA(
- name, displayname, description, private=self.is_private_team)
+ self.ubuntu, name, displayname, description,
+ private=self.is_private_team)
self.next_url = canonical_url(ppa)
@property
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2014-07-29 07:31:06 +0000
+++ lib/lp/soyuz/model/archive.py 2014-08-01 09:33:16 +0000
@@ -2137,15 +2137,15 @@
job.destroySelf()
-def validate_ppa(owner, proposed_name, private=False):
+def validate_ppa(owner, distribution, proposed_name, private=False):
"""Can 'person' create a PPA called 'proposed_name'?
:param owner: The proposed owner of the PPA.
:param proposed_name: The proposed name.
:param private: Whether or not to make it private.
"""
+ assert proposed_name is not None
creator = getUtility(ILaunchBag).user
- ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
if private:
# NOTE: This duplicates the policy in lp/soyuz/configure.zcml
# which says that one needs 'launchpad.Admin' permission to set
@@ -2160,20 +2160,24 @@
if owner.is_team and (
owner.membership_policy in INCLUSIVE_TEAM_POLICY):
return "Open teams cannot have PPAs."
- if proposed_name is not None and proposed_name == ubuntu.name:
+ if not distribution.supports_ppas:
+ return "%s does not support PPAs." % distribution.displayname
+ if proposed_name == distribution.name:
return (
"A PPA cannot have the same name as its distribution.")
- if proposed_name is None:
- proposed_name = 'ppa'
+ if proposed_name == "ubuntu":
+ return (
+ 'A PPA cannot be named "ubuntu".')
try:
- owner.getPPAByName(ubuntu, proposed_name)
+ owner.getPPAByName(distribution, proposed_name)
except NoSuchPPA:
return None
else:
- text = "You already have a PPA named '%s'." % proposed_name
+ text = "You already have a PPA for %s named '%s'." % (
+ distribution.displayname, proposed_name)
if owner.is_team:
- text = "%s already has a PPA named '%s'." % (
- owner.displayname, proposed_name)
+ text = "%s already has a PPA for %s named '%s'." % (
+ owner.displayname, distribution.displayname, proposed_name)
return text
=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt 2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-workflow.txt 2014-08-01 09:33:16 +0000
@@ -507,7 +507,7 @@
>>> print_feedback_messages(browser2.contents)
There is 1 error.
- You already have a PPA named 'boomppa'.
+ You already have a PPA for Ubuntu named 'boomppa'.
>>> print browser2.getControl(name="field.name").value
boomppa
@@ -591,7 +591,7 @@
>>> print_feedback_messages(cprov_browser.contents)
There is 1 error.
- You already have a PPA named 'ppa'.
+ You already have a PPA for Ubuntu named 'ppa'.
If the PPA is named as the distribution it is targeted for it cannot
be created, mainly because of the way we publish repositories
=== modified file 'lib/lp/soyuz/stories/webservice/xx-person-createppa.txt'
--- lib/lp/soyuz/stories/webservice/xx-person-createppa.txt 2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/stories/webservice/xx-person-createppa.txt 2014-08-01 09:33:16 +0000
@@ -2,9 +2,11 @@
>>> from zope.component import getUtility
>>> from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+ >>> from lp.registry.interfaces.distribution import IDistributionSet
>>> from lp.testing import celebrity_logged_in
>>> from lp.testing.sampledata import ADMIN_EMAIL
>>> login(ADMIN_EMAIL)
+ >>> getUtility(IDistributionSet)['ubuntutest'].supports_ppas = True
>>> owner = factory.makePerson()
>>> url = "/~%s" % owner.name
>>> logout()
@@ -16,13 +18,12 @@
... owner, permission=OAuthPermission.WRITE_PRIVATE)
>>> print ppa_owner_webservice.named_post(
- ... ppa_owner['self_link'], 'createPPA', {}, name=None,
- ... displayname='My shiny new PPA', description='Shinyness!',
- ... )
+ ... ppa_owner['self_link'], 'createPPA', {}, distribution='/ubuntu',
+ ... name='yay', displayname='My shiny new PPA', description='Shinyness!')
HTTP/1.1 201 Created
Status: 201
...
- Location: http://api.launchpad.dev/.../+archive/ubuntu/ppa
+ Location: http://api.launchpad.dev/.../+archive/ubuntu/yay
...
>>> print ppa_owner_webservice.named_post(
@@ -40,7 +41,7 @@
>>> print_self_link_of_entries(webservice.get(
... ppa_owner['ppas_collection_link']).jsonBody())
- http://api.launchpad.dev/beta/~.../+archive/ubuntu/ppa
+ http://api.launchpad.dev/beta/~.../+archive/ubuntu/yay
They aren't a commercial admin, so they cannot create private PPAs.
@@ -59,7 +60,7 @@
>>> print_self_link_of_entries(webservice.get(
... ppa_owner['ppas_collection_link']).jsonBody())
- http://api.launchpad.dev/beta/~.../+archive/ubuntu/ppa
+ http://api.launchpad.dev/beta/~.../+archive/ubuntu/yay
However, we can grant them commercial admin access:
@@ -85,8 +86,8 @@
>>> print_self_link_of_entries(webservice.get(
... ppa_owner['ppas_collection_link']).jsonBody())
- http://api.launchpad.dev/.../+archive/ubuntu/ppa
http://api.launchpad.dev/.../+archive/ubuntu/secret
+ http://api.launchpad.dev/.../+archive/ubuntu/yay
And the PPA is, of course, private:
@@ -94,3 +95,39 @@
... ppa_owner['self_link'], 'getPPAByName', name='secret').jsonBody()
>>> ppa['private']
True
+
+It's possible to create PPAs for all sorts of distributions.
+
+ >>> print ppa_owner_webservice.named_post(
+ ... ppa_owner['self_link'], 'createPPA', {}, distribution='/ubuntutest',
+ ... name='ppa')
+ HTTP/1.1 201 Created
+ Status: 201
+ ...
+ Location: http://api.launchpad.dev/.../+archive/ubuntutest/ppa
+ ...
+
+But not for distributions that don't have PPAs enabled.
+
+ >>> print ppa_owner_webservice.named_post(
+ ... ppa_owner['self_link'], 'createPPA', {}, distribution='/redhat',
+ ... name='ppa')
+ HTTP/1.1 400 Bad Request
+ Status: 400
+ ...
+ Red Hat does not support PPAs.
+
+
+== Defaults ==
+
+createPPA's distribution and name arguments were added years after the
+method, so they remain optional and default to Ubuntu and "ppa"
+respectively.
+
+ >>> print ppa_owner_webservice.named_post(
+ ... ppa_owner['self_link'], 'createPPA', {})
+ HTTP/1.1 201 Created
+ Status: 201
+ ...
+ Location: http://api.launchpad.dev/.../+archive/ubuntu/ppa
+ ...
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2014-07-29 06:25:35 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2014-08-01 09:33:16 +0000
@@ -1717,22 +1717,49 @@
layer = DatabaseFunctionalLayer
+ def setUp(self):
+ super(TestValidatePPA, self).setUp()
+ self.ubuntu = getUtility(IDistributionSet)['ubuntu']
+ self.ubuntutest = getUtility(IDistributionSet)['ubuntutest']
+ with admin_logged_in():
+ self.ubuntutest.supports_ppas = True
+
def test_open_teams(self):
team = self.factory.makeTeam()
self.assertEqual(
- 'Open teams cannot have PPAs.', validate_ppa(team, None))
+ 'Open teams cannot have PPAs.',
+ validate_ppa(team, self.ubuntu, "ppa"))
def test_distribution_name(self):
ppa_owner = self.factory.makePerson()
self.assertEqual(
'A PPA cannot have the same name as its distribution.',
- validate_ppa(ppa_owner, 'ubuntu'))
+ validate_ppa(ppa_owner, self.ubuntu, 'ubuntu'))
+
+ def test_ubuntu_name(self):
+ # Disambiguating old-style URLs relies on there never being a
+ # PPA named "ubuntu".
+ ppa_owner = self.factory.makePerson()
+ self.assertEqual(
+ 'A PPA cannot be named "ubuntu".',
+ validate_ppa(ppa_owner, self.ubuntutest, 'ubuntu'))
+
+ def test_distro_unsupported(self):
+ ppa_owner = self.factory.makePerson()
+ distro = self.factory.makeDistribution(displayname="Unbuntu")
+ self.assertEqual(
+ "Unbuntu does not support PPAs.",
+ validate_ppa(ppa_owner, distro, "ppa"))
+ with admin_logged_in():
+ distro.supports_ppas = True
+ self.assertIs(None, validate_ppa(ppa_owner, distro, "ppa"))
def test_private_ppa_standard_user(self):
ppa_owner = self.factory.makePerson()
with person_logged_in(ppa_owner):
errors = validate_ppa(
- ppa_owner, self.factory.getUniqueString(), private=True)
+ ppa_owner, self.ubuntu, self.factory.getUniqueString(),
+ private=True)
self.assertEqual(
'%s is not allowed to make private PPAs' % (ppa_owner.name,),
errors)
@@ -1741,7 +1768,7 @@
owner = self.factory.makePerson()
self.factory.grantCommercialSubscription(owner)
with person_logged_in(owner):
- errors = validate_ppa(owner, 'ppa', private=True)
+ errors = validate_ppa(owner, self.ubuntu, 'ppa', private=True)
self.assertIsNone(errors)
def test_private_ppa_commercial_admin(self):
@@ -1752,32 +1779,34 @@
with person_logged_in(ppa_owner):
self.assertIsNone(
validate_ppa(
- ppa_owner, self.factory.getUniqueString(), private=True))
+ ppa_owner, self.ubuntu, self.factory.getUniqueString(),
+ private=True))
def test_private_ppa_admin(self):
ppa_owner = self.factory.makeAdministrator()
with person_logged_in(ppa_owner):
self.assertIsNone(
validate_ppa(
- ppa_owner, self.factory.getUniqueString(), private=True))
+ ppa_owner, self.ubuntu, self.factory.getUniqueString(),
+ private=True))
def test_two_ppas(self):
ppa = self.factory.makeArchive(name='ppa')
self.assertEqual(
- "You already have a PPA named 'ppa'.",
- validate_ppa(ppa.owner, 'ppa'))
+ "You already have a PPA for Ubuntu named 'ppa'.",
+ validate_ppa(ppa.owner, self.ubuntu, 'ppa'))
def test_two_ppas_with_team(self):
team = self.factory.makeTeam(
membership_policy=TeamMembershipPolicy.MODERATED)
self.factory.makeArchive(owner=team, name='ppa')
self.assertEqual(
- "%s already has a PPA named 'ppa'." % team.displayname,
- validate_ppa(team, 'ppa'))
+ "%s already has a PPA for Ubuntu named 'ppa'." % team.displayname,
+ validate_ppa(team, self.ubuntu, 'ppa'))
def test_valid_ppa(self):
ppa_owner = self.factory.makePerson()
- self.assertIsNone(validate_ppa(ppa_owner, None))
+ self.assertIsNone(validate_ppa(ppa_owner, self.ubuntu, "ppa"))
def test_private_team_private_ppa(self):
# Folk with launchpad.Edit on a private team can make private PPAs for
@@ -1791,7 +1820,8 @@
private_team.addMember(
team_admin, team_owner, status=TeamMembershipStatus.ADMIN)
with person_logged_in(team_admin):
- result = validate_ppa(private_team, 'ppa', private=True)
+ result = validate_ppa(
+ private_team, self.ubuntu, 'ppa', private=True)
self.assertIsNone(result)
def test_private_team_public_ppa(self):
@@ -1805,7 +1835,8 @@
private_team.addMember(
team_admin, team_owner, status=TeamMembershipStatus.ADMIN)
with person_logged_in(team_admin):
- result = validate_ppa(private_team, 'ppa', private=False)
+ result = validate_ppa(
+ private_team, self.ubuntu, 'ppa', private=False)
self.assertEqual(
'Private teams may not have public archives.', result)
Follow ups
