Code review comment for lp:~julian-edwards/launchpad/ppa-copy-to-main-bug-426163

Revision history for this message
Julian Edwards (julian-edwards) wrote :

= Summary =
Ensure that packages copied from the primary archive into a PPA don't get a
non-main component

== Proposed fix ==
Right now, when packages are copied from Ubuntu into a PPA, if the source was
in a non-main component then that component is retained in the PPA. This is
bad because PPAs are not supposed to have non-main components!

== Pre-implementation notes ==
I chatted with Celso about this. The actual fix for this is quite trivial;
that is to override the component when the publishing record in the PPA is
created. However, we identified more than one place where publications are
created and this overriding happens. Therefore, this branch contains some
refactoring.

== Implementation details ==
The various methods that created publishing records are now refactored into
the IPublishingSet utility. There's two new methods, one for source and one
for binary. They both override the component to main if the context archive
is a PPA.

File-by-file changes:

* lib/lp/soyuz/doc/publishing.txt:
  I did a moin -> reST conversion. Also updated tests for the new methods on
IPublishingSet. Had to fixe the last test in the file which was copying stuff
into a PPA and expecting a non-main component!

* lib/lp/soyuz/model/publishing.py
  Fixed some code that creates publications so that they use the utility.
  Also added the new utility methods here.

* lib/lp/soyuz/model/queue.py
  Calling the new utility methods instead of creating publishing records
directly.

* lib/lp/soyuz/stories/soyuz/xx-queue-pages-delayed-copies.txt
  This test got broken by the changes because it was relying on the fact that
the publishing records had no security wrapper. Now that they created in a
utility, they will get a wrapper, and this has the knock on effect of also
putting things like SourcePackageReleaseFile in a wrapper which means the
pagetest interaction can't process the upload. I made it call out to a script
instead which runs zopeless. (the objects are *only* ever created in zopeless
scripts in production)

== Tests ==
bin/test -cvvt publishing.txt -t xx-queue-pages-delayed-copies.txt

== Demo and Q/A ==
Can test this on dogfood by using the +copy-packages page to copy some
packages into a PPA. If the packages appear in main, then it's all good!

= 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/queue.py
  lib/lp/soyuz/stories/soyuz/xx-queue-pages-delayed-copies.txt
  lib/lp/soyuz/model/publishing.py
  lib/lp/soyuz/doc/publishing.txt
  lib/lp/soyuz/interfaces/publishing.py

== Pylint notices ==

lib/lp/soyuz/interfaces/publishing.py
    32: [F0401] Unable to import 'lazr.enum' (No module named enum)
    39: [F0401] Unable to import 'lazr.restful.fields' (No module named
restful)
    40: [F0401] Unable to import 'lazr.restful.declarations' (No module named
restful)

« Back to merge proposal