launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06062
[Merge] lp:~cjwatson/launchpad/archive-copy-packages-source-series into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/archive-copy-packages-source-series into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #913722 in Launchpad itself: "Archive.copyPackages doesn't allow specifying a source series"
https://bugs.launchpad.net/launchpad/+bug/913722
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/archive-copy-packages-source-series/+merge/87942
== Summary ==
Archive.copyPackages doesn't allow specifying a source distroseries. This means that autosyncs from Debian using this method copy the newest version available no matter what - up to and including versions in experimental!
https://lists.ubuntu.com/archives/ubuntu-release/2012-January/000672.html
== Proposed fix ==
Add an optional from_series parameter.
== Implementation details ==
I made from_series be a distroseries name rather than an object, matching to_series. This necessitated some mild tedium in Archive._text_to_series to fetch series from the source distribution.
== Tests ==
bin/test -vvct test_archive_webservice -t soyuz/doc/archive.txt -t soyuz.tests.test_archive
== Demo and Q/A ==
Find a package in dogfood that has dogfood/Ubuntu/oneiric (or precise) < dogfood/Debian/wheezy < dogfood/Debian/sid; hopefully there's at least one. Use Archive.copyPackages(from_series='wheezy') and make sure that the wheezy version gets copied and not the sid version.
== lint ==
./lib/lp/soyuz/model/archive.py
348: redefinition of function 'private' from line 344
Just lint being confused about setter properties.
--
https://code.launchpad.net/~cjwatson/launchpad/archive-copy-packages-source-series/+merge/87942
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/archive-copy-packages-source-series into lp:launchpad.
=== modified file 'lib/lp/soyuz/browser/tests/test_archive_webservice.py'
--- lib/lp/soyuz/browser/tests/test_archive_webservice.py 2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_webservice.py 2012-01-09 14:58:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -338,13 +338,14 @@
with person_logged_in(target_archive.owner):
target_archive.newComponentUploader(uploader_dude, "universe")
transaction.commit()
- return (source_archive, source_name, target_archive, to_pocket,
- to_series, uploader_dude, sponsored_dude, version)
+ return (source, source_archive, source_name, target_archive,
+ to_pocket, to_series, uploader_dude, sponsored_dude, version)
def test_copyPackage(self):
"""Basic smoke test"""
- (source_archive, source_name, target_archive, to_pocket, to_series,
- uploader_dude, sponsored_dude, version) = self.setup_data()
+ (source, source_archive, source_name, target_archive, to_pocket,
+ to_series, uploader_dude, sponsored_dude,
+ version) = self.setup_data()
ws_target_archive = self.wsObject(target_archive, user=uploader_dude)
ws_source_archive = self.wsObject(source_archive)
@@ -363,8 +364,9 @@
def test_copyPackages(self):
"""Basic smoke test"""
- (source_archive, source_name, target_archive, to_pocket, to_series,
- uploader_dude, sponsored_dude, version) = self.setup_data()
+ (source, source_archive, source_name, target_archive, to_pocket,
+ to_series, uploader_dude, sponsored_dude,
+ version) = self.setup_data()
ws_target_archive = self.wsObject(target_archive, user=uploader_dude)
ws_source_archive = self.wsObject(source_archive)
@@ -373,7 +375,8 @@
ws_target_archive.copyPackages(
source_names=[source_name], from_archive=ws_source_archive,
to_pocket=to_pocket.name, to_series=to_series.name,
- include_binaries=False, sponsored=ws_sponsored_dude)
+ from_series=source.distroseries.name, include_binaries=False,
+ sponsored=ws_sponsored_dude)
transaction.commit()
job_source = getUtility(IPlainPackageCopyJobSource)
=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt 2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/doc/archive.txt 2012-01-09 14:58:27 +0000
@@ -2342,6 +2342,25 @@
...
NoSuchDistroSeries: No such distribution series: 'badseries'.
+If a package exists in multiple distroseries, we can use the `from_series`
+parameter to select the distroseries to synchronise from:
+
+ >>> test_publisher.addFakeChroots(breezy_autotest)
+ >>> discard = test_publisher.getPubSource(
+ ... sourcename="package-multiseries", version="1.0",
+ ... archive=cprov.archive, status=PackagePublishingStatus.PUBLISHED)
+ >>> discard = test_publisher.getPubSource(
+ ... sourcename="package-multiseries", version="1.1",
+ ... distroseries=breezy_autotest, archive=cprov.archive,
+ ... status=PackagePublishingStatus.PUBLISHED)
+ >>> mark.archive.syncSources(
+ ... ["package-multiseries"], cprov.archive, "release",
+ ... from_series="hoary", person=mark)
+ >>> mark_multiseries = mark.archive.getPublishedSources(
+ ... name="package-multiseries").one()
+ >>> print mark_multiseries.sourcepackagerelease.version
+ 1.0
+
We can also specify a single source to be copied with the `syncSource`
call. This allows a version to be specified so older versions can be
pulled.
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2012-01-01 02:58:52 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2012-01-09 14:58:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0211,E0213
@@ -1268,7 +1268,14 @@
from_archive=Reference(schema=Interface),
#Really IArchive, see below
to_pocket=TextLine(title=_("Pocket name")),
- to_series=TextLine(title=_("Distroseries name"), required=False),
+ to_series=TextLine(
+ title=_("Distroseries name"),
+ description=_("The distro series to copy packages into."),
+ required=False),
+ from_series=TextLine(
+ title=_("Distroseries name"),
+ description=_("The distro series to copy packages from."),
+ required=False),
include_binaries=Bool(
title=_("Include Binaries"),
description=_("Whether or not to copy binaries already built for"
@@ -1282,7 +1289,7 @@
@export_write_operation()
@operation_for_version('devel')
def copyPackages(source_names, from_archive, to_pocket, person,
- to_series=None, include_binaries=False,
+ to_series=None, from_series=None, include_binaries=False,
sponsored=None):
"""Copy multiple named sources into this archive from another.
@@ -1298,6 +1305,7 @@
:param from_archive: the source archive from which to copy.
:param to_pocket: the target pocket (as a string).
:param to_series: the target distroseries (as a string).
+ :param from_series: the source distroseries (as a string).
:param include_binaries: optional boolean, controls whether or not
the published binaries for each given source should also be
copied along with the source.
@@ -1325,7 +1333,14 @@
from_archive=Reference(schema=Interface),
#Really IArchive, see below
to_pocket=TextLine(title=_("Pocket name")),
- to_series=TextLine(title=_("Distroseries name"), required=False),
+ to_series=TextLine(
+ title=_("Distroseries name"),
+ description=_("The distro series to copy packages into."),
+ required=False),
+ from_series=TextLine(
+ title=_("Distroseries name"),
+ description=_("The distro series to copy packages from."),
+ required=False),
include_binaries=Bool(
title=_("Include Binaries"),
description=_("Whether or not to copy binaries already built for"
@@ -1335,7 +1350,7 @@
# Source_names is a string because exporting a SourcePackageName is
# rather nonsensical as it only has id and name columns.
def syncSources(source_names, from_archive, to_pocket, to_series=None,
- include_binaries=False, person=None):
+ from_series=None, include_binaries=False, person=None):
"""Synchronise (copy) named sources into this archive from another.
It will copy the most recent PUBLISHED versions of the named
@@ -1350,6 +1365,7 @@
:param from_archive: the source archive from which to copy.
:param to_pocket: the target pocket (as a string).
:param to_series: the target distroseries (as a string).
+ :param from_series: the source distroseries (as a string).
:param include_binaries: optional boolean, controls whether or not
the published binaries for each given source should also be
copied along with the source.
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2012-01-06 11:08:30 +0000
+++ lib/lp/soyuz/model/archive.py 2012-01-09 14:58:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0611,W0212
@@ -1526,11 +1526,12 @@
reason)
def syncSources(self, source_names, from_archive, to_pocket,
- to_series=None, include_binaries=False, person=None):
+ to_series=None, from_series=None, include_binaries=False,
+ person=None):
"""See `IArchive`."""
# Find and validate the source package names in source_names.
sources = self._collectLatestPublishedSources(
- from_archive, source_names)
+ from_archive, from_series, source_names)
self._copySources(
sources, to_pocket, to_series, include_binaries,
person=person)
@@ -1591,13 +1592,13 @@
sponsored=sponsored)
def copyPackages(self, source_names, from_archive, to_pocket,
- person, to_series=None, include_binaries=None,
- sponsored=None):
+ person, to_series=None, from_series=None,
+ include_binaries=None, sponsored=None):
"""See `IArchive`."""
self._checkCopyPackageFeatureFlags()
sources = self._collectLatestPublishedSources(
- from_archive, source_names)
+ from_archive, from_series, source_names)
if not sources:
raise CannotCopy(
"None of the supplied package names are published")
@@ -1635,13 +1636,16 @@
copy_policy=PackageCopyPolicy.MASS_SYNC,
include_binaries=include_binaries, sponsored=sponsored)
- def _collectLatestPublishedSources(self, from_archive, source_names):
+ def _collectLatestPublishedSources(self, from_archive, from_series,
+ source_names):
"""Private helper to collect the latest published sources for an
archive.
:raises NoSuchSourcePackageName: If any of the source_names do not
exist.
"""
+ from_series_obj = self._text_to_series(
+ from_series, distribution=from_archive.distribution)
# XXX bigjools bug=810421
# This code is inefficient. It should try to bulk load all the
# sourcepackagenames and publications instead of iterating.
@@ -1654,7 +1658,7 @@
# Grabbing the item at index 0 ensures it's the most recent
# publication.
published_sources = from_archive.getPublishedSources(
- name=name, exact_match=True,
+ name=name, distroseries=from_series_obj, exact_match=True,
status=(PackagePublishingStatus.PUBLISHED,
PackagePublishingStatus.PENDING))
first_source = published_sources.first()
@@ -1662,10 +1666,12 @@
sources.append(first_source)
return sources
- def _text_to_series(self, to_series):
+ def _text_to_series(self, to_series, distribution=None):
+ if distribution is None:
+ distribution = self.distribution
if to_series is not None:
result = getUtility(IDistroSeriesSet).queryByName(
- self.distribution, to_series)
+ distribution, to_series)
if result is None:
raise NoSuchDistroSeries(to_series)
series = result
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2012-01-09 14:58:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test Archive features."""
@@ -443,7 +443,7 @@
["1.0", "1.1", "2.0"],
[sourcepackagename, sourcepackagename, other_spn])
pubs = removeSecurityProxy(archive)._collectLatestPublishedSources(
- archive, ["foo"])
+ archive, None, ["foo"])
self.assertEqual(1, len(pubs))
self.assertEqual('1.1', pubs[0].source_package_version)
@@ -460,10 +460,32 @@
["1.0", "1.1", "2.0"],
[sourcepackagename, sourcepackagename, other_spn])
pubs = removeSecurityProxy(archive)._collectLatestPublishedSources(
- archive, ["foo"])
+ archive, None, ["foo"])
self.assertEqual(1, len(pubs))
self.assertEqual('1.0', pubs[0].source_package_version)
+ def test_collectLatestPublishedSources_multiple_distroseries(self):
+ # The helper method selects the correct publication from multiple
+ # distroseries.
+ sourcepackagename = self.factory.makeSourcePackageName(name="foo")
+ archive = self.factory.makeArchive()
+ distroseries_one = self.factory.makeDistroSeries(
+ distribution=archive.distribution)
+ distroseries_two = self.factory.makeDistroSeries(
+ distribution=archive.distribution)
+ self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename=sourcepackagename, archive=archive,
+ distroseries=distroseries_one, version="1.0",
+ status=PackagePublishingStatus.PUBLISHED)
+ self.factory.makeSourcePackagePublishingHistory(
+ sourcepackagename=sourcepackagename, archive=archive,
+ distroseries=distroseries_two, version="1.1",
+ status=PackagePublishingStatus.PUBLISHED)
+ pubs = removeSecurityProxy(archive)._collectLatestPublishedSources(
+ archive, distroseries_one.name, ["foo"])
+ self.assertEqual(1, len(pubs))
+ self.assertEqual("1.0", pubs[0].source_package_version)
+
class TestArchiveCanUpload(TestCaseWithFactory):
"""Test the various methods that verify whether uploads are allowed to
@@ -2302,6 +2324,45 @@
to_pocket.name, to_series=to_series.name, include_binaries=False,
person=person)
+ def test_copyPackages_with_multiple_distroseries(self):
+ # The from_series parameter selects a source distroseries.
+ (source, source_archive, source_name, target_archive, to_pocket,
+ to_series, version) = self._setup_copy_data()
+ new_distroseries = self.factory.makeDistroSeries(
+ distribution=source_archive.distribution)
+ new_version = "%s.1" % version
+ new_spr = self.factory.makeSourcePackageRelease(
+ archive=source_archive, distroseries=new_distroseries,
+ sourcepackagename=source_name, version=new_version)
+ self.factory.makeSourcePackagePublishingHistory(
+ archive=source_archive, distroseries=new_distroseries,
+ sourcepackagerelease=new_spr)
+
+ with person_logged_in(target_archive.owner):
+ target_archive.copyPackages(
+ [source_name], source_archive, to_pocket.name,
+ to_series=to_series.name,
+ from_series=source.distroseries.name, include_binaries=False,
+ person=target_archive.owner)
+
+ # There should be one copy job with the correct version.
+ job_source = getUtility(IPlainPackageCopyJobSource)
+ copy_job = job_source.getActiveJobs(target_archive).one()
+ self.assertEqual(version, copy_job.package_version)
+
+ # If we now do another copy without the from_series parameter, it
+ # selects the newest version in the source archive.
+ with person_logged_in(target_archive.owner):
+ target_archive.copyPackages(
+ [source_name], source_archive, to_pocket.name,
+ to_series=to_series.name, include_binaries=False,
+ person=target_archive.owner)
+
+ copy_jobs = job_source.getActiveJobs(target_archive)
+ self.assertEqual(2, copy_jobs.count())
+ self.assertEqual(copy_job, copy_jobs[0])
+ self.assertEqual(new_version, copy_jobs[1].package_version)
+
class TestgetAllPublishedBinaries(TestCaseWithFactory):
Follow ups
