Merge lp:~robru/cupstream2distro/check-approved-uploaders-for-publish into lp:cupstream2distro

[Robert Bruce Park]

Call checkUpload to see if publisher has upload rights.

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1066. By Robert Bruce Park on 2015-09-01

Catch ClientError.

1067. By Robert Bruce Park on 2015-09-01

Log PublishError properly.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1066
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/733/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/733/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1068. By Robert Bruce Park on 2015-09-01

Absorb redundant confirm_publisher_identity.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1068
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/734/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/734/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1069. By Robert Bruce Park on 2015-09-01

More logging.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1069
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/735/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/735/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1070. By Robert Bruce Park on 2015-09-02

No more honor system for who is acking packages.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1070
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/736/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/736/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1071. By Robert Bruce Park on 2015-09-02

Enforce checkupload for all manual sources but merges only with diffs.

1072. By Robert Bruce Park on 2015-09-02

Debug logging.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1072
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/737/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/737/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1073. By Robert Bruce Park on 2015-09-02

Stop generating XML artifacts.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1073
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/738/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/738/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1074. By Robert Bruce Park on 2015-09-02

Fail properly.

1075. By Robert Bruce Park on 2015-09-02

-rf

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1075
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/739/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/739/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/check-approved-uploaders-for-publish]

1076. By Robert Bruce Park on 2015-09-02

Set self.authorized after, just to be sure.

[Robert Bruce Park (robru)]

Looking good to me in staging, just waiting for some love from one of the core devs I emailed to double check.

review: Approve

=== modified file 'citrain/jenkins-templates/build.xml.tmpl'
--- citrain/jenkins-templates/build.xml.tmpl	2015-03-13 04:48:32 +0000
+++ citrain/jenkins-templates/build.xml.tmpl	2015-09-02 03:52:46 +0000
@@ -85,7 +85,7 @@
 [ "$DEBUG" = "true" ] && set -x
 
 # Cleanup workspace
-pwd|grep -q workspace && rm -f *
+rm -rf $WORKSPACE/*
 
 cd {SILOS_DIR}/{SILO_NAME}
 

=== modified file 'citrain/jenkins-templates/publish.xml.tmpl'
--- citrain/jenkins-templates/publish.xml.tmpl	2015-03-13 04:48:32 +0000
+++ citrain/jenkins-templates/publish.xml.tmpl	2015-09-02 03:52:46 +0000
@@ -25,11 +25,6 @@
           <description>Ack packaging changes</description>
           <defaultValue>false</defaultValue>
         </hudson.model.BooleanParameterDefinition>
-        <hudson.model.StringParameterDefinition>
-          <name>PUBLISHER</name>
-          <description>If you sponsor this publishing for someone else, file this entry with his/her launchpad nickname. It defaults to your user id.</description>
-          <defaultValue></defaultValue>
-        </hudson.model.StringParameterDefinition>
         <hudson.model.BooleanParameterDefinition>
           <name>IGNORE_VERSIONDESTINATION</name>
           <description>Ignore if the latest version in destination doesn't match when prepare was started</description>
@@ -63,32 +58,31 @@
 
 [ "$DEBUG" = "true" ] &amp;&amp; set -x
 
-# Cleanup workspace
-pwd|grep -q workspace &amp;&amp; rm -f *
+rm -rf $WORKSPACE/*
 
 cd {SILOS_DIR}/{SILO_NAME}
 
 {BINDIR}/publisher.py
-
-# copy artefacts
-cp *diff publisher.xml packagelist* $WORKSPACE/ 2&gt;/dev/null || true
-
-# create rsync out dir
+RETVAL=$?
+
+cp *diff packagelist* $WORKSPACE/ 2&gt;/dev/null || true
+
 mkdir -p {RSYNC_OUTDIR}
 mv packagelist_rsync_* {RSYNC_OUTDIR} 2&gt;/dev/null || true
+
+# ubuntu/ directory only contains distro versions, and only used for diffing.
+# This cuts our disk usage in half:
+rm -rf ubuntu/ build-area/
+
+exit $RETVAL
 </command>
     </hudson.tasks.Shell>
   </builders>
   <publishers>
     <hudson.tasks.ArtifactArchiver>
-      <artifacts>*.diff,*xml,packagelist*</artifacts>
+      <artifacts>*</artifacts>
       <latestOnly>false</latestOnly>
     </hudson.tasks.ArtifactArchiver>
-    <hudson.tasks.junit.JUnitResultArchiver>
-      <testResults>*.xml</testResults>
-      <keepLongStdio>false</keepLongStdio>
-      <testDataPublishers/>
-    </hudson.tasks.junit.JUnitResultArchiver>
   </publishers>
   <buildWrappers>
     <org.jenkinsci.plugins.builduser.BuildUser plugin="build-user-vars-plugin@1.3"/>

=== modified file 'citrain/publisher.py'
--- citrain/publisher.py	2015-08-22 04:36:15 +0000
+++ citrain/publisher.py	2015-09-02 03:52:46 +0000
@@ -22,11 +22,8 @@
 ACK_PACKAGING
     Publish packaging changes. Only done after manual reviews of the diff.
 
-PUBLISHER
-    Launchpad nickname of the person publishing that package.
-
 BUILD_USER_ID
-    This provides a default value if PUBLISHER is unset (Jenkins sets this one)
+    Launchpad nickname of the person running this job.
 
 IGNORE_VERSIONDESTINATION
     Ignore if the latest version in archive doesn't match what was prepared.
@@ -39,11 +36,9 @@
 """
 
 import logging
-import glob
 import sys
 import os
 
-from xml.sax.saxutils import escape
 from collections import defaultdict
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
@@ -59,31 +54,16 @@
 from cupstream2distro.project import DotProject
 from cupstream2distro.revno import Rev
 from cupstream2distro.settings import (
-    PUBLISHER_ARTEFACTS_FILENAME,
-    PACKAGING_DIFF,
     STABLE_OVERLAY_PPA,
 )
 from cupstream2distro.utils import (
     SILO_DIR,
     env,
     log_value_of,
-    os_path_join_safe,
     run_script,
-    suppress,
-    utf8_open,
 )
 
 
-UNSTABLE_XML_STRING = """\
-<testsuite errors="0" failures="{}" name="" tests="1" time="0.1">
-  <testcase classname="MarkUnstable" name="Publisher" time="0.0">{}
-  </testcase>
-</testsuite>
-"""
-FAILURE_TEMPLATE = """
-    <failure type="exception">{}</failure>"""
-
-
 def get_distro_and_series(series):
     """Parse the distro and series name from a series link."""
     env.DISTRO = series.distribution.name
@@ -96,8 +76,6 @@
     """Prevent the publish job from being run simultaneously."""
     if is_publishing:
         raise PublishError('The publisher job is already running. Skipping.')
-    with suppress(OSError):
-        os.remove(os_path_join_safe(SILO_DIR(), PUBLISHER_ARTEFACTS_FILENAME))
 
 
 def check_versions_at_destination(dest, source_names):
@@ -152,31 +130,6 @@
         raise PublishError('Some merges have unbuilt revisions.')
 
 
-def confirm_publisher_identity(lp_people):
-    """Identify the person who is running this job."""
-    nick = env.PUBLISHER or env.BUILD_USER_ID
-    try:
-        logging.debug('lp.people["{}"]: {}'.format(nick, lp_people[nick]))
-    except KeyError:
-        raise PublishError('{} is not a valid launchpad user id.'.format(nick))
-    logging.info('Publishing the silo as {}.'.format(nick))
-    env.PUBLISHER = nick
-
-
-def ack_packaging_diffs():
-    """Prevent publishing silos with packaging diffs without an ACK."""
-    if env.ACK_PACKAGING != 'true':
-        diffs = glob.glob(SILO_DIR(PACKAGING_DIFF.format('*')))
-        for diff in diffs:
-            basename = os.path.basename(diff)
-            logging.error('Packaging changes found: {}'.format(basename))
-        if diffs:
-            generate_xml_artifacts(diffs)
-            raise PublishError('Packaging changes need manual ACKing.')
-    else:
-        logging.warning('Honoring your request to ACK packaging changes.')
-
-
 # FIXME: Call this during the build job, so we have access to it sooner.
 def push_branches(mps, lp_load):
     """Push all local branches to Launchpad."""
@@ -260,22 +213,14 @@
                 source.source_package_version)
 
 
-def generate_xml_artifacts(details=[]):
-    """Generate a fake test name xml for marking the build as unstable."""
-    failure = ''.join(FAILURE_TEMPLATE.format(escape(d)) for d in details)
-    path = os_path_join_safe(SILO_DIR(), PUBLISHER_ARTEFACTS_FILENAME)
-    with utf8_open(path, 'w') as data:
-        data.write(UNSTABLE_XML_STRING.format(1 if details else 0, failure))
-
-
 def main():
     """Conduct a beautiful orchestra of publication."""
     silo_state = SiloState.safe_load(env.SILONAME)
     mergemanager = MergeManager(silo_state)
-    mergemanager.do('diff')
     dest = silo_state.dest
     ppa = silo_state.ppa
     try:
+        mergemanager.do('diff', 'checkupload', 'ackaging')
         prevent_duplicate_publishing(silo_state.is_publishing)
         get_distro_and_series(silo_state.series)
         silo_state.set_publishing()
@@ -283,11 +228,8 @@
         check_versions_at_destination(dest, silo_state.all_projects)
         check_for_unapproved(silo_state.mps, lp.load)
         check_for_unbuilt_revids(silo_state.mps, lp.load)
-        confirm_publisher_identity(lp.people)
-        ack_packaging_diffs()
         push_branches(silo_state.mps, lp.load)
         initiate_publication(dest, ppa, silo_state.set_package_version_list)
-        generate_xml_artifacts()
         silo_state.set_published()
         silo_state.set_migrating()
         silo_state.mark_others_dirty()

=== modified file 'citrain/recipes/base.py'
--- citrain/recipes/base.py	2015-08-28 20:32:47 +0000
+++ citrain/recipes/base.py	2015-09-02 03:52:46 +0000
@@ -26,9 +26,13 @@
 
 sys.path.append(os.path.dirname(os.path.dirname(__file__)))
 
+from os.path import exists
 from glob import glob
 
+from lazr.restfulclient.errors import ClientError
+
 from citrain.prepare_silo import clean_source
+from cupstream2distro.launchpadmanager import lp
 from cupstream2distro.packagemanager import Package
 from cupstream2distro.archive import Archive, ArchWatcher, newest
 from cupstream2distro.branchhandling import Branch
@@ -36,11 +40,13 @@
 from cupstream2distro.version import V
 from cupstream2distro.errors import (
     BuildError,
+    PublishError,
     MergeError,
     MigrationError,
 )
 from cupstream2distro.settings import (
     DEPWAIT_TIMEOUT,
+    PACKAGING_DIFF,
     PPA_TIMEOUT,
     TIME_BETWEEN_PPA_CHECKS,
 )
@@ -83,6 +89,8 @@
 
 class BuildBase(Package, Branch, Archive):
     """Outlines build steps common to all types of package builds."""
+    blame = lp.people[env.BUILD_USER_ID] if env.BUILD_USER_ID else None
+    authorized = None
     failures = set()
     states = dict()
 
@@ -253,7 +261,7 @@
 
     def diff_phase(self):
         """Generate a diff of the uploaded source compared to distro."""
-        if os.path.exists(self.path):
+        if exists(self.path):
             logging.info('Generating diff for {}...'.format(self.name))
             path = SILO_DIR('ubuntu', self.name)
             dest_version = newest(self.dest, source_name=self.name)
@@ -264,6 +272,41 @@
                 for new in glob(SILO_DIR(dsc)):
                     self.generate_package_diffs(old, new)
 
+    def checkupload_phase(self):
+        """Confirm that the silo publisher has upload rights."""
+        sourcepub = self.dest.getPublishedSources(
+            distro_series=self.series,
+            source_name=self.name,
+            status='Published',
+            pocket='Release',
+            exact_match=True)[0]
+        try:
+            self.dest.checkUpload(
+                component=sourcepub.component_name,
+                sourcepackagename=self.name,
+                distroseries=self.series,
+                pocket='Proposed',
+                person=self.blame)
+            self.authorized = True
+        except ClientError:
+            self.authorized = False
+        logging.debug('{} authorized to upload {}? {}'.format(
+            self.blame, self.name, self.authorized))
+
+    def enforce_authorization(self):
+        """Enforce checkupload requirements."""
+        if not self.authorized:
+            raise PublishError('{} not authorized to upload {}'.format(
+                self.blame.name, self.name))
+
+    def ackaging_phase(self):
+        """Ensure that packaging changes are correctly ACKed."""
+        if self.authorized and env.ACK_PACKAGING == 'true':
+            logging.info('Honoring your request to ACK packaging changes.')
+        elif exists(SILO_DIR(PACKAGING_DIFF.format(self.name))):
+            self.enforce_authorization()
+            raise PublishError('Packaging changes need manual ACKing.')
+
     def migration_phase(self):
         """Identify the location of this package in its migration."""
         version = DotProject(self.name).published_version

=== modified file 'citrain/recipes/manual.py'
--- citrain/recipes/manual.py	2015-08-21 05:46:52 +0000
+++ citrain/recipes/manual.py	2015-09-02 03:52:46 +0000
@@ -60,3 +60,8 @@
 
         # Proceed with standard diffing.
         super().diff_phase()
+
+    def ackaging_phase(self):
+        """All manual sources must be published by correct uploaders."""
+        self.enforce_authorization()
+        super().ackaging_phase()

=== modified file 'cupstream2distro/settings.py'
--- cupstream2distro/settings.py	2015-08-28 20:32:47 +0000
+++ cupstream2distro/settings.py	2015-09-02 03:52:46 +0000
@@ -57,7 +57,6 @@
 
 FULL_DIFF = '{}_content.diff'
 PACKAGING_DIFF = '{}_packaging_changes.diff'
-PUBLISHER_ARTEFACTS_FILENAME = 'publisher.xml'
 
 OLD_STACK_DIR = 'old'
 

=== modified file 'tests/strings.py'
--- tests/strings.py	2015-08-22 03:06:08 +0000
+++ tests/strings.py	2015-09-02 03:52:46 +0000
@@ -141,23 +141,6 @@
 
 NO_SYMBOLS = MULTIPLE_SYMBOLS.replace('0replaceme', '42.0.0')
 
-XML_TEMPLATE = """\
-<testsuite errors="0" failures="{}" name="" tests="1" time="0.1">
-  <testcase classname="MarkUnstable" name="Publisher" time="0.0">{}
-  </testcase>
-</testsuite>
-"""
-
-XML_NO_ERROR = XML_TEMPLATE.format(0, '')
-
-XML_1_ERROR = XML_TEMPLATE.format(1, """
-    <failure type="exception">one issue</failure>""")
-
-XML_2_ERRORS = XML_TEMPLATE.format(1, """
-    <failure type="exception">one issue</failure>
-    <failure type="exception">a second issue</failure>""")
-
-
 TAGLIST = """\
 0.2.0+14.04.20140217.1-0ubuntu1 254
 0.2.0+14.10.20140714.2-0ubuntu1       257

=== modified file 'tests/unit/test_recipe_base.py'
--- tests/unit/test_recipe_base.py	2015-08-28 20:32:47 +0000
+++ tests/unit/test_recipe_base.py	2015-09-02 03:52:46 +0000
@@ -23,6 +23,7 @@
 
 from citrain.recipes.base import (
     BuildBase,
+    ClientError,
     DEPWAIT_TIMEOUT as DW,
     PPA_TIMEOUT as PT,
 )
@@ -30,11 +31,15 @@
 from cupstream2distro.errors import (
     BuildError,
     CITrainError,
+    PublishError,
     MergeError,
     MigrationError,
 )
 
 
+N = '\n'
+
+
 class BuildFake(object):
     """Mimic a launchpadlib build object."""
     def __init__(self, arch, status=''):
@@ -412,6 +417,72 @@
             status='Published')
         build.generate_package_diffs.assert_called_once_with(None, 'new_dsc')
 
+    @patch('citrain.recipes.base.lp')
+    def test_buildbase_checkupload_phase(self, lp_mock):
+        """Ensure that we call checkupload correctly."""
+        src = Mock()
+        robru = Mock()
+        env.BUILD_USER_ID = 'robru'
+        lp_mock.people = dict(robru=robru)
+        build = BuildBase('qt', self.series, self.dest, self.ppa)
+        build.blame = robru
+        build.dest.getPublishedSources.return_value = [src]
+        build.checkupload_phase()
+        build.dest.getPublishedSources.assert_called_once_with(
+            distro_series=self.series,
+            status='Published',
+            pocket='Release',
+            exact_match=True,
+            source_name='qt')
+        build.dest.checkUpload.assert_called_once_with(
+            component=src.component_name,
+            distroseries=self.series,
+            sourcepackagename='qt',
+            pocket='Proposed',
+            person=robru)
+        self.assertTrue(build.authorized)
+
+    @patch('citrain.recipes.base.lp')
+    def test_buildbase_checkupload_phase_failed(self, lp_mock):
+        """Ensure that we report invalid upload rights."""
+        src = Mock()
+        robru = Mock()
+        env.BUILD_USER_ID = 'robru'
+        lp_mock.people = dict(robru=robru)
+        build = BuildBase('qt', self.series, self.dest, self.ppa)
+        build.blame = Mock()
+        build.dest.getPublishedSources.return_value = [src]
+        build.dest.checkUpload.side_effect = ClientError(None, None)
+        build.checkupload_phase()
+        self.assertFalse(build.authorized)
+
+    def test_buildbase_ackaging_phase_authorized_success(self):
+        """Ensure that authorized people can ACK."""
+        env.ACK_PACKAGING = 'true'
+        build = BuildBase('qt', self.series, self.dest, self.ppa)
+        build.authorized = True
+        build.ackaging_phase()
+
+    @patch('citrain.recipes.base.exists', Mock(return_value=True))
+    def test_buildbase_ackaging_phase_authorized_fail(self):
+        """Ensure that authorized people can ACK."""
+        env.ACK_PACKAGING = 'false'
+        build = BuildBase('qt', self.series, self.dest, self.ppa)
+        build.authorized = True
+        with self.assertRaisesRegexp(PublishError, 'need manual ACKing'):
+            build.ackaging_phase()
+
+    @patch('citrain.recipes.base.exists', Mock(return_value=True))
+    def test_buildbase_ackaging_phase_authorized_unauthorized(self):
+        """Ensure that authorized people can ACK."""
+        env.ACK_PACKAGING = 'true'
+        build = BuildBase('qt', self.series, self.dest, self.ppa)
+        build.authorized = False
+        build.blame = Mock()
+        build.blame.name = 'you are'
+        with self.assertRaisesRegexp(PublishError, 'you are not authorized'):
+            build.ackaging_phase()
+
     @patch('citrain.recipes.base.DotProject')
     def test_buildbase_migration_phase(self, dp_mock):
         """Report the location status of this package."""

=== modified file 'tests/unit/test_recipe_manual.py'
--- tests/unit/test_recipe_manual.py	2015-08-21 05:46:52 +0000
+++ tests/unit/test_recipe_manual.py	2015-09-02 03:52:46 +0000
@@ -72,3 +72,14 @@
         manual.get_from_archive.assert_called_once_with(
             self.tempdir + '/', manual.silo_ppa)
         BuildBase.diff_phase.assert_called_once_with()
+
+    def test_manual_ackaging(self):
+        """Ensure that authorization always enforced in manual packages."""
+        env.ACK_PACKAGING = 'true'
+        manual = Manual('flappy', self.series, self.dest, self.ppa)
+        manual.blame = Mock()
+        manual.blame.name = 'stop'
+        manual.authorized = True
+        manual.enforce_authorization = Mock()
+        manual.ackaging_phase()
+        manual.enforce_authorization.assert_called_once_with()

=== modified file 'tests/unit/test_script_publisher.py'
--- tests/unit/test_script_publisher.py	2015-08-22 03:16:02 +0000
+++ tests/unit/test_script_publisher.py	2015-09-02 03:52:46 +0000
@@ -21,14 +21,11 @@
 
 from tests.unit.test_recipe_base import SourceFake
 from tests.unit import CITrainScriptTestCase
-from tests import strings as s
 
-from cupstream2distro.utils import env, os_path_join_safe, utf8_open
+from cupstream2distro.utils import env
 from cupstream2distro.errors import PublishError, BranchError
-from cupstream2distro.settings import PACKAGING_DIFF
-
-
-PUB = 'publisher.xml'
+
+
 N = '\n'
 
 
@@ -54,24 +51,6 @@
         env.DISTRO = 'ubuntu'
         env.SERIES = 'vivid'
 
-    def test_generate_xml_artefacts_no_issue(self):
-        """Generate the xml jenkins artefacts when no issue occured."""
-        self.script.generate_xml_artifacts([])
-        with utf8_open(os_path_join_safe(self.tempdir, PUB)) as data:
-            self.assertEqual(data.read().split(N), s.XML_NO_ERROR.split(N))
-
-    def test_generate_xml_artefacts_one_failure(self):
-        """Generate the xml jenkins artefacts when there is one failure."""
-        self.script.generate_xml_artifacts(['one issue'])
-        with utf8_open(os_path_join_safe(self.tempdir, PUB)) as data:
-            self.assertEqual(data.read().split(N), s.XML_1_ERROR.split(N))
-
-    def test_generate_xml_artefacts_two_failures(self):
-        """Generate the xml artefacts when there is more than one failure."""
-        self.script.generate_xml_artifacts(['one issue', 'a second issue'])
-        with utf8_open(os_path_join_safe(self.tempdir, PUB)) as data:
-            self.assertEqual(data.read().split(N), s.XML_2_ERRORS.split(N))
-
     def test_get_distro_and_series(self):
         """Given a series URL, identify the distro and series."""
         env.DISTRO, env.SERIES = ('', '')
@@ -287,8 +266,6 @@
     def test_prevent_duplicate_publishing_ok(self):
         """Ensure we can publish ok the first time."""
         self.script.prevent_duplicate_publishing(False)
-        self.script.os.remove.assert_called_once_with(
-            os_path_join_safe(self.tempdir, PUB))
 
     def test_prevent_duplicate_publishing(self):
         """Ensure main function cowardly refuses to run twice."""
@@ -425,63 +402,6 @@
             call('example.com/1234'),
         ])
 
-    def test_confirm_publisher_identity_success(self):
-        """Ensure that we can correctly identify the publisher."""
-        lp_people = defaultdict(str)
-        env.PUBLISHER = ''
-        env.BUILD_USER_ID = 'vorlon'
-        self.script.confirm_publisher_identity(lp_people)
-        self.assertEqual(lp_people, {'vorlon': ''})
-        self.assertEqual(env.PUBLISHER, 'vorlon')
-
-    def test_confirm_publisher_identity_failure(self):
-        """Ensure that we fail to publish without identifying the publisher."""
-        lp_people = dict()
-        env.PUBLISHER = ''
-        env.BUILD_USER_ID = 'vorlon'
-        with self.assertRaisesRegexp(PublishError, 'not a valid launchpad'):
-            self.script.confirm_publisher_identity(lp_people)
-        self.assertEqual(lp_people, {})
-        self.assertEqual(env.PUBLISHER, '')
-
-    def test_ack_packaging_diffs_acked(self):
-        """ack_packaging_diffs acknowledges ACK."""
-        self.script.generate_xml_artifacts = Mock()
-        env.ACK_PACKAGING = 'true'
-        self.script.ack_packaging_diffs()
-        self.assertEqual(self.script.glob.glob.call_count, 0)
-        self.assertEqual(self.script.logging.error.call_count, 0)
-        self.assertEqual(self.script.generate_xml_artifacts.call_count, 0)
-        self.script.logging.warning.assert_called_once_with(
-            'Honoring your request to ACK packaging changes.')
-
-    def test_ack_packaging_diffs_none_to_ack(self):
-        """ack_packaging_diffs approved of no diffs present."""
-        env.ACK_PACKAGING = 'false'
-        self.script.generate_xml_artifacts = Mock()
-        self.script.glob.glob.return_value = []
-        self.script.ack_packaging_diffs()
-        self.script.glob.glob.assert_called_once_with(
-            os_path_join_safe(self.tempdir, PACKAGING_DIFF.format('*')))
-        self.assertEqual(self.script.logging.error.call_count, 0)
-        self.assertEqual(self.script.generate_xml_artifacts.call_count, 0)
-
-    def test_ack_packaging_diffs_halts_when_diff_found(self):
-        """ack_packaging_diffs hates those diffs you have!"""
-        env.ACK_PACKAGING = 'false'
-        self.script.generate_xml_artifacts = Mock()
-        self.script.glob.glob.return_value = ['/my/diff1', '/my/diff2']
-        with self.assertRaisesRegexp(PublishError, 'need manual ACKing'):
-            self.script.ack_packaging_diffs()
-        self.script.glob.glob.assert_called_once_with(
-            os_path_join_safe(self.tempdir, PACKAGING_DIFF.format('*')))
-        self.assertEquals(
-            self.script.logging.error.mock_calls,
-            [call('Packaging changes found: diff1'),
-             call('Packaging changes found: diff2')])
-        self.script.generate_xml_artifacts.assert_called_once_with(
-            self.script.glob.glob.return_value)
-
     def test_copy_to_dest(self):
         """Ensure that we poke the LP API correctly for copyPackage."""
         sources = [
@@ -523,11 +443,8 @@
         self.script.check_versions_at_destination = Mock()
         self.script.check_for_unapproved = Mock()
         self.script.check_for_unbuilt_revids = Mock()
-        self.script.confirm_publisher_identity = Mock()
-        self.script.ack_packaging_diffs = Mock()
         self.script.push_branches = Mock()
         self.script.initiate_publication = Mock()
-        self.script.generate_xml_artifacts = Mock()
         self.script.SiloState.safe_load.return_value.series = \
             'https://api.launchpad.net/devel/ubuntu/vivid'
 
@@ -540,7 +457,7 @@
         self.assertEqual(self.script.main(), 0)
         self.script.MergeManager.assert_called_once_with(state)
         self.script.MergeManager.return_value.do.assert_called_once_with(
-            'diff')
+            'diff', 'checkupload', 'ackaging')
         self.script.prevent_duplicate_publishing.assert_called_once_with(
             state.is_publishing)
         state.set_publishing.assert_called_once_with()
@@ -550,14 +467,10 @@
             state.mps, self.script.lp.load)
         self.script.check_for_unbuilt_revids.assert_called_once_with(
             state.mps, self.script.lp.load)
-        self.script.confirm_publisher_identity.assert_called_once_with(
-            self.script.lp.people)
-        self.script.ack_packaging_diffs.assert_called_once_with()
         self.script.push_branches.assert_called_once_with(
             state.mps, self.script.lp.load)
         self.script.initiate_publication.assert_called_once_with(
             dest, ppa, state.set_package_version_list)
-        self.script.generate_xml_artifacts.assert_called_once_with()
         state.set_published.assert_called_once_with()
         state.set_migrating.assert_called_once_with()
         self.assertEqual(state.save_config.mock_calls, [call(), call()])
@@ -579,8 +492,6 @@
             state.mps, self.script.lp.load)
         self.script.check_for_unbuilt_revids.assert_called_once_with(
             state.mps, self.script.lp.load)
-        self.script.confirm_publisher_identity.assert_called_once_with(
-            self.script.lp.people)
         self.script.push_branches.assert_called_once_with(
             state.mps, self.script.lp.load)
         self.assertEqual(self.script.initiate_publication.call_count, 0)