Launchpad itself

Merge lp:~cjwatson/launchpad/improve-dpkg-architecture-cache into lp:launchpad

  1. improve-dpkg-architecture-cache
  2. Merge into devel
Proposed by Colin Watson
Status: Merged
Merged at revision: 18494
Proposed branch: lp:~cjwatson/launchpad/improve-dpkg-architecture-cache
Merge into: lp:launchpad
Diff against target: 123 lines (+55/-6)
2 files modified
lib/lp/soyuz/adapters/buildarch.py (+10/-5)
lib/lp/soyuz/adapters/tests/test_buildarch.py (+45/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/improve-dpkg-architecture-cache
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+332853@code.launchpad.net

Commit message

Improve DpkgArchitectureCache's timeline handling, and speed it up a bit in some cases.

Description of the change

We can only really do so much to make this faster without https://bugs.debian.org/771058, but the nested list comprehension I wrote five years was very confusing for 2017-me to read, so I took the opportunity to make it short-circuit properly once any wildcard has matched for a given architecture.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/soyuz/adapters/buildarch.py'
--- lib/lp/soyuz/adapters/buildarch.py 2017-02-18 01:48:31 +0000
+++ lib/lp/soyuz/adapters/buildarch.py 2017-10-26 11:47:59 +0000
@@ -29,8 +29,9 @@
29 env = dict(os.environ)29 env = dict(os.environ)
30 env["DEB_HOST_ARCH"] = arch30 env["DEB_HOST_ARCH"] = arch
31 action = timeline.start(31 action = timeline.start(
32 "dpkg-architecture", "-i%s" % wildcard,32 "dpkg-architecture",
33 "DEB_HOST_ARCH=%s" % arch)33 "-i%s DEB_HOST_ARCH=%s" % (wildcard, arch),
34 allow_nested=True)
34 try:35 try:
35 ret = (subprocess.call(command, env=env) == 0)36 ret = (subprocess.call(command, env=env) == 0)
36 finally:37 finally:
@@ -39,9 +40,13 @@
39 return self._matches[(arch, wildcard)]40 return self._matches[(arch, wildcard)]
4041
41 def findAllMatches(self, arches, wildcards):42 def findAllMatches(self, arches, wildcards):
42 return list(sorted(set(43 matches = set()
43 arch for arch in arches for wildcard in wildcards44 for arch in arches:
44 if self.match(arch, wildcard))))45 for wildcard in wildcards:
46 if self.match(arch, wildcard):
47 matches.add(arch)
48 break
49 return list(sorted(matches))
4550
4651
47dpkg_architecture = DpkgArchitectureCache()52dpkg_architecture = DpkgArchitectureCache()
4853
=== modified file 'lib/lp/soyuz/adapters/tests/test_buildarch.py'
--- lib/lp/soyuz/adapters/tests/test_buildarch.py 2015-12-30 23:36:39 +0000
+++ lib/lp/soyuz/adapters/tests/test_buildarch.py 2017-10-26 11:47:59 +0000
@@ -1,34 +1,70 @@
1# Copyright 2009-2013 Canonical Ltd. This software is licensed under the1# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4__metaclass__ = type4__metaclass__ = type
55
6from testtools.matchers import (
7 MatchesListwise,
8 MatchesStructure,
9 )
10
6from lp.soyuz.adapters.buildarch import (11from lp.soyuz.adapters.buildarch import (
7 determine_architectures_to_build,12 determine_architectures_to_build,
8 DpkgArchitectureCache,13 DpkgArchitectureCache,
9 )14 )
10from lp.testing import TestCase15from lp.testing import TestCase
16from lp.testing.fixture import CaptureTimeline
1117
1218
13class TestDpkgArchitectureCache(TestCase):19class TestDpkgArchitectureCache(TestCase):
1420
21 def setUp(self):
22 super(TestDpkgArchitectureCache, self).setUp()
23 self.timeline = self.useFixture(CaptureTimeline()).timeline
24
25 def assertTimeline(self, expected_details):
26 matchers = []
27 for expected_detail in expected_details:
28 matchers.append(MatchesStructure.byEquality(
29 category='dpkg-architecture-start', detail=expected_detail))
30 matchers.append(MatchesStructure.byEquality(
31 category='dpkg-architecture-stop', detail=expected_detail))
32 self.assertThat(self.timeline.actions, MatchesListwise(matchers))
33
15 def test_multiple(self):34 def test_multiple(self):
16 self.assertContentEqual(35 self.assertContentEqual(
17 ['amd64', 'armhf'],36 ['amd64', 'armhf'],
18 DpkgArchitectureCache().findAllMatches(37 DpkgArchitectureCache().findAllMatches(
19 ['amd64', 'i386', 'armhf'], ['amd64', 'armhf']))38 ['amd64', 'i386', 'armhf'], ['amd64', 'armhf']))
39 self.assertTimeline([
40 '-iamd64 DEB_HOST_ARCH=amd64',
41 '-iamd64 DEB_HOST_ARCH=i386',
42 '-iarmhf DEB_HOST_ARCH=i386',
43 '-iamd64 DEB_HOST_ARCH=armhf',
44 '-iarmhf DEB_HOST_ARCH=armhf',
45 ])
2046
21 def test_any(self):47 def test_any(self):
22 self.assertContentEqual(48 self.assertContentEqual(
23 ['amd64', 'i386', 'kfreebsd-amd64'],49 ['amd64', 'i386', 'kfreebsd-amd64'],
24 DpkgArchitectureCache().findAllMatches(50 DpkgArchitectureCache().findAllMatches(
25 ['amd64', 'i386', 'kfreebsd-amd64'], ['any']))51 ['amd64', 'i386', 'kfreebsd-amd64'], ['any']))
52 self.assertTimeline([
53 '-iany DEB_HOST_ARCH=amd64',
54 '-iany DEB_HOST_ARCH=i386',
55 '-iany DEB_HOST_ARCH=kfreebsd-amd64',
56 ])
2657
27 def test_all(self):58 def test_all(self):
28 self.assertContentEqual(59 self.assertContentEqual(
29 [],60 [],
30 DpkgArchitectureCache().findAllMatches(61 DpkgArchitectureCache().findAllMatches(
31 ['amd64', 'i386', 'kfreebsd-amd64'], ['all']))62 ['amd64', 'i386', 'kfreebsd-amd64'], ['all']))
63 self.assertTimeline([
64 '-iall DEB_HOST_ARCH=amd64',
65 '-iall DEB_HOST_ARCH=i386',
66 '-iall DEB_HOST_ARCH=kfreebsd-amd64',
67 ])
3268
33 def test_partial_wildcards(self):69 def test_partial_wildcards(self):
34 self.assertContentEqual(70 self.assertContentEqual(
@@ -36,6 +72,14 @@
36 DpkgArchitectureCache().findAllMatches(72 DpkgArchitectureCache().findAllMatches(
37 ['amd64', 'i386', 'kfreebsd-amd64', 'kfreebsd-i386'],73 ['amd64', 'i386', 'kfreebsd-amd64', 'kfreebsd-i386'],
38 ['linux-any', 'any-amd64']))74 ['linux-any', 'any-amd64']))
75 self.assertTimeline([
76 '-ilinux-any DEB_HOST_ARCH=amd64',
77 '-ilinux-any DEB_HOST_ARCH=i386',
78 '-ilinux-any DEB_HOST_ARCH=kfreebsd-amd64',
79 '-iany-amd64 DEB_HOST_ARCH=kfreebsd-amd64',
80 '-ilinux-any DEB_HOST_ARCH=kfreebsd-i386',
81 '-iany-amd64 DEB_HOST_ARCH=kfreebsd-i386',
82 ])
3983
4084
41class TestDetermineArchitecturesToBuild(TestCase):85class TestDetermineArchitecturesToBuild(TestCase):