launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17075
[Merge] lp:~wgrant/launchpad/cross-distro-ppa-uploads into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/cross-distro-ppa-uploads into lp:launchpad with lp:~wgrant/launchpad/ppa-distro-traversal as a prerequisite.
Commit message:
Support /~person/distro/ppa[/suite] paths in archiveuploader, for non-Ubuntu PPAs. Also drop support for /~person/distro/suite (deprecated five years ago), as it's ambiguous with /~person/distro/ppa.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/cross-distro-ppa-uploads/+merge/225662
Support /~person/distro/ppa[/suite] paths in archiveuploader, for non-Ubuntu PPAs. Also drop support for /~person/distro/suite (deprecated five years ago), as it's ambiguous with /~person/distro/ppa.
--
https://code.launchpad.net/~wgrant/launchpad/cross-distro-ppa-uploads/+merge/225662
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/cross-distro-ppa-uploads into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/tests/upload-path-parsing.txt'
--- lib/lp/archiveuploader/tests/upload-path-parsing.txt 2014-07-03 04:51:30 +0000
+++ lib/lp/archiveuploader/tests/upload-path-parsing.txt 2014-07-07 02:47:53 +0000
@@ -78,70 +78,77 @@
starts with '~', then the subsequent text in this term will be looked
up as a Person in Launchpad.
- >>> check_upload('~cprov/ppa/ubuntu')
+ >>> from zope.component import getUtility
+ >>> from lp.registry.interfaces.distribution import IDistributionSet
+ >>> from lp.registry.interfaces.person import IPersonSet
+ >>> from lp.soyuz.enums import ArchivePurpose
+ >>> from lp.soyuz.interfaces.archive import IArchiveSet
+ >>> debian = getUtility(IDistributionSet).getByName('debian')
+ >>> cprov = getUtility(IPersonSet).getByName('cprov')
+ >>> copy_archive = getUtility(IArchiveSet).new(
+ ... ArchivePurpose.PPA, cprov, 'ppa', distribution=debian)
+
+ >>> check_upload('~cprov/ubuntu/ppa')
Archive: ppa
Distribution: ubuntu
Suite: None
- >>> check_upload('~does-not-exist/ppa/ubuntu')
+ >>> check_upload('~cprov/debian/ppa')
+ Archive: ppa
+ Distribution: debian
+ Suite: None
+
+ >>> check_upload('~does-not-exist/ubuntu/ppa')
Traceback (most recent call last):
...
PPAUploadPathError: Could not find person or team named
'does-not-exist'.
-The second path term is assumed to be a PPA name. Since for a long
-time named-PPA was not supported and uploads didn't need to consider
-it, we still pointing uploads where the PPA name was omitted to the
-default PPA (the one named 'ppa').
-
- >>> check_upload('~cprov/ubuntu')
+ >>> check_upload('~cprov/notbuntu/ppa')
+ Traceback (most recent call last):
+ ...
+ PPAUploadPathError: Could not find distribution 'notbuntu'.
+
+Two deprecated PPA paths are still supported for compatibility. Before
+mid-2014 all PPAs were for Ubuntu, and the distribution came after the
+PPA name and could be omitted. Before 2009 each user had at most one
+PPA, so the name was omitted -- we assume "ppa".
+
+ >>> check_upload('~cprov/ppa/ubuntu')
Archive: ppa
Distribution: ubuntu
Suite: None
-
-PPA upload paths missing the 'distribution' part default to 'ubuntu'
-distribution.
-
>>> check_upload('~cprov/ppa')
Archive: ppa
Distribution: ubuntu
Suite: None
+ >>> check_upload('~cprov/ubuntu')
+ Archive: ppa
+ Distribution: ubuntu
+ Suite: None
+
PPA uploads also support overrides to the changesfile suite when it's
-valid. It's also supported for uploads omitting the PPA name.
+valid. It's also supported for uploads to the deprecated paths.
+
+ >>> check_upload('~cprov/ubuntu/ppa/warty-backports')
+ Archive: ppa
+ Distribution: ubuntu
+ Suite: warty-backports
>>> check_upload('~cprov/ppa/ubuntu/warty-backports')
Archive: ppa
Distribution: ubuntu
Suite: warty-backports
- >>> check_upload('~cprov/ubuntu/warty-backports')
- Archive: ppa
- Distribution: ubuntu
- Suite: warty-backports
-
>>> check_upload('~cprov/ppa/ubuntu/boing')
Traceback (most recent call last):
...
PPAUploadPathError: Could not find suite 'boing'.
-A PPA upload for an conflicting distribution results in an
-error. Currently all PPAs in Launchpad are targeted to 'ubuntu'.
-
- >>> check_upload('~cprov/ppa/debian')
- Traceback (most recent call last):
- ...
- PPAUploadPathError: PPA for Celso Providelo only supports uploads
- to 'ubuntu' distribution.
-
We will disable Celso's default PPA and uploads to it will result in
an error.
- >>> from zope.component import getUtility
- >>> from lp.registry.interfaces.distribution import IDistributionSet
- >>> from lp.registry.interfaces.person import IPersonSet
-
- >>> cprov = getUtility(IPersonSet).getByName('cprov')
>>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
>>> cprov_ppa = cprov.getPPAByName(ubuntu, 'ppa')
>>> cprov_ppa.disable()
@@ -149,32 +156,42 @@
>>> import transaction
>>> transaction.commit()
+ >>> check_upload('~cprov/ubuntu/ppa')
+ Traceback (most recent call last):
+ ...
+ PPAUploadPathError: PPA for Celso Providelo is disabled.
+
>>> check_upload('~cprov/ppa/ubuntu')
Traceback (most recent call last):
...
PPAUploadPathError: PPA for Celso Providelo is disabled.
- >>> check_upload('~cprov/ubuntu')
- Traceback (most recent call last):
- ...
- PPAUploadPathError: PPA for Celso Providelo is disabled.
-
Uploading to named PPA that does not exist fails.
+ >>> check_upload('~cprov/ubuntu/beta')
+ Traceback (most recent call last):
+ ...
+ PPAUploadPathError: Could not find a PPA owned by 'cprov' for
+ 'ubuntu' named 'beta'.
+
>>> check_upload('~cprov/beta/ubuntu')
Traceback (most recent call last):
...
- PPAUploadPathError: Could not find a PPA named 'beta' for 'cprov'.
+ PPAUploadPathError: Could not find a PPA owned by 'cprov' for
+ 'ubuntu' named 'beta'.
We will create a 'beta' PPA for Celso.
- >>> from lp.soyuz.enums import ArchivePurpose
- >>> from lp.soyuz.interfaces.archive import IArchiveSet
>>> beta_ppa = getUtility(IArchiveSet).new(
... ArchivePurpose.PPA, cprov, 'beta')
And the upload now found its way to the new named PPA.
+ >>> check_upload('~cprov/ubuntu/beta')
+ Archive: beta
+ Distribution: ubuntu
+ Suite: None
+
>>> check_upload('~cprov/beta/ubuntu')
Archive: beta
Distribution: ubuntu
@@ -203,7 +220,14 @@
An extra path part that cannot be processed for PPA uploads.
- >>> check_upload('~cprov/ppa/ubuntu/warty/ding-dong')
+ >>> check_upload('~cprov/ubuntu/ppa/warty/ding-dong')
+ Traceback (most recent call last):
+ ...
+ UploadPathError: Path format mismatch.
+
+A PPA upload missing '~':
+
+ >>> check_upload('cprov/ubuntu/ppa')
Traceback (most recent call last):
...
UploadPathError: Path format mismatch.
@@ -215,7 +239,7 @@
...
UploadPathError: Could not find distribution 'cprov'.
-A named PPA upload missing '~'.
+An old-style named PPA upload missing '~'.
>>> check_upload('cprov/ppa/ubuntu')
Traceback (most recent call last):
=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py 2014-07-03 04:51:30 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py 2014-07-07 02:47:53 +0000
@@ -57,7 +57,6 @@
from zope.component import getUtility
from lp.app.errors import NotFoundError
-from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.archiveuploader.livefsupload import LiveFSUpload
from lp.archiveuploader.nascentupload import (
EarlyReturnUploadError,
@@ -784,12 +783,22 @@
We do this by analysing the path to which the user has uploaded,
ie. the relative path within the upload folder to the changes file.
- The valid paths are:
- '' - default distro, ubuntu
- '<distroname>' - given distribution
- '~<personname>[/ppa_name]/<distroname>[/distroseriesname]' - given ppa,
- distribution and optionally a distroseries. If ppa_name is not
- specified it will default to the one referenced by IPerson.archive.
+ Current paths are:
+ /<distro>[/suite] - any primary archive
+ /~<person>/<distro>/<ppa>[/suite] - any PPA
+
+ One deprecated form is still supported for the Ubuntu primary archive:
+ / - Ubuntu primary archive
+
+ Three deprecated forms are still supported for Ubuntu PPAs:
+ /~<person>/<ppa>/ubuntu[/suite] - any Ubuntu PPA
+ /~<person>/<ppa> - any Ubuntu PPA
+ /~<person>/ubuntu - Ubuntu PPA named "ppa"
+
+ The original /~<person>/ubuntu/<suite> form is no longer supported
+ as it clashes with /~<person>/<distro>/<ppa>
+
+ suite is an optional distroseries with an optional pocket suffix.
I raises UploadPathError if something was wrong when parsing it.
@@ -802,7 +811,7 @@
if (not first_path.startswith('~') and not first_path.isdigit()
and len(parts) <= 2):
- # Distribution upload (<distro>[/distroseries]). Always targeted to
+ # Distribution upload (<distro>[/suite]). Always targeted to
# the corresponding primary archive.
distribution, suite_name = _getDistributionAndSuite(
parts, UploadPathError)
@@ -811,7 +820,13 @@
elif (first_path.startswith('~') and
len(parts) >= 2 and
len(parts) <= 4):
- # PPA upload (~<person>[/ppa_name]/<distro>[/distroseries]).
+ # PPA uploads look like (~<person>/<distro>/<ppa_name>[/suite]).
+ #
+ # Additionally, three deprecated formats are still supported for
+ # Ubuntu PPAs:
+ # (~<person>/<ppa_name>/ubuntu[/suite])
+ # (~<person>/<ppa_name>)
+ # (~<person>/ubuntu)
# Skip over '~' from the person name.
person_name = first_path[1:]
@@ -820,26 +835,35 @@
raise PPAUploadPathError(
"Could not find person or team named '%s'." % person_name)
- ppa_name = parts[1]
-
- # Compatibilty feature for allowing unamed-PPA upload paths
- # for a certain period of time.
- if ppa_name == 'ubuntu':
- ppa_name = 'ppa'
- distribution_and_suite = parts[1:]
+ if len(parts) == 2:
+ distro_name = 'ubuntu'
+ suite = []
+ if parts[1] == 'ubuntu':
+ # Old nameless path.
+ ppa_name = 'ppa'
+ else:
+ # Old named but distroless path.
+ ppa_name = parts[1]
else:
- distribution_and_suite = parts[2:]
+ if parts[2] == 'ubuntu':
+ # Old named path with PPA name before distro.
+ distro_name = 'ubuntu'
+ ppa_name = parts[1]
+ else:
+ # Modern path with distro, name, suite.
+ distro_name = parts[1]
+ ppa_name = parts[2]
+ suite = parts[3:]
+
+ distribution, suite_name = _getDistributionAndSuite(
+ [distro_name] + suite, PPAUploadPathError)
try:
- archive = person.getPPAByName(
- getUtility(ILaunchpadCelebrities).ubuntu, ppa_name)
+ archive = person.getPPAByName(distribution, ppa_name)
except NoSuchPPA:
raise PPAUploadPathError(
- "Could not find a PPA named '%s' for '%s'."
- % (ppa_name, person_name))
-
- distribution, suite_name = _getDistributionAndSuite(
- distribution_and_suite, PPAUploadPathError)
+ "Could not find a PPA owned by '%s' for '%s' named '%s'."
+ % (person.name, distribution.name, ppa_name))
elif first_path.isdigit():
# This must be a binary upload from a build slave.
Follow ups