Merge ~cjwatson/launchpad-buildd:recipe-create-homedir into launchpad-buildd:master

[Colin Watson]

sourcepackagerecipe: Create /home/buildd inside chroot if necessary

Recently-built chroots for mantic no longer include /home/buildd. This is probably a reasonable cleanup, but it broke recipe builds.

[Jürgen Gmach (jugmac00)]

Could you please share your thoughts why you did not add an extra test?

review: Approve

5d290dd... by Colin Watson on 2023-09-18

Update tests for creating /home/buildd

f41cce5... by Colin Watson on 2023-09-18

Simplify comment slightly

[Colin Watson (cjwatson)]

Er, oops, apparently I hadn't run the tests at all! I've sorted out tests now.

diff --git a/debian/changelog b/debian/changelog
index 0bdad53..fd9125d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+launchpad-buildd (235) UNRELEASED; urgency=medium
+
+  * sourcepackagerecipe: Create /home/buildd inside the chroot if it doesn't
+    already exist.
+
+ -- Colin Watson <[e-mail]>  Fri, 15 Sep 2023 17:30:59 +0100
+
 launchpad-buildd (234) focal; urgency=medium
 
   [ Colin Watson ]
diff --git a/lpbuildd/sourcepackagerecipe.py b/lpbuildd/sourcepackagerecipe.py
index 250c3b5..00db21a 100644
--- a/lpbuildd/sourcepackagerecipe.py
+++ b/lpbuildd/sourcepackagerecipe.py
@@ -6,6 +6,7 @@
 
 import os
 import re
+import subprocess
 
 from lpbuildd.builder import get_build_path
 from lpbuildd.debian import DebianBuildManager, DebianBuildState
@@ -17,19 +18,6 @@ RETCODE_FAILURE_INSTALL_BUILD_DEPS = 202
 RETCODE_FAILURE_BUILD_SOURCE_PACKAGE = 203
 
 
-def splat_file(path, contents):
-    """Write a string to the specified path.
-
-    :param path: The path to store the string in.
-    :param contents: The string to write to the file.
-    """
-    file_obj = open(path, "w")
-    try:
-        file_obj.write(contents)
-    finally:
-        file_obj.close()
-
-
 def get_chroot_path(home, build_id, *extra):
     """Return a path within the chroot.
 
@@ -81,9 +69,26 @@ class SourcePackageRecipeBuildManager(DebianBuildManager):
 
     def doRunBuild(self):
         """Run the build process to build the source package."""
-        os.makedirs(get_chroot_path(self.home, self._buildid, "work"))
-        recipe_path = get_chroot_path(self.home, self._buildid, "work/recipe")
-        splat_file(recipe_path, self.recipe_text)
+        work_dir = os.path.join(os.environ["HOME"], "work")
+        self.backend.run(["mkdir", "-p", work_dir])
+        # buildrecipe needs to be able to write directly to the work
+        # directory.  (That directory needs to be inside the chroot so that
+        # buildrecipe can run dpkg-buildpackage on it from inside the
+        # chroot.)
+        subprocess.run(
+            [
+                "sudo",
+                "chown",
+                "-R",
+                "buildd:",
+                get_chroot_path(self.home, self._buildid, "work"),
+            ],
+            check=True,
+        )
+        with self.backend.open(
+            os.path.join(work_dir, "recipe"), "w"
+        ) as recipe_file:
+            recipe_file.write(self.recipe_text)
         args = ["buildrecipe"]
         if self.git:
             args.append("--git")
diff --git a/lpbuildd/tests/test_sourcepackagerecipe.py b/lpbuildd/tests/test_sourcepackagerecipe.py
index ca14223..4ff7004 100644
--- a/lpbuildd/tests/test_sourcepackagerecipe.py
+++ b/lpbuildd/tests/test_sourcepackagerecipe.py
@@ -6,6 +6,7 @@ import shutil
 import tempfile
 from textwrap import dedent
 
+from systemfixtures import FakeProcesses
 from testtools import TestCase
 from testtools.deferredruntest import AsynchronousDeferredRunTest
 from twisted.internet import defer
@@ -83,17 +84,40 @@ class TestSourcePackageRecipeBuildManagerIteration(TestCase):
         }
         if git:
             extra_args["git"] = True
+        original_backend_name = self.buildmanager.backend_name
+        self.buildmanager.backend_name = "fake"
         self.buildmanager.initiate({}, "chroot.tar.gz", extra_args)
+        self.buildmanager.backend_name = original_backend_name
 
         # Skip states that are done in DebianBuildManager to the state
         # directly before BUILD_RECIPE.
         self.buildmanager._state = SourcePackageRecipeBuildState.UPDATE
 
         # BUILD_RECIPE: Run the builder's payload to build the source package.
+        processes_fixture = self.useFixture(FakeProcesses())
+        processes_fixture.add(lambda _: {}, name="sudo")
         yield self.buildmanager.iterate(0)
         self.assertEqual(
             SourcePackageRecipeBuildState.BUILD_RECIPE, self.getState()
         )
+        self.assertEqual(
+            [(["mkdir", "-p", os.path.join(os.environ["HOME"], "work")],)],
+            self.buildmanager.backend.run.extract_args(),
+        )
+        self.assertEqual(
+            [
+                [
+                    "sudo",
+                    "chown",
+                    "-R",
+                    "buildd:",
+                    os.path.join(
+                        self.chrootdir, os.environ["HOME"][1:], "work"
+                    ),
+                ]
+            ],
+            [proc._args["args"] for proc in processes_fixture.procs],
+        )
         expected_command = ["sharepath/bin/buildrecipe", "buildrecipe"]
         if git:
             expected_command.append("--git")