launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23896
[Merge] lp:~cjwatson/launchpad-buildd/recipe-currentlybuilding-permissions into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/recipe-currentlybuilding-permissions into lp:launchpad-buildd.
Commit message:
Fix recipe building to not rely on /CurrentlyBuilding existing in base images.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1841075 in launchpad-buildd: "buildrecipe fails if CurrentlyBuilding doesn't exist in chroot"
https://bugs.launchpad.net/launchpad-buildd/+bug/1841075
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/recipe-currentlybuilding-permissions/+merge/371672
This involves a bit of copy-and-paste from Chroot.copy_in, since buildrecipe doesn't use the Operation framework yet.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/recipe-currentlybuilding-permissions into lp:launchpad-buildd.
=== modified file 'bin/buildrecipe'
--- bin/buildrecipe 2017-07-25 22:11:19 +0000
+++ bin/buildrecipe 2019-08-22 16:24:05 +0000
@@ -1,5 +1,5 @@
#!/usr/bin/python -u
-# Copyright 2010, 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""A script that builds a package from a recipe and a chroot."""
@@ -12,6 +12,7 @@
import os
import pwd
import socket
+import stat
from subprocess import (
PIPE,
Popen,
@@ -20,6 +21,7 @@
check_output,
)
import sys
+import tempfile
from textwrap import dedent
from debian.deb822 import Deb822
@@ -260,8 +262,6 @@
def installBuildDeps(self):
"""Install the build-depends of the source tree."""
package = self.getPackageName()
- currently_building_path = os.path.join(
- self.chroot_path, 'CurrentlyBuilding')
currently_building_contents = (
'Package: %s\n'
'Suite: %s\n'
@@ -269,9 +269,11 @@
'Purpose: %s\n'
'Build-Debug-Symbols: no\n' %
(package, self.suite, self.component, self.archive_purpose))
- currently_building = open(currently_building_path, 'w')
- currently_building.write(currently_building_contents)
- currently_building.close()
+ with tempfile.NamedTemporaryFile(mode='w+') as currently_building:
+ currently_building.write(currently_building_contents)
+ currently_building.flush()
+ os.fchmod(currently_building.fileno(), 0o644)
+ self.copy_in(currently_building.name, '/CurrentlyBuilding')
self.setUpAptArchive(package)
return self.chroot(
['apt-get', 'build-dep', '-y', '--only-source', package])
@@ -286,8 +288,29 @@
print("Running in chroot: %s" %
' '.join("'%s'" % arg for arg in args))
sys.stdout.flush()
- return call([
- '/usr/bin/sudo', '/usr/sbin/chroot', self.chroot_path] + args)
+ return call(['sudo', '/usr/sbin/chroot', self.chroot_path] + args)
+
+ def copy_in(self, source_path, target_path):
+ """Copy a file into the target environment.
+
+ The target file will be owned by root/root and have the same
+ permission mode as the source file.
+
+ :param source_path: the path to the file that should be copied from
+ the host system.
+ :param target_path: the path where the file should be installed
+ inside the target environment, relative to the target
+ environment's root.
+ """
+ # Use install(1) so that we can end up with root/root ownership with
+ # a minimum of subprocess calls; the buildd user may not make sense
+ # in the target.
+ mode = stat.S_IMODE(os.stat(source_path).st_mode)
+ full_target_path = os.path.join(
+ self.chroot_path, target_path.lstrip("/"))
+ check_call(
+ ["sudo", "install", "-o", "root", "-g", "root", "-m", "%o" % mode,
+ source_path, full_target_path])
def buildSourcePackage(self):
"""Build the source package.
=== modified file 'debian/changelog'
--- debian/changelog 2019-06-18 16:54:33 +0000
+++ debian/changelog 2019-08-22 16:24:05 +0000
@@ -1,3 +1,10 @@
+launchpad-buildd (177) UNRELEASED; urgency=medium
+
+ * Fix recipe building to not rely on /CurrentlyBuilding existing in base
+ images (LP: #1841075).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx> Thu, 22 Aug 2019 17:16:24 +0100
+
launchpad-buildd (176) xenial; urgency=medium
* Don't rely on /CurrentlyBuilding existing in base images.
=== modified file 'lpbuildd/tests/test_buildrecipe.py'
--- lpbuildd/tests/test_buildrecipe.py 2017-10-27 07:42:35 +0000
+++ lpbuildd/tests/test_buildrecipe.py 2019-08-22 16:24:05 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014 Canonical Ltd. This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from __future__ import print_function
@@ -9,11 +9,20 @@
import imp
import os
import shutil
+import stat
import sys
import tempfile
from textwrap import dedent
+from fixtures import MockPatchObject
+import six
+from systemfixtures import FakeProcesses
from testtools import TestCase
+from testtools.matchers import (
+ Equals,
+ MatchesListwise,
+ StartsWith,
+ )
@contextmanager
@@ -30,6 +39,23 @@
"buildrecipe", "bin/buildrecipe").RecipeBuilder
+class RanCommand(MatchesListwise):
+
+ def __init__(self, *args):
+ args_matchers = [
+ Equals(arg) if isinstance(arg, six.string_types) else arg
+ for arg in args]
+ super(RanCommand, self).__init__(args_matchers)
+
+
+class RanInChroot(RanCommand):
+
+ def __init__(self, home_dir, *args):
+ super(RanInChroot, self).__init__(
+ "sudo", "/usr/sbin/chroot",
+ os.path.join(home_dir, "build-1", "chroot-autobuild"), *args)
+
+
class TestRecipeBuilder(TestCase):
def setUp(self):
super(TestRecipeBuilder, self).setUp()
@@ -143,6 +169,71 @@
self.assertIn(
"Build-Depends: debhelper (>= 9~), libfoo-dev\n", sources_text)
- # XXX cjwatson 2015-06-15: We should unit-test enableAptArchive and
- # installBuildDeps too, but it involves a lot of mocks. For now,
- # integration testing is probably more useful.
+ def test_installBuildDeps(self):
+ processes_fixture = self.useFixture(FakeProcesses())
+ processes_fixture.add(lambda _: {}, name="sudo")
+ copies = {}
+
+ def mock_copy_in(source_path, target_path):
+ with open(source_path, "rb") as source:
+ copies[target_path] = (
+ source.read(), os.fstat(source.fileno()).st_mode)
+
+ self.useFixture(
+ MockPatchObject(self.builder, "copy_in", mock_copy_in))
+ self.builder.source_dir_relative = os.path.join(
+ self.builder.work_dir_relative, "tree", "foo")
+ changelog_path = os.path.join(
+ self.builder.work_dir, "tree", "foo", "debian", "changelog")
+ control_path = os.path.join(
+ self.builder.work_dir, "tree", "foo", "debian", "control")
+ os.makedirs(os.path.dirname(changelog_path))
+ with open(changelog_path, "w") as changelog:
+ # Not a valid changelog, but only the first line matters here.
+ print("foo (1.0-1) bionic; urgency=medium", file=changelog)
+ with open(control_path, "w") as control:
+ print(dedent("""\
+ Source: foo
+ Build-Depends: debhelper (>= 9~), libfoo-dev
+
+ Package: foo
+ Depends: ${shlibs:Depends}"""),
+ file=control)
+ self.assertEqual(0, self.builder.installBuildDeps())
+ self.assertThat(
+ [proc._args["args"] for proc in processes_fixture.procs],
+ MatchesListwise([
+ RanInChroot(
+ self.home_dir, "apt-get",
+ "-o", StartsWith("Dir::Etc::sourcelist="),
+ "-o", "APT::Get::List-Cleanup=false",
+ "update"),
+ RanCommand(
+ "sudo", "mv",
+ os.path.join(
+ self.builder.apt_dir, "buildrecipe-archive.list"),
+ os.path.join(
+ self.builder.apt_sources_list_dir,
+ "buildrecipe-archive.list")),
+ RanInChroot(
+ self.home_dir, "apt-get",
+ "build-dep", "-y", "--only-source", "foo"),
+ ]))
+ self.assertEqual(
+ (dedent("""\
+ Package: foo
+ Suite: grumpy
+ Component: main
+ Purpose: PPA
+ Build-Debug-Symbols: no
+ """).encode("UTF-8"), stat.S_IFREG | 0o644),
+ copies["/CurrentlyBuilding"])
+ # This is still in the temporary location, since we mocked the "sudo
+ # mv" command.
+ with open(os.path.join(
+ self.builder.apt_dir,
+ "buildrecipe-archive.list")) as tmp_list:
+ self.assertEqual(
+ "deb-src [trusted=yes] file://%s ./\n" %
+ self.builder.apt_dir_relative,
+ tmp_list.read())
