← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wgrant/launchpad/buildbase-must-die into lp:launchpad/devel

 

William Grant has proposed merging lp:~wgrant/launchpad/buildbase-must-die into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Wellington saw the creation of BuildBase as a base class to sit under BinaryPackageBuild and SourcePackageRecipeBuild. It didn't have a table, so fields were duplicated in its subclasses. A couple of months ago, a new PackageBuild class and table were created to replace it, calling into BuildBase's methods to do its work. Now that both build types have been ported to PackageBuild, we can move BuildBase's functionality into PackageBuild, and remove BuildBase itself.

This branch does a pretty straight move of BuildBase's methods to PackageBuild and PackageBuildDerived. I've de-static'd some methods where it was trivial, but other cleanup can wait for a subsequent branch.

lp.buildmaster.model.buildbase is gone entirely, but lp.buildmaster.interfaces.buildbase retains two members (BUILDD_MANAGER_LOG_NAME and BuildStatus), since this branch is big enough. I'll move them afterwards.
-- 
https://code.launchpad.net/~wgrant/launchpad/buildbase-must-die/+merge/33505
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/buildbase-must-die into lp:launchpad/devel.
=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2010-08-22 18:31:30 +0000
+++ lib/canonical/launchpad/security.py	2010-08-24 10:48:48 +0000
@@ -1645,14 +1645,14 @@
             return None
 
     def _getBuild(self):
-        """Get `IBuildBase` associated with this job, if any."""
+        """Get `IPackageBuild` associated with this job, if any."""
         if IBuildFarmBuildJob.providedBy(self.obj):
             return self.obj.build
         else:
             return None
 
     def _checkBuildPermission(self, user=None):
-        """Check access to `IBuildBase` for this job."""
+        """Check access to `IPackageBuild` for this job."""
         permission = ViewBinaryPackageBuild(self.obj.build)
         if user is None:
             return permission.checkUnauthenticated()

=== modified file 'lib/lp/buildmaster/interfaces/buildbase.py'
--- lib/lp/buildmaster/interfaces/buildbase.py	2010-08-20 20:31:18 +0000
+++ lib/lp/buildmaster/interfaces/buildbase.py	2010-08-24 10:48:48 +0000
@@ -10,35 +10,12 @@
 __all__ = [
     'BUILDD_MANAGER_LOG_NAME',
     'BuildStatus',
-    'IBuildBase',
     ]
 
 from lazr.enum import (
     DBEnumeratedType,
     DBItem,
     )
-from lazr.restful.declarations import exported
-from lazr.restful.fields import Reference
-from zope.interface import (
-    Attribute,
-    Interface,
-    )
-from zope.schema import (
-    Choice,
-    Datetime,
-    Object,
-    TextLine,
-    Timedelta,
-    )
-
-from canonical.launchpad import _
-from canonical.launchpad.interfaces.librarian import ILibraryFileAlias
-from lp.buildmaster.interfaces.builder import IBuilder
-from lp.buildmaster.interfaces.buildfarmjob import BuildFarmJobType
-from lp.buildmaster.interfaces.buildqueue import IBuildQueue
-from lp.registry.interfaces.distribution import IDistribution
-from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.soyuz.interfaces.archive import IArchive
 
 
 BUILDD_MANAGER_LOG_NAME = "slave-scanner"
@@ -122,178 +99,3 @@
         buildlog, datebuilt, duration, builder, etc) and the buildd admins
         will be notified via process-upload about the reason of the rejection.
         """)
-
-
-class IBuildBase(Interface):
-    """Common interface shared by farm jobs that build a package."""
-    # XXX 2010-04-21 michael.nelson bug=567922. This interface
-    # can be removed once all *Build classes inherit from
-    # IBuildFarmJob/IPackageBuild. Until that time, to allow the shared
-    # implementation of handling build status, IBuildBase needs to
-    # provide aliases for buildstate, buildlog and datebuilt as follows:
-    # status => buildstate
-    # log => buildlog
-    # date_finished => datebuilt
-    status = Choice(
-            title=_('State'), required=True, vocabulary=BuildStatus,
-            description=_("The current build state."))
-    log = Object(
-        schema=ILibraryFileAlias, required=False,
-        title=_("The LibraryFileAlias containing the entire buildlog."))
-    date_finished = Datetime(
-            title=_('Date built'), required=False,
-            description=_("The time when the build result got collected."))
-
-
-    build_farm_job_type = Choice(
-        title=_("Job type"), required=True, readonly=True,
-        vocabulary=BuildFarmJobType,
-        description=_("The specific type of job."))
-
-    # XXX: wgrant 2010-01-20 bug=507712: Most of these attribute names
-    # are bad.
-    datecreated = exported(
-        Datetime(
-            title=_('Date created'), required=True, readonly=True,
-            description=_("The time when the build request was created.")))
-
-    buildstate = exported(status)
-
-    date_first_dispatched = exported(
-        Datetime(
-            title=_('Date first dispatched'), required=False,
-            description=_("The actual build start time. Set when the build "
-                          "is dispatched the first time and not changed in "
-                          "subsequent build attempts.")))
-
-    builder = Object(
-        title=_("Builder"), schema=IBuilder, required=False,
-        description=_("The Builder which address this build request."))
-
-    datebuilt = exported(date_finished)
-
-    buildduration = Timedelta(
-        title=_("Build Duration"), required=False,
-        description=_("Build duration interval, calculated when the "
-                      "build result gets collected."))
-
-    buildlog = log
-
-    build_log_url = exported(
-        TextLine(
-            title=_("Build Log URL"), required=False,
-            description=_("A URL for the build log. None if there is no "
-                          "log available.")))
-
-    buildqueue_record = Object(
-        schema=IBuildQueue, required=True,
-        title=_("Corresponding BuildQueue record"))
-
-    is_private = Attribute("Whether the build should be treated as private.")
-
-    policy_name = TextLine(
-        title=_("Policy name"), required=True,
-        description=_("The upload policy to use for handling these builds."))
-
-    archive = exported(
-        Reference(
-            title=_("Archive"), schema=IArchive,
-            required=True, readonly=True,
-            description=_("The Archive context for this build.")))
-
-    current_component = Attribute(
-        "Component where the source related to this build was last "
-        "published.")
-
-    pocket = exported(
-        Choice(
-            title=_('Pocket'), required=True,
-            vocabulary=PackagePublishingPocket,
-            description=_("The build targeted pocket.")))
-
-    dependencies = exported(
-        TextLine(
-            title=_("Dependencies"), required=False,
-            description=_("Debian-like dependency line that must be satisfied"
-                          " before attempting to build this request.")))
-
-    distribution = exported(
-        Reference(
-            schema=IDistribution,
-            title=_("Distribution"), required=True,
-            description=_("Shortcut for its distribution.")))
-
-    upload_log = Object(
-        schema=ILibraryFileAlias, required=False,
-        title=_("The LibraryFileAlias containing the upload log for "
-                "build resulting in an upload that could not be processed "
-                "successfully. Otherwise it will be None."))
-
-    upload_log_url = exported(
-        TextLine(
-            title=_("Upload Log URL"), required=False,
-            description=_("A URL for failed upload logs."
-                          "Will be None if there was no failure.")))
-
-    title = exported(TextLine(title=_("Title"), required=False))
-
-    def getUploaderCommand(build, upload_leaf, uploader_logfilename):
-        """Get the command to run as the uploader.
-
-        :return: A list of command line arguments, beginning with the
-            executable.
-        """
-
-    def getUploadLogContent(root, leaf):
-        """Retrieve the upload log contents.
-
-        :param root: Root directory for the uploads
-        :param leaf: Leaf for this particular upload
-        :return: Contents of log file or message saying no log file was found.
-        """
-
-    def handleStatus(status, librarian, slave_status):
-        """Handle a finished build status from a slave.
-
-        :param status: Slave build status string with 'BuildStatus.' stripped.
-        :param slave_status: A dict as returned by IBuilder.slaveStatus
-        """
-
-    def getLogFromSlave(build):
-        """Get last buildlog from slave.
-
-        Invoke getFileFromSlave method with 'buildlog' identifier.
-        """
-
-    def queueBuild(build, suspended=False):
-        """Create a BuildQueue entry for this build.
-
-        :param suspended: Whether the associated `Job` instance should be
-            created in a suspended state.
-        """
-
-    def estimateDuration():
-        """Estimate the build duration."""
-
-    def storeBuildInfo(build, librarian, slave_status):
-        """Store available information for the build job.
-
-        Subclasses can override this as needed, and call it from custom status
-        handlers, but it should not be called externally.
-        """
-
-    def verifySuccessfulUpload():
-        """Verify that the upload of this build completed succesfully."""
-
-    def storeUploadLog(content):
-        """Store the given content as the build upload_log.
-
-        :param content: string containing the upload-processor log output for
-            the binaries created in this build.
-        """
-
-    def notify(extra_info=None):
-        """Notify current build state to related people via email."""
-
-    def makeJob():
-        """Construct and return an `IBuildFarmJob` for this build."""

=== removed file 'lib/lp/buildmaster/model/buildbase.py'
--- lib/lp/buildmaster/model/buildbase.py	2010-08-20 20:31:18 +0000
+++ lib/lp/buildmaster/model/buildbase.py	1970-01-01 00:00:00 +0000
@@ -1,480 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-# pylint: disable-msg=E0211,E0213
-
-
-"""Common build base classes."""
-
-
-from __future__ import with_statement
-
-__metaclass__ = type
-
-__all__ = [
-    'handle_status_for_build',
-    'BuildBase',
-    ]
-
-from cStringIO import StringIO
-import datetime
-import logging
-import os
-import subprocess
-
-import pytz
-from storm.store import Store
-from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
-
-from canonical.config import config
-from canonical.database.sqlbase import (
-    clear_current_connection_cache,
-    cursor,
-    flush_database_updates,
-    )
-from canonical.launchpad.helpers import filenameToContentType
-from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
-from canonical.librarian.utils import copy_and_close
-from lp.buildmaster.interfaces.buildbase import (
-    BUILDD_MANAGER_LOG_NAME,
-    BuildStatus,
-    )
-from lp.buildmaster.model.buildqueue import BuildQueue
-from lp.registry.interfaces.pocket import pocketsuffix
-
-
-UPLOAD_LOG_FILENAME = 'uploader.log'
-
-
-def handle_status_for_build(build, status, librarian, slave_status,
-                            build_class=None):
-    """Find and call the correct method for handling the build status.
-
-    This is extracted from build base so that the implementation
-    can be shared by the newer IPackageBuild class.
-    """
-    logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
-
-    if build_class is None:
-        build_class = BuildBase
-    method = getattr(build_class, '_handleStatus_' + status, None)
-
-    if method is None:
-        logger.critical("Unknown BuildStatus '%s' for builder '%s'"
-                        % (status, build.buildqueue_record.builder.url))
-        return
-
-    method(build, librarian, slave_status, logger)
-
-
-class BuildBase:
-    """A mixin class providing functionality for farm jobs that build a
-    package.
-
-    Note: this class does not implement IBuildBase as we currently duplicate
-    the properties defined on IBuildBase on the inheriting class tables.
-    BuildBase cannot therefore implement IBuildBase itself, as storm requires
-    that the corresponding __storm_table__ be defined for the class. Instead,
-    the classes using the BuildBase mixin must ensure that they implement
-    IBuildBase.
-    """
-    policy_name = 'buildd'
-
-    @staticmethod
-    def getUploadDirLeaf(build_cookie, now=None):
-        """See `IPackageBuild`."""
-        # UPLOAD_LEAF: <TIMESTAMP>-<BUILD-COOKIE>
-        if now is None:
-            now = datetime.datetime.now()
-        return '%s-%s' % (now.strftime("%Y%m%d-%H%M%S"), build_cookie)
-
-    @staticmethod
-    def getUploadDir(upload_leaf):
-        """Return the directory that things will be stored in."""
-        return os.path.join(config.builddmaster.root, 'incoming', upload_leaf)
-
-    @staticmethod
-    def getUploaderCommand(build, upload_leaf, uploader_logfilename):
-        """See `IBuildBase`."""
-        root = os.path.abspath(config.builddmaster.root)
-        uploader_command = list(config.builddmaster.uploader.split())
-
-        # add extra arguments for processing a binary upload
-        extra_args = [
-            "--log-file", "%s" % uploader_logfilename,
-            "-d", "%s" % build.distribution.name,
-            "-s", "%s" % (build.distro_series.name +
-                          pocketsuffix[build.pocket]),
-            "-b", "%s" % build.id,
-            "-J", "%s" % upload_leaf,
-            '--context=%s' % build.policy_name,
-            "%s" % root,
-            ]
-
-        uploader_command.extend(extra_args)
-        return uploader_command
-
-    def _getProxiedFileURL(self, library_file):
-        """Return the 'http_url' of a `ProxiedLibraryFileAlias`."""
-        # Avoiding circular imports.
-        from canonical.launchpad.browser.librarian import (
-            ProxiedLibraryFileAlias)
-
-        proxied_file = ProxiedLibraryFileAlias(library_file, self)
-        return proxied_file.http_url
-
-    @property
-    def build_log_url(self):
-        """See `IBuildBase`."""
-        if self.buildlog is None:
-            return None
-        return self._getProxiedFileURL(self.buildlog)
-
-    @staticmethod
-    def getUploadLogContent(root, leaf):
-        """Retrieve the upload log contents.
-
-        :param root: Root directory for the uploads
-        :param leaf: Leaf for this particular upload
-        :return: Contents of log file or message saying no log file was found.
-        """
-        # Retrieve log file content.
-        possible_locations = (
-            'failed', 'failed-to-move', 'rejected', 'accepted')
-        for location_dir in possible_locations:
-            log_filepath = os.path.join(root, location_dir, leaf,
-                UPLOAD_LOG_FILENAME)
-            if os.path.exists(log_filepath):
-                with open(log_filepath, 'r') as uploader_log_file:
-                    return uploader_log_file.read()
-        else:
-            return 'Could not find upload log file'
-
-    @property
-    def upload_log_url(self):
-        """See `IBuildBase`."""
-        if self.upload_log is None:
-            return None
-        return self._getProxiedFileURL(self.upload_log)
-
-    def handleStatus(self, status, librarian, slave_status):
-        """See `IBuildBase`."""
-        return handle_status_for_build(
-            self, status, librarian, slave_status, self.__class__)
-
-    @staticmethod
-    def _handleStatus_OK(build, librarian, slave_status, logger):
-        """Handle a package that built successfully.
-
-        Once built successfully, we pull the files, store them in a
-        directory, store build information and push them through the
-        uploader.
-        """
-        filemap = slave_status['filemap']
-
-        logger.info("Processing successful build %s from builder %s" % (
-            build.buildqueue_record.specific_job.build.title,
-            build.buildqueue_record.builder.name))
-        # Explode before collect a binary that is denied in this
-        # distroseries/pocket
-        if not build.archive.allowUpdatesToReleasePocket():
-            assert build.distro_series.canUploadToPocket(build.pocket), (
-                "%s (%s) can not be built for pocket %s: illegal status"
-                % (build.title, build.id, build.pocket.name))
-
-        # ensure we have the correct build root as:
-        # <BUILDMASTER_ROOT>/incoming/<UPLOAD_LEAF>/<TARGET_PATH>/[FILES]
-        root = os.path.abspath(config.builddmaster.root)
-
-        # create a single directory to store build result files
-        upload_leaf = build.getUploadDirLeaf(
-            '%s-%s' % (build.id, build.buildqueue_record.id))
-        upload_dir = build.getUploadDir(upload_leaf)
-        logger.debug("Storing build result at '%s'" % upload_dir)
-
-        # Build the right UPLOAD_PATH so the distribution and archive
-        # can be correctly found during the upload:
-        #       <archive_id>/distribution_name
-        # for all destination archive types.
-        archive = build.archive
-        distribution_name = build.distribution.name
-        target_path = '%s/%s' % (archive.id, distribution_name)
-        upload_path = os.path.join(upload_dir, target_path)
-        os.makedirs(upload_path)
-
-        slave = removeSecurityProxy(build.buildqueue_record.builder.slave)
-        successful_copy_from_slave = True
-        for filename in filemap:
-            logger.info("Grabbing file: %s" % filename)
-            out_file_name = os.path.join(upload_path, filename)
-            # If the evaluated output file name is not within our
-            # upload path, then we don't try to copy this or any
-            # subsequent files.
-            if not os.path.realpath(out_file_name).startswith(upload_path):
-                successful_copy_from_slave = False
-                logger.warning(
-                    "A slave tried to upload the file '%s' "
-                    "for the build %d." % (filename, build.id))
-                break
-            out_file = open(out_file_name, "wb")
-            slave_file = slave.getFile(filemap[filename])
-            copy_and_close(slave_file, out_file)
-
-        # We only attempt the upload if we successfully copied all the
-        # files from the slave.
-        if successful_copy_from_slave:
-            uploader_logfilename = os.path.join(
-                upload_dir, UPLOAD_LOG_FILENAME)
-            uploader_command = build.getUploaderCommand(
-                build, upload_leaf, uploader_logfilename)
-            logger.debug("Saving uploader log at '%s'" % uploader_logfilename)
-
-            logger.info("Invoking uploader on %s" % root)
-            logger.info("%s" % uploader_command)
-
-            uploader_process = subprocess.Popen(
-                uploader_command, stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE)
-
-            # Nothing should be written to the stdout/stderr.
-            upload_stdout, upload_stderr = uploader_process.communicate()
-
-            # XXX cprov 2007-04-17: we do not check uploader_result_code
-            # anywhere. We need to find out what will be best strategy
-            # when it failed HARD (there is a huge effort in process-upload
-            # to not return error, it only happen when the code is broken).
-            uploader_result_code = uploader_process.returncode
-            logger.info("Uploader returned %d" % uploader_result_code)
-
-        # Quick and dirty hack to carry on on process-upload failures
-        if os.path.exists(upload_dir):
-            logger.warning("The upload directory did not get moved.")
-            failed_dir = os.path.join(root, "failed-to-move")
-            if not os.path.exists(failed_dir):
-                os.mkdir(failed_dir)
-            os.rename(upload_dir, os.path.join(failed_dir, upload_leaf))
-
-        # The famous 'flush_updates + clear_cache' will make visible
-        # the DB changes done in process-upload, considering that the
-        # transaction was set with ISOLATION_LEVEL_READ_COMMITED
-        # isolation level.
-        cur = cursor()
-        cur.execute('SHOW transaction_isolation')
-        isolation_str = cur.fetchone()[0]
-        assert isolation_str == 'read committed', (
-            'BuildMaster/BuilderGroup transaction isolation should be '
-            'ISOLATION_LEVEL_READ_COMMITTED (not "%s")' % isolation_str)
-
-        original_slave = build.buildqueue_record.builder.slave
-
-        # XXX Robert Collins, Celso Providelo 2007-05-26 bug=506256:
-        # 'Refreshing' objects  procedure  is forced on us by using a
-        # different process to do the upload, but as that process runs
-        # in the same unix account, it is simply double handling and we
-        # would be better off to do it within this process.
-        flush_database_updates()
-        clear_current_connection_cache()
-
-        # XXX cprov 2007-06-15: Re-issuing removeSecurityProxy is forced on
-        # us by sqlobject refreshing the builder object during the
-        # transaction cache clearing. Once we sort the previous problem
-        # this step should probably not be required anymore.
-        build.buildqueue_record.builder.setSlaveForTesting(
-            removeSecurityProxy(original_slave))
-
-        # Store build information, build record was already updated during
-        # the binary upload.
-        build.storeBuildInfo(build, librarian, slave_status)
-
-        # Retrive the up-to-date build record and perform consistency
-        # checks. The build record should be updated during the binary
-        # upload processing, if it wasn't something is broken and needs
-        # admins attention. Even when we have a FULLYBUILT build record,
-        # if it is not related with at least one binary, there is also
-        # a problem.
-        # For both situations we will mark the builder as FAILEDTOUPLOAD
-        # and the and update the build details (datebuilt, duration,
-        # buildlog, builder) in LP. A build-failure-notification will be
-        # sent to the lp-build-admin celebrity and to the sourcepackagerelease
-        # uploader about this occurrence. The failure notification will
-        # also contain the information required to manually reprocess the
-        # binary upload when it was the case.
-        if (build.status != BuildStatus.FULLYBUILT or
-            not successful_copy_from_slave or
-            not build.verifySuccessfulUpload()):
-            logger.warning("Build %s upload failed." % build.id)
-            build.status = BuildStatus.FAILEDTOUPLOAD
-            uploader_log_content = build.getUploadLogContent(root,
-                upload_leaf)
-            # Store the upload_log_contents in librarian so it can be
-            # accessed by anyone with permission to see the build.
-            build.storeUploadLog(uploader_log_content)
-            # Notify the build failure.
-            build.notify(extra_info=uploader_log_content)
-        else:
-            logger.info(
-                "Gathered %s %d completely" % (
-                build.__class__.__name__, build.id))
-
-        # Release the builder for another job.
-        build.buildqueue_record.builder.cleanSlave()
-        # Remove BuildQueue record.
-        build.buildqueue_record.destroySelf()
-
-    @staticmethod
-    def _handleStatus_PACKAGEFAIL(build, librarian, slave_status, logger):
-        """Handle a package that had failed to build.
-
-        Build has failed when trying the work with the target package,
-        set the job status as FAILEDTOBUILD, store available info and
-        remove Buildqueue entry.
-        """
-        build.status = BuildStatus.FAILEDTOBUILD
-        build.storeBuildInfo(build, librarian, slave_status)
-        build.buildqueue_record.builder.cleanSlave()
-        build.notify()
-        build.buildqueue_record.destroySelf()
-
-    @staticmethod
-    def _handleStatus_DEPFAIL(build, librarian, slave_status, logger):
-        """Handle a package that had missing dependencies.
-
-        Build has failed by missing dependencies, set the job status as
-        MANUALDEPWAIT, store available information, remove BuildQueue
-        entry and release builder slave for another job.
-        """
-        build.status = BuildStatus.MANUALDEPWAIT
-        build.storeBuildInfo(build, librarian, slave_status)
-        logger.critical("***** %s is MANUALDEPWAIT *****"
-                        % build.buildqueue_record.builder.name)
-        build.buildqueue_record.builder.cleanSlave()
-        build.buildqueue_record.destroySelf()
-
-    @staticmethod
-    def _handleStatus_CHROOTFAIL(build, librarian, slave_status,
-                                 logger):
-        """Handle a package that had failed when unpacking the CHROOT.
-
-        Build has failed when installing the current CHROOT, mark the
-        job as CHROOTFAIL, store available information, remove BuildQueue
-        and release the builder.
-        """
-        build.status = BuildStatus.CHROOTWAIT
-        build.storeBuildInfo(build, librarian, slave_status)
-        logger.critical("***** %s is CHROOTWAIT *****" %
-                        build.buildqueue_record.builder.name)
-        build.buildqueue_record.builder.cleanSlave()
-        build.notify()
-        build.buildqueue_record.destroySelf()
-
-    @staticmethod
-    def _handleStatus_BUILDERFAIL(build, librarian, slave_status, logger):
-        """Handle builder failures.
-
-        Build has been failed when trying to build the target package,
-        The environment is working well, so mark the job as NEEDSBUILD again
-        and 'clean' the builder to do another jobs.
-        """
-        logger.warning("***** %s has failed *****"
-                       % build.buildqueue_record.builder.name)
-        build.buildqueue_record.builder.failBuilder(
-            "Builder returned BUILDERFAIL when asked for its status")
-        # simply reset job
-        build.storeBuildInfo(build, librarian, slave_status)
-        build.buildqueue_record.reset()
-
-    @staticmethod
-    def _handleStatus_GIVENBACK(build, librarian, slave_status, logger):
-        """Handle automatic retry requested by builder.
-
-        GIVENBACK pseudo-state represents a request for automatic retry
-        later, the build records is delayed by reducing the lastscore to
-        ZERO.
-        """
-        logger.warning("***** %s is GIVENBACK by %s *****"
-                       % (build.buildqueue_record.specific_job.build.title,
-                          build.buildqueue_record.builder.name))
-        build.storeBuildInfo(build, librarian, slave_status)
-        # XXX cprov 2006-05-30: Currently this information is not
-        # properly presented in the Web UI. We will discuss it in
-        # the next Paris Summit, infinity has some ideas about how
-        # to use this content. For now we just ensure it's stored.
-        build.buildqueue_record.builder.cleanSlave()
-        build.buildqueue_record.reset()
-
-    @staticmethod
-    def getLogFromSlave(build):
-        """See `IBuildBase`."""
-        return build.buildqueue_record.builder.transferSlaveFileToLibrarian(
-            'buildlog', build.buildqueue_record.getLogFileName(),
-            build.is_private)
-
-    @staticmethod
-    def storeBuildInfo(build, librarian, slave_status):
-        """See `IBuildBase`."""
-        # XXX michaeln 2010-05-05 bug=567922
-        # As this method is temporarily static until BuildBase is
-        # removed and the implementation moved to PackageBuild,
-        # self.attr_name is temporarily build.attr_name, which
-        # means we cannot set the build attributes.
-        naked_build = removeSecurityProxy(build)
-        naked_build.log = build.getLogFromSlave(build)
-        naked_build.builder = build.buildqueue_record.builder
-        # XXX cprov 20060615 bug=120584: Currently buildduration includes
-        # the scanner latency, it should really be asking the slave for
-        # the duration spent building locally.
-        naked_build.date_finished = datetime.datetime.now(pytz.UTC)
-        if slave_status.get('dependencies') is not None:
-            build.dependencies = unicode(slave_status.get('dependencies'))
-        else:
-            build.dependencies = None
-
-    @staticmethod
-    def createUploadLog(build, content, filename=None):
-        """Creates a file on the librarian for the upload log.
-
-        :return: ILibraryFileAlias for the upload log file.
-        """
-        # The given content is stored in the librarian, restricted as
-        # necessary according to the targeted archive's privacy.  The content
-        # object's 'upload_log' attribute will point to the
-        # `LibrarianFileAlias`.
-
-        assert build.upload_log is None, (
-            "Upload log information already exists and cannot be overridden.")
-
-        if filename is None:
-            filename = 'upload_%s_log.txt' % build.id
-        contentType = filenameToContentType(filename)
-        file_size = len(content)
-        file_content = StringIO(content)
-        restricted = build.is_private
-
-        return getUtility(ILibraryFileAliasSet).create(
-            filename, file_size, file_content, contentType=contentType,
-            restricted=restricted)
-
-    def storeUploadLog(self, content):
-        """See `IBuildBase`."""
-        library_file = self.createUploadLog(self, content)
-        self.upload_log = library_file
-
-    @staticmethod
-    def queueBuild(build, suspended=False):
-        """See `IBuildBase`"""
-        specific_job = build.makeJob()
-
-        # This build queue job is to be created in a suspended state.
-        if suspended:
-            specific_job.job.suspend()
-
-        duration_estimate = build.estimateDuration()
-        queue_entry = BuildQueue(
-            estimated_duration=duration_estimate,
-            job_type=build.build_farm_job_type,
-            job=specific_job.job, processor=specific_job.processor,
-            virtualized=specific_job.virtualized)
-        Store.of(build).add(queue_entry)
-        return queue_entry
-

=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py	2010-08-20 20:31:18 +0000
+++ lib/lp/buildmaster/model/packagebuild.py	2010-08-24 10:48:48 +0000
@@ -9,11 +9,19 @@
     ]
 
 
+import datetime
+import logging
+import os.path
+import subprocess
+
+from cStringIO import StringIO
 from lazr.delegates import delegates
+import pytz
 from storm.expr import Desc
 from storm.locals import (
     Int,
     Reference,
+    Store,
     Storm,
     Unicode,
     )
@@ -22,37 +30,53 @@
     classProvides,
     implements,
     )
+from zope.security.proxy import removeSecurityProxy
 
+from canonical.config import config
 from canonical.database.enumcol import DBEnum
+from canonical.database.sqlbase import (
+    clear_current_connection_cache,
+    cursor,
+    flush_database_updates,
+    )
 from canonical.launchpad.browser.librarian import ProxiedLibraryFileAlias
+from canonical.launchpad.helpers import filenameToContentType
+from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.launchpad.interfaces.lpstorm import IMasterStore
 from canonical.launchpad.webapp.interfaces import (
     DEFAULT_FLAVOR,
     IStoreSelector,
     MAIN_STORE,
     )
-from lp.buildmaster.interfaces.buildbase import BuildStatus
+from canonical.librarian.utils import copy_and_close
+from lp.buildmaster.interfaces.buildbase import (
+    BUILDD_MANAGER_LOG_NAME,
+    BuildStatus,
+    )
 from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSource
 from lp.buildmaster.interfaces.packagebuild import (
     IPackageBuild,
     IPackageBuildSet,
     IPackageBuildSource,
     )
-from lp.buildmaster.model.buildbase import (
-    BuildBase,
-    handle_status_for_build,
-    )
 from lp.buildmaster.model.buildfarmjob import (
     BuildFarmJob,
     BuildFarmJobDerived,
     )
-from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.buildmaster.model.buildqueue import BuildQueue
+from lp.registry.interfaces.pocket import (
+    PackagePublishingPocket,
+    pocketsuffix,
+    )
 from lp.soyuz.adapters.archivedependencies import (
     default_component_dependency_name,
     )
 from lp.soyuz.interfaces.component import IComponentSet
 
 
+UPLOAD_LOG_FILENAME = 'uploader.log'
+
+
 class PackageBuild(BuildFarmJobDerived, Storm):
     """An implementation of `IBuildFarmJob` for package builds."""
 
@@ -117,7 +141,7 @@
 
     @property
     def upload_log_url(self):
-        """See `IBuildBase`."""
+        """See `IPackageBuild`."""
         if self.upload_log is None:
             return None
         return ProxiedLibraryFileAlias(self.upload_log, self).http_url
@@ -131,52 +155,120 @@
 
     @property
     def is_private(self):
-        """See `IBuildBase`"""
+        """See `IBuildFarmJob`"""
         return self.archive.private
 
     def getUploadDirLeaf(self, build_cookie, now=None):
         """See `IPackageBuild`."""
-        return BuildBase.getUploadDirLeaf(build_cookie, now)
+        # UPLOAD_LEAF: <TIMESTAMP>-<BUILD-COOKIE>
+        if now is None:
+            now = datetime.datetime.now()
+        return '%s-%s' % (now.strftime("%Y%m%d-%H%M%S"), build_cookie)
 
     def getUploadDir(self, upload_leaf):
         """See `IPackageBuild`."""
-        return BuildBase.getUploadDir(upload_leaf)
+        return os.path.join(config.builddmaster.root, 'incoming', upload_leaf)
 
     @staticmethod
     def getUploaderCommand(package_build, upload_leaf, upload_logfilename):
         """See `IPackageBuild`."""
-        return BuildBase.getUploaderCommand(
-            package_build, upload_leaf, upload_logfilename)
+        root = os.path.abspath(config.builddmaster.root)
+        uploader_command = list(config.builddmaster.uploader.split())
+
+        # add extra arguments for processing a binary upload
+        extra_args = [
+            "--log-file", "%s" % upload_logfilename,
+            "-d", "%s" % package_build.distribution.name,
+            "-s", "%s" % (package_build.distro_series.name +
+                          pocketsuffix[package_build.pocket]),
+            "-b", "%s" % package_build.id,
+            "-J", "%s" % upload_leaf,
+            '--context=%s' % package_build.policy_name,
+            "%s" % root,
+            ]
+
+        uploader_command.extend(extra_args)
+        return uploader_command
 
     @staticmethod
     def getLogFromSlave(package_build):
         """See `IPackageBuild`."""
-        return BuildBase.getLogFromSlave(package_build)
+        builder = package_build.buildqueue_record.builder
+        return builder.transferSlaveFileToLibrarian(
+            'buildlog', package_build.buildqueue_record.getLogFileName(),
+            package_build.is_private)
 
     @staticmethod
     def getUploadLogContent(root, leaf):
-        """See `IPackageBuild`."""
-        return BuildBase.getUploadLogContent(root, leaf)
+        """Retrieve the upload log contents.
+
+        :param root: Root directory for the uploads
+        :param leaf: Leaf for this particular upload
+        :return: Contents of log file or message saying no log file was found.
+        """
+        # Retrieve log file content.
+        possible_locations = (
+            'failed', 'failed-to-move', 'rejected', 'accepted')
+        for location_dir in possible_locations:
+            log_filepath = os.path.join(root, location_dir, leaf,
+                UPLOAD_LOG_FILENAME)
+            if os.path.exists(log_filepath):
+                with open(log_filepath, 'r') as uploader_log_file:
+                    return uploader_log_file.read()
+        else:
+            return 'Could not find upload log file'
 
     def estimateDuration(self):
         """See `IPackageBuild`."""
         raise NotImplementedError
 
     @staticmethod
-    def storeBuildInfo(package_build, librarian, slave_status):
+    def storeBuildInfo(build, librarian, slave_status):
         """See `IPackageBuild`."""
-        return BuildBase.storeBuildInfo(package_build, librarian,
-                                        slave_status)
+        naked_build = removeSecurityProxy(build)
+        naked_build.log = build.getLogFromSlave(build)
+        naked_build.builder = build.buildqueue_record.builder
+        # XXX cprov 20060615 bug=120584: Currently buildduration includes
+        # the scanner latency, it should really be asking the slave for
+        # the duration spent building locally.
+        naked_build.date_finished = datetime.datetime.now(pytz.UTC)
+        if slave_status.get('dependencies') is not None:
+            build.dependencies = unicode(slave_status.get('dependencies'))
+        else:
+            build.dependencies = None
 
     def verifySuccessfulUpload(self):
         """See `IPackageBuild`."""
         raise NotImplementedError
 
+    def createUploadLog(self, content, filename=None):
+        """Creates a file on the librarian for the upload log.
+
+        :return: ILibraryFileAlias for the upload log file.
+        """
+        # The given content is stored in the librarian, restricted as
+        # necessary according to the targeted archive's privacy.  The content
+        # object's 'upload_log' attribute will point to the
+        # `LibrarianFileAlias`.
+
+        assert self.upload_log is None, (
+            "Upload log information already exists and cannot be overridden.")
+
+        if filename is None:
+            filename = 'upload_%s_log.txt' % self.id
+        contentType = filenameToContentType(filename)
+        file_size = len(content)
+        file_content = StringIO(content)
+        restricted = self.is_private
+
+        return getUtility(ILibraryFileAliasSet).create(
+            filename, file_size, file_content, contentType=contentType,
+            restricted=restricted)
+
     def storeUploadLog(self, content):
         """See `IPackageBuild`."""
         filename = "upload_%s_log.txt" % self.build_farm_job.id
-        library_file = BuildBase.createUploadLog(
-            self, content, filename=filename)
+        library_file = self.createUploadLog(content, filename=filename)
         self.upload_log = library_file
 
     def notify(self, extra_info=None):
@@ -202,39 +294,263 @@
 
     def queueBuild(self, suspended=False):
         """See `IPackageBuild`."""
-        return BuildBase.queueBuild(self, suspended=suspended)
+        specific_job = self.makeJob()
+
+        # This build queue job is to be created in a suspended state.
+        if suspended:
+            specific_job.job.suspend()
+
+        duration_estimate = self.estimateDuration()
+        queue_entry = BuildQueue(
+            estimated_duration=duration_estimate,
+            job_type=self.build_farm_job_type,
+            job=specific_job.job, processor=specific_job.processor,
+            virtualized=specific_job.virtualized)
+        Store.of(self).add(queue_entry)
+        return queue_entry
 
     def handleStatus(self, status, librarian, slave_status):
         """See `IPackageBuild`."""
-        return handle_status_for_build(self, status, librarian, slave_status)
+        logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
+        method = getattr(self, '_handleStatus_' + status, None)
+        if method is None:
+            logger.critical("Unknown BuildStatus '%s' for builder '%s'"
+                            % (status, self.buildqueue_record.builder.url))
+            return
+        method(librarian, slave_status, logger)
 
-    # The following private handlers currently re-use the BuildBase
-    # implementation until it is no longer in use. If we find in the
-    # future that it would be useful to delegate these also, they can be
-    # added to IBuildFarmJob or IPackageBuild as necessary.
     def _handleStatus_OK(self, librarian, slave_status, logger):
-        return BuildBase._handleStatus_OK(
-            self, librarian, slave_status, logger)
+        """Handle a package that built successfully.
+
+        Once built successfully, we pull the files, store them in a
+        directory, store build information and push them through the
+        uploader.
+        """
+        filemap = slave_status['filemap']
+
+        logger.info("Processing successful build %s from builder %s" % (
+            self.buildqueue_record.specific_job.build.title,
+            self.buildqueue_record.builder.name))
+        # Explode before collect a binary that is denied in this
+        # distroseries/pocket
+        if not self.archive.allowUpdatesToReleasePocket():
+            assert self.distro_series.canUploadToPocket(self.pocket), (
+                "%s (%s) can not be built for pocket %s: illegal status"
+                % (self.title, self.id, self.pocket.name))
+
+        # ensure we have the correct build root as:
+        # <BUILDMASTER_ROOT>/incoming/<UPLOAD_LEAF>/<TARGET_PATH>/[FILES]
+        root = os.path.abspath(config.builddmaster.root)
+
+        # create a single directory to store build result files
+        upload_leaf = self.getUploadDirLeaf(
+            '%s-%s' % (self.id, self.buildqueue_record.id))
+        upload_dir = self.getUploadDir(upload_leaf)
+        logger.debug("Storing build result at '%s'" % upload_dir)
+
+        # Build the right UPLOAD_PATH so the distribution and archive
+        # can be correctly found during the upload:
+        #       <archive_id>/distribution_name
+        # for all destination archive types.
+        archive = self.archive
+        distribution_name = self.distribution.name
+        target_path = '%s/%s' % (archive.id, distribution_name)
+        upload_path = os.path.join(upload_dir, target_path)
+        os.makedirs(upload_path)
+
+        slave = removeSecurityProxy(self.buildqueue_record.builder.slave)
+        successful_copy_from_slave = True
+        for filename in filemap:
+            logger.info("Grabbing file: %s" % filename)
+            out_file_name = os.path.join(upload_path, filename)
+            # If the evaluated output file name is not within our
+            # upload path, then we don't try to copy this or any
+            # subsequent files.
+            if not os.path.realpath(out_file_name).startswith(upload_path):
+                successful_copy_from_slave = False
+                logger.warning(
+                    "A slave tried to upload the file '%s' "
+                    "for the build %d." % (filename, self.id))
+                break
+            out_file = open(out_file_name, "wb")
+            slave_file = slave.getFile(filemap[filename])
+            copy_and_close(slave_file, out_file)
+
+        # We only attempt the upload if we successfully copied all the
+        # files from the slave.
+        if successful_copy_from_slave:
+            uploader_logfilename = os.path.join(
+                upload_dir, UPLOAD_LOG_FILENAME)
+            uploader_command = self.getUploaderCommand(
+                self, upload_leaf, uploader_logfilename)
+            logger.debug("Saving uploader log at '%s'" % uploader_logfilename)
+
+            logger.info("Invoking uploader on %s" % root)
+            logger.info("%s" % uploader_command)
+
+            uploader_process = subprocess.Popen(
+                uploader_command, stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE)
+
+            # Nothing should be written to the stdout/stderr.
+            upload_stdout, upload_stderr = uploader_process.communicate()
+
+            # XXX cprov 2007-04-17: we do not check uploader_result_code
+            # anywhere. We need to find out what will be best strategy
+            # when it failed HARD (there is a huge effort in process-upload
+            # to not return error, it only happen when the code is broken).
+            uploader_result_code = uploader_process.returncode
+            logger.info("Uploader returned %d" % uploader_result_code)
+
+        # Quick and dirty hack to carry on on process-upload failures
+        if os.path.exists(upload_dir):
+            logger.warning("The upload directory did not get moved.")
+            failed_dir = os.path.join(root, "failed-to-move")
+            if not os.path.exists(failed_dir):
+                os.mkdir(failed_dir)
+            os.rename(upload_dir, os.path.join(failed_dir, upload_leaf))
+
+        # The famous 'flush_updates + clear_cache' will make visible
+        # the DB changes done in process-upload, considering that the
+        # transaction was set with ISOLATION_LEVEL_READ_COMMITED
+        # isolation level.
+        cur = cursor()
+        cur.execute('SHOW transaction_isolation')
+        isolation_str = cur.fetchone()[0]
+        assert isolation_str == 'read committed', (
+            'BuildMaster/BuilderGroup transaction isolation should be '
+            'ISOLATION_LEVEL_READ_COMMITTED (not "%s")' % isolation_str)
+
+        original_slave = self.buildqueue_record.builder.slave
+
+        # XXX Robert Collins, Celso Providelo 2007-05-26 bug=506256:
+        # 'Refreshing' objects  procedure  is forced on us by using a
+        # different process to do the upload, but as that process runs
+        # in the same unix account, it is simply double handling and we
+        # would be better off to do it within this process.
+        flush_database_updates()
+        clear_current_connection_cache()
+
+        # XXX cprov 2007-06-15: Re-issuing removeSecurityProxy is forced on
+        # us by sqlobject refreshing the builder object during the
+        # transaction cache clearing. Once we sort the previous problem
+        # this step should probably not be required anymore.
+        self.buildqueue_record.builder.setSlaveForTesting(
+            removeSecurityProxy(original_slave))
+
+        # Store build information, build record was already updated during
+        # the binary upload.
+        self.storeBuildInfo(self, librarian, slave_status)
+
+        # Retrive the up-to-date build record and perform consistency
+        # checks. The build record should be updated during the binary
+        # upload processing, if it wasn't something is broken and needs
+        # admins attention. Even when we have a FULLYBUILT build record,
+        # if it is not related with at least one binary, there is also
+        # a problem.
+        # For both situations we will mark the builder as FAILEDTOUPLOAD
+        # and the and update the build details (datebuilt, duration,
+        # buildlog, builder) in LP. A build-failure-notification will be
+        # sent to the lp-build-admin celebrity and to the sourcepackagerelease
+        # uploader about this occurrence. The failure notification will
+        # also contain the information required to manually reprocess the
+        # binary upload when it was the case.
+        if (self.status != BuildStatus.FULLYBUILT or
+            not successful_copy_from_slave or
+            not self.verifySuccessfulUpload()):
+            logger.warning("Build %s upload failed." % self.id)
+            self.status = BuildStatus.FAILEDTOUPLOAD
+            uploader_log_content = self.getUploadLogContent(root,
+                upload_leaf)
+            # Store the upload_log_contents in librarian so it can be
+            # accessed by anyone with permission to see the build.
+            self.storeUploadLog(uploader_log_content)
+            # Notify the build failure.
+            self.notify(extra_info=uploader_log_content)
+        else:
+            logger.info(
+                "Gathered %s %d completely" % (
+                self.__class__.__name__, self.id))
+
+        # Release the builder for another job.
+        self.buildqueue_record.builder.cleanSlave()
+        # Remove BuildQueue record.
+        self.buildqueue_record.destroySelf()
 
     def _handleStatus_PACKAGEFAIL(self, librarian, slave_status, logger):
-        return BuildBase._handleStatus_PACKAGEFAIL(
-            self, librarian, slave_status, logger)
+        """Handle a package that had failed to build.
+
+        Build has failed when trying the work with the target package,
+        set the job status as FAILEDTOBUILD, store available info and
+        remove Buildqueue entry.
+        """
+        self.status = BuildStatus.FAILEDTOBUILD
+        self.storeBuildInfo(self, librarian, slave_status)
+        self.buildqueue_record.builder.cleanSlave()
+        self.notify()
+        self.buildqueue_record.destroySelf()
 
     def _handleStatus_DEPFAIL(self, librarian, slave_status, logger):
-        return BuildBase._handleStatus_DEPFAIL(
-            self, librarian, slave_status, logger)
+        """Handle a package that had missing dependencies.
+
+        Build has failed by missing dependencies, set the job status as
+        MANUALDEPWAIT, store available information, remove BuildQueue
+        entry and release builder slave for another job.
+        """
+        self.status = BuildStatus.MANUALDEPWAIT
+        self.storeBuildInfo(self, librarian, slave_status)
+        logger.critical("***** %s is MANUALDEPWAIT *****"
+                        % self.buildqueue_record.builder.name)
+        self.buildqueue_record.builder.cleanSlave()
+        self.buildqueue_record.destroySelf()
 
     def _handleStatus_CHROOTFAIL(self, librarian, slave_status, logger):
-        return BuildBase._handleStatus_CHROOTFAIL(
-            self, librarian, slave_status, logger)
+        """Handle a package that had failed when unpacking the CHROOT.
+
+        Build has failed when installing the current CHROOT, mark the
+        job as CHROOTFAIL, store available information, remove BuildQueue
+        and release the builder.
+        """
+        self.status = BuildStatus.CHROOTWAIT
+        self.storeBuildInfo(self, librarian, slave_status)
+        logger.critical("***** %s is CHROOTWAIT *****" %
+                        self.buildqueue_record.builder.name)
+        self.buildqueue_record.builder.cleanSlave()
+        self.notify()
+        self.buildqueue_record.destroySelf()
 
     def _handleStatus_BUILDERFAIL(self, librarian, slave_status, logger):
-        return BuildBase._handleStatus_BUILDERFAIL(
-            self, librarian, slave_status, logger)
+        """Handle builder failures.
+
+        Build has been failed when trying to build the target package,
+        The environment is working well, so mark the job as NEEDSBUILD again
+        and 'clean' the builder to do another jobs.
+        """
+        logger.warning("***** %s has failed *****"
+                       % self.buildqueue_record.builder.name)
+        self.buildqueue_record.builder.failBuilder(
+            "Builder returned BUILDERFAIL when asked for its status")
+        # simply reset job
+        self.storeBuildInfo(self, librarian, slave_status)
+        self.buildqueue_record.reset()
 
     def _handleStatus_GIVENBACK(self, librarian, slave_status, logger):
-        return BuildBase._handleStatus_GIVENBACK(
-            self, librarian, slave_status, logger)
+        """Handle automatic retry requested by builder.
+
+        GIVENBACK pseudo-state represents a request for automatic retry
+        later, the build records is delayed by reducing the lastscore to
+        ZERO.
+        """
+        logger.warning("***** %s is GIVENBACK by %s *****"
+                       % (self.buildqueue_record.specific_job.build.title,
+                          self.buildqueue_record.builder.name))
+        self.storeBuildInfo(self, librarian, slave_status)
+        # XXX cprov 2006-05-30: Currently this information is not
+        # properly presented in the Web UI. We will discuss it in
+        # the next Paris Summit, infinity has some ideas about how
+        # to use this content. For now we just ensure it's stored.
+        self.buildqueue_record.builder.cleanSlave()
+        self.buildqueue_record.reset()
 
 
 class PackageBuildSet:

=== removed file 'lib/lp/buildmaster/tests/test_buildbase.py'
--- lib/lp/buildmaster/tests/test_buildbase.py	2010-08-20 20:31:18 +0000
+++ lib/lp/buildmaster/tests/test_buildbase.py	1970-01-01 00:00:00 +0000
@@ -1,225 +0,0 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-from __future__ import with_statement
-
-"""Tests for `IBuildBase`.
-
-   XXX 2010-04-26 michael.nelson bug=567922.
-   These tests should be moved into test_packagebuild when buildbase is
-   deleted. For the moment, test_packagebuild inherits these tests to
-   ensure the new classes pass too.
-"""
-__metaclass__ = type
-
-from datetime import datetime
-import os
-
-from zope.security.proxy import removeSecurityProxy
-
-from canonical.config import config
-from canonical.database.constants import UTC_NOW
-from canonical.testing.layers import LaunchpadZopelessLayer
-from lp.buildmaster.interfaces.buildbase import BuildStatus
-from lp.buildmaster.model.buildbase import BuildBase
-from lp.registry.interfaces.pocket import pocketsuffix
-from lp.soyuz.tests.soyuzbuilddhelpers import WaitingSlave
-from lp.testing import TestCase
-from lp.testing.fakemethod import FakeMethod
-
-
-class TestBuildBaseMixin:
-    """Tests for `IBuildBase`."""
-
-    def test_getUploadDirLeaf(self):
-        # getUploadDirLeaf returns the current time, followed by the build
-        # cookie.
-        now = datetime.now()
-        build_cookie = self.factory.getUniqueString()
-        upload_leaf = self.package_build.getUploadDirLeaf(
-            build_cookie, now=now)
-        self.assertEqual(
-            '%s-%s' % (now.strftime("%Y%m%d-%H%M%S"), build_cookie),
-            upload_leaf)
-
-    def test_getUploadDir(self):
-        # getUploadDir is the absolute path to the directory in which things
-        # are uploaded to.
-        build_cookie = self.factory.getUniqueInteger()
-        upload_leaf = self.package_build.getUploadDirLeaf(build_cookie)
-        upload_dir = self.package_build.getUploadDir(upload_leaf)
-        self.assertEqual(
-            os.path.join(config.builddmaster.root, 'incoming', upload_leaf),
-            upload_dir)
-
-
-class TestBuildBase(TestCase, TestBuildBaseMixin):
-
-    def setUp(self):
-        """Create the package build for testing."""
-        super(TestBuildBase, self).setUp()
-        self.package_build = BuildBase()
-
-
-class TestGetUploadMethodsMixin:
-    """Tests for `IBuildBase` that need objects from the rest of Launchpad."""
-
-    layer = LaunchpadZopelessLayer
-
-    def makeBuild(self):
-        """Allow classes to override the build with which the test runs.
-
-        XXX michaeln 2010-06-03 bug=567922
-        Until buildbase is removed, we need to ensure these tests
-        run against new IPackageBuild builds (BinaryPackageBuild)
-        and the IBuildBase builds (SPRecipeBuild). They assume the build
-        is successfully built and check that incorrect upload paths will
-        set the status to FAILEDTOUPLOAD.
-        """
-        raise NotImplemented
-
-    def setUp(self):
-        super(TestGetUploadMethodsMixin, self).setUp()
-        self.build = self.makeBuild()
-
-    def test_getUploadLogContent_nolog(self):
-        """If there is no log file there, a string explanation is returned.
-        """
-        self.useTempDir()
-        self.assertEquals('Could not find upload log file',
-            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
-
-    def test_getUploadLogContent_only_dir(self):
-        """If there is a directory but no log file, expect the error string,
-        not an exception."""
-        self.useTempDir()
-        os.makedirs("accepted/myleaf")
-        self.assertEquals('Could not find upload log file',
-            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
-
-    def test_getUploadLogContent_readsfile(self):
-        """If there is a log file, return its contents."""
-        self.useTempDir()
-        os.makedirs("accepted/myleaf")
-        with open('accepted/myleaf/uploader.log', 'w') as f:
-            f.write('foo')
-        self.assertEquals('foo',
-            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
-
-    def test_getUploaderCommand(self):
-        upload_leaf = self.factory.getUniqueString('upload-leaf')
-        config_args = list(config.builddmaster.uploader.split())
-        log_file = self.factory.getUniqueString('logfile')
-        config_args.extend(
-            ['--log-file', log_file,
-             '-d', self.build.distribution.name,
-             '-s', (self.build.distro_series.name
-                    + pocketsuffix[self.build.pocket]),
-             '-b', str(self.build.id),
-             '-J', upload_leaf,
-             '--context=%s' % self.build.policy_name,
-             os.path.abspath(config.builddmaster.root),
-             ])
-        uploader_command = self.build.getUploaderCommand(
-            self.build, upload_leaf, log_file)
-        self.assertEqual(config_args, uploader_command)
-
-
-class TestHandleStatusMixin:
-    """Tests for `IBuildBase`s handleStatus method.
-
-    Note: these tests do *not* test the updating of the build
-    status to FULLYBUILT as this happens during the upload which
-    is stubbed out by a mock function.
-    """
-
-    layer = LaunchpadZopelessLayer
-
-    def makeBuild(self):
-        """Allow classes to override the build with which the test runs.
-
-        XXX michaeln 2010-06-03 bug=567922
-        Until buildbase is removed, we need to ensure these tests
-        run against new IPackageBuild builds (BinaryPackageBuild)
-        and the IBuildBase builds (SPRecipeBuild). They assume the build
-        is successfully built and check that incorrect upload paths will
-        set the status to FAILEDTOUPLOAD.
-        """
-        raise NotImplementedError
-
-    def setUp(self):
-        super(TestHandleStatusMixin, self).setUp()
-        self.build = self.makeBuild()
-        # For the moment, we require a builder for the build so that
-        # handleStatus_OK can get a reference to the slave.
-        builder = self.factory.makeBuilder()
-        self.build.buildqueue_record.builder = builder
-        self.build.buildqueue_record.setDateStarted(UTC_NOW)
-        self.slave = WaitingSlave('BuildStatus.OK')
-        self.slave.valid_file_hashes.append('test_file_hash')
-        builder.setSlaveForTesting(self.slave)
-
-        # We overwrite the buildmaster root to use a temp directory.
-        tmp_dir = self.makeTemporaryDirectory()
-        tmp_builddmaster_root = """
-        [builddmaster]
-        root: %s
-        """ % tmp_dir
-        config.push('tmp_builddmaster_root', tmp_builddmaster_root)
-
-        # We stub out our builds getUploaderCommand() method so
-        # we can check whether it was called as well as
-        # verifySuccessfulUpload().
-        self.fake_getUploaderCommand = FakeMethod(
-            result=['echo', 'noop'])
-        removeSecurityProxy(self.build).getUploaderCommand = (
-            self.fake_getUploaderCommand)
-        removeSecurityProxy(self.build).verifySuccessfulUpload = FakeMethod(
-            result=True)
-
-    def test_handleStatus_OK_normal_file(self):
-        # A filemap with plain filenames should not cause a problem.
-        # The call to handleStatus will attempt to get the file from
-        # the slave resulting in a URL error in this test case.
-        self.build.handleStatus('OK', None, {
-                'filemap': {'myfile.py': 'test_file_hash'},
-                })
-
-        self.assertEqual(BuildStatus.FULLYBUILT, self.build.status)
-        self.assertEqual(1, self.fake_getUploaderCommand.call_count)
-
-    def test_handleStatus_OK_absolute_filepath(self):
-        # A filemap that tries to write to files outside of
-        # the upload directory will result in a failed upload.
-        self.build.handleStatus('OK', None, {
-            'filemap': {'/tmp/myfile.py': 'test_file_hash'},
-            })
-        self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
-        self.assertEqual(0, self.fake_getUploaderCommand.call_count)
-
-    def test_handleStatus_OK_relative_filepath(self):
-        # A filemap that tries to write to files outside of
-        # the upload directory will result in a failed upload.
-        self.build.handleStatus('OK', None, {
-            'filemap': {'../myfile.py': 'test_file_hash'},
-            })
-        self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
-        self.assertEqual(0, self.fake_getUploaderCommand.call_count)
-
-    def test_handleStatus_OK_sets_build_log(self):
-        # The build log is set during handleStatus.
-        removeSecurityProxy(self.build).log = None
-        self.assertEqual(None, self.build.log)
-        self.build.handleStatus('OK', None, {
-                'filemap': {'myfile.py': 'test_file_hash'},
-                })
-        self.assertNotEqual(None, self.build.log)
-
-    def test_date_finished_set(self):
-        # The date finished is updated during handleStatus_OK.
-        removeSecurityProxy(self.build).date_finished = None
-        self.assertEqual(None, self.build.date_finished)
-        self.build.handleStatus('OK', None, {
-                'filemap': {'myfile.py': 'test_file_hash'},
-                })
-        self.assertNotEqual(None, self.build.date_finished)

=== modified file 'lib/lp/buildmaster/tests/test_packagebuild.py'
--- lib/lp/buildmaster/tests/test_packagebuild.py	2010-08-20 20:31:18 +0000
+++ lib/lp/buildmaster/tests/test_packagebuild.py	2010-08-24 10:48:48 +0000
@@ -5,14 +5,21 @@
 
 __metaclass__ = type
 
+from datetime import datetime
 import hashlib
+import os.path
 
 from storm.store import Store
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
-from canonical.testing.layers import LaunchpadFunctionalLayer
+from canonical.config import config
+from canonical.database.constants import UTC_NOW
+from canonical.testing.layers import (
+    LaunchpadFunctionalLayer,
+    LaunchpadZopelessLayer,
+    )
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.buildmaster.interfaces.buildfarmjob import BuildFarmJobType
 from lp.buildmaster.interfaces.packagebuild import (
@@ -21,13 +28,17 @@
     IPackageBuildSource,
     )
 from lp.buildmaster.model.packagebuild import PackageBuild
-from lp.buildmaster.tests.test_buildbase import TestBuildBaseMixin
-from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.pocket import (
+    PackagePublishingPocket,
+    pocketsuffix,
+    )
+from lp.soyuz.tests.soyuzbuilddhelpers import WaitingSlave
 from lp.testing import (
     login,
     login_person,
     TestCaseWithFactory,
     )
+from lp.testing.fakemethod import FakeMethod
 
 
 class TestPackageBuildBase(TestCaseWithFactory):
@@ -49,21 +60,6 @@
             status=status, pocket=pocket)
 
 
-class TestBuildBaseMethods(TestPackageBuildBase, TestBuildBaseMixin):
-    """The new PackageBuild class provides the same methods as the BuildBase.
-
-    XXX 2010-04-21 michael.nelson bug=567922.
-    Until the BuildBase class and its tests are removed, we re-use the tests
-    here to ensure that there is no divergence. Once BuildBase is removed the
-    tests can be moved into TestPackageBuild.
-    """
-    layer = LaunchpadFunctionalLayer
-
-    def setUp(self):
-        super(TestBuildBaseMethods, self).setUp()
-        self.package_build = self.makePackageBuild()
-
-
 class TestPackageBuild(TestPackageBuildBase):
     """Tests for the package build object."""
 
@@ -181,6 +177,27 @@
         self.failUnlessEqual(
             u'My deps', self.package_build.dependencies)
 
+    def test_getUploadDirLeaf(self):
+        # getUploadDirLeaf returns the current time, followed by the build
+        # cookie.
+        now = datetime.now()
+        build_cookie = self.factory.getUniqueString()
+        upload_leaf = self.package_build.getUploadDirLeaf(
+            build_cookie, now=now)
+        self.assertEqual(
+            '%s-%s' % (now.strftime("%Y%m%d-%H%M%S"), build_cookie),
+            upload_leaf)
+
+    def test_getUploadDir(self):
+        # getUploadDir is the absolute path to the directory in which things
+        # are uploaded to.
+        build_cookie = self.factory.getUniqueInteger()
+        upload_leaf = self.package_build.getUploadDirLeaf(build_cookie)
+        upload_dir = self.package_build.getUploadDir(upload_leaf)
+        self.assertEqual(
+            os.path.join(config.builddmaster.root, 'incoming', upload_leaf),
+            upload_dir)
+
 
 class TestPackageBuildSet(TestPackageBuildBase):
 
@@ -221,3 +238,151 @@
             self.package_builds[:1],
             self.package_build_set.getBuildsForArchive(
                 self.archive, pocket=PackagePublishingPocket.UPDATES))
+
+
+class TestGetUploadMethodsMixin:
+    """Tests for `IPackageBuild` that need objects from the rest of LP."""
+
+    layer = LaunchpadZopelessLayer
+
+    def makeBuild(self):
+        """Allow classes to override the build with which the test runs."""
+        raise NotImplemented
+
+    def setUp(self):
+        super(TestGetUploadMethodsMixin, self).setUp()
+        self.build = self.makeBuild()
+
+    def test_getUploadLogContent_nolog(self):
+        """If there is no log file there, a string explanation is returned.
+        """
+        self.useTempDir()
+        self.assertEquals('Could not find upload log file',
+            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
+
+    def test_getUploadLogContent_only_dir(self):
+        """If there is a directory but no log file, expect the error string,
+        not an exception."""
+        self.useTempDir()
+        os.makedirs("accepted/myleaf")
+        self.assertEquals('Could not find upload log file',
+            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
+
+    def test_getUploadLogContent_readsfile(self):
+        """If there is a log file, return its contents."""
+        self.useTempDir()
+        os.makedirs("accepted/myleaf")
+        with open('accepted/myleaf/uploader.log', 'w') as f:
+            f.write('foo')
+        self.assertEquals('foo',
+            self.build.getUploadLogContent(os.getcwd(), "myleaf"))
+
+    def test_getUploaderCommand(self):
+        upload_leaf = self.factory.getUniqueString('upload-leaf')
+        config_args = list(config.builddmaster.uploader.split())
+        log_file = self.factory.getUniqueString('logfile')
+        config_args.extend(
+            ['--log-file', log_file,
+             '-d', self.build.distribution.name,
+             '-s', (self.build.distro_series.name
+                    + pocketsuffix[self.build.pocket]),
+             '-b', str(self.build.id),
+             '-J', upload_leaf,
+             '--context=%s' % self.build.policy_name,
+             os.path.abspath(config.builddmaster.root),
+             ])
+        uploader_command = self.build.getUploaderCommand(
+            self.build, upload_leaf, log_file)
+        self.assertEqual(config_args, uploader_command)
+
+
+class TestHandleStatusMixin:
+    """Tests for `IPackageBuild`s handleStatus method.
+
+    Note: these tests do *not* test the updating of the build
+    status to FULLYBUILT as this happens during the upload which
+    is stubbed out by a mock function.
+    """
+
+    layer = LaunchpadZopelessLayer
+
+    def makeBuild(self):
+        """Allow classes to override the build with which the test runs."""
+        raise NotImplementedError
+
+    def setUp(self):
+        super(TestHandleStatusMixin, self).setUp()
+        self.build = self.makeBuild()
+        # For the moment, we require a builder for the build so that
+        # handleStatus_OK can get a reference to the slave.
+        builder = self.factory.makeBuilder()
+        self.build.buildqueue_record.builder = builder
+        self.build.buildqueue_record.setDateStarted(UTC_NOW)
+        self.slave = WaitingSlave('BuildStatus.OK')
+        self.slave.valid_file_hashes.append('test_file_hash')
+        builder.setSlaveForTesting(self.slave)
+
+        # We overwrite the buildmaster root to use a temp directory.
+        tmp_dir = self.makeTemporaryDirectory()
+        tmp_builddmaster_root = """
+        [builddmaster]
+        root: %s
+        """ % tmp_dir
+        config.push('tmp_builddmaster_root', tmp_builddmaster_root)
+
+        # We stub out our builds getUploaderCommand() method so
+        # we can check whether it was called as well as
+        # verifySuccessfulUpload().
+        self.fake_getUploaderCommand = FakeMethod(
+            result=['echo', 'noop'])
+        removeSecurityProxy(self.build).getUploaderCommand = (
+            self.fake_getUploaderCommand)
+        removeSecurityProxy(self.build).verifySuccessfulUpload = FakeMethod(
+            result=True)
+
+    def test_handleStatus_OK_normal_file(self):
+        # A filemap with plain filenames should not cause a problem.
+        # The call to handleStatus will attempt to get the file from
+        # the slave resulting in a URL error in this test case.
+        self.build.handleStatus('OK', None, {
+                'filemap': {'myfile.py': 'test_file_hash'},
+                })
+
+        self.assertEqual(BuildStatus.FULLYBUILT, self.build.status)
+        self.assertEqual(1, self.fake_getUploaderCommand.call_count)
+
+    def test_handleStatus_OK_absolute_filepath(self):
+        # A filemap that tries to write to files outside of
+        # the upload directory will result in a failed upload.
+        self.build.handleStatus('OK', None, {
+            'filemap': {'/tmp/myfile.py': 'test_file_hash'},
+            })
+        self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
+        self.assertEqual(0, self.fake_getUploaderCommand.call_count)
+
+    def test_handleStatus_OK_relative_filepath(self):
+        # A filemap that tries to write to files outside of
+        # the upload directory will result in a failed upload.
+        self.build.handleStatus('OK', None, {
+            'filemap': {'../myfile.py': 'test_file_hash'},
+            })
+        self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
+        self.assertEqual(0, self.fake_getUploaderCommand.call_count)
+
+    def test_handleStatus_OK_sets_build_log(self):
+        # The build log is set during handleStatus.
+        removeSecurityProxy(self.build).log = None
+        self.assertEqual(None, self.build.log)
+        self.build.handleStatus('OK', None, {
+                'filemap': {'myfile.py': 'test_file_hash'},
+                })
+        self.assertNotEqual(None, self.build.log)
+
+    def test_date_finished_set(self):
+        # The date finished is updated during handleStatus_OK.
+        removeSecurityProxy(self.build).date_finished = None
+        self.assertEqual(None, self.build.date_finished)
+        self.build.handleStatus('OK', None, {
+                'filemap': {'myfile.py': 'test_file_hash'},
+                })
+        self.assertNotEqual(None, self.build.date_finished)

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-08-24 10:48:48 +0000
@@ -46,7 +46,6 @@
     )
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.buildmaster.interfaces.buildfarmjob import BuildFarmJobType
-from lp.buildmaster.model.buildbase import BuildBase
 from lp.buildmaster.model.buildfarmjob import BuildFarmJobOldDerived
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.buildmaster.model.packagebuild import (
@@ -128,7 +127,7 @@
 
     @property
     def distribution(self):
-        """See `IBuildBase`."""
+        """See `IPackageBuild`."""
         return self.distroseries.distribution
 
     is_virtualized = True
@@ -160,7 +159,7 @@
 
     @property
     def buildqueue_record(self):
-        """See `IBuildBase`."""
+        """See `IBuildFarmJob`."""
         store = Store.of(self)
         results = store.find(
             BuildQueue,
@@ -278,7 +277,7 @@
         return specific_job
 
     def estimateDuration(self):
-        """See `IBuildBase`."""
+        """See `IPackageBuild`."""
         median = self.recipe.getMedianBuildDuration()
         if median is not None:
             return median
@@ -288,7 +287,7 @@
         return self.source_package_release is not None
 
     def notify(self, extra_info=None):
-        """See `IBuildBase`."""
+        """See `IPackageBuild`."""
         mailer = SourcePackageRecipeBuildMailer.forStatus(self)
         mailer.sendAll()
 
@@ -327,13 +326,13 @@
         except KeyError:
             raise NotFoundError(filename)
 
-    @staticmethod
-    def _handleStatus_OK(build, librarian, slave_status, logger):
-        """See `IBuildBase`."""
-        BuildBase._handleStatus_OK(build, librarian, slave_status, logger)
+    def _handleStatus_OK(self, librarian, slave_status, logger):
+        """See `IPackageBuild`."""
+        super(SourcePackageRecipeBuild, self)._handleStatus_OK(
+            librarian, slave_status, logger)
         # base implementation doesn't notify on success.
-        if build.status == BuildStatus.FULLYBUILT:
-            build.notify()
+        if self.status == BuildStatus.FULLYBUILT:
+            self.notify()
 
 
 class SourcePackageRecipeBuildJob(BuildFarmJobOldDerived, Storm):

=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-08-20 20:31:18 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-08-24 10:48:48 +0000
@@ -26,7 +26,7 @@
 from lp.app.errors import NotFoundError
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
-from lp.buildmaster.tests.test_buildbase import (
+from lp.buildmaster.tests.test_packagebuild import (
     TestGetUploadMethodsMixin,
     TestHandleStatusMixin,
     )
@@ -77,7 +77,7 @@
             requester=person)
 
     def test_providesInterfaces(self):
-        # SourcePackageRecipeBuild provides IBuildBase and
+        # SourcePackageRecipeBuild provides IPackageBuild and
         # ISourcePackageRecipeBuild.
         spb = self.makeSourcePackageRecipeBuild()
         self.assertProvides(spb, ISourcePackageRecipeBuild)
@@ -386,12 +386,12 @@
 
 class TestGetUploadMethodsForSPRecipeBuild(
     MakeSPRecipeBuildMixin, TestGetUploadMethodsMixin, TestCaseWithFactory):
-    """IBuildBase.getUpload-related methods work with SPRecipe builds."""
+    """IPackageBuild.getUpload-related methods work with SPRecipe builds."""
 
 
 class TestHandleStatusForSPRBuild(
     MakeSPRecipeBuildMixin, TestHandleStatusMixin, TestCaseWithFactory):
-    """IBuildBase.handleStatus works with SPRecipe builds."""
+    """IPackageBuild.handleStatus works with SPRecipe builds."""
 
 
 def test_suite():

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2010-08-20 20:31:18 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2010-08-24 10:48:48 +0000
@@ -357,7 +357,7 @@
         self.buildqueue_record.manualScore(score)
 
     def makeJob(self):
-        """See `IBuildBase`."""
+        """See `IBuildFarmJob`."""
         store = Store.of(self)
         job = Job()
         store.add(job)
@@ -499,7 +499,7 @@
             debug_package=debug_package)
 
     def estimateDuration(self):
-        """See `IBuildBase`."""
+        """See `IPackageBuild`."""
         # Always include the primary archive when looking for
         # past build times (just in case that none can be found
         # in a PPA or copy archive).
@@ -559,7 +559,7 @@
         return self.binarypackages.count() > 0
 
     def notify(self, extra_info=None):
-        """See `IBuildBase`.
+        """See `IPackageBuild`.
 
         If config.buildmaster.build_notification is disable, simply
         return.

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-08-20 20:31:18 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2010-08-24 10:48:48 +0000
@@ -19,7 +19,7 @@
 from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
 from lp.buildmaster.model.buildqueue import BuildQueue
-from lp.buildmaster.tests.test_buildbase import (
+from lp.buildmaster.tests.test_packagebuild import (
     TestGetUploadMethodsMixin,
     TestHandleStatusMixin,
     )
@@ -453,9 +453,9 @@
 class TestGetUploadMethodsForBinaryPackageBuild(
     MakeBinaryPackageBuildMixin, TestGetUploadMethodsMixin,
     TestCaseWithFactory):
-    """IBuildBase.getUpload-related methods work with binary builds."""
+    """IPackageBuild.getUpload-related methods work with binary builds."""
 
 
 class TestHandleStatusForBinaryPackageBuild(
     MakeBinaryPackageBuildMixin, TestHandleStatusMixin, TestCaseWithFactory):
-    """IBuildBase.handleStatus works with binary builds."""
+    """IPackageBuild.handleStatus works with binary builds."""


Follow ups