launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17485
[Merge] lp:~wgrant/launchpad/bug-1350208 into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-1350208 into lp:launchpad.
Requested reviews:
William Grant (wgrant): code
Related bugs:
Bug #1063188 in Launchpad itself: "Launchpad doesn't try to build the "all" packages if i386 isn't in the Architecture field"
https://bugs.launchpad.net/launchpad/+bug/1063188
Bug #1350208 in Launchpad itself: "createMissingBuilds misbehaves when copying between series with different nominatedarchindep"
https://bugs.launchpad.net/launchpad/+bug/1350208
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1350208/+merge/240808
--
https://code.launchpad.net/~wgrant/launchpad/bug-1350208/+merge/240808
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/soyuz/adapters/buildarch.py'
--- lib/lp/soyuz/adapters/buildarch.py 2013-05-31 04:27:26 +0000
+++ lib/lp/soyuz/adapters/buildarch.py 2014-11-06 05:30:00 +0000
@@ -37,7 +37,7 @@
def determine_architectures_to_build(hintlist, archive, distroseries,
- legal_archseries):
+ legal_archseries, need_arch_indep):
"""Return a list of architectures for which this publication should build.
This function answers the question: given a list of architectures and
@@ -78,7 +78,7 @@
# 'all' is only used as a last resort, to create an arch-indep build
# where no builds would otherwise exist.
- if len(build_tags) == 0 and 'all' in hint_archs:
+ if need_arch_indep and len(build_tags) == 0 and 'all' in hint_archs:
nominated_arch = distroseries.nominatedarchindep
if nominated_arch in legal_archseries:
build_tags = set([nominated_arch.architecturetag])
=== modified file 'lib/lp/soyuz/adapters/tests/test_buildarch.py'
--- lib/lp/soyuz/adapters/tests/test_buildarch.py 2013-09-11 06:05:44 +0000
+++ lib/lp/soyuz/adapters/tests/test_buildarch.py 2014-11-06 05:30:00 +0000
@@ -34,7 +34,7 @@
if arch.architecturetag in allowed_arch_tags]
architectures = determine_architectures_to_build(
pub.sourcepackagerelease.architecturehintlist, pub.archive,
- self.publisher.breezy_autotest, allowed_archs)
+ self.publisher.breezy_autotest, allowed_archs, True)
self.assertContentEqual(
expected_arch_tags, [a.architecturetag for a in architectures])
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py 2014-11-06 00:33:31 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py 2014-11-06 05:30:00 +0000
@@ -13,7 +13,10 @@
]
import datetime
-from operator import itemgetter
+from operator import (
+ attrgetter,
+ itemgetter,
+ )
import apt_pkg
import pytz
@@ -1383,58 +1386,83 @@
if not distroarch.processor.restricted or
distroarch.processor in archive.enabled_restricted_processors]
- def _createMissingBuildForArchitecture(self, sourcepackagerelease,
- archive, arch, pocket,
- logger=None):
- """Create a build for a given architecture if it doesn't exist yet.
-
- Return the just-created `IBinaryPackageBuild` record already
- scored or None if a suitable build is already present.
- """
- exact_build = self.getBySourceAndLocation(
- sourcepackagerelease, archive, arch)
- if exact_build is not None:
- return None
-
- build_candidate = self.findBuiltOrPublishedBySourceAndArchive(
- sourcepackagerelease, archive).get(arch.architecturetag)
- if build_candidate is not None:
- return None
-
- build = self.new(
- source_package_release=sourcepackagerelease,
- distro_arch_series=arch, archive=archive, pocket=pocket)
- # Create the builds in suspended mode for disabled archives.
- build_queue = build.queueBuild(suspended=not archive.enabled)
- Store.of(build).flush()
-
- if logger is not None:
- logger.debug(
- "Created %s [%d] in %s (%d)"
- % (build.title, build.id, build.archive.displayname,
- build_queue.lastscore))
-
- return build
-
def createForSource(self, sourcepackagerelease, archive, distroseries,
pocket, architectures_available=None, logger=None):
"""See `ISourcePackagePublishingHistory`."""
+
+ # Exclude any architectures which already have built or copied
+ # binaries. A new build with the same archtag could never
+ # succeed; its files would conflict during upload.
+ relevant_builds = self.findBuiltOrPublishedBySourceAndArchive(
+ sourcepackagerelease, archive).values()
+
+ # Find any architectures that already have a build that exactly
+ # matches, regardless of status. We can't create a second build
+ # with the same (SPR, Archive, DAS).
+ # XXX wgrant 2014-11-06: Should use a bulk query.
+ for das in distroseries.architectures:
+ build = self.getBySourceAndLocation(
+ sourcepackagerelease, archive, das)
+ if build is not None:
+ relevant_builds.append(build)
+
+ skip_archtags = set(
+ bpb.distro_arch_series.architecturetag for bpb in relevant_builds)
+ # We need to assign the arch-indep role to a build unless an
+ # arch-indep build has already succeeded, or another build in
+ # this series already has it set.
+ need_arch_indep = not any(bpb.arch_indep for bpb in relevant_builds)
+
+ # Find the architectures for which the source should end up with
+ # binaries, parsing architecturehintlist as you'd expect.
+ # For an architecturehintlist of just 'all', this will
+ # be the current nominatedarchindep if need_arch_indep,
+ # otherwise nothing.
if architectures_available is None:
architectures_available = list(
distroseries.buildable_architectures)
-
architectures_available = self._getAllowedArchitectures(
archive, architectures_available)
-
- build_architectures = determine_architectures_to_build(
+ candidate_architectures = determine_architectures_to_build(
sourcepackagerelease.architecturehintlist, archive, distroseries,
- architectures_available)
-
- builds = []
- for arch in build_architectures:
- build_candidate = self._createMissingBuildForArchitecture(
- sourcepackagerelease, archive, arch, pocket, logger=logger)
- if build_candidate is not None:
- builds.append(build_candidate)
-
- return builds
+ architectures_available, need_arch_indep)
+
+ # Filter out any architectures for which we earlier found sufficient
+ # builds.
+ needed_architectures = [
+ das for das in candidate_architectures
+ if das.architecturetag not in skip_archtags]
+ if not needed_architectures:
+ return []
+
+ arch_indep_das = None
+ if need_arch_indep:
+ # The ideal arch_indep build is nominatedarchindep. But if
+ # that isn't a build we would otherwise create, use the DAS
+ # with the lowest Processor.id.
+ # XXX wgrant 2014-11-06: The fact that production's
+ # Processor 1 is i386, a good arch-indep candidate, is a
+ # total coincidence and this isn't a hack. I promise.
+ if distroseries.nominatedarchindep in needed_architectures:
+ arch_indep_das = distroseries.nominatedarchindep
+ else:
+ arch_indep_das = sorted(
+ needed_architectures, key=attrgetter('processor.id'))[0]
+
+ # Create builds for the remaining architectures.
+ new_builds = []
+ for das in needed_architectures:
+ build = self.new(
+ source_package_release=sourcepackagerelease,
+ distro_arch_series=das, archive=archive, pocket=pocket,
+ arch_indep=das == arch_indep_das)
+ new_builds.append(build)
+ # Create the builds in suspended mode for disabled archives.
+ build_queue = build.queueBuild(suspended=not archive.enabled)
+ Store.of(build).flush()
+ if logger is not None:
+ logger.debug(
+ "Created %s [%d] in %s (%d)"
+ % (build.title, build.id, build.archive.displayname,
+ build_queue.lastscore))
+ return new_builds
=== modified file 'lib/lp/soyuz/tests/test_build_set.py'
--- lib/lp/soyuz/tests/test_build_set.py 2014-11-06 03:33:36 +0000
+++ lib/lp/soyuz/tests/test_build_set.py 2014-11-06 05:30:00 +0000
@@ -256,6 +256,16 @@
self.distroseries.nominatedarchindep = self.distroseries['sparc']
self.addFakeChroots(self.distroseries)
+ self.distroseries2 = self.factory.makeDistroSeries(
+ distribution=self.distro, name="dumb")
+ for name, arch in (('avr', self.avr), ('sparc', self.sparc),
+ ('x32', self.x32)):
+ self.factory.makeDistroArchSeries(
+ architecturetag=name, processor=arch,
+ distroseries=self.distroseries2, supports_virtualized=True)
+ self.distroseries2.nominatedarchindep = self.distroseries2['x32']
+ self.addFakeChroots(self.distroseries2)
+
def getPubSource(self, architecturehintlist):
"""Return a mock source package publishing record for the archive
and architecture used in this testcase.
@@ -281,6 +291,17 @@
self.assertContentEqual(expected.items(), actual.items())
self.assertEqual(len(actual), len(builds))
+ def completeBuilds(self, builds, success_map):
+ for build in builds:
+ success_or_failure = success_map.get(
+ build.distro_arch_series.architecturetag, None)
+ if success_or_failure is not None:
+ build.updateStatus(
+ BuildStatus.FULLYBUILT if success_or_failure
+ else BuildStatus.FAILEDTOBUILD)
+ del success_map[build.distro_arch_series.architecturetag]
+ self.assertContentEqual([], success_map)
+
def test__getAllowedArchitectures_restricted(self):
"""Test _getAllowedArchitectures doesn't return unrestricted
archs.
@@ -353,6 +374,88 @@
builds = self.createBuilds(spr, self.distroseries)
self.assertBuildsMatch({'sparc': True, 'avr': False}, builds)
+ def test_createForSource_arch_indep_from_scratch(self):
+ """createForSource() sets arch_indep=True on builds for the
+ nominatedarchindep architecture when no builds already exist.
+ """
+ spr = self.factory.makeSourcePackageRelease(architecturehintlist='any')
+ builds = self.createBuilds(spr, self.distroseries)
+ self.assertBuildsMatch({'sparc': True, 'avr': False}, builds)
+
+ def test_createForSource_any_with_nai_change(self):
+ # A new non-arch-indep build is created for a new
+ # nominatedarchindep architecture if arch-indep has already
+ # built elsewhere.
+ #
+ # This is most important when copying with binaries between
+ # series with different nominatedarchdep (bug #1350208).
+ spr = self.factory.makeSourcePackageRelease(architecturehintlist='any')
+ builds = self.createBuilds(spr, self.distroseries)
+ self.assertBuildsMatch({'sparc': True, 'avr': False}, builds)
+ self.completeBuilds(builds, {'sparc': True, 'avr': True})
+ # The new nominatedarchindep needs to be built, but we already
+ # have arch-indep binaries so arch_indep is False.
+ new_builds = self.createBuilds(spr, self.distroseries2)
+ self.assertBuildsMatch({'x32': False}, new_builds)
+
+ def test_createForSource_any_with_nai_change_and_fail(self):
+ # When the previous arch-indep build has failed, and
+ # nominatedarchindep has changed in the new series, the new
+ # nominatedarchindep has arch_indep=True while the other arch
+ # has arch_indep=False.
+ spr = self.factory.makeSourcePackageRelease(architecturehintlist='any')
+ builds = self.createBuilds(spr, self.distroseries)
+ self.assertBuildsMatch({'sparc': True, 'avr': False}, builds)
+ self.completeBuilds(builds, {'sparc': False, 'avr': True})
+ # The new nominatedarchindep needs to be built, and the previous
+ # nominatedarchindep build failed. We end up with two new
+ # builds, and arch_indep on nominatedarchindep.
+ new_builds = self.createBuilds(spr, self.distroseries2)
+ self.assertBuildsMatch({'x32': True, 'sparc': False}, new_builds)
+
+ def test_createForSource_all_with_nai_change(self):
+ # If we only need arch-indep binaries and they've already built
+ # successfully, no build is created for the new series, even if
+ # nominatedarchindep has changed.
+ spr = self.factory.makeSourcePackageRelease(architecturehintlist='all')
+ builds = self.createBuilds(spr, self.distroseries)
+ self.assertBuildsMatch({'sparc': True}, builds)
+ self.completeBuilds(builds, {'sparc': True})
+ # Despite there being no build for the new nominatedarchindep,
+ # the old arch-indep build is sufficient and no new record is
+ # created.
+ new_builds = self.createBuilds(spr, self.distroseries2)
+ self.assertBuildsMatch({}, new_builds)
+
+ def test_createForSource_all_with_nai_change_and_fail(self):
+ # If the previous arch-indep sole build failed, a new arch-indep
+ # build is created for nominatedarchindep.
+ spr = self.factory.makeSourcePackageRelease(architecturehintlist='all')
+ builds = self.createBuilds(spr, self.distroseries)
+ self.assertBuildsMatch({'sparc': True}, builds)
+ self.completeBuilds(builds, {'sparc': False})
+ # Despite there being no build for the new nominatedarchindep,
+ # the old arch-indep build is sufficient and no new record is
+ # created.
+ new_builds = self.createBuilds(spr, self.distroseries2)
+ self.assertBuildsMatch({'x32': True}, new_builds)
+
+ def test_createForSource_all_and_other_archs(self):
+ # If a source package specifies both 'all' and a set of
+ # architectures that doesn't include nominatedarchindep,
+ # arch_indep is set on the available DistroArchSeries with the
+ # oldest Processor.
+ # This is mostly a hack to avoid hardcoding a preference for
+ # the faster x86-family architectures, so we don't accidentally
+ # build documentation on hppa.
+ spr = self.factory.makeSourcePackageRelease(
+ architecturehintlist='all sparc avr')
+ builds = self.createBuilds(spr, self.distroseries2)
+ self.assertBuildsMatch({'sparc': False, 'avr': True}, builds)
+ self.completeBuilds(builds, {'sparc': True, 'avr': True})
+ new_builds = self.createBuilds(spr, self.distroseries)
+ self.assertBuildsMatch({}, new_builds)
+
class TestFindBuiltOrPublishedBySourceAndArchive(TestCaseWithFactory):
"""Tests for findBuiltOrPublishedBySourceAndArchive()."""
References
