Code review comment for lp:~jml/launchpad/package-permission-love

Hello reviewer,

This branch is a very important branch, and a very risky one too. Please take
extra care when reviewing it. It's important because much of the Ubuntu
Distributed Development work depends on it. It's risky because it changes
poorly-understood security code.

This extracts out a function called 'verify_upload' from the nascentupload
code in Soyuz. The function is intended to provide a canonical answer to the
question, "can this person upload to this package in this archive?".

Sadly, it doesn't actually provide a canonical answer just yet. There's logic
in the upload policy code that needs to be moved into it. Specifically, it
needs to grow pocket-based permission checks. However, since the branch is
quite large already, I think I'll save that for another change.

The function has been placed in 'lp.archiveuploader.permission'. I don't think
this is a good long-term location, and I'm happy to move it if you think of a
better one. However, I think I'd prefer to wait until 'verify_upload' checks
the pocket as well before I encourage its re-use by putting it in a more
public location.

The implementation of the function is quite straight-forward, if you know what
it's supposed to do. I hope that the unit tests and the docstring explain this
clearly.

I probably got a bit ahead of myself with the exceptions. I'm happy to change
them on request.

Since this is a refactoring patch, and since I don't _really_ understand the
code, I wanted to make as few behavioural changes as possible. However, I was
forced to make some:

  * The error message for when you cannot upload to a particular component no
    longer includes the name of the DSC file. wgrant informed me that the file
    name was never very useful anyway.

  * The binary upload check happens much sooner. It has nothing to do with
    ACLs, so we might as well check it first.

I had to do a fair bit of exploratory work to get this patch up-and-running,
particularly to get the unit tests looking like actual unit tests. Not all of
it ended up being used, but it's all actually useful. Here's a run-down:

 * 'makeComponent' added to the object factory.

 * 'makeArchive' can now create non-PPA archives.

 * Added 'makePackageset'.

 * Tweaked a decorator to use 'mergeFunctionMetadata'. This means that it'll
   have a useful name when it appears in exception stack traces.

 * Changed one of the soyuz helpers, 'getPubSource' so that you can use it to
   get a SourcePackagePublishingHistory record without actually doing a
   simulated upload.

 * Added a launchpad.Edit permission policy for packagesets. This allows them
   to be used in a non-Zopeless environment.

I also did a few incidental cleanups:

 * Changed a docstring in archivepermisson.py to actually match reality. I
   checked this one with Muharem, who was the original author.

 * Re-used an existing local variable in a permission check, rather than
   violating the Law of Demeter.

 * Various whitespace and lint.

Thanks for getting to the end of the cover letter! I'd like to get this patch
right, so please ask as many questions as possible.