Launchpad itself

Merge lp:~julian-edwards/launchpad/copy-archive-fix-component into lp:launchpad

  1. copy-archive-fix-component
  2. Merge into devel
Proposed by Julian Edwards
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 11088
Proposed branch: lp:~julian-edwards/launchpad/copy-archive-fix-component
Merge into: lp:launchpad
Diff against target: 92 lines (+21/-17)
2 files modified
lib/lp/soyuz/model/packagecloner.py (+7/-2)
lib/lp/soyuz/scripts/tests/test_populatearchive.py (+14/-15)
To merge this branch: bzr merge lp:~julian-edwards/launchpad/copy-archive-fix-component
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) release-critical Approve
Jeroen T. Vermeulen (community) Approve
Review via email: mp+28871@code.launchpad.net

Description of the change

= Summary =
Creation of COPY (rebuild) archives doesn't respect the specified component.

This fix is required for an urgent cherry pick as the Linaro guys need a
rebuild for ARM set up.

== Pre-implementation notes ==
Chatted to Muharem who identified the culprit code which is an SQL query that
ignores the component specified for the copy operation.

== Implementation details ==
Add the component to the query if it's been specified. Also fix a bunch of
tests that relied on the broken behaviour.

== Tests ==
bin/test -cvv test_populatearchive

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/model/packagecloner.py
  lib/lp/soyuz/scripts/tests/test_populatearchive.py

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Looks good.

review: Approve
Revision history for this message
Francis J. Lacoste (flacoste) :
review: Approve (release-critical)
Revision history for this message
James Westby (james-w) wrote :

Hi Julian,

You should fix the docstring of the test that asserts the behaviour
that we don't want.

57 When copying you specify a component, but that component doesn't
58 limit the packages copied. We create a source in main and one in
59 universe, and then copy with --component main, and expect to see
60 both sources in the copy.

Thanks,

James

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/model/packagecloner.py'
2--- lib/lp/soyuz/model/packagecloner.py 2010-01-12 08:36:58 +0000
3+++ lib/lp/soyuz/model/packagecloner.py 2010-06-30 15:09:31 +0000
4@@ -305,7 +305,7 @@
5 to be copied.
6 """
7 store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
8- store.execute('''
9+ query = '''
10 INSERT INTO SourcePackagePublishingHistory (
11 sourcepackagerelease, distroseries, status, component,
12 section, archive, datecreated, datepublished, pocket)
13@@ -321,7 +321,12 @@
14 UTC_NOW, destination.pocket, origin.distroseries,
15 PackagePublishingStatus.PENDING,
16 PackagePublishingStatus.PUBLISHED,
17- origin.pocket, origin.archive))
18+ origin.pocket, origin.archive)
19+
20+ if origin.component:
21+ query += "and spph.component = %s" % sqlvalues(origin.component)
22+
23+ store.execute(query)
24
25 def packageSetDiff(self, origin, destination, logger=None):
26 """Please see `IPackageCloner`."""
27
28=== modified file 'lib/lp/soyuz/scripts/tests/test_populatearchive.py'
29--- lib/lp/soyuz/scripts/tests/test_populatearchive.py 2010-06-22 11:14:02 +0000
30+++ lib/lp/soyuz/scripts/tests/test_populatearchive.py 2010-06-30 15:09:31 +0000
31@@ -119,7 +119,6 @@
32 '--to-distribution', distro_name, '--to-suite', 'hoary',
33 '--to-archive', archive_name, '--to-user', 'salgado', '--reason',
34 '"copy archive from %s"' % datetime.ctime(datetime.utcnow()),
35- '--component', 'main'
36 ]
37
38 # Start archive population now!
39@@ -206,7 +205,7 @@
40 def getScript(self, test_args=None):
41 """Return an ArchivePopulator instance."""
42 if test_args is None:
43- test_args = []
44+ test_args = []
45 script = ArchivePopulator("test copy archives", test_args=test_args)
46 script.logger = QuietFakeLogger()
47 script.txn = self.layer.txn
48@@ -502,24 +501,24 @@
49 architectures=["386", "amd64"])
50 self.checkBuilds(copy_archive, [package_info, package_info])
51
52- def testCopyArchiveCopiesAllComponents(self):
53- """Test that packages from all components are copied.
54+ def testCopyArchiveCopiesRightComponents(self):
55+ """Test that packages from the right components are copied.
56
57- When copying you specify a component, but that component doesn't
58+ When copying you specify a component, that component should
59 limit the packages copied. We create a source in main and one in
60 universe, and then copy with --component main, and expect to see
61- both sources in the copy.
62+ only main in the copy.
63 """
64- package_infos = [
65- PackageInfo(
66+ package_info_universe = PackageInfo(
67 "bzr", "2.1", status=PackagePublishingStatus.PUBLISHED,
68- component="universe"),
69- PackageInfo(
70+ component="universe")
71+ package_info_main = PackageInfo(
72 "apt", "2.2", status=PackagePublishingStatus.PUBLISHED,
73- component="main")]
74- copy_archive, distroseries = self.makeCopyArchive(package_infos,
75- component="main")
76- self.checkBuilds(copy_archive, package_infos)
77+ component="main")
78+ package_infos_both = [package_info_universe, package_info_main]
79+ copy_archive, distroseries = self.makeCopyArchive(
80+ package_infos_both, component="main")
81+ self.checkBuilds(copy_archive, [package_info_main])
82
83 def testCopyFromPPA(self):
84 """Test we can create a copy archive with a PPA as the source."""
85@@ -545,7 +544,7 @@
86 archive_name = self.getTargetArchiveName(distroseries.distribution)
87 copy_archive = self.copyArchive(
88 distroseries, archive_name, owner, from_user=ppa_owner_name,
89- from_archive=ppa_name)
90+ from_archive=ppa_name, component=package_info.component)
91 self.checkCopiedSources(
92 copy_archive, distroseries, [package_info])
93