Merge ~cjwatson/launchpad-buildd:py3-fix-buildrecipe into launchpad-buildd:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: b61f70289cccfde7585d99abe736b2a00217f331
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad-buildd:py3-fix-buildrecipe
Merge into: launchpad-buildd:master
Diff against target: 281 lines (+128/-33)
3 files modified
bin/buildrecipe (+21/-32)
debian/changelog (+1/-0)
lpbuildd/tests/test_buildrecipe.py (+106/-1)
Reviewer Review Type Date Requested Status
Tom Wardill (community) Approve
Review via email: mp+391378@code.launchpad.net

Commit message

Fix bytes/text handling in RecipeBuilder.buildTree

To post a comment you must log in.
Revision history for this message
Tom Wardill (twom) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/bin/buildrecipe b/bin/buildrecipe
2index f3034e7..b447f01 100755
3--- a/bin/buildrecipe
4+++ b/bin/buildrecipe
5@@ -14,13 +14,7 @@ import os
6 import pwd
7 import socket
8 import stat
9-from subprocess import (
10- PIPE,
11- Popen,
12- call,
13- check_call,
14- check_output,
15- )
16+import subprocess
17 import sys
18 import tempfile
19 from textwrap import dedent
20@@ -35,18 +29,15 @@ RETCODE_FAILURE_INSTALL_BUILD_DEPS = 202
21 RETCODE_FAILURE_BUILD_SOURCE_PACKAGE = 203
22
23
24-def call_report_rusage(args, env):
25+def call_report(args, env):
26 """Run a subprocess.
27
28- Report that it was run, and the resources used, and complain if it fails.
29+ Report that it was run and complain if it fails.
30
31- :return: The process wait status.
32+ :return: The process exit status.
33 """
34 print('RUN %r' % args)
35- proc = Popen(args, env=env)
36- pid, status, rusage = os.wait4(proc.pid, 0)
37- print(rusage)
38- return status
39+ return subprocess.call(args, env=env)
40
41
42 class RecipeBuilder:
43@@ -102,31 +93,28 @@ class RecipeBuilder:
44 assert not os.path.exists(self.tree_path)
45 recipe_path = os.path.join(self.work_dir, 'recipe')
46 manifest_path = os.path.join(self.tree_path, 'manifest')
47- recipe_file = open(recipe_path, 'rb')
48- try:
49+ with open(recipe_path) as recipe_file:
50 recipe = recipe_file.read()
51- finally:
52- recipe_file.close()
53 # As of bzr 2.2, a defined identity is needed. In this case, we're
54 # using buildd@<hostname>.
55 hostname = socket.gethostname()
56 email = 'buildd@%s' % hostname
57- lsb_release = Popen(['/usr/bin/sudo',
58+ lsb_release = subprocess.Popen(['sudo',
59 '/usr/sbin/chroot', self.chroot_path, 'lsb_release',
60- '-r', '-s'], stdout=PIPE)
61+ '-r', '-s'], stdout=subprocess.PIPE, universal_newlines=True)
62 distroseries_version = lsb_release.communicate()[0].rstrip()
63 assert lsb_release.returncode == 0
64
65 if self.git:
66 print('Git version:')
67- check_call(['git', '--version'])
68- print(check_output(
69- ['dpkg-query', '-W', 'git-build-recipe']).rstrip(
70- '\n').replace('\t', ' '))
71+ subprocess.check_call(['git', '--version'])
72+ print(subprocess.check_output(
73+ ['dpkg-query', '-W', 'git-build-recipe'],
74+ universal_newlines=True).rstrip('\n').replace('\t', ' '))
75 else:
76 print('Bazaar versions:')
77- check_call(['bzr', 'version'])
78- check_call(['bzr', 'plugins'])
79+ subprocess.check_call(['bzr', 'version'])
80+ subprocess.check_call(['bzr', 'plugins'])
81
82 print('Building recipe:')
83 print(recipe)
84@@ -149,7 +137,7 @@ class RecipeBuilder:
85 '--append-version', '~ubuntu%s.1' % distroseries_version,
86 recipe_path, self.tree_path,
87 ])
88- retcode = call_report_rusage(cmd, env=env)
89+ retcode = call_report(cmd, env=env)
90 if retcode != 0:
91 return retcode
92 (source,) = [name for name in os.listdir(self.tree_path)
93@@ -217,14 +205,14 @@ class RecipeBuilder:
94 file=conf)
95 ftparchive_env = dict(os.environ)
96 ftparchive_env.pop("APT_CONFIG", None)
97- ret = call(
98+ ret = subprocess.call(
99 ["apt-ftparchive", "-q=2", "generate", conf_path],
100 env=ftparchive_env)
101 if ret != 0:
102 return ret
103
104 with open(os.path.join(self.apt_dir, "Release"), "w") as release:
105- return call(
106+ return subprocess.call(
107 ["apt-ftparchive", "-q=2", "-c", conf_path, "release",
108 self.apt_dir],
109 stdout=release, env=ftparchive_env)
110@@ -252,7 +240,7 @@ class RecipeBuilder:
111 if ret == 0:
112 list_path = os.path.join(
113 self.apt_sources_list_dir, "buildrecipe-archive.list")
114- return call(['sudo', 'mv', tmp_list_path, list_path])
115+ return subprocess.call(['sudo', 'mv', tmp_list_path, list_path])
116 return ret
117
118 def setUpAptArchive(self, package):
119@@ -296,7 +284,8 @@ class RecipeBuilder:
120 print("Running in chroot: %s" %
121 ' '.join("'%s'" % arg for arg in args))
122 sys.stdout.flush()
123- return call(['sudo', '/usr/sbin/chroot', self.chroot_path] + args)
124+ return subprocess.call(
125+ ['sudo', '/usr/sbin/chroot', self.chroot_path] + args)
126
127 def copy_in(self, source_path, target_path):
128 """Copy a file into the target environment.
129@@ -316,7 +305,7 @@ class RecipeBuilder:
130 mode = stat.S_IMODE(os.stat(source_path).st_mode)
131 full_target_path = os.path.join(
132 self.chroot_path, target_path.lstrip("/"))
133- check_call(
134+ subprocess.check_call(
135 ["sudo", "install", "-o", "root", "-g", "root", "-m", "%o" % mode,
136 source_path, full_target_path])
137
138diff --git a/debian/changelog b/debian/changelog
139index e46111d..547c167 100644
140--- a/debian/changelog
141+++ b/debian/changelog
142@@ -1,6 +1,7 @@
143 launchpad-buildd (193) UNRELEASED; urgency=medium
144
145 * Fix handling of bytes arguments passed to BuildManager.runSubProcess.
146+ * Fix bytes/text handling in RecipeBuilder.buildTree.
147
148 -- Colin Watson <cjwatson@ubuntu.com> Thu, 24 Sep 2020 16:45:27 +0100
149
150diff --git a/lpbuildd/tests/test_buildrecipe.py b/lpbuildd/tests/test_buildrecipe.py
151index ba93dfe..a4da180 100644
152--- a/lpbuildd/tests/test_buildrecipe.py
153+++ b/lpbuildd/tests/test_buildrecipe.py
154@@ -7,6 +7,7 @@ __metaclass__ = type
155
156 from contextlib import contextmanager
157 import imp
158+import io
159 import os
160 import shutil
161 import stat
162@@ -14,7 +15,10 @@ import sys
163 import tempfile
164 from textwrap import dedent
165
166-from fixtures import MockPatchObject
167+from fixtures import (
168+ MockPatch,
169+ MockPatchObject,
170+ )
171 import six
172 from systemfixtures import FakeProcesses
173 from testtools import TestCase
174@@ -82,6 +86,107 @@ class TestRecipeBuilder(TestCase):
175 self.resetEnvironment()
176 super(TestRecipeBuilder, self).tearDown()
177
178+ def test_buildTree_git(self):
179+ def fake_git(args):
180+ if args["args"][1] == "--version":
181+ print("git version x.y.z")
182+ return {}
183+ else:
184+ return {"returncode": 1}
185+
186+ def fake_git_build_recipe(args):
187+ print("dummy recipe build")
188+ os.makedirs(os.path.join(self.builder.tree_path, "foo"))
189+ return {}
190+
191+ processes_fixture = self.useFixture(FakeProcesses())
192+ processes_fixture.add(
193+ lambda _: {"stdout": io.StringIO(u"5.10\n")}, name="sudo")
194+ processes_fixture.add(fake_git, name="git")
195+ processes_fixture.add(
196+ lambda _: {"stdout": io.StringIO(u"git-build-recipe\tx.y.z\n")},
197+ name="dpkg-query")
198+ processes_fixture.add(fake_git_build_recipe, name="git-build-recipe")
199+ self.builder = RecipeBuilder(
200+ self.build_id, "Recipe Builder", "builder@example.org>", "grumpy",
201+ "grumpy", "main", "PPA", git=True)
202+ with open(os.path.join(self.builder.work_dir, "recipe"), "w") as f:
203+ f.write("dummy recipe contents\n")
204+ mock_stdout = six.StringIO()
205+ self.useFixture(MockPatch("sys.stdout", mock_stdout))
206+ self.assertEqual(0, self.builder.buildTree())
207+ self.assertEqual(
208+ os.path.join(self.builder.work_dir_relative, "tree", "foo"),
209+ self.builder.source_dir_relative)
210+ expected_recipe_command = [
211+ "git-build-recipe", "--safe", "--no-build",
212+ "--manifest", os.path.join(self.builder.tree_path, "manifest"),
213+ "--distribution", "grumpy", "--allow-fallback-to-native",
214+ "--append-version", u"~ubuntu5.10.1",
215+ os.path.join(self.builder.work_dir, "recipe"),
216+ self.builder.tree_path,
217+ ]
218+ self.assertEqual(
219+ dedent("""\
220+ Git version:
221+ git version x.y.z
222+ git-build-recipe x.y.z
223+ Building recipe:
224+ dummy recipe contents
225+
226+ RUN %s
227+ dummy recipe build
228+ """) % repr(expected_recipe_command),
229+ mock_stdout.getvalue())
230+
231+ def test_buildTree_bzr(self):
232+ def fake_bzr(args):
233+ if args["args"][1] == "version":
234+ print("bzr version x.y.z")
235+ return {}
236+ elif args["args"][1] == "plugins":
237+ print("bzr-plugin x.y.z")
238+ return {}
239+ elif "dailydeb" in args["args"][1:]:
240+ print("dummy recipe build")
241+ os.makedirs(os.path.join(self.builder.tree_path, "foo"))
242+ return {}
243+ else:
244+ return {"returncode": 1}
245+
246+ processes_fixture = self.useFixture(FakeProcesses())
247+ processes_fixture.add(
248+ lambda _: {"stdout": io.StringIO(u"5.10\n")}, name="sudo")
249+ processes_fixture.add(fake_bzr, name="bzr")
250+ with open(os.path.join(self.builder.work_dir, "recipe"), "w") as f:
251+ f.write("dummy recipe contents\n")
252+ mock_stdout = six.StringIO()
253+ self.useFixture(MockPatch("sys.stdout", mock_stdout))
254+ self.assertEqual(0, self.builder.buildTree())
255+ self.assertEqual(
256+ os.path.join(self.builder.work_dir_relative, "tree", "foo"),
257+ self.builder.source_dir_relative)
258+ expected_recipe_command = [
259+ "bzr", "-Derror", "dailydeb", "--safe", "--no-build",
260+ "--manifest", os.path.join(self.builder.tree_path, "manifest"),
261+ "--distribution", "grumpy", "--allow-fallback-to-native",
262+ "--append-version", u"~ubuntu5.10.1",
263+ os.path.join(self.builder.work_dir, "recipe"),
264+ self.builder.tree_path,
265+ ]
266+ self.assertEqual(
267+ dedent("""\
268+ Bazaar versions:
269+ bzr version x.y.z
270+ bzr-plugin x.y.z
271+ Building recipe:
272+ dummy recipe contents
273+
274+ RUN %s
275+ dummy recipe build
276+ """) % repr(expected_recipe_command),
277+ mock_stdout.getvalue())
278+
279 def test_makeDummyDsc(self):
280 self.builder.source_dir_relative = os.path.join(
281 self.builder.work_dir_relative, "tree", "foo")

Subscribers

People subscribed via source and target branches