launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02332
[Merge] lp:~wgrant/launchpad/bug-701383-ppa-component-override into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-701383-ppa-component-override into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#701383 PPA component override should not be hardcoded everywhere
https://bugs.launchpad.net/bugs/701383
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-701383-ppa-component-override/+merge/46221
PPAs only support the 'main' component, with other components being overridden whenever publications are created. This restriction is not well encapsulated, with various parts of Soyuz and Code hardcoding 'main' for PPAs.
This branch moves most of the logic into Archive. Archive.default_component now specifies the target override component for single-component archives, and Archive.getComponentsForSeries specifies the permitted set of components. Both are cached, so they can be called cheaply. Callsites no longer have to hardcode PPA-based checks, instead just checking the archive's permitted and default components.
I think I've replaced every use of 'main' outside archive.py and tests. However, some instances of 'universe' and 'multiverse' remain due to more complex component mapping rules that apply to the primary archive :(
Implementation details
----------------------
Archive.getComponentsForSeries needed to be quick, so I refactored it to wrap both Archive.default_component and DistroSeries.components, both of which are cachedproperties. DistroSeries.components is now a list instead of a ResultSet, because ResultSets are too lazy for caching to be effective.
I've added a new ArchivePurpose tuple, FULL_COMPONENT_SUPPORT, enumerating those purposes that have the full set of archive components (ie. not partner archives or PPAs).
A side-effect of this change is that all partner publications will be overridden to 'partner'. However, all publications to partner are meant to be in 'partner' anyway, and the lack of this override has caused problems in past, so this is a positive thing.
--
https://code.launchpad.net/~wgrant/launchpad/bug-701383-ppa-component-override/+merge/46221
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-701383-ppa-component-override into lp:launchpad.
=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py 2011-01-10 03:22:42 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py 2011-01-14 03:33:01 +0000
@@ -62,7 +62,6 @@
from lp.code.model.sourcepackagerecipedata import SourcePackageRecipeData
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.job.model.job import Job
-from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
from lp.soyuz.model.buildfarmbuildjob import BuildFarmBuildJob
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
@@ -94,10 +93,11 @@
@property
def current_component(self):
- # Since recipes only build into PPAs, they should always build
- # in main. This will need to change once other archives are
- # supported.
- return getUtility(IComponentSet)['main']
+ # Only PPAs currently have a sane default component at the
+ # moment, but we only support recipes for PPAs.
+ component = self.archive.default_component
+ assert component is not None
+ return component
distroseries_id = Int(name='distroseries', allow_none=True)
distroseries = Reference(distroseries_id, 'DistroSeries.id')
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2010-12-21 00:26:41 +0000
+++ lib/lp/registry/model/distroseries.py 2011-01-14 03:33:01 +0000
@@ -270,18 +270,18 @@
""" % self.id,
clauseTables=["ComponentSelection"])
- @property
+ @cachedproperty
def components(self):
"""See `IDistroSeries`."""
# XXX julian 2007-06-25
# This is filtering out the partner component for now, until
# the second stage of the partner repo arrives in 1.1.8.
- return Component.select("""
+ return list(Component.select("""
ComponentSelection.distroseries = %s AND
Component.id = ComponentSelection.component AND
Component.name != 'partner'
""" % self.id,
- clauseTables=["ComponentSelection"])
+ clauseTables=["ComponentSelection"]))
@property
def answers_usage(self):
@@ -1540,7 +1540,7 @@
latest_releases = series.getCurrentSourceReleases(
packagenames)
for spph in spphs:
- latest_release = latest_releases.get(spph.meta_sourcepackage, None)
+ latest_release = latest_releases.get(spph.meta_sourcepackage)
if latest_release is not None and apt_pkg.VersionCompare(
latest_release.version, spph.source_package_version) > 0:
version = latest_release
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2011-01-03 22:15:46 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2011-01-14 03:33:01 +0000
@@ -21,6 +21,7 @@
'CannotUploadToPPA',
'CannotUploadToPocket',
'DistroSeriesNotFound',
+ 'FULL_COMPONENT_SUPPORT',
'IArchive',
'IArchiveAppend',
'IArchiveEdit',
@@ -334,6 +335,10 @@
debug_archive = Attribute(
"The archive into which debug binaries should be uploaded.")
+ default_component = Attribute(
+ "The default component for this archive. Publications without a "
+ "valid component will be assigned this one.")
+
archive_url = Attribute("External archive URL.")
is_ppa = Attribute("True if this archive is a PPA.")
@@ -1670,4 +1675,10 @@
ArchivePurpose.COPY,
)
+FULL_COMPONENT_SUPPORT = (
+ ArchivePurpose.PRIMARY,
+ ArchivePurpose.DEBUG,
+ ArchivePurpose.COPY,
+ )
+
# Circular dependency issues fixed in _schema_circular_imports.py
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2011-01-03 22:15:46 +0000
+++ lib/lp/soyuz/model/archive.py 2011-01-14 03:33:01 +0000
@@ -89,7 +89,10 @@
from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
from lp.registry.model.teammembership import TeamParticipation
from lp.services.job.interfaces.job import JobStatus
-from lp.services.propertycache import get_property_cache
+from lp.services.propertycache import (
+ cachedproperty,
+ get_property_cache,
+ )
from lp.soyuz.adapters.archivedependencies import expand_dependencies
from lp.soyuz.adapters.packagelocation import PackageLocation
from lp.soyuz.enums import (
@@ -112,6 +115,7 @@
CannotUploadToPPA,
default_name_by_purpose,
DistroSeriesNotFound,
+ FULL_COMPONENT_SUPPORT,
IArchive,
IArchiveSet,
IDistributionArchive,
@@ -415,6 +419,16 @@
else:
return self
+ @cachedproperty
+ def default_component(self):
+ """See `IArchive`."""
+ if self.is_partner:
+ return getUtility(IComponentSet)['partner']
+ elif self.is_ppa:
+ return getUtility(IComponentSet)['main']
+ else:
+ return None
+
@property
def archive_url(self):
"""See `IArchive`."""
@@ -841,12 +855,13 @@
return permission
def getComponentsForSeries(self, distroseries):
- if self.is_partner:
- return [getUtility(IComponentSet)['partner']]
- elif self.is_ppa:
- return [getUtility(IComponentSet)['main']]
- else:
+ """See `IArchive`."""
+ # DistroSeries.components and Archive.default_component are
+ # cachedproperties, so this is fairly cheap.
+ if self.purpose in FULL_COMPONENT_SUPPORT:
return distroseries.components
+ else:
+ return [self.default_component]
def updateArchiveCache(self):
"""See `IArchive`."""
@@ -948,9 +963,10 @@
raise ArchiveDependencyError(
"Non-primary archives only support the RELEASE pocket.")
if (component is not None and
- component.id is not getUtility(IComponentSet)['main'].id):
+ component != dependency.default_component):
raise ArchiveDependencyError(
- "Non-primary archives only support the 'main' component.")
+ "Non-primary archives only support the '%s' component." %
+ dependency.default_component.name)
return ArchiveDependency(
archive=self, dependency=dependency, pocket=pocket,
@@ -1086,7 +1102,7 @@
# interface will no longer require them because we can
# then relax the database constraint on
# ArchivePermission.
- component_or_package = getUtility(IComponentSet)['main']
+ component_or_package = self.default_component
# Flatly refuse uploads to copy archives, at least for now.
if self.is_copy:
@@ -1234,8 +1250,10 @@
else:
name = None
- if name is None or name != 'main':
- raise InvalidComponent("Component for PPAs should be 'main'")
+ if name != self.default_component.name:
+ raise InvalidComponent(
+ "Component for PPAs should be '%s'" %
+ self.default_component.name)
permission_set = getUtility(IArchivePermissionSet)
return permission_set.newComponentUploader(
=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py 2010-12-16 20:01:09 +0000
+++ lib/lp/soyuz/model/publishing.py 2011-01-14 03:33:01 +0000
@@ -83,7 +83,6 @@
BuildSetStatus,
IBinaryPackageBuildSet,
)
-from lp.soyuz.interfaces.component import IComponentSet
from lp.soyuz.interfaces.publishing import (
active_publishing_status,
IBinaryPackageFilePublishing,
@@ -120,6 +119,21 @@
'pool', poolify(source_name, component_name))
+def maybe_override_component(archive, distroseries, component):
+ """Override the component to fit in the archive, if possible.
+
+ If the archive has a default component, and it forbids use of the
+ requested component in the requested series, use the default.
+
+ If there is no default, just return the given component.
+ """
+ permitted_components = archive.getComponentsForSeries(distroseries)
+ if (component not in permitted_components and
+ archive.default_component is not None):
+ return archive.default_component
+ return component
+
+
class FilePublishingBase:
"""Base class to publish files in the archive."""
@@ -817,7 +831,6 @@
assert self.component in (
self.archive.getComponentsForSeries(self.distroseries))
-
def _proxied_urls(self, files, parent):
"""Run the files passed through `ProxiedLibraryFileAlias`."""
return [
@@ -1244,19 +1257,10 @@
def copyBinariesTo(self, binaries, distroseries, pocket, archive):
"""See `IPublishingSet`."""
-
- # If the target archive is a ppa then we will need to override
- # the component for each copy - so lookup the main component
- # here once.
- override_component = None
- if archive.is_ppa:
- override_component = getUtility(IComponentSet)['main']
-
secure_copies = []
for binary in binaries:
binarypackagerelease = binary.binarypackagerelease
- target_component = override_component or binary.component
# XXX 2010-09-28 Julian bug=649859
# This piece of code duplicates the logic in
@@ -1293,16 +1297,10 @@
distroarchseries=distroarchseries)
if binary_in_destination.count() == 0:
- pub = BinaryPackagePublishingHistory(
- archive=archive,
- binarypackagerelease=binarypackagerelease,
- distroarchseries=distroarchseries,
- component=target_component,
- section=binary.section,
- priority=binary.priority,
- status=PackagePublishingStatus.PENDING,
- datecreated=UTC_NOW,
- pocket=pocket)
+ pub = self.newBinaryPublication(
+ archive, binarypackagerelease, distroarchseries,
+ binary.component, binary.section, binary.priority,
+ pocket)
secure_copies.append(pub)
return secure_copies
@@ -1311,15 +1309,12 @@
distroarchseries, component, section, priority,
pocket):
"""See `IPublishingSet`."""
- if archive.is_ppa:
- # PPA component must always be 'main', so we override it
- # here.
- component = getUtility(IComponentSet)['main']
pub = BinaryPackagePublishingHistory(
archive=archive,
binarypackagerelease=binarypackagerelease,
distroarchseries=distroarchseries,
- component=component,
+ component=maybe_override_component(
+ archive, distroarchseries.distroseries, component),
section=section,
priority=priority,
status=PackagePublishingStatus.PENDING,
@@ -1332,16 +1327,13 @@
distroseries, component, section, pocket,
ancestor=None):
"""See `IPublishingSet`."""
- if archive.is_ppa:
- # PPA component must always be 'main', so we override it
- # here.
- component = getUtility(IComponentSet)['main']
pub = SourcePackagePublishingHistory(
distroseries=distroseries,
pocket=pocket,
archive=archive,
sourcepackagerelease=sourcepackagerelease,
- component=component,
+ component=maybe_override_component(
+ archive, distroseries, component),
section=section,
status=PackagePublishingStatus.PENDING,
datecreated=UTC_NOW,
=== modified file 'lib/lp/soyuz/scripts/initialise_distroseries.py'
--- lib/lp/soyuz/scripts/initialise_distroseries.py 2010-12-21 00:26:41 +0000
+++ lib/lp/soyuz/scripts/initialise_distroseries.py 2011-01-14 03:33:01 +0000
@@ -121,7 +121,7 @@
raise InitialisationError(error)
if self.distroseries.architectures.count():
raise InitialisationError(error)
- if self.distroseries.components.count():
+ if len(self.distroseries.components):
raise InitialisationError(error)
if self.distroseries.sections.count():
raise InitialisationError(error)
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2010-12-13 06:44:16 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2011-01-14 03:33:01 +0000
@@ -27,6 +27,7 @@
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
from lp.services.job.interfaces.job import JobStatus
+from lp.services.propertycache import clear_property_cache
from lp.services.worlddata.interfaces.country import ICountrySet
from lp.soyuz.enums import (
ArchivePurpose,
@@ -1474,10 +1475,11 @@
# The primary archive uses the series' defined components.
archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
self.assertEquals(
- 0, archive.getComponentsForSeries(self.series).count())
+ 0, len(archive.getComponentsForSeries(self.series)))
ComponentSelection(distroseries=self.series, component=self.comp1)
ComponentSelection(distroseries=self.series, component=self.comp2)
+ clear_property_cache(self.series)
self.assertEquals(
set((self.comp1, self.comp2)),
@@ -1501,6 +1503,26 @@
[main_comp], list(archive.getComponentsForSeries(self.series)))
+class TestDefaultComponent(TestCaseWithFactory):
+ """Tests for Archive.default_component."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_default_component_for_other_archives(self):
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
+ self.assertIs(None, archive.default_component)
+
+ def test_default_component_for_partner(self):
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PARTNER)
+ self.assertEquals(
+ getUtility(IComponentSet)['partner'], archive.default_component)
+
+ def test_default_component_for_ppas(self):
+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+ self.assertEquals(
+ getUtility(IComponentSet)['main'], archive.default_component)
+
+
class TestGetPockets(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
