Launchpad itself

Merge lp:~wgrant/launchpad/more-a-f-cleanup into lp:launchpad

  1. more-a-f-cleanup
  2. Merge into devel
Proposed by William Grant
Status: Merged
Approved by: Jonathan Lange
Approved revision: no longer in the source branch.
Merged at revision: 11733
Proposed branch: lp:~wgrant/launchpad/more-a-f-cleanup
Merge into: lp:launchpad
Diff against target: 473 lines (+105/-102)
6 files modified
lib/lp/archivepublisher/ftparchive.py (+44/-51)
lib/lp/archivepublisher/publishing.py (+11/-19)
lib/lp/archivepublisher/tests/apt-data/apt.conf (+30/-30)
lib/lp/archivepublisher/tests/test_ftparchive.py (+16/-0)
lib/lp/registry/interfaces/pocket.py (+3/-0)
lib/lp/registry/model/distribution.py (+1/-2)
To merge this branch: bzr merge lp:~wgrant/launchpad/more-a-f-cleanup
Reviewer Review Type Date Requested Status
Jonathan Lange (community) Approve
Review via email: mp+38646@code.launchpad.net

Commit message

Clean up use of pocketsuffix and suites in lp.archivepublisher.

Description of the change

This branch is just a cleanup of lp.archivepublisher, mainly related to pocket handling.

Nothing should use pocketsuffix directly -- if iteration over the pockets is wanted, PackagePublishingPocket.items is your friend. If you want to create a suite name, use distroseries.getSuite(pocket).

I also renamed a few insanities like 'dr_pocketed' and 'full_name' to 'suite', as they should have been from the start. suffixpocket also turns out to be unused in archivepublisher, so I moved it back to Registry with the rest.

apt.conf's diff is just a reordering of stanzas to match the proper order of pockets as defined in PPP, rather than the pocketsuffix dict order.

To post a comment you must log in.
Revision history for this message
Jonathan Lange (jml) wrote :

On Sun, Oct 17, 2010 at 11:19 AM, William Grant <email address hidden> wrote:
> William Grant has proposed merging lp:~wgrant/launchpad/more-a-f-cleanup into lp:launchpad/devel.
>
> Requested reviews:
>  Launchpad code reviewers (launchpad-reviewers)
>
>
> This branch is just a cleanup of lp.archivepublisher, mainly related to pocket handling.
>
> Nothing should use pocketsuffix directly -- if iteration over the pockets is wanted, PackagePublishingPocket.items is your friend. If you want to create a suite name, use distroseries.getSuite(pocket).
>

Heck yes.

> I also renamed a few insanities like 'dr_pocketed' and 'full_name' to 'suite', as they should have been from the start. suffixpocket also turns out to be unused in archivepublisher, so I moved it back to Registry with the rest.
>
> apt.conf's diff is just a reordering of stanzas to match the proper order of pockets as defined in PPP, rather than the pocketsuffix dict order.
>

Sounds good.

The code looks good too. All of the changes seem pretty mechanical to
me, but well worth it.

Thanks,
jml

Revision history for this message
Jonathan Lange (jml) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/ftparchive.py'
2--- lib/lp/archivepublisher/ftparchive.py 2010-10-15 11:13:01 +0000
3+++ lib/lp/archivepublisher/ftparchive.py 2010-10-17 10:19:44 +0000
4@@ -23,10 +23,7 @@
5 MAIN_STORE,
6 )
7 from lp.archivepublisher.utils import process_in_batches
8-from lp.registry.interfaces.pocket import (
9- PackagePublishingPocket,
10- pocketsuffix,
11- )
12+from lp.registry.interfaces.pocket import PackagePublishingPocket
13 from lp.registry.model.sourcepackagename import SourcePackageName
14 from lp.soyuz.enums import PackagePublishingStatus
15 from lp.soyuz.model.component import Component
16@@ -227,26 +224,25 @@
17 ".".join(["override", distroseries.name, comp,
18 "debian-installer"]))
19
20- full_distroseries_name = distroseries.name + pocketsuffix[pocket]
21-
22- # Touch the source file lists and override files
23- f_touch(self._config.overrideroot,
24- ".".join(["override", full_distroseries_name, comp]))
25- f_touch(self._config.overrideroot,
26- ".".join(["override", full_distroseries_name, "extra", comp]))
27- f_touch(self._config.overrideroot,
28- ".".join(["override", full_distroseries_name, comp, "src"]))
29-
30- f_touch(self._config.overrideroot,
31- "_".join([full_distroseries_name, comp, "source"]))
32+ suite = distroseries.getSuite(pocket)
33+
34+ # Create empty override lists.
35+ for path in ((comp, ), ("extra", comp), (comp, "src")):
36+ f_touch(os.path.join(
37+ self._config.overrideroot,
38+ ".".join(("override", suite) + path)))
39+
40+ # Create empty file lists.
41+ def touch_list(*parts):
42+ f_touch(os.path.join(
43+ self._config.overrideroot,
44+ "_".join((suite, ) + parts)))
45+ touch_list(comp, "source")
46
47 for arch in self._config.archTagsForSeries(distroseries.name):
48 # Touch more file lists for the archs.
49- f_touch(self._config.overrideroot,
50- "_".join([full_distroseries_name, comp, "binary-"+arch]))
51- f_touch(self._config.overrideroot,
52- "_".join([full_distroseries_name, comp,
53- "debian-installer", "binary-"+arch]))
54+ touch_list(comp, "binary-" + arch)
55+ touch_list(comp, "debian-installer", "binary-" + arch)
56
57 #
58 # Override Generation
59@@ -291,7 +287,7 @@
60 SourcePackagePublishingHistory.status ==
61 PackagePublishingStatus.PUBLISHED)
62
63- suite = distroseries.name + pocketsuffix[pocket]
64+ suite = distroseries.getSuite(pocket)
65 def add_suite(result):
66 name, component, section = result
67 return (name, suite, component, section)
68@@ -346,7 +342,7 @@
69 BinaryPackagePublishingHistory.status ==
70 PackagePublishingStatus.PUBLISHED)
71
72- suite = distroseries.name + pocketsuffix[pocket]
73+ suite = distroseries.getSuite(pocket)
74 def add_suite(result):
75 name, component, section, priority = result
76 return (name, suite, component, section, priority.title.lower())
77@@ -571,7 +567,7 @@
78 SourcePackageFilePublishing.publishingstatus ==
79 PackagePublishingStatus.PUBLISHED)
80
81- suite = distroseries.name + pocketsuffix[pocket]
82+ suite = distroseries.getSuite(pocket)
83 def add_suite(result):
84 name, filename, component = result
85 return (name, suite, filename, component)
86@@ -608,7 +604,7 @@
87 BinaryPackageFilePublishing.publishingstatus ==
88 PackagePublishingStatus.PUBLISHED)
89
90- suite = distroseries.name + pocketsuffix[pocket]
91+ suite = distroseries.getSuite(pocket)
92 def add_suite(result):
93 name, filename, component, architecturetag = result
94 architecture = 'binary-' + architecturetag
95@@ -621,7 +617,7 @@
96 def generateFileLists(self, fullpublish=False):
97 """Collect currently published FilePublishings and write filelists."""
98 for distroseries in self.distroseries:
99- for pocket in pocketsuffix:
100+ for pocket in PackagePublishingPocket.items:
101 if not fullpublish:
102 if not self.publisher.isDirty(distroseries, pocket):
103 continue
104@@ -746,7 +742,7 @@
105 # each of the distroseries we've touched
106 for distroseries_name in self._config.distroSeriesNames():
107 distroseries = self.distro[distroseries_name]
108- for pocket in pocketsuffix:
109+ for pocket in PackagePublishingPocket.items:
110
111 if not fullpublish:
112 if not self.publisher.isDirty(distroseries, pocket):
113@@ -759,8 +755,7 @@
114 if not self.publisher.isAllowed(distroseries, pocket):
115 continue
116
117- self.generateConfigForPocket(
118- apt_config, distroseries, distroseries_name, pocket)
119+ self.generateConfigForPocket(apt_config, distroseries, pocket)
120
121 # And now return that string.
122 s = apt_config.getvalue()
123@@ -772,20 +767,19 @@
124 fp.close()
125 return apt_config_filename
126
127- def generateConfigForPocket(self, apt_config, distroseries,
128- distroseries_name, pocket):
129+ def generateConfigForPocket(self, apt_config, distroseries, pocket):
130 """Generates the config stanza for an individual pocket."""
131- dr_pocketed = distroseries_name + pocketsuffix[pocket]
132-
133- archs = self._config.archTagsForSeries(distroseries_name)
134- comps = self._config.componentsForSeries(distroseries_name)
135-
136- self.log.debug("Generating apt config for %s" % dr_pocketed)
137+ suite = distroseries.getSuite(pocket)
138+
139+ archs = self._config.archTagsForSeries(distroseries.name)
140+ comps = self._config.componentsForSeries(distroseries.name)
141+
142+ self.log.debug("Generating apt config for %s" % suite)
143 apt_config.write(STANZA_TEMPLATE % {
144 "LISTPATH": self._config.overrideroot,
145- "DISTRORELEASE": dr_pocketed,
146- "DISTRORELEASEBYFILE": dr_pocketed,
147- "DISTRORELEASEONDISK": dr_pocketed,
148+ "DISTRORELEASE": suite,
149+ "DISTRORELEASEBYFILE": suite,
150+ "DISTRORELEASEONDISK": suite,
151 "ARCHITECTURES": " ".join(archs + ["source"]),
152 "SECTIONS": " ".join(comps),
153 "EXTENSIONS": ".deb",
154@@ -793,28 +787,28 @@
155 "DISTS": os.path.basename(self._config.distsroot),
156 "HIDEEXTRA": ""})
157
158- if archs and dr_pocketed in self._di_release_components:
159- for component in self._di_release_components[dr_pocketed]:
160+ if archs and suite in self._di_release_components:
161+ for component in self._di_release_components[suite]:
162 apt_config.write(STANZA_TEMPLATE % {
163 "LISTPATH": self._config.overrideroot,
164- "DISTRORELEASEONDISK": "%s/%s" % (dr_pocketed, component),
165- "DISTRORELEASEBYFILE": "%s_%s" % (dr_pocketed, component),
166- "DISTRORELEASE": "%s.%s" % (dr_pocketed, component),
167+ "DISTRORELEASEONDISK": "%s/%s" % (suite, component),
168+ "DISTRORELEASEBYFILE": "%s_%s" % (suite, component),
169+ "DISTRORELEASE": "%s.%s" % (suite, component),
170 "ARCHITECTURES": " ".join(archs),
171 "SECTIONS": "debian-installer",
172 "EXTENSIONS": ".udeb",
173 "CACHEINSERT": "debian-installer-",
174 "DISTS": os.path.basename(self._config.distsroot),
175- "HIDEEXTRA": "// "
176+ "HIDEEXTRA": "// ",
177 })
178
179 # XXX: 2006-08-24 kiko: Why do we do this directory creation here?
180 for comp in comps:
181- component_path = os.path.join(self._config.distsroot,
182- dr_pocketed, comp)
183+ component_path = os.path.join(
184+ self._config.distsroot, suite, comp)
185 base_paths = [component_path]
186- if dr_pocketed in self._di_release_components:
187- if comp in self._di_release_components[dr_pocketed]:
188+ if suite in self._di_release_components:
189+ if comp in self._di_release_components[suite]:
190 base_paths.append(os.path.join(component_path,
191 "debian-installer"))
192 for base_path in base_paths:
193@@ -822,4 +816,3 @@
194 safe_mkdir(os.path.join(base_path, "source"))
195 for arch in archs:
196 safe_mkdir(os.path.join(base_path, "binary-"+arch))
197-
198
199=== modified file 'lib/lp/archivepublisher/publishing.py'
200--- lib/lp/archivepublisher/publishing.py 2010-10-15 10:34:04 +0000
201+++ lib/lp/archivepublisher/publishing.py 2010-10-17 10:19:44 +0000
202@@ -3,7 +3,6 @@
203
204 __all__ = [
205 'Publisher',
206- 'suffixpocket',
207 'getPublisher',
208 ]
209
210@@ -35,10 +34,7 @@
211 get_ppa_reference,
212 RepositoryIndexFile,
213 )
214-from lp.registry.interfaces.pocket import (
215- PackagePublishingPocket,
216- pocketsuffix,
217- )
218+from lp.registry.interfaces.pocket import PackagePublishingPocket
219 from lp.soyuz.enums import (
220 ArchivePurpose,
221 ArchiveStatus,
222@@ -48,9 +44,6 @@
223 from lp.soyuz.interfaces.component import IComponentSet
224
225
226-suffixpocket = dict((v, k) for (k, v) in pocketsuffix.items())
227-
228-
229 def reorder_components(components):
230 """Return a list of the components provided.
231
232@@ -195,7 +188,7 @@
233 self.log.debug("* Step A: Publishing packages")
234
235 for distroseries in self.distro.series:
236- for pocket, suffix in pocketsuffix.items():
237+ for pocket in PackagePublishingPocket.items:
238 if (self.allowed_suites and not (distroseries.name, pocket) in
239 self.allowed_suites):
240 self.log.debug(
241@@ -235,7 +228,7 @@
242
243 # Loop for each pocket in each distroseries:
244 for distroseries in self.distro.series:
245- for pocket, suffix in pocketsuffix.items():
246+ for pocket in PackagePublishingPocket.items:
247 if self.cannotModifySuite(distroseries, pocket):
248 # We don't want to mark release pockets dirty in a
249 # stable distroseries, no matter what other bugs
250@@ -295,7 +288,7 @@
251 """
252 self.log.debug("* Step C': write indexes directly from DB")
253 for distroseries in self.distro:
254- for pocket, suffix in pocketsuffix.items():
255+ for pocket in PackagePublishingPocket.items:
256 if not is_careful:
257 if not self.isDirty(distroseries, pocket):
258 self.log.debug("Skipping index generation for %s/%s" %
259@@ -323,8 +316,7 @@
260 """
261 self.log.debug("* Step D: Generating Release files.")
262 for distroseries in self.distro:
263- for pocket, suffix in pocketsuffix.items():
264-
265+ for pocket in PackagePublishingPocket.items:
266 if not is_careful:
267 if not self.isDirty(distroseries, pocket):
268 self.log.debug("Skipping release files for %s/%s" %
269@@ -341,7 +333,7 @@
270 Write contents using LP info to an extra plain file (Packages.lp
271 and Sources.lp .
272 """
273- suite_name = distroseries.name + pocketsuffix[pocket]
274+ suite_name = distroseries.getSuite(pocket)
275 self.log.debug("Generate Indexes for %s/%s"
276 % (suite_name, component.name))
277
278@@ -475,11 +467,11 @@
279 else:
280 drsummary += pocket.name.capitalize()
281
282- full_name = distroseries.getSuite(pocket)
283+ suite = distroseries.getSuite(pocket)
284 release_file = Release()
285 release_file["Origin"] = self._getOrigin()
286 release_file["Label"] = self._getLabel()
287- release_file["Suite"] = full_name
288+ release_file["Suite"] = suite
289 release_file["Version"] = distroseries.version
290 release_file["Codename"] = distroseries.name
291 release_file["Date"] = datetime.utcnow().strftime(
292@@ -491,7 +483,7 @@
293 release_file["Description"] = drsummary
294
295 for filename in sorted(list(all_files), key=os.path.dirname):
296- entry = self._readIndexFileContents(full_name, filename)
297+ entry = self._readIndexFileContents(suite, filename)
298 if entry is None:
299 continue
300 release_file.setdefault("MD5Sum", []).append({
301@@ -508,7 +500,7 @@
302 "size": len(entry)})
303
304 f = open(os.path.join(
305- self._config.distsroot, full_name, "Release"), "w")
306+ self._config.distsroot, suite, "Release"), "w")
307 try:
308 release_file.dump(f, "utf-8")
309 finally:
310@@ -521,7 +513,7 @@
311
312 # Sign the repository.
313 archive_signer = IArchiveSigningKey(self.archive)
314- archive_signer.signRepository(full_name)
315+ archive_signer.signRepository(suite)
316
317 def _writeSuiteArchOrSource(self, distroseries, pocket, component,
318 file_stub, arch_name, arch_path,
319
320=== modified file 'lib/lp/archivepublisher/tests/apt-data/apt.conf'
321--- lib/lp/archivepublisher/tests/apt-data/apt.conf 2006-08-24 11:57:44 +0000
322+++ lib/lp/archivepublisher/tests/apt-data/apt.conf 2010-10-17 10:19:44 +0000
323@@ -37,21 +37,6 @@
324 }
325
326
327-tree "dists/breezy-autotest-backports"
328-{
329- FileList "/var/tmp/archive/ubuntutest-overrides/breezy-autotest-backports_$(SECTION)_binary-$(ARCH)";
330- SourceFileList "/var/tmp/archive/ubuntutest-overrides/breezy-autotest-backports_$(SECTION)_source";
331- Sections "main restricted universe multiverse";
332- Architectures "source";
333- BinOverride "override.breezy-autotest-backports.$(SECTION)";
334- SrcOverride "override.breezy-autotest-backports.$(SECTION).src";
335- ExtraOverride "override.breezy-autotest-backports.extra.$(SECTION)";
336- Packages::Extensions ".deb";
337- BinCacheDB "packages-$(ARCH).db";
338- Contents " ";
339-}
340-
341-
342 tree "dists/breezy-autotest-security"
343 {
344 FileList "/var/tmp/archive/ubuntutest-overrides/breezy-autotest-security_$(SECTION)_binary-$(ARCH)";
345@@ -97,6 +82,21 @@
346 }
347
348
349+tree "dists/breezy-autotest-backports"
350+{
351+ FileList "/var/tmp/archive/ubuntutest-overrides/breezy-autotest-backports_$(SECTION)_binary-$(ARCH)";
352+ SourceFileList "/var/tmp/archive/ubuntutest-overrides/breezy-autotest-backports_$(SECTION)_source";
353+ Sections "main restricted universe multiverse";
354+ Architectures "source";
355+ BinOverride "override.breezy-autotest-backports.$(SECTION)";
356+ SrcOverride "override.breezy-autotest-backports.$(SECTION).src";
357+ ExtraOverride "override.breezy-autotest-backports.extra.$(SECTION)";
358+ Packages::Extensions ".deb";
359+ BinCacheDB "packages-$(ARCH).db";
360+ Contents " ";
361+}
362+
363+
364 tree "dists/hoary-test"
365 {
366 FileList "/var/tmp/archive/ubuntutest-overrides/hoary-test_$(SECTION)_binary-$(ARCH)";
367@@ -172,21 +172,6 @@
368 }
369
370
371-tree "dists/hoary-test-backports"
372-{
373- FileList "/var/tmp/archive/ubuntutest-overrides/hoary-test-backports_$(SECTION)_binary-$(ARCH)";
374- SourceFileList "/var/tmp/archive/ubuntutest-overrides/hoary-test-backports_$(SECTION)_source";
375- Sections "main restricted universe multiverse";
376- Architectures "amd64 i386 source";
377- BinOverride "override.hoary-test-backports.$(SECTION)";
378- SrcOverride "override.hoary-test-backports.$(SECTION).src";
379- ExtraOverride "override.hoary-test-backports.extra.$(SECTION)";
380- Packages::Extensions ".deb";
381- BinCacheDB "packages-$(ARCH).db";
382- Contents " ";
383-}
384-
385-
386 tree "dists/hoary-test-security"
387 {
388 FileList "/var/tmp/archive/ubuntutest-overrides/hoary-test-security_$(SECTION)_binary-$(ARCH)";
389@@ -231,3 +216,18 @@
390 Contents " ";
391 }
392
393+
394+tree "dists/hoary-test-backports"
395+{
396+ FileList "/var/tmp/archive/ubuntutest-overrides/hoary-test-backports_$(SECTION)_binary-$(ARCH)";
397+ SourceFileList "/var/tmp/archive/ubuntutest-overrides/hoary-test-backports_$(SECTION)_source";
398+ Sections "main restricted universe multiverse";
399+ Architectures "amd64 i386 source";
400+ BinOverride "override.hoary-test-backports.$(SECTION)";
401+ SrcOverride "override.hoary-test-backports.$(SECTION).src";
402+ ExtraOverride "override.hoary-test-backports.extra.$(SECTION)";
403+ Packages::Extensions ".deb";
404+ BinCacheDB "packages-$(ARCH).db";
405+ Contents " ";
406+}
407+
408
409=== modified file 'lib/lp/archivepublisher/tests/test_ftparchive.py'
410--- lib/lp/archivepublisher/tests/test_ftparchive.py 2010-10-17 04:13:24 +0000
411+++ lib/lp/archivepublisher/tests/test_ftparchive.py 2010-10-17 10:19:44 +0000
412@@ -348,6 +348,22 @@
413
414 fa.createEmptyPocketRequests(fullpublish=True)
415
416+ # createEmptyPocketRequests creates empty override and file
417+ # listings.
418+ lists = (
419+ 'hoary-test-updates_main_source',
420+ 'hoary-test-updates_main_binary-i386',
421+ 'hoary-test-updates_main_debian-installer_binary-i386',
422+ 'override.hoary-test-updates.main',
423+ 'override.hoary-test-updates.extra.main',
424+ 'override.hoary-test-updates.main.src',
425+ )
426+
427+ for listname in lists:
428+ path = os.path.join(self._config.overrideroot, listname)
429+ self.assertTrue(os.path.exists(path))
430+ self.assertEquals("", open(path).read())
431+
432 # XXX cprov 2007-03-21: see above, byte-to-byte configuration
433 # comparing is weak.
434 apt_conf = fa.generateConfig(fullpublish=True)
435
436=== modified file 'lib/lp/registry/interfaces/pocket.py'
437--- lib/lp/registry/interfaces/pocket.py 2010-10-15 10:11:43 +0000
438+++ lib/lp/registry/interfaces/pocket.py 2010-10-17 10:19:44 +0000
439@@ -12,6 +12,7 @@
440 __all__ = [
441 'PackagePublishingPocket',
442 'pocketsuffix',
443+ 'suffixpocket',
444 ]
445
446 from lazr.enum import (
447@@ -76,3 +77,5 @@
448 PackagePublishingPocket.PROPOSED: "-proposed",
449 PackagePublishingPocket.BACKPORTS: "-backports",
450 }
451+
452+suffixpocket = dict((v, k) for (k, v) in pocketsuffix.items())
453
454=== modified file 'lib/lp/registry/model/distribution.py'
455--- lib/lp/registry/model/distribution.py 2010-10-17 04:13:24 +0000
456+++ lib/lp/registry/model/distribution.py 2010-10-17 10:19:44 +0000
457@@ -129,6 +129,7 @@
458 validate_public_person,
459 )
460 from lp.registry.interfaces.pillar import IPillarNameSet
461+from lp.registry.interfaces.pocket import suffixpocket
462 from lp.registry.interfaces.series import SeriesStatus
463 from lp.registry.interfaces.sourcepackagename import ISourcePackageName
464 from lp.registry.model.announcement import MakesAnnouncements
465@@ -918,8 +919,6 @@
466
467 def getDistroSeriesAndPocket(self, distroseries_name):
468 """See `IDistribution`."""
469- from lp.archivepublisher.publishing import suffixpocket
470-
471 # Get the list of suffixes.
472 suffixes = [suffix for suffix, ignored in suffixpocket.items()]
473 # Sort it longest string first.