rejected builds for synced packages send mail to Debian maintainer

[Ansgar Burchardt]

enet was synced from Debian and Launchpad sent me a mail:

----
from: Ubuntu Installer <email address hidden>
subject: enet_1.3.3-2_i386.changes rejected
to: Ansgar Burchardt <email address hidden>
Date: Mon, 17 Oct 2011 14:26:28 -0000
Reply-To: Ubuntu Installer <email address hidden>

Rejected:
Require Pre-Depends: dpkg (>= 1.15.6) when using xz compression.
Require Pre-Depends: dpkg (>= 1.15.6) when using xz compression.
Require Pre-Depends: dpkg (>= 1.15.6) when using xz compression.
Require Pre-Depends: dpkg (>= 1.15.6) when using xz compression.
----

I do not think Ubuntu should send suchmails to the original maintainer in Debian. (I should not subscribed to the package in Launchpad as I follow Ubuntu bug mail via the Debian Games group / gmane.)

In addition, unless you want to manually change all packages in Debian using XZ in Ubuntu, the required pre-dependency on dpkg should be dropped (IMO).

Ansgar

[Colin Watson (cjwatson)]

I agree that this is a regression. We should not be mailing Debian maintainers for activity in Ubuntu just because their name was on the package we synced. To see why not, imagine the full set of Debian derivatives.

Ansgar, we have no real choice regarding xz and dpkg; our last LTS release didn't have a sufficient version of dpkg, and we need to support direct LTS->LTS upgrades, so this pre-dependency has to be there for us until after 12.04 releases. If that means we have to temporarily diverge from Debian for a bunch more packages, then that's unfortunate but so be it.

[Jeroen T. Vermeulen (jtv)]

Technically this looks like one of those start-pulling-a-loose-thread-end-up-without-your-sweater problems. Nested deep down in the bowels of the package-copying mechanism, we have clever notification functions that cleverly figure out what's going on and who needs to be notified.

There seem to be a lot of possible scenarios that this code can be dragged into, so we'll want to start by reconstructing what they are, and who gets notified when. Then we pinpoint the changes we need, and come up with a more transparent way to connect high-level actions with notification policy.

Apparently lp.soyuz.adapters.notification.get_recipients is adding at least one recipient that we don't want in this case: the maintainer.

[Launchpad QA Bot (lpqabot)]

Fixed in stable r14202 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/14202>.

[Jeroen T. Vermeulen (jtv)]

Haven't managed to Q/A the cleanups on dogfood: uploads are broken and I can't request syncs. Since there are no functional changes (and there are plenty of high-level scenario tests) I'll mark this as untestable.

Again, this first branch is cleanups only; rolling it out does not make this bug Fix Released.

[Bryce Harrington (bryce)]

Hi Jeroen, what's the status on this bug currently?

[Jeroen T. Vermeulen (jtv)]

Blocked on “more critical” issues. Our board is painted in shades of red these days.

[Jeroen T. Vermeulen (jtv)]

I'm back to working on this, but still lost in building a scenario we could use for an acceptance test.

We do know that the actual error message comes out of verifyFormat. Here's the call stack, as far as I can reconstruct:

BaseBinaryUploadFile.verifyFormat
 ← BaseBinaryUploadFile.verify (e.g. for a DebBinaryUploadFile)
   ← DSCFile.checkFiles (calls on DSCUploadedFile, child of NascentUploadFile)
     ← DSCFile.verify
       ← NascentUpload.process
         ← {Build,User}UploadHandler._processUpload
           ← UploadHandler.processChangesFile
             ← {Build,User}UploadHandler.process
               ← UploadProcessor.processUploadQueue
                 ← ProcessUpload.main

The call to notify() that we want to catch probably originates somewhere in NascentUpload.process or {Build,User}UploadHandler._processUpload. Unfortunately there are many code paths that can end up calling notify().

That's control flow. But we also have data flow to worry about: as I understand it, the source package release gets synced from Debian into Ubuntu. That triggers builds for the appropriate architectures, resulting in binary uploads, which is where the error happens.

It looks like we can tell that notifying the maintainer is inappropriate if the upload's target archive is a different one than the source package release. Here's what we decided… Compare BinaryPackageBuild archive to target archive: if different, do not notify addressees from changes file (maintainer, changed-by).

[Jeroen T. Vermeulen (jtv)]

The notification seems to be generated from NascentUpload.process, and go through lp.soyuz.adapters.notification.notify:

NascentUpload.process
  → DSCFile.verify
    → DSCFile.checkFiles
      → NascentUpload.run_and_collect_errors(verify)
        → BaseBinaryUploadFile.verify
          → BaseBinaryUploadFile.verifyFormat [generates error message]
        → [appends to self.rejections/self.warnings]

Notification is triggered from NascentUpload.do_reject:

NascentUpload.do_reject
  → NascentUpload.rejection_message
  → PackageUpload.notify
    → lp.soyuz.adapters.notification.notify

[Jeroen T. Vermeulen (jtv)]

Julian suggests this procedure for Q/A testing:

1. Sync something from Debian.
2. Wait for it to finish building, so that it's in the "waiting to upload" state.
3. Find the files in buildmaster/incoming.
4. Sabotage the .deb file so that it will fail its md5 check.

[Launchpad QA Bot (lpqabot)]

Fixed in stable r14645 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/14645>.