Comment 1 for bug 499438

Michael Nelson (michael.nelson) wrote :

Pre-implementation notes requiring feedback:

After a brief survey of the code in lp.archiveuploader.changesfile.ChangesFile.__init__(), it seems that we generate an oops (rather than rejecting the upload with a nice email) if:

 1. The changes file can't be parsed correctly,
 2. *any* of the mandatory fields are not present (currently source, binary, architecture, version, distribution, maintainer, files, changes),
 3. the format of the changes file is not supported, or
 4. the signature could not be verified (for a number of reasons).

I assume (2) is happening because if we can't verify the maintainer (one of the mandatory fields), we know we can't email them, but I'm not sure why we've included the other fields in this check.

So, the plan is to update (2) above simply to:

2. The maintainer field is not present (hrm, should really be changed-by? What's the reason that changed-by is not already a mandatory field in this check?)

and remove (3) altogether from __init__, and then add two new methods:

ChangesFile.checkMandatoryFields()
ChangesFile.checkChangesFormat()

which can be used by NascentUpload.process (which calls run_and_reject_on_errors).

The result will be that we only oops in situations where we don't have a signer or maintainer email.

Note: at first I was confused why we don't include changed-by as a mandatory field, but realised it might be because it's not required by policy (http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-debianchangesfiles). But we do raise an UploadError during checkAddresses later if it's not present (which doesn't generate an oops. but rejects the upload, which will try to email the signer as per normal).

Note: currently PackageUpload._getRecipients() always includes either the signer or the changed-by address. But even though these may be present, an email will still not be sent if they have no preferredEmail etc.