launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17376
[Merge] lp:~wgrant/launchpad/ppa-rtm-updates into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/ppa-rtm-updates into lp:launchpad.
Commit message:
PPA vocabularies now use Archive.reference as the term token.
Requested reviews:
William Grant (wgrant): code
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/ppa-rtm-updates/+merge/232853
Fixed version of https://code.launchpad.net/~cjohnston/launchpad/ppa-rtm-updates/+merge/230142.
--
https://code.launchpad.net/~wgrant/launchpad/ppa-rtm-updates/+merge/232853
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/code/javascript/tests/test_requestbuild_overlay.html'
--- lib/lp/code/javascript/tests/test_requestbuild_overlay.html 2012-10-26 09:54:28 +0000
+++ lib/lp/code/javascript/tests/test_requestbuild_overlay.html 2014-09-01 07:45:54 +0000
@@ -1,6 +1,6 @@
<!DOCTYPE html>
<!--
-Copyright 2012 Canonical Ltd. This software is licensed under the
+Copyright 2012-2014 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -73,7 +73,7 @@
<script type="text/x-template" id="requestbuilds-form-template">
<div class="form" id="launchpad-form-widgets">
<select id="field.archive" name="field.archive" size="1" >
- <option selected="selected" value="mark/ppa">
+ <option selected="selected" value="~mark/ubuntu/ppa">
PPA for Mark Shuttleworth</option>
</select>
<input name="field.archive-empty-marker" type="hidden" value="1" />
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2014-08-01 08:47:00 +0000
+++ lib/lp/soyuz/browser/archive.py 2014-09-01 07:45:54 +0000
@@ -1391,6 +1391,7 @@
for archive in archives:
label = '%s [%s]' % (archive.displayname, archive.reference)
terms.append(SimpleTerm(archive, archive.reference, label))
+ terms.sort(key=lambda x: x.value.reference)
return SimpleVocabulary(terms)
@@ -1627,10 +1628,8 @@
canonical_url(dependency), archive_dependency.title)
else:
dependency_label = archive_dependency.title
- dependency_token = '%s/%s' % (
- dependency.owner.name, dependency.name)
term = SimpleTerm(
- dependency, dependency_token, dependency_label)
+ dependency, dependency.reference, dependency_label)
terms.append(term)
return form.Fields(
List(__name__='selected_dependencies',
=== modified file 'lib/lp/soyuz/browser/tests/archive-views.txt'
--- lib/lp/soyuz/browser/tests/archive-views.txt 2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/browser/tests/archive-views.txt 2014-09-01 07:45:54 +0000
@@ -716,7 +716,7 @@
Let's emulate a dependency addition. Note that the form contains, a
empty 'selected_dependencies' (as it was rendered in the empty
-request) and 'dependency_candidate' contains a valid PPA owner name.
+request) and 'dependency_candidate' contains a valid PPA name.
Validation checks are documented in
pagetests/ppa/xx-edit-dependencies.txt.
@@ -724,7 +724,7 @@
... cprov.archive, name="+edit-dependencies",
... form={
... 'field.selected_dependencies': [],
- ... 'field.dependency_candidate': 'mark/ppa',
+ ... 'field.dependency_candidate': '~mark/ubuntu/ppa',
... 'field.primary_dependencies': 'UPDATES',
... 'field.primary_components': 'ALL_COMPONENTS',
... 'field.actions.save': 'Save',
@@ -762,7 +762,7 @@
PPA for Mark Shuttleworth
>>> print dependency.token
- mark/ppa
+ ~mark/ubuntu/ppa
>>> print dependency.title.escapedtext
<a href="http://launchpad.dev/~mark/+archive/ubuntu/ppa";>PPA for Mark
@@ -794,7 +794,7 @@
PPA for Mark Shuttleworth
>>> print dependency.token
- mark/ppa
+ ~mark/ubuntu/ppa
>>> print dependency.title
PPA for Mark Shuttleworth
@@ -805,7 +805,7 @@
>>> view = create_initialized_view(
... cprov.archive, name="+edit-dependencies",
... form={
- ... 'field.selected_dependencies': ['mark/ppa'],
+ ... 'field.selected_dependencies': ['~mark/ubuntu/ppa'],
... 'field.dependency_candidate': '',
... 'field.primary_dependencies': 'UPDATES',
... 'field.primary_components': 'ALL_COMPONENTS',
@@ -849,7 +849,7 @@
>>> view.widgets.get('primary_dependencies')._getCurrentValue()
<DBItem PackagePublishingPocket.UPDATES, (20) Updates>
-A similar widget is used for the primary archive component overrides ,
+A similar widget is used for the primary archive component overrides,
which contains two pre-defined options. By default all PPAs use all
ubuntu components available to satisfy build dependencies, i.e. the
'multiverse' component.
@@ -965,7 +965,7 @@
<DBItem PackagePublishingPocket.UPDATES, (20) Updates>
Dependencies on private PPAs can be only set if the user performing
-the action also have permission to view the private PPA and if the
+the action also has permission to view the private PPA and if the
context PPA is also private.
The latter guarantee that the P3A buildd_secret won't get exposed in
@@ -988,7 +988,7 @@
>>> add_private_form = {
... 'field.selected_dependencies': [],
- ... 'field.dependency_candidate': 'pirulito-team/ppa',
+ ... 'field.dependency_candidate': '~pirulito-team/ubuntu/ppa',
... 'field.primary_dependencies': 'UPDATES',
... 'field.primary_components': 'FOLLOW_PRIMARY',
... 'field.actions.save': 'Save',
=== modified file 'lib/lp/soyuz/doc/vocabularies.txt'
--- lib/lp/soyuz/doc/vocabularies.txt 2013-07-12 06:14:22 +0000
+++ lib/lp/soyuz/doc/vocabularies.txt 2014-09-01 07:45:54 +0000
@@ -189,13 +189,13 @@
* value: the IArchive object;
* title: the first line of the PPA description text.
- >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+ >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
>>> print cprov_term.token
- cprov/ppa
+ ~cprov/ubuntu/ppa
>>> print cprov_term.value
- <Archive ...>
+ <... lp.soyuz.model.archive.Archive instance ...>
>>> print cprov_term.title
packages to help my friends.
@@ -216,15 +216,15 @@
>>> cprov_search = vocabulary.search(u'cprov')
>>> print_search_results(cprov_search)
- cprov/ppa: packages to help my friends.
+ ~cprov/ubuntu/ppa: packages to help my friends.
>>> celso_search = vocabulary.search(u'celso')
>>> print_search_results(celso_search)
- cprov/ppa: packages to help my friends.
+ ~cprov/ubuntu/ppa: packages to help my friends.
>>> friends_search = vocabulary.search(u'friends')
>>> print_search_results(friends_search)
- cprov/ppa: packages to help my friends.
+ ~cprov/ubuntu/ppa: packages to help my friends.
We will create an additional PPA for Celso named 'testing'
@@ -242,15 +242,15 @@
>>> cprov_search = vocabulary.search(u'cprov')
>>> print_search_results(cprov_search)
- cprov/ppa: packages to help my friends.
- cprov/testing: testing packages.
+ ~cprov/ubuntu/ppa: packages to help my friends.
+ ~cprov/ubuntu/testing: testing packages.
The vocabulary search also supports specific named PPA lookups
follwing the same combined syntax used to build unique tokens.
- >>> named_search = vocabulary.search(u'cprov/testing')
+ >>> named_search = vocabulary.search(u'~cprov/ubuntu/testing')
>>> print_search_results(named_search)
- cprov/testing: testing packages.
+ ~cprov/ubuntu/testing: testing packages.
As mentioned the PPA vocabulary term title only contains the first
line of the PPA description.
@@ -258,14 +258,14 @@
>>> cprov.archive.description = "Single line."
>>> flush_database_updates()
- >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+ >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
>>> print cprov_term.title
Single line.
>>> cprov.archive.description = "First line\nSecond line."
>>> flush_database_updates()
- >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+ >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
>>> print cprov_term.title
First line
@@ -274,7 +274,7 @@
>>> cprov.archive.description = None
>>> flush_database_updates()
- >>> cprov_term = vocabulary.getTermByToken('cprov/ppa')
+ >>> cprov_term = vocabulary.getTermByToken('~cprov/ubuntu/ppa')
>>> print cprov_term.title
No description available
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2014-08-14 10:08:28 +0000
+++ lib/lp/soyuz/model/archive.py 2014-09-01 07:45:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database class for table Archive."""
@@ -1063,6 +1063,11 @@
raise ArchiveDependencyError(
"You don't have permission to use this dependency.")
return
+ if not dependency.enabled:
+ raise ArchiveDependencyError("Dependencies must not be disabled.")
+ if dependency.distribution != self.distribution:
+ raise ArchiveDependencyError(
+ "Dependencies must be for the same distribution.")
if dependency.private and not self.private:
raise ArchiveDependencyError(
"Public PPAs cannot depend on private ones.")
@@ -1076,7 +1081,6 @@
raise ArchiveDependencyError(
"Non-primary archives only support the '%s' component." %
dependency.default_component.name)
-
return ArchiveDependency(
archive=self, dependency=dependency, pocket=pocket,
component=component)
@@ -2625,6 +2629,10 @@
def get_archive_privacy_filter(user):
+ """Get a simplified Archive privacy Storm filter.
+
+ Incorrect and deprecated. Use get_enabled_archive_filter instead.
+ """
if user is None:
privacy_filter = Not(Archive._private)
elif IPersonRoles(user).in_admin:
=== modified file 'lib/lp/soyuz/stories/ppa/xx-edit-dependencies.txt'
--- lib/lp/soyuz/stories/ppa/xx-edit-dependencies.txt 2014-07-24 09:37:03 +0000
+++ lib/lp/soyuz/stories/ppa/xx-edit-dependencies.txt 2014-09-01 07:45:54 +0000
@@ -112,7 +112,7 @@
is rendered on top of the page and the list of dependencies available
for removal is updated.
- >>> admin_browser.getControl("Add PPA dependency").value = 'mark/ppa'
+ >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
>>> admin_browser.getControl("Save").click()
>>> print_feedback_messages(admin_browser.contents)
Dependency added: PPA for Mark Shuttleworth
@@ -123,7 +123,7 @@
Trying to add a dependency that is already recorded results in a error.
- >>> admin_browser.getControl("Add PPA dependency").value = 'mark/ppa'
+ >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
>>> admin_browser.getControl("Save").click()
>>> print_feedback_messages(admin_browser.contents)
There is 1 error.
@@ -132,7 +132,7 @@
Trying to add a dependency for the context PPA itself also results in
a error.
- >>> admin_browser.getControl("Add PPA dependency").value = 'cprov/ppa'
+ >>> admin_browser.getControl("Add PPA dependency").value = '~cprov/ubuntu/ppa'
>>> admin_browser.getControl("Save").click()
>>> print_feedback_messages(admin_browser.contents)
There is 1 error.
@@ -140,7 +140,7 @@
If it's a new dependency everything is fine.
- >>> admin_browser.getControl("Add PPA dependency").value = 'no-priv/ppa'
+ >>> admin_browser.getControl("Add PPA dependency").value = '~no-priv/ubuntu/ppa'
>>> admin_browser.getControl("Save").click()
>>> print_feedback_messages(admin_browser.contents)
Dependency added: PPA for No Privileges Person
@@ -152,7 +152,7 @@
PPA for Mark Shuttleworth
PPA for No Privileges Person
-The dependencies are presented in a separated section (bellow the
+The dependencies are presented in a separated section (below the
sources.list widget).
>>> user_browser.open('http://launchpad.dev/~cprov/+archive/ubuntu/ppa')
@@ -196,7 +196,7 @@
http://launchpad.dev/~no-priv/+archive/ubuntu/ppa
When accessed by their owners, a PPA depending on disabled archives
-will additionally show an warning for uploaders . This way PPA
+will additionally show an warning for uploaders. This way a PPA
maintainer can react to this problem.
>>> cprov_browser.open('http://launchpad.dev/~cprov/+archive/ubuntu/ppa')
@@ -234,7 +234,7 @@
>>> admin_browser.getControl(
... name="field.selected_dependencies").value = [
- ... 'mark/ppa', 'no-priv/ppa']
+ ... '~mark/ubuntu/ppa', '~no-priv/ubuntu/ppa']
>>> admin_browser.getControl("Save").click()
>>> print_feedback_messages(admin_browser.contents)
Dependencies removed:
@@ -258,6 +258,43 @@
... user_browser.contents, 'archive-dependencies')
None
+We should also make sure that a user is unable to add a disabled PPA as a
+dependency.
+
+ # Disable Mark's PPA.
+ >>> login('foo.bar@xxxxxxxxxxxxx')
+ >>> from zope.component import getUtility
+ >>> from lp.registry.interfaces.person import IPersonSet
+ >>> mark = getUtility(IPersonSet).getByName('mark')
+ >>> mark.archive.disable()
+ >>> logout()
+
+ # Attempt to add Mark's PPA
+ >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
+ >>> admin_browser.getControl("Save").click()
+ >>> print_feedback_messages(admin_browser.contents)
+ There is 1 error.
+ Invalid value
+
+ # When the page is reloaded, there shouldn't be any dependencies.
+ >>> admin_browser.reload()
+ >>> print_ppa_dependencies(admin_browser.contents)
+ No dependencies recorded for this PPA yet.
+
+Re-enable Mark's PPA for subsequent tests.
+
+ >>> login('foo.bar@xxxxxxxxxxxxx')
+ >>> mark.archive.enable()
+ >>> logout()
+
+Clear the page.
+
+ >>> admin_browser.getControl("Add PPA dependency").value = ''
+ >>> admin_browser.getControl("Save").click()
+ >>> admin_browser.reload()
+ >>> print_ppa_dependencies(admin_browser.contents)
+ No dependencies recorded for this PPA yet.
+
== Primary dependencies ==
A user can modify how a PPA depends on its corresponding
@@ -418,9 +455,9 @@
The form can perform multiple actions in a single submit.
-First we will create a PPA dependency for No privileged' PPA.
+First we will create a PPA dependency for 'No privileged' PPA.
- >>> admin_browser.getControl("Add PPA dependency").value = 'no-priv/ppa'
+ >>> admin_browser.getControl("Add PPA dependency").value = '~no-priv/ubuntu/ppa'
>>> admin_browser.getControl("Save").click()
>>> print_feedback_messages(admin_browser.contents)
Dependency added: PPA for No Privileges Person
@@ -450,12 +487,12 @@
RELEASE.
>>> admin_browser.getControl(
- ... name="field.selected_dependencies").value = ['no-priv/ppa']
+ ... name="field.selected_dependencies").value = ['~no-priv/ubuntu/ppa']
>>> admin_browser.getControl(
... "Use all Ubuntu components available.").selected = True
- >>> admin_browser.getControl("Add PPA dependency").value = 'mark/ppa'
+ >>> admin_browser.getControl("Add PPA dependency").value = '~mark/ubuntu/ppa'
>>> admin_browser.getControl(
... "Basic (only released packages).").selected = True
@@ -513,9 +550,9 @@
>>> admin_browser.getLink('Edit PPA dependencies').click()
- >>> admin_browser.getControl("Add PPA dependency").value = 'no-priv/ppa'
+ >>> admin_browser.getControl("Add PPA dependency").value = '~no-priv/ubuntu/ppa'
>>> admin_browser.getControl(
- ... name="field.selected_dependencies").value = ['mark/ppa']
+ ... name="field.selected_dependencies").value = ['~mark/ubuntu/ppa']
>>> admin_browser.getControl(
... "Default (security dependencies and recommended updates)."
... ).selected = True
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2014-08-07 07:53:27 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2014-09-01 07:45:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test Archive features."""
@@ -1415,6 +1415,31 @@
PackagePublishingPocket.RELEASE)
self.assertContentEqual(archive.dependencies, [archive_dependency])
+ def test_dependency_has_different_distribution(self):
+ # A public archive may not depend on a private archive.
+ archive = self.factory.makeArchive()
+ distro = self.factory.makeDistribution()
+ dependency = self.factory.makeArchive(
+ distribution=distro, owner=archive.owner)
+ with person_logged_in(archive.owner):
+ with ExpectedException(
+ ArchiveDependencyError,
+ "Dependencies must be for the same distribution."):
+ archive.addArchiveDependency(
+ dependency, PackagePublishingPocket.RELEASE)
+
+ def test_dependency_is_disabled(self):
+ # A public archive may not depend on a private archive.
+ archive = self.factory.makeArchive()
+ dependency = self.factory.makeArchive(
+ owner=archive.owner, enabled=False)
+ with person_logged_in(archive.owner):
+ with ExpectedException(
+ ArchiveDependencyError,
+ "Dependencies must not be disabled."):
+ archive.addArchiveDependency(
+ dependency, PackagePublishingPocket.RELEASE)
+
class TestArchiveDependencies(TestCaseWithFactory):
=== modified file 'lib/lp/soyuz/tests/test_vocabularies.py'
--- lib/lp/soyuz/tests/test_vocabularies.py 2014-06-11 08:29:40 +0000
+++ lib/lp/soyuz/tests/test_vocabularies.py 2014-09-01 07:45:54 +0000
@@ -22,5 +22,5 @@
term = vocab.toTerm(archive)
self.assertThat(term, MatchesStructure.byEquality(
value=archive,
- token='%s/%s' % (archive.owner.name, archive.name),
+ token=archive.reference,
title='No description available'))
=== modified file 'lib/lp/soyuz/vocabularies.py'
--- lib/lp/soyuz/vocabularies.py 2013-09-10 06:28:26 +0000
+++ lib/lp/soyuz/vocabularies.py 2014-09-01 07:45:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the GNU
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the GNU
# Affero General Public License version 3 (see the file LICENSE).
"""Soyuz vocabularies."""
@@ -20,6 +20,7 @@
from zope.component import getUtility
from zope.interface import implements
from zope.schema.vocabulary import SimpleTerm
+from zope.security.interfaces import Unauthorized
from lp.registry.model.distroseries import DistroSeries
from lp.registry.model.person import Person
@@ -32,7 +33,11 @@
SQLObjectVocabularyBase,
)
from lp.soyuz.enums import ArchivePurpose
-from lp.soyuz.model.archive import Archive
+from lp.soyuz.interfaces.archive import IArchiveSet
+from lp.soyuz.model.archive import (
+ Archive,
+ get_enabled_archive_filter,
+ )
from lp.soyuz.model.component import Component
from lp.soyuz.model.distroarchseries import DistroArchSeries
from lp.soyuz.model.processor import Processor
@@ -87,7 +92,11 @@
_table = Archive
_orderBy = ['Person.name, Archive.name']
_clauseTables = ['Person']
+ # This should probably also filter by privacy, but that becomes
+ # problematic when you need to remove a dependency that you can no
+ # longer see.
_filter = And(
+ Archive._enabled == True,
Person.q.id == Archive.q.ownerID,
Archive.q.purpose == ArchivePurpose.PPA)
displayname = 'Select a PPA'
@@ -95,35 +104,20 @@
def toTerm(self, archive):
"""See `IVocabulary`."""
- description = archive.description
- if description:
- summary = description.splitlines()[0]
- else:
- summary = "No description available"
-
- token = '%s/%s' % (archive.owner.name, archive.name)
-
- return SimpleTerm(archive, token, summary)
+ summary = "No description available"
+ try:
+ if archive.description:
+ summary = archive.description.splitlines()[0]
+ except Unauthorized:
+ pass
+ return SimpleTerm(archive, archive.reference, summary)
def getTermByToken(self, token):
"""See `IVocabularyTokenized`."""
- try:
- owner_name, archive_name = token.split('/')
- except ValueError:
- raise LookupError(token)
-
- clause = And(
- self._filter,
- Person.name == owner_name,
- Archive.name == archive_name)
-
- obj = self._table.selectOne(
- clause, clauseTables=self._clauseTables)
-
- if obj is None:
- raise LookupError(token)
- else:
- return self.toTerm(obj)
+ obj = getUtility(IArchiveSet).getByReference(token)
+ if obj is None or not obj.enabled or not obj.is_ppa:
+ raise LookupError(token)
+ return self.toTerm(obj)
def search(self, query, vocab_filter=None):
"""Return a resultset of archives.
@@ -135,17 +129,27 @@
query = query.lower()
+ if query.startswith('~'):
+ query = query.strip('~')
try:
- owner_name, archive_name = query.split('/')
+ query_split = query.split('/')
+ if len(query_split) == 3:
+ owner_name, distro_name, archive_name = query_split
+ else:
+ owner_name, archive_name = query_split
except ValueError:
- clause = And(
- self._filter,
- Or(fti_search(Archive, query), fti_search(Person, query)))
+ search_clause = Or(
+ fti_search(Archive, query), fti_search(Person, query))
else:
- clause = And(
- self._filter, Person.name == owner_name,
- Archive.name == archive_name)
+ search_clause = And(
+ Person.name == owner_name, Archive.name == archive_name)
+ clause = And(
+ self._filter,
+ get_enabled_archive_filter(
+ getUtility(ILaunchBag).user, purpose=ArchivePurpose.PPA,
+ include_public=True),
+ search_clause)
return self._table.select(
clause, orderBy=self._orderBy, clauseTables=self._clauseTables)
References
