launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/currentlybuilding-permissions into lp:launchpad-buildd.

Commit message:
Don't rely on /CurrentlyBuilding existing in base images.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/currentlybuilding-permissions/+merge/368984

The newish Backend.copy_in interface doesn't rely on the file already existing in the base image with suitable permissions, which doesn't seem like a sensible thing for us to require of base images.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/currentlybuilding-permissions into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2019-06-10 13:03:11 +0000
+++ debian/changelog	2019-06-18 15:34:08 +0000
@@ -1,3 +1,9 @@
+launchpad-buildd (176) UNRELEASED; urgency=medium
+
+  * Don't rely on /CurrentlyBuilding existing in base images.
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 18 Jun 2019 16:26:28 +0100
+
 launchpad-buildd (175) xenial; urgency=medium
 
   * Allow configuring APT or snap store proxies via a new [proxy]

=== modified file 'lpbuildd/binarypackage.py'
--- lpbuildd/binarypackage.py	2019-02-12 10:35:12 +0000
+++ lpbuildd/binarypackage.py	2019-06-18 15:34:08 +0000
@@ -146,8 +146,6 @@
                 ['sudo', 'install', '-o', 'root', '-g', 'root', '-m', '0644',
                  schroot_file.name, self.schroot_config_path])
 
-        currently_building_path = os.path.join(
-            self.chroot_path, 'CurrentlyBuilding')
         currently_building_contents = (
             'Package: %s\n'
             'Component: %s\n'
@@ -157,8 +155,11 @@
                self.archive_purpose))
         if self.build_debug_symbols:
             currently_building_contents += 'Build-Debug-Symbols: yes\n'
-        with open(currently_building_path, 'w') as currently_building:
+        with tempfile.NamedTemporaryFile(mode='w+') as currently_building:
             currently_building.write(currently_building_contents)
+            currently_building.flush()
+            os.fchmod(currently_building.fileno(), 0o644)
+            self.backend.copy_in(currently_building.name, '/CurrentlyBuilding')
 
         args = ["sbuild-package", self._buildid, self.arch_tag]
         args.append(self.suite)

=== modified file 'lpbuildd/tests/test_binarypackage.py'
--- lpbuildd/tests/test_binarypackage.py	2019-02-12 10:41:20 +0000
+++ lpbuildd/tests/test_binarypackage.py	2019-06-18 15:34:08 +0000
@@ -6,6 +6,7 @@
 from functools import partial
 import os
 import shutil
+import stat
 import subprocess
 import tempfile
 from textwrap import dedent
@@ -199,6 +200,10 @@
     def test_with_debug_symbols(self):
         # A build with debug symbols sets up /CurrentlyBuilding
         # appropriately, and does not pass DEB_BUILD_OPTIONS.
+        self.addCleanup(
+            setattr, self.buildmanager, 'backend_name',
+            self.buildmanager.backend_name)
+        self.buildmanager.backend_name = 'fake'
         self.buildmanager.initiate(
             {'foo_1.dsc': ''}, 'chroot.tar.gz',
             {'distribution': 'ubuntu', 'series': 'warty', 'suite': 'warty',
@@ -216,19 +221,24 @@
              'foo_1.dsc'],
             env_matcher=Not(Contains('DEB_BUILD_OPTIONS')), final=True)
         self.assertFalse(self.builder.wasCalled('chrootFail'))
-        with open(os.path.join(self.chrootdir, 'CurrentlyBuilding')) as cb:
-            self.assertEqual(dedent("""\
+        self.assertEqual(
+            (dedent("""\
                 Package: foo
                 Component: main
                 Suite: warty
                 Purpose: PRIMARY
                 Build-Debug-Symbols: yes
-                """), cb.read())
+                """).encode('UTF-8'), stat.S_IFREG | 0o644),
+            self.buildmanager.backend.backend_fs['/CurrentlyBuilding'])
 
     @defer.inlineCallbacks
     def test_without_debug_symbols(self):
         # A build with debug symbols sets up /CurrentlyBuilding
         # appropriately, and passes DEB_BUILD_OPTIONS=noautodbgsym.
+        self.addCleanup(
+            setattr, self.buildmanager, 'backend_name',
+            self.buildmanager.backend_name)
+        self.buildmanager.backend_name = 'fake'
         self.buildmanager.initiate(
             {'foo_1.dsc': ''}, 'chroot.tar.gz',
             {'distribution': 'ubuntu', 'series': 'warty', 'suite': 'warty',
@@ -248,13 +258,14 @@
                 {'DEB_BUILD_OPTIONS': Equals('noautodbgsym')}),
             final=True)
         self.assertFalse(self.builder.wasCalled('chrootFail'))
-        with open(os.path.join(self.chrootdir, 'CurrentlyBuilding')) as cb:
-            self.assertEqual(dedent("""\
+        self.assertEqual(
+            (dedent("""\
                 Package: foo
                 Component: main
                 Suite: warty
                 Purpose: PRIMARY
-                """), cb.read())
+                """).encode('UTF-8'), stat.S_IFREG | 0o644),
+            self.buildmanager.backend.backend_fs['/CurrentlyBuilding'])
 
     @defer.inlineCallbacks
     def test_abort_sbuild(self):