Merge lp:~michael.nelson/launchpad/567922-binarypackagebuild-packagebuild-3 into lp:launchpad/db-devel

[Michael Nelson]

Please read the description of the previous MP at:

https://code.edge.launchpad.net/~michael.nelson/launchpad/567922-binarypackagebuild-packagebuild-2/+merge/24211

This branch adds the last three interface methods to IPackageBuild: verifySuccessfulUpload, storeUploadLog and notify, as well as handleStatus() on IPackageBuildDerived (it should not be delegated like the other methods, as it needs to then call sub-handles such as _handleStatus_OK() etc., which need to be called from the top of the chain).

The same tests to run.

The next branch steps back from all this re-shuffling and cleans up the results.

[Jelmer Vernooij (jelmer)]

Sorry, still haven't sorted out my mailing list subscriptions. I promise I'll do that for the next one and reply inline...

Making the BuildBase methods static is ugly, though I can understand why you're doing it and it seems like a reasonable thing to do for the moment.
Some minor issues:

The various versions of _set_build_farm_job have an extra heading space in their docstring.

The extra_info parameter to IPackageBuild.verify is not documented in its interface.

Also, this code will unfortunately conflict once my simplify-uploadprocess branch lands.

review: Approve (code*)

[Jelmer Vernooij (jelmer)]

It'd be great if this branch can also use Store.flush rather than flush_database_updates as we discussed on IRC.

[Michael Nelson (michael.nelson)]

On Thu, Apr 29, 2010 at 3:52 PM, Jelmer Vernooij <email address hidden> wrote:
> Review: Approve code*
> Sorry, still haven't sorted out my mailing list subscriptions. I promise I'll do that for the next one and reply inline...
>
> Making the BuildBase methods static is ugly, though I can understand why you're doing it and it seems like a reasonable thing to do for the moment.

Yep - it's temporary just so there's only one implementation while I
keep refactoring (and tests continue to pass). I've updated the
BRANCH.TODO to reflect that (which will ensure the code doesn't land
until it is fixed).

> Some minor issues:
>
> The various versions of _set_build_farm_job have an extra heading space in their docstring.

Fixed.

>
> The extra_info parameter to IPackageBuild.verify is not documented in its interface.

Fixed.

>
> Also, this code will unfortunately conflict once my simplify-uploadprocess branch lands.

Yeah, that's ok. I'm expecting to be resolving conflicts for the next while :)

Incremental diff attached. And I'd already replaced
flush_database_updates as discussed in a previous rev.

Thanks!

=== modified file 'BRANCH.TODO'
--- BRANCH.TODO	2010-04-13 19:17:03 +0000
+++ BRANCH.TODO	2010-05-05 15:49:24 +0000
@@ -2,3 +2,7 @@
 # landing. There is a test to ensure it is empty in trunk. If there is
 # stuff still here when you are ready to land, the items should probably
 # be converted to bugs so they can be scheduled.
+
+The static methods on IBuildFarmJob/IPackageBuild should be reverted
+to normal methods once all *Build classes have transitioned to the new
+model and IBuildBase is removed.

=== modified file 'lib/lp/buildmaster/doc/buildfarmjob.txt'
--- lib/lp/buildmaster/doc/buildfarmjob.txt	2010-04-16 14:37:33 +0000
+++ lib/lp/buildmaster/doc/buildfarmjob.txt	2010-05-05 15:49:24 +0000
@@ -1,7 +1,8 @@
 BuildFarmJob
 ============
 
-    >>> from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
+    >>> from lp.buildmaster.interfaces.buildfarmjob import (
+    ...     BuildFarmJobType, IBuildFarmJob)
     >>> from lp.buildmaster.model.buildfarmjob import (
     ...     BuildFarmJob, BuildFarmJobDerived)
 
@@ -9,12 +10,14 @@
 specific build farm job classes will automatically delegate to
 BuildFarmJob by inheriting BuildFarmJobDerived.
 
-    >>> buildfarmjob = BuildFarmJob()
+    >>> buildfarmjob = BuildFarmJob(BuildFarmJobType.PACKAGEBUILD)
     >>> verifyObject(IBuildFarmJob, buildfarmjob)
     True
 
-The BuildFarmJob class does not (yet) do a lot itself, other than
-providing default implementations for its methods.
+The BuildFarmJob class provides many of the common attributes used by
+different types of build farm jobs, as well as default implementations
+for the common methods.  The BuildFarmJob class provides default
+implementations for many build-related methods.
 
     >>> print buildfarmjob.getLogFileName()
     buildlog.txt
@@ -24,6 +27,7 @@
     ...
     NotImplementedError
 
+For more details, please refer to lp.buildmaster.tests.test_buildfarmjob.
 
 BuildFarmJobDerived
 ===================
@@ -40,4 +44,3 @@
 instance of a specific build-farm job implementation associated with a
 given Job instance which must be called in the context of a concrete
 derived class. See lp.buildmaster.tests.test_buildqueue for examples.
-

=== modified file 'lib/lp/buildmaster/interfaces/buildbase.py'
--- lib/lp/buildmaster/interfaces/buildbase.py	2010-05-05 15:49:22 +0000
+++ lib/lp/buildmaster/interfaces/buildbase.py	2010-05-05 15:49:24 +0000
@@ -228,7 +228,7 @@
 >>>>>>> MERGE-SOURCE
         """
 
-    def handleStatus(status, librarian, slave_status):
+    def handleStatus(build, status, librarian, slave_status):
         """Handle a finished build status from a slave.
 
         :param status: Slave build status string with 'BuildStatus.' stripped.

=== modified file 'lib/lp/buildmaster/interfaces/packagebuild.py'
--- lib/lp/buildmaster/interfaces/packagebuild.py	2010-05-05 15:49:22 +0000
+++ lib/lp/buildmaster/interfaces/packagebuild.py	2010-05-05 15:49:24 +0000
@@ -6,6 +6,7 @@
 __all__ = [
     'IPackageBuild',
     'IPackageBuildSource',
+    'IPackageBuildDerived',
     ]
 
 
@@ -118,6 +119,25 @@
         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.
+
+        :param extra_info: Optional extra information that will be included
+            in the notification email. If the notification is for a
+            failed-to-upload error then this must be the content of the
+            upload log.
+        """
+
 
 class IPackageBuildSource(Interface):
     """A utility of this interface used to create _things_."""
@@ -129,3 +149,15 @@
         :param pocket: An item of `PackagePublishingPocket`.
         :param dependencies: An optional debian-like dependency line.
         """
+
+
+class IPackageBuildDerived(Interface):
+    """Classes deriving from IPackageBuild inherit the default handleStatus.
+    """
+    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
+        """
+

=== modified file 'lib/lp/buildmaster/model/buildbase.py'
--- lib/lp/buildmaster/model/buildbase.py	2010-05-05 15:49:22 +0000
+++ lib/lp/buildmaster/model/buildbase.py	2010-05-05 15:49:24 +0000
@@ -125,20 +125,31 @@
             return None
         return self._getProxiedFileURL(self.upload_log)
 
-    def handleStatus(self, status, librarian, slave_status):
+    @staticmethod
+    def handleStatus(build, status, librarian, slave_status):
         """See `IBuildBase`."""
         logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
 
-        method = getattr(self, '_handleStatus_' + status, None)
+        method = getattr(build, '_handleStatus_' + status, None)
 
         if method is None:
+<<<<<<< TREE
             logger.critical("Unknown BuildStatus '%s' for builder '%s'"
                             % (status, self.buildqueue_record.builder.url))
+=======
+            if build.buildqueue_record is not None:
+                logger.critical("Unknown BuildStatus '%s' for builder '%s'"
+                                % (status, build.buildqueue_record.builder.url))
+            else:
+                logger.critical("Unknown BuildStatus '%s' for %r"
+                                % (status, build))
+>>>>>>> MERGE-SOURCE
             return
 
-        method(librarian, slave_status, logger)
+        method(build, librarian, slave_status, logger)
 
 <<<<<<< TREE
+    def _handleStatus_OK(self, librarian, slave_status, logger):
 =======
     def processUpload(self, leaf, root, logger):
         """Process an upload.
@@ -168,8 +179,9 @@
             ztm=ZopelessTransactionManager, log=logger)
         processor.processUploadQueue(leaf)
 
+    @staticmethod
+    def _handleStatus_OK(build, librarian, slave_status, logger):
 >>>>>>> MERGE-SOURCE
-    def _handleStatus_OK(self, librarian, slave_status, logger):
         """Handle a package that built successfully.
 
         Once built successfully, we pull the files, store them in a
@@ -179,36 +191,41 @@
         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))
+            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 self.archive.allowUpdatesToReleasePocket():
-            assert self.distroseries.canUploadToPocket(self.pocket), (
+        if not build.archive.allowUpdatesToReleasePocket():
+            assert build.distroseries.canUploadToPocket(build.pocket), (
                 "%s (%s) can not be built for pocket %s: illegal status"
-                % (self.title, self.id, self.pocket.name))
+                % (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 = self.getUploadDirLeaf(
-            '%s-%s' % (self.id, self.buildqueue_record.id))
-        upload_dir = self.getUploadDir(upload_leaf)
+        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.
+<<<<<<< TREE
         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)
+=======
+        upload_path = os.path.join(upload_dir, str(build.archive.id),
+                                   build.distribution.name)
+>>>>>>> MERGE-SOURCE
         os.makedirs(upload_path)
 
-        slave = removeSecurityProxy(self.buildqueue_record.builder.slave)
+        slave = removeSecurityProxy(build.buildqueue_record.builder.slave)
         successful_copy_from_slave = True
         for filename in filemap:
             logger.info("Grabbing file: %s" % filename)
@@ -220,7 +237,7 @@
                 successful_copy_from_slave = False
                 logger.warning(
                     "A slave tried to upload the file '%s' "
-                    "for the build %d." % (filename, self.id))
+                    "for the build %d." % (filename, build.id))
                 break
             out_file = open(out_file_name, "wb")
             slave_file = slave.getFile(filemap[filename])
@@ -229,6 +246,7 @@
         # We only attempt the upload if we successfully copied all the
         # files from the slave.
         if successful_copy_from_slave:
+<<<<<<< TREE
             uploader_logfilename = os.path.join(
                 upload_dir, UPLOAD_LOG_FILENAME)
             uploader_command = self.getUploaderCommand(
@@ -251,6 +269,14 @@
             # 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)
+=======
+            logger.info("Invoking uploader on %s for %s" % (root, upload_leaf))
+            upload_logger = BufferLogger()
+            upload_log = build.processUpload(upload_leaf, root, upload_logger)
+            uploader_log_content = upload_logger.buffer.getvalue()
+        else:
+            uploader_log_content = 'Copy from slave was unsuccessful.'
+>>>>>>> MERGE-SOURCE
 
         # Quick and dirty hack to carry on on process-upload failures
         if os.path.exists(upload_dir):
@@ -290,7 +316,7 @@
 
         # Store build information, build record was already updated during
         # the binary upload.
-        self.storeBuildInfo(self, librarian, slave_status)
+        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
@@ -305,56 +331,65 @@
         # 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.buildstate != BuildStatus.FULLYBUILT or
+        if (build.buildstate != BuildStatus.FULLYBUILT or
             not successful_copy_from_slave or
+<<<<<<< TREE
             not self.verifySuccessfulUpload()):
             logger.warning("Build %s upload failed." % self.id)
             self.buildstate = BuildStatus.FAILEDTOUPLOAD
             uploader_log_content = self.getUploadLogContent(root,
                 upload_leaf)
+=======
+            not build.verifySuccessfulUpload()):
+            logger.warning("Build %s upload failed." % build.id)
+            build.buildstate = BuildStatus.FAILEDTOUPLOAD
+>>>>>>> MERGE-SOURCE
             # 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)
+            build.storeUploadLog(uploader_log_content)
             # Notify the build failure.
-            self.notify(extra_info=uploader_log_content)
+            build.notify(extra_info=uploader_log_content)
         else:
             logger.info(
                 "Gathered %s %d completely" % (
-                self.__class__.__name__, self.id))
+                build.__class__.__name__, build.id))
 
         # Release the builder for another job.
-        self.buildqueue_record.builder.cleanSlave()
+        build.buildqueue_record.builder.cleanSlave()
         # Remove BuildQueue record.
-        self.buildqueue_record.destroySelf()
+        build.buildqueue_record.destroySelf()
 
-    def _handleStatus_PACKAGEFAIL(self, librarian, slave_status, logger):
+    @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.
         """
-        self.buildstate = BuildStatus.FAILEDTOBUILD
-        self.storeBuildInfo(self, librarian, slave_status)
-        self.buildqueue_record.builder.cleanSlave()
-        self.notify()
-        self.buildqueue_record.destroySelf()
+        build.buildstate = BuildStatus.FAILEDTOBUILD
+        build.storeBuildInfo(build, librarian, slave_status)
+        build.buildqueue_record.builder.cleanSlave()
+        build.notify()
+        build.buildqueue_record.destroySelf()
 
-    def _handleStatus_DEPFAIL(self, librarian, slave_status, logger):
+    @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.
         """
-        self.buildstate = BuildStatus.MANUALDEPWAIT
-        self.storeBuildInfo(self, librarian, slave_status)
+        build.buildstate = BuildStatus.MANUALDEPWAIT
+        build.storeBuildInfo(build, librarian, slave_status)
         logger.critical("***** %s is MANUALDEPWAIT *****"
-                        % self.buildqueue_record.builder.name)
-        self.buildqueue_record.builder.cleanSlave()
-        self.buildqueue_record.destroySelf()
+                        % build.buildqueue_record.builder.name)
+        build.buildqueue_record.builder.cleanSlave()
+        build.buildqueue_record.destroySelf()
 
-    def _handleStatus_CHROOTFAIL(self, librarian, slave_status,
+    @staticmethod
+    def _handleStatus_CHROOTFAIL(build, librarian, slave_status,
                                  logger):
         """Handle a package that had failed when unpacking the CHROOT.
 
@@ -362,15 +397,16 @@
         job as CHROOTFAIL, store available information, remove BuildQueue
         and release the builder.
         """
-        self.buildstate = BuildStatus.CHROOTWAIT
-        self.storeBuildInfo(self, librarian, slave_status)
+        build.buildstate = BuildStatus.CHROOTWAIT
+        build.storeBuildInfo(build, librarian, slave_status)
         logger.critical("***** %s is CHROOTWAIT *****" %
-                        self.buildqueue_record.builder.name)
-        self.buildqueue_record.builder.cleanSlave()
-        self.notify()
-        self.buildqueue_record.destroySelf()
+                        build.buildqueue_record.builder.name)
+        build.buildqueue_record.builder.cleanSlave()
+        build.notify()
+        build.buildqueue_record.destroySelf()
 
-    def _handleStatus_BUILDERFAIL(self, librarian, slave_status, logger):
+    @staticmethod
+    def _handleStatus_BUILDERFAIL(build, librarian, slave_status, logger):
         """Handle builder failures.
 
         Build has been failed when trying to build the target package,
@@ -378,14 +414,15 @@
         and 'clean' the builder to do another jobs.
         """
         logger.warning("***** %s has failed *****"
-                       % self.buildqueue_record.builder.name)
-        self.buildqueue_record.builder.failBuilder(
+                       % build.buildqueue_record.builder.name)
+        build.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()
+        build.storeBuildInfo(build, librarian, slave_status)
+        build.buildqueue_record.reset()
 
-    def _handleStatus_GIVENBACK(self, librarian, slave_status, logger):
+    @staticmethod
+    def _handleStatus_GIVENBACK(build, librarian, slave_status, logger):
         """Handle automatic retry requested by builder.
 
         GIVENBACK pseudo-state represents a request for automatic retry
@@ -393,15 +430,15 @@
         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)
+                       % (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.
-        self.buildqueue_record.builder.cleanSlave()
-        self.buildqueue_record.reset()
+        build.buildqueue_record.builder.cleanSlave()
+        build.buildqueue_record.reset()
 
     @staticmethod
     def getLogFromSlave(build):
@@ -428,25 +465,34 @@
         else:
             build.dependencies = None
 
-    def storeUploadLog(self, content):
-        """See `IBuildBase`."""
+    @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 self.upload_log is None, (
+        assert build.upload_log is None, (
             "Upload log information already exists and cannot be overridden.")
 
-        filename = 'upload_%s_log.txt' % self.id
+        if filename is None:
+            filename = 'upload_%s_log.txt' % build.id
         contentType = filenameToContentType(filename)
         file_size = len(content)
         file_content = StringIO(content)
-        restricted = self.is_private
+        restricted = build.is_private
 
-        library_file = getUtility(ILibraryFileAliasSet).create(
+        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
 
     def queueBuild(self, suspended=False):

=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py	2010-05-05 15:49:22 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py	2010-05-05 15:49:24 +0000
@@ -201,9 +201,15 @@
         self._set_build_farm_job()
 
     def _set_build_farm_job(self):
-        """Set the build farm job to which we will delegate.
-
-        Sub-classes can override as required.
+        """Set the default build farm job to which we will delegate.
+
+        Sub-classes should override as required.
+
+        XXX 2010-04-27 michael.nelson bug=570939
+        This only exists because certain classes assume that
+        BuildFarmJob/PackageBuild are in-memory objects that simply
+        provide methods to update the associated builds.
+        We can remove it once the above bug is completed.
         """
         self.build_farm_job = BuildFarmJob(
             job_type=BuildFarmJobType.PACKAGEBUILD)

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py	2010-04-12 16:23:13 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py	2010-05-05 15:49:24 +0000
@@ -183,8 +183,9 @@
 
         # XXX: dsilvers 2005-03-02: Confirm the builder has the right build?
 
-        queueItem.specific_job.build.handleStatus(
-            build_status, librarian, slave_status)
+        build = queueItem.specific_job.build
+        build.handleStatus(
+            build, build_status, librarian, slave_status)
 
 
 class IdleBuildBehavior(BuildFarmJobBehaviorBase):

=== modified file 'lib/lp/buildmaster/model/packagebuild.py'
--- lib/lp/buildmaster/model/packagebuild.py	2010-05-05 15:49:22 +0000
+++ lib/lp/buildmaster/model/packagebuild.py	2010-05-05 15:49:24 +0000
@@ -22,7 +22,7 @@
 from lp.buildmaster.interfaces.buildbase import BuildStatus
 from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSource
 from lp.buildmaster.interfaces.packagebuild import (
-    IPackageBuild, IPackageBuildSource)
+    IPackageBuild, IPackageBuildDerived, IPackageBuildSource)
 from lp.buildmaster.model.buildbase import BuildBase
 from lp.buildmaster.model.buildfarmjob import BuildFarmJobDerived
 from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -62,7 +62,7 @@
     def __init__(self, build):
         """Construct a PackageBuild.
 
-        XXX 2010-04-21 michael.nelson bug=536700
+        XXX 2010-04-21 michael.nelson bug=570939
         This initialiser is only used by IBuildFarmJobDerived classes
         that are not yet expecting a concrete BuildFarmJob (and so are
         expecting to pass in the build to which they refer, such as
@@ -196,10 +196,54 @@
         """See `IPackageBuild`."""
         return BuildBase.storeBuildInfo(self, librarian, slave_status)
 
-
-class PackageBuildDerived(BuildFarmJobDerived):
-    """Override the base delegate to use a build farm job specific to
-    packages.
-    """
-    def _set_build_farm_job(self):
-        self.build_farm_job = PackageBuild(self.build)
+    def verifySuccessfulUpload(self):
+        """See `IPackageBuild`."""
+        raise NotImplementedError
+
+    def storeUploadLog(self, content):
+        """See `IPackageBuild`."""
+        filename = "upload_%s_log.txt" % self.build_farm_job.id
+        library_file = BuildBase.createUploadLog(
+            self, content, filename=filename)
+        self.upload_log = library_file
+
+    def notify(self, extra_info=None):
+        """See `IPackageBuild`."""
+        raise NotImplementedError
+
+
+class PackageBuildDerived:
+    """See `IPackageBuildDerived`."""
+    implements(IPackageBuildDerived)
+
+    def handleStatus(self, status, librarian, slave_status):
+        """See `IPackageBuildDerived`."""
+        return BuildBase.handleStatus(self, status, librarian, slave_status)
+
+    # 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)
+
+    def _handleStatus_PACKAGEFAIL(self, librarian, slave_status, logger):
+        return BuildBase._handleStatus_PACKAGEFAIL(
+            self, librarian, slave_status, logger)
+
+    def _handleStatus_DEPFAIL(self, librarian, slave_status, logger):
+        return BuildBase._handleStatus_DEPFAIL(
+            self, librarian, slave_status, logger)
+
+    def _handleStatus_CHROOTFAIL(self, librarian, slave_status, logger):
+        return BuildBase._handleStatus_CHROOTFAIL(
+            self, librarian, slave_status, logger)
+
+    def _handleStatus_BUILDERFAIL(self, librarian, slave_status, logger):
+        return BuildBase._handleStatus_BUILDERFAIL(
+            self, librarian, slave_status, logger)
+
+    def _handleStatus_GIVENBACK(self, librarian, slave_status, logger):
+        return BuildBase._handleStatus_GIVENBACK(
+            self, librarian, slave_status, logger)

=== modified file 'lib/lp/buildmaster/tests/test_buildbase.py'
--- lib/lp/buildmaster/tests/test_buildbase.py	2010-05-05 15:49:22 +0000
+++ lib/lp/buildmaster/tests/test_buildbase.py	2010-05-05 15:49:24 +0000
@@ -192,7 +192,7 @@
         # 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, {
+        self.build.handleStatus(self.build, 'OK', None, {
                 'filemap': { 'myfile.py': 'test_file_hash'},
                 })
 
@@ -202,7 +202,7 @@
     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, {
+        self.build.handleStatus(self.build, 'OK', None, {
             'filemap': { '/tmp/myfile.py': 'test_file_hash'},
             })
         self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.buildstate)
@@ -211,7 +211,7 @@
     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, {
+        self.build.handleStatus(self.build, 'OK', None, {
             'filemap': { '../myfile.py': 'test_file_hash'},
             })
         self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.buildstate)

=== modified file 'lib/lp/buildmaster/tests/test_packagebuild.py'
--- lib/lp/buildmaster/tests/test_packagebuild.py	2010-05-05 15:49:22 +0000
+++ lib/lp/buildmaster/tests/test_packagebuild.py	2010-05-05 15:49:24 +0000
@@ -6,12 +6,12 @@
 __metaclass__ = type
 
 import unittest
+import hashlib
 
 from storm.store import Store
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
-from canonical.database.sqlbase import flush_database_updates
 from canonical.testing.layers import LaunchpadFunctionalLayer
 
 from lp.buildmaster.interfaces.buildfarmjob import BuildFarmJobType
@@ -74,8 +74,8 @@
 
     def test_saves_record(self):
         # A package build can be stored in the database.
-        flush_database_updates()
         store = Store.of(self.package_build)
+        store.flush()
         retrieved_build = store.find(
             PackageBuild,
             PackageBuild.id == self.package_build.id).one()
@@ -86,6 +86,9 @@
         self.assertRaises(NotImplementedError, self.package_build.getTitle)
         self.assertRaises(
             NotImplementedError, self.package_build.estimateDuration)
+        self.assertRaises(
+            NotImplementedError, self.package_build.verifySuccessfulUpload)
+        self.assertRaises(NotImplementedError, self.package_build.notify)
 
     def test_default_values(self):
         # PackageBuild has a number of default values.
@@ -105,15 +108,25 @@
                 self.package_build.build_farm_job.id),
             log_url)
 
+    def test_storeUploadLog(self):
+        # The given content is uploaded to the librarian and linked as
+        # the upload log.
+        self.package_build.storeUploadLog("Some content")
+        self.failIfEqual(None, self.package_build.upload_log)
+        self.failUnlessEqual(
+            hashlib.sha1("Some content").hexdigest(),
+            self.package_build.upload_log.content.sha1)
+
     def test_upload_log_url(self):
         # The url of the upload log file is determined by the PackageBuild.
-        lfa = self.factory.makeLibraryFileAlias('myuploadlog.txt')
-        removeSecurityProxy(self.package_build).upload_log = lfa
+        Store.of(self.package_build).flush()
+        build_id = self.package_build.build_farm_job.id
+        self.package_build.storeUploadLog("Some content")
         log_url = self.package_build.upload_log_url
         self.failUnlessEqual(
             'http://launchpad.dev/~joe/'
-            '+archive/ppa/+build/%d/+files/myuploadlog.txt' % (
-                self.package_build.build_farm_job.id),
+            '+archive/ppa/+build/%d/+files/upload_%d_log.txt' % (
+                build_id, build_id),
             log_url)
 
 

=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-05-05 15:49:22 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-05-05 15:49:24 +0000
@@ -27,8 +27,8 @@
 from lp.buildmaster.interfaces.buildfarmjob import BuildFarmJobType
 from lp.buildmaster.model.buildbase import BuildBase
 from lp.buildmaster.model.buildqueue import BuildQueue
-from lp.buildmaster.model.packagebuild import (
-    PackageBuildDerived)
+from lp.buildmaster.model.buildfarmjob import BuildFarmJobDerived
+from lp.buildmaster.model.packagebuild import PackageBuild
 from lp.code.interfaces.sourcepackagerecipebuild import (
     ISourcePackageRecipeBuildJob, ISourcePackageRecipeBuildJobSource,
     ISourcePackageRecipeBuild, ISourcePackageRecipeBuildSource)
@@ -199,7 +199,7 @@
         return
 
 
-class SourcePackageRecipeBuildJob(PackageBuildDerived, Storm):
+class SourcePackageRecipeBuildJob(BuildFarmJobDerived, Storm):
     classProvides(ISourcePackageRecipeBuildJobSource)
     implements(ISourcePackageRecipeBuildJob)
 
@@ -222,6 +222,12 @@
         self.job = job
         super(SourcePackageRecipeBuildJob, self).__init__()
 
+    def _set_build_farm_job(self):
+        """Setup the IBuildFarmJob delegate.
+
+        We override this to provide a delegate specific to package builds."""
+        self.build_farm_job = PackageBuild(self.build)
+
     @classmethod
     def new(cls, build, job):
         """See `ISourcePackageRecipeBuildJobSource`."""

=== modified file 'lib/lp/soyuz/model/buildpackagejob.py'
--- lib/lp/soyuz/model/buildpackagejob.py	2010-05-05 15:49:22 +0000
+++ lib/lp/soyuz/model/buildpackagejob.py	2010-05-05 15:49:24 +0000
@@ -18,8 +18,8 @@
 from canonical.database.sqlbase import sqlvalues
 
 from lp.buildmaster.interfaces.buildbase import BuildStatus
-from lp.buildmaster.model.packagebuild import (
-    PackageBuildDerived)
+from lp.buildmaster.model.buildfarmjob import BuildFarmJobDerived
+from lp.buildmaster.model.packagebuild import PackageBuild
 from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.soyuz.interfaces.archive import ArchivePurpose
@@ -28,7 +28,7 @@
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 
 
-class BuildPackageJob(PackageBuildDerived, Storm):
+class BuildPackageJob(BuildFarmJobDerived, Storm):
     """See `IBuildPackageJob`."""
     implements(IBuildPackageJob)
 
@@ -42,10 +42,15 @@
     build = Reference(build_id, 'BinaryPackageBuild.id')
 
     def __init__(self, build, job):
-        """ Setup the IBuildFarmJob delegation when new items are created."""
         self.build, self.job = build, job
         super(BuildPackageJob, self).__init__()
 
+    def _set_build_farm_job(self):
+        """Setup the IBuildFarmJob delegate.
+
+        We override this to provide a delegate specific to package builds."""
+        self.build_farm_job = PackageBuild(self.build)
+
     def score(self):
         """See `IBuildPackageJob`."""
         score_pocketname = {

=== modified file 'lib/lp/translations/model/translationtemplatesbuildjob.py'
--- lib/lp/translations/model/translationtemplatesbuildjob.py	2010-04-28 08:24:54 +0000
+++ lib/lp/translations/model/translationtemplatesbuildjob.py	2010-05-05 15:49:24 +0000
@@ -22,7 +22,8 @@
 
 from lp.buildmaster.interfaces.buildfarmjob import BuildFarmJobType
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
-from lp.buildmaster.model.buildfarmjob import BuildFarmJobDerived
+from lp.buildmaster.model.buildfarmjob import (
+    BuildFarmJob, BuildFarmJobDerived)
 from lp.buildmaster.model.buildqueue import BuildQueue
 from lp.code.interfaces.branchjob import IRosettaUploadJobSource
 from lp.buildmaster.interfaces.buildfarmbranchjob import IBuildFarmBranchJob
@@ -49,6 +50,14 @@
     def __init__(self, branch_job):
         super(TranslationTemplatesBuildJob, self).__init__(branch_job)
 
+    def _set_build_farm_job(self):
+        """Setup the IBuildFarmJob delegate.
+
+        We override this to provide a non-database delegate that simply
+        provides required functionality to the queue system."""
+        self.build_farm_job = BuildFarmJob(
+            job_type=BuildFarmJobType.TRANSLATIONTEMPLATESBUILD)
+
     def score(self):
         """See `IBuildFarmJob`."""
         # Hard-code score for now; anything other than 1000 is probably