launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01043
[Merge] lp:~jelmer/launchpad/archiveuploader-build-handling into lp:launchpad/devel
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/archiveuploader-build-handling into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
Rather than always using the object with command-line options that gets passed around deep inside the archiveuploader code to obtain the build id, this branch changes the code to allow the build object to be passed in from the caller.
It will still fall back to attempting to use options.buildid if the build was not passed in.
This branch is a prerequisite for a followup branch for proper handling of source package recipe builds with the new --builds option.
tests: ./bin/test lp.archiveuploader
--
https://code.launchpad.net/~jelmer/launchpad/archiveuploader-build-handling/+merge/35528
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/archiveuploader-build-handling into lp:launchpad/devel.
=== modified file 'lib/lp/archiveuploader/dscfile.py'
--- lib/lp/archiveuploader/dscfile.py 2010-09-09 17:02:33 +0000
+++ lib/lp/archiveuploader/dscfile.py 2010-09-15 13:21:14 +0000
@@ -632,32 +632,12 @@
def findBuild(self):
"""Find and return the SourcePackageRecipeBuild, if one is specified.
-
- If by any chance an inconsistent build was found this method will
- raise UploadError resulting in a upload rejection.
"""
build_id = getattr(self.policy.options, 'buildid', None)
if build_id is None:
return None
- build = getUtility(ISourcePackageRecipeBuildSource).getById(build_id)
-
- # The master verifies the status to confirm successful upload.
- build.status = BuildStatus.FULLYBUILT
- # If this upload is successful, any existing log is wrong and
- # unuseful.
- build.upload_log = None
-
- # Sanity check; raise an error if the build we've been
- # told to link to makes no sense.
- if (build.pocket != self.policy.pocket or
- build.distroseries != self.policy.distroseries or
- build.archive != self.policy.archive):
- raise UploadError(
- "Attempt to upload source specifying "
- "recipe build %s, where it doesn't fit." % build.id)
-
- return build
+ return getUtility(ISourcePackageRecipeBuildSource).getById(build_id)
def storeInDatabase(self, build):
"""Store DSC information as a SourcePackageRelease record.
=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py 2010-08-27 14:27:22 +0000
+++ lib/lp/archiveuploader/nascentupload.py 2010-09-15 13:21:14 +0000
@@ -137,7 +137,7 @@
raise FatalUploadError(str(e))
return cls(changesfile, policy, logger)
- def process(self):
+ def process(self, build=None):
"""Process this upload, checking it against policy, loading it into
the database if it seems okay.
@@ -200,7 +200,7 @@
self.overrideArchive()
# Check upload rights for the signer of the upload.
- self.verify_acl()
+ self.verify_acl(build)
# Perform policy checks.
policy.checkUpload(self)
@@ -483,7 +483,7 @@
#
# Signature and ACL stuff
#
- def verify_acl(self):
+ def verify_acl(self, build=None):
"""Check the signer's upload rights.
The signer must have permission to upload to either the component
@@ -511,7 +511,7 @@
source_name = getUtility(
ISourcePackageNameSet).queryByName(self.changes.dsc.package)
- rejection_reason = archive.checkUpload(
+ rejection_reason = self.policy.archive.checkUpload(
uploader, self.policy.distroseries, source_name,
self.changes.dsc.component, self.policy.pocket, not self.is_new)
@@ -824,7 +824,7 @@
#
# Actually processing accepted or rejected uploads -- and mailing people
#
- def do_accept(self, notify=True):
+ def do_accept(self, notify=True, build=None):
"""Accept the upload into the queue.
This *MAY* in extreme cases cause a database error and thus
@@ -834,13 +834,14 @@
constraint.
:param notify: True to send an email, False to not send one.
+ :param build: The build associated with this upload.
"""
if self.is_rejected:
self.reject("Alas, someone called do_accept when we're rejected")
self.do_reject(notify)
return False
try:
- self.storeObjectsInDatabase()
+ self.storeObjectsInDatabase(build=build)
# Send the email.
# There is also a small corner case here where the DB transaction
@@ -923,7 +924,7 @@
#
# Inserting stuff in the database
#
- def storeObjectsInDatabase(self):
+ def storeObjectsInDatabase(self, build=None):
"""Insert this nascent upload into the database."""
# Queue entries are created in the NEW state by default; at the
@@ -940,6 +941,8 @@
if self.sourceful:
assert self.changes.dsc, "Sourceful upload lacks DSC."
build = self.changes.dsc.findBuild()
+ if build is not None:
+ self.changes.dsc.checkBuild(build)
sourcepackagerelease = self.changes.dsc.storeInDatabase(build)
package_upload_source = self.queue_root.addSource(
sourcepackagerelease)
@@ -966,6 +969,7 @@
bpfs_to_create = sorted(
self.changes.binary_package_files,
key=lambda file: file.ddeb_file is not None)
+ assert build is None or len(bpfs_to_create) == 1
for binary_package_file in bpfs_to_create:
if self.sourceful:
# The reason we need to do this verification
@@ -980,11 +984,19 @@
sourcepackagerelease = (
binary_package_file.findSourcePackageRelease())
- build = binary_package_file.findBuild(sourcepackagerelease)
+ if build is None:
+ build = binary_package_file.findBuild(
+ sourcepackagerelease)
+ if build.source_package_release != sourcepackagerelease:
+ raise AssertionError(
+ "Attempt to upload binaries specifying build %s, "
+ "where they don't fit." % build.id)
+ binary_package_file.checkBuild(build)
assert self.queue_root.pocket == build.pocket, (
"Binary was not build for the claimed pocket.")
binary_package_file.storeInDatabase(build)
processed_builds.append(build)
+ build = None
# Store the related builds after verifying they were built
# from the same source.
=== modified file 'lib/lp/archiveuploader/nascentuploadfile.py'
--- lib/lp/archiveuploader/nascentuploadfile.py 2010-09-02 16:28:50 +0000
+++ lib/lp/archiveuploader/nascentuploadfile.py 2010-09-15 13:21:14 +0000
@@ -33,6 +33,7 @@
from canonical.encoding import guess as guess_encoding
from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
from canonical.librarian.utils import filechunks
+from lp.app.errors import NotFoundError
from lp.archiveuploader.utils import (
determine_source_file_type,
prefix_multi_line_string,
@@ -338,6 +339,13 @@
"""Return an ISection for self.section_name."""
return getUtility(ISectionSet)[self.section_name]
+ def checkBuild(self, build):
+ """Check the status of the build this file is part of.
+
+ :param build: an `IPackageBuild` instance
+ """
+ raise NotImplementedError(self.checkBuild)
+
def extractUserDefinedFields(self, control):
"""Extract the user defined fields out of a control file list.
"""
@@ -381,6 +389,23 @@
yield UploadError("%s: should be %s according to changes file."
% (filename_version, version_chopped))
+ def checkBuild(self, build):
+ """See PackageUploadFile."""
+ # The master verifies the status to confirm successful upload.
+ build.status = BuildStatus.FULLYBUILT
+ # If this upload is successful, any existing log is wrong and
+ # unuseful.
+ build.upload_log = None
+
+ # Sanity check; raise an error if the build we've been
+ # told to link to makes no sense.
+ if (build.pocket != self.policy.pocket or
+ build.distroseries != self.policy.distroseries or
+ build.archive != self.policy.archive):
+ raise UploadError(
+ "Attempt to upload source specifying "
+ "recipe build %s, where it doesn't fit." % build.id)
+
class BaseBinaryUploadFile(PackageUploadFile):
"""Base methods for binary upload modeling."""
@@ -834,8 +859,6 @@
in this case, change this build to be FULLYBUILT.
- Create a new build in FULLYBUILT status.
- If by any chance an inconsistent build was found this method will
- raise UploadError resulting in a upload rejection.
"""
build_id = getattr(self.policy.options, 'buildid', None)
dar = self.policy.distroseries[self.archtag]
@@ -861,25 +884,35 @@
# Ensure gathered binary is related to a FULLYBUILT build
# record. It will be check in slave-scanner procedure to
# certify that the build was processed correctly.
- build.status = BuildStatus.FULLYBUILT
- # Also purge any previous failed upload_log stored, so its
- # content can be garbage-collected since it's not useful
- # anymore.
- build.upload_log = None
+ return build
+
+ def checkBuild(self, build):
+ """See PackageUploadFile."""
+ try:
+ dar = self.policy.distroseries[self.archtag]
+ except NotFoundError:
+ raise UploadError(
+ "Upload to unknown architecture %s for distroseries %s" %
+ (self.archtag, self.policy.distroseries))
+
+ # Ensure gathered binary is related to a FULLYBUILT build
+ # record. It will be check in slave-scanner procedure to
+ # certify that the build was processed correctly.
+ build.status = BuildStatus.FULLYBUILT
+ # Also purge any previous failed upload_log stored, so its
+ # content can be garbage-collected since it's not useful
+ # anymore.
+ build.upload_log = None
# Sanity check; raise an error if the build we've been
- # told to link to makes no sense (ie. is not for the right
- # source package).
- if (build.source_package_release != sourcepackagerelease or
- build.pocket != self.policy.pocket or
+ # told to link to makes no sense.
+ if (build.pocket != self.policy.pocket or
build.distro_arch_series != dar or
build.archive != self.policy.archive):
raise UploadError(
"Attempt to upload binaries specifying "
"build %s, where they don't fit." % build.id)
- return build
-
def storeInDatabase(self, build):
"""Insert this binary release and build into the database."""
# Reencode everything we are supplying, because old packages
=== modified file 'lib/lp/archiveuploader/tests/test_nascentuploadfile.py'
--- lib/lp/archiveuploader/tests/test_nascentuploadfile.py 2010-09-03 06:06:40 +0000
+++ lib/lp/archiveuploader/tests/test_nascentuploadfile.py 2010-09-15 13:21:14 +0000
@@ -20,8 +20,11 @@
from lp.archiveuploader.nascentuploadfile import (
CustomUploadFile,
DebBinaryUploadFile,
+ UploadError,
)
+from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.archiveuploader.tests import AbsolutelyAnythingGoesUploadPolicy
+from lp.buildmaster.enums import BuildStatus
from lp.soyuz.enums import PackageUploadCustomFormat
from lp.testing import TestCaseWithFactory
@@ -34,6 +37,7 @@
self.logger = BufferLogger()
self.policy = AbsolutelyAnythingGoesUploadPolicy()
self.distro = self.factory.makeDistribution()
+ self.policy.pocket = PackagePublishingPocket.RELEASE
self.policy.archive = self.factory.makeArchive(
distribution=self.distro)
@@ -217,6 +221,34 @@
release = uploadfile.storeInDatabase(None)
self.assertEquals(u"http://samba.org/~jelmer/bzr";, release.homepage)
+ def test_checkBuild(self):
+ # checkBuild() verifies consistency with a build.
+ build = self.factory.makeSourcePackageRecipeBuild(
+ pocket=self.policy.pocket, distroseries=self.policy.distroseries,
+ archive=self.policy.archive)
+ dsc = self.getBaseDsc()
+ uploadfile = self.createDSCFile(
+ "foo.dsc", dsc, "main/net", "extra", "dulwich", "0.42",
+ self.createChangesFile("foo.changes", self.getBaseChanges()))
+ uploadfile.checkBuild(build)
+ # checkBuild() sets the build status to FULLYBUILT and
+ # removes the upload log.
+ self.assertEquals(BuildStatus.FULLYBUILT, build.status)
+ self.assertIs(None, build.upload_log)
+
+ def test_checkBuild_inconsistent(self):
+ # checkBuild() raises UploadError if inconsistencies between build
+ # and upload file are found.
+ build = self.factory.makeSourcePackageRecipeBuild(
+ pocket=self.policy.pocket,
+ distroseries=self.factory.makeDistroSeries(),
+ archive=self.policy.archive)
+ dsc = self.getBaseDsc()
+ uploadfile = self.createDSCFile(
+ "foo.dsc", dsc, "main/net", "extra", "dulwich", "0.42",
+ self.createChangesFile("foo.changes", self.getBaseChanges()))
+ self.assertRaises(UploadError, uploadfile.checkBuild, build)
+
class DebBinaryUploadFileTests(PackageUploadFileTestCase):
"""Tests for DebBinaryUploadFile."""
@@ -326,3 +358,32 @@
bpr = uploadfile.storeInDatabase(build)
self.assertEquals(
u"http://samba.org/~jelmer/dulwich";, bpr.homepage)
+
+ def test_checkBuild(self):
+ # checkBuild() verifies consistency with a build.
+ das = self.factory.makeDistroArchSeries(
+ distroseries=self.policy.distroseries, architecturetag="i386")
+ build = self.factory.makeBinaryPackageBuild(
+ distroarchseries=das,
+ archive=self.policy.archive)
+ uploadfile = self.createDebBinaryUploadFile(
+ "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
+ None)
+ uploadfile.checkBuild(build)
+ # checkBuild() sets the build status to FULLYBUILT and
+ # removes the upload log.
+ self.assertEquals(BuildStatus.FULLYBUILT, build.status)
+ self.assertIs(None, build.upload_log)
+
+ def test_checkBuild_inconsistent(self):
+ # checkBuild() raises UploadError if inconsistencies between build
+ # and upload file are found.
+ das = self.factory.makeDistroArchSeries(
+ distroseries=self.policy.distroseries, architecturetag="amd64")
+ build = self.factory.makeBinaryPackageBuild(
+ distroarchseries=das,
+ archive=self.policy.archive)
+ uploadfile = self.createDebBinaryUploadFile(
+ "foo_0.42_i386.deb", "main/python", "unknown", "mypkg", "0.42",
+ None)
+ self.assertRaises(UploadError, uploadfile.checkBuild, build)
=== modified file 'lib/lp/archiveuploader/uploadprocessor.py'
--- lib/lp/archiveuploader/uploadprocessor.py 2010-08-27 11:19:54 +0000
+++ lib/lp/archiveuploader/uploadprocessor.py 2010-09-15 13:21:14 +0000
@@ -220,7 +220,7 @@
[changes_file] = self.locateChangesFiles(upload_path)
logger.debug("Considering changefile %s" % changes_file)
result = self.processChangesFile(
- upload_path, changes_file, logger)
+ upload_path, changes_file, logger, build)
except (KeyboardInterrupt, SystemExit):
raise
except:
@@ -360,7 +360,8 @@
os.path.join(relative_path, filename))
return self.orderFilenames(changes_files)
- def processChangesFile(self, upload_path, changes_file, logger=None):
+ def processChangesFile(self, upload_path, changes_file, logger=None,
+ build=None):
"""Process a single changes file.
This is done by obtaining the appropriate upload policy (according
@@ -456,7 +457,7 @@
result = UploadStatusEnum.ACCEPTED
try:
- upload.process()
+ upload.process(build)
except UploadPolicyError, e:
upload.reject("UploadPolicyError escaped upload.process: "
"%s " % e)
@@ -497,7 +498,8 @@
upload.do_reject(notify)
self.ztm.abort()
else:
- successful = upload.do_accept(notify=notify)
+ successful = upload.do_accept(
+ notify=notify, build=build)
if not successful:
result = UploadStatusEnum.REJECTED
logger.info(
