Merge lp:~cjwatson/launchpad/new-python-apt into lp:launchpad
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Merged at revision: | 14618 | ||||
| Proposed branch: | lp:~cjwatson/launchpad/new-python-apt | ||||
| Merge into: | lp:launchpad | ||||
| Diff against target: |
929 lines (+154/-170) 21 files modified
cronscripts/publishing/maintenance-check.py (+7/-7) lib/lp/archivepublisher/domination.py (+2/-2) lib/lp/archivepublisher/tests/test_dominator.py (+2/-2) lib/lp/archiveuploader/dscfile.py (+1/-1) lib/lp/archiveuploader/nascentupload.py (+1/-1) lib/lp/archiveuploader/nascentuploadfile.py (+41/-58) lib/lp/archiveuploader/tagfiles.py (+1/-1) lib/lp/archiveuploader/tests/test_tagfiles.py (+6/-7) lib/lp/registry/browser/distroseries.py (+1/-1) lib/lp/registry/browser/sourcepackage.py (+5/-5) lib/lp/registry/model/distroseries.py (+1/-1) lib/lp/registry/model/distroseriesdifference.py (+6/-6) lib/lp/soyuz/browser/binarypackagerelease.py (+2/-2) lib/lp/soyuz/browser/packagerelationship.py (+6/-1) lib/lp/soyuz/model/binarypackagebuild.py (+13/-7) lib/lp/soyuz/model/sourcepackagerelease.py (+2/-2) lib/lp/soyuz/scripts/gina/archive.py (+9/-9) lib/lp/soyuz/scripts/packagecopier.py (+2/-2) lib/lp/soyuz/tests/test_publish_archive_indexes.py (+27/-32) lib/lp_sitecustomize.py (+0/-4) scripts/ftpmaster-tools/sync-source.py (+19/-19) |
||||
| To merge this branch: | bzr merge lp:~cjwatson/launchpad/new-python-apt | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Benji York (community) | code | 2011-12-14 | Approve on 2011-12-19 |
|
Review via email:
|
|||
Commit Message
[r=benji][bug=551510] Port to new python-apt API.
Description of the Change
= Summary =
Port Launchpad to the new python-apt API, introduced in lucid. Launchpad currently has to ignore a DeprecationWarning for its use of the old API; this is obviously bad since it might be removed at some point.
== Proposed fix ==
The proposed fix is fairly obvious for the most part; I searched for '(import|from) apt' and ran python-apt's utils/migrate-
== Implementation details ==
I didn't discuss this with anyone before implementation since it seemed likely to be fairly mechanical. Maybe that was a mistake, since a few things did come up:
* The new apt_pkg.
* cronscripts/
* The replacements for apt_inst.
* apt_pkg.TagFile objects are iterable. Where appropriate, I made use of this rather than using the 'step' (previously 'Step') method, which is ugly because it mutates the object.
== Tests ==
bin/test -vvct 'archivepublish
== Demo and Q/A ==
This is mainly internal rearrangement, and I don't think any particular Q/A is required, although I'm happy to be corrected. The only untested code being modified here is scripts/
== lint ==
I cleaned up some pre-existing lint. What's left is:
./cronscripts/
86: Line exceeds 78 characters.
86: E501 line too long (84 characters)
./lib/lp/
123: Line exceeds 78 characters.
145: Line exceeds 78 characters.
123: E501 line too long (83 characters)
145: E501 line too long (89 characters)
These are long URLs and didn't really seem worth adjusting to satisfy lint. (I hope to rewrite maintenance-
./lib/lp/
205: redefinition of function 'copyright' from line 196
lint is wrong; this is a setter property.
./scripts/
27: '_pythonpath' imported but unused
Usual lint wrongness.
| Colin Watson (cjwatson) wrote : | # |
I think "ar" is correct here. A .deb is an ar archive of three members, two of which are compressed tar archives; but the contents of the inner tar archives aren't checked here. The constructor in question is debDebFile:
http://
That might fail, for example, if the "debian-binary" member is missing, which isn't a tar archive.
However, I can see a confusion between "members of the ar archive" and "members which are ar archives" here, so I've rephrased the comment:
# We get an error from the constructor if the .deb does not
- # contain all the expected ar members.
+ # contain all the expected top-level members (debian-binary,
+ # control.tar.gz, and data.tar.*).

This branch looks good. I concur with the decision to remap ">" and
"<"".
The only issue I found was that I think "ar" in this comment (line 304 le.py) should be "tar":
of the diff, from nascentuploadfi
except SystemError, error:
# We get an error from the constructor if the .deb does not
# contain all the expected ar members.
yield UploadError(error)