Run prefer_system_check in a temporary dir

bits_helpers/build.py

@@ -22,6 +22,7 @@
 from textwrap import dedent
 from collections import OrderedDict
 from shlex import quote
+import tempfile
 
 import concurrent.futures
 import importlib
@@ -506,6 +507,10 @@ def doBuild(args, parser):
   install_wrapper_script("git", workDir)
 
   with DockerRunner(args.dockerImage, args.docker_extra_args) as getstatusoutput_docker:
+    def performPreferCheckWithTempDir(pkg, cmd):
+      with tempfile.TemporaryDirectory(prefix=f"bits_prefer_check_{pkg['package']}_") as temp_dir:
+        return getstatusoutput_docker(cmd, cwd=temp_dir)
+
     systemPackages, ownPackages, failed, validDefaults = \
       getPackageList(packages                = packages,
                      specs                   = specs,
@@ -516,8 +521,8 @@ def doBuild(args, parser):
                      disable                 = args.disable,
                      force_rebuild           = args.force_rebuild,
                      defaults                = args.defaults,
-                     performPreferCheck      = lambda pkg, cmd: getstatusoutput_docker(cmd),
-                     performRequirementCheck = lambda pkg, cmd: getstatusoutput_docker(cmd),
+                     performPreferCheck      = performPreferCheckWithTempDir,
+                     performRequirementCheck = performPreferCheckWithTempDir,
                      performValidateDefaults = lambda spec: validateDefaults(spec, args.defaults),
                      overrides               = overrides,
                      taps                    = taps,

bits_helpers/cmd.py

@@ -37,9 +37,9 @@ def getoutput(command, timeout=None):
   return decode_with_fallback(stdout)
 
 
-def getstatusoutput(command, timeout=None):
+def getstatusoutput(command, timeout=None, cwd=None):
   """Run command and return its return code and output (stdout and stderr)."""
-  proc = Popen(command, shell=isinstance(command, str), stdout=PIPE, stderr=STDOUT)
+  proc = Popen(command, shell=isinstance(command, str), stdout=PIPE, stderr=STDOUT, cwd=cwd)
   try:
     merged_output, _ = proc.communicate(timeout=timeout)
   except TimeoutExpired:
@@ -91,11 +91,12 @@ def __enter__(self):
       cmd += [self._docker_image, "sleep", "inf"]
       self._container = getoutput(cmd).strip()
 
-    def getstatusoutput_docker(cmd):
+    def getstatusoutput_docker(cmd, cwd=None):
       if self._container is None:
-        return getstatusoutput("{} -c {}".format(BASH, quote(cmd)))
+        return getstatusoutput("{} -c {}".format(BASH, quote(cmd)), cwd=cwd)
       return getstatusoutput("docker container exec {} bash -c {}"
-                             .format(quote(self._container), quote(cmd)))
+                             .format(quote(self._container), quote(cmd)),
+                             cwd=cwd)
 
     return getstatusoutput_docker
 

bits_helpers/doctor.py

@@ -8,6 +8,7 @@
 from bits_helpers.log import logger
 from bits_helpers.utilities import getPackageList, parseDefaults, readDefaults, validateDefaults
 from bits_helpers.cmd import getstatusoutput, DockerRunner
+import tempfile
 
 def prunePaths(workDir) -> None:
   for x in ["PATH", "LD_LIBRARY_PATH", "DYLD_LIBRARY_PATH"]:
@@ -140,6 +141,9 @@ def performValidateDefaults(spec):
     return (ok,msg,valid)
 
   with DockerRunner(args.dockerImage, args.docker_extra_args) as getstatusoutput_docker:
+    def performPreferCheckWithTempDir(pkg, cmd):
+      with tempfile.TemporaryDirectory(prefix=f"bits_prefer_check_{pkg['package']}_") as temp_dir:
+        return getstatusoutput_docker(cmd, cwd=temp_dir)
     fromSystem, own, failed, validDefaults = \
       getPackageList(packages                = packages,
                      specs                   = specs,
@@ -149,8 +153,8 @@ def performValidateDefaults(spec):
                      architecture            = args.architecture,
                      disable                 = args.disable,
                      defaults                = args.defaults,
-                     performPreferCheck      = lambda pkg, cmd: checkPreferSystem(pkg, cmd, homebrew_replacement, getstatusoutput_docker),
-                     performRequirementCheck = lambda pkg, cmd: checkRequirements(pkg, cmd, homebrew_replacement, getstatusoutput_docker),
+                     performPreferCheck      = performPreferCheckWithTempDir,
+                     performRequirementCheck = performPreferCheckWithTempDir,
                      performValidateDefaults = performValidateDefaults,
                      overrides               = overrides,
                      taps                    = taps,

tests/test_cmd.py

@@ -26,15 +26,15 @@ def test_DockerRunner(self, mock_getstatusoutput, mock_getoutput):
             mock_getoutput.assert_called_with(["docker", "run", "--detach", "--rm", "--entrypoint=",
                                                "extra arg", "image", "sleep", "inf"])
             getstatusoutput_docker("echo foo")
-            mock_getstatusoutput.assert_called_with("docker container exec container-id bash -c 'echo foo'")
+            mock_getstatusoutput.assert_called_with("docker container exec container-id bash -c 'echo foo'", cwd=None)
         mock_getstatusoutput.assert_called_with("docker container kill container-id")
 
         mock_getoutput.reset_mock()
         mock_getstatusoutput.reset_mock()
         with DockerRunner("") as getstatusoutput_docker:
             mock_getoutput.assert_not_called()
             getstatusoutput_docker("echo foo")
-            mock_getstatusoutput.assert_called_with("/bin/bash -c 'echo foo'")
+            mock_getstatusoutput.assert_called_with("/bin/bash -c 'echo foo'", cwd=None)
             mock_getstatusoutput.reset_mock()
         mock_getstatusoutput.assert_not_called()
 

tests/test_packagelist.py

@@ -1,6 +1,7 @@
 from textwrap import dedent
 import unittest
 from unittest import mock
+import tempfile
 
 from bits_helpers.cmd import getstatusoutput
 from bits_helpers.utilities import getPackageList
@@ -66,9 +67,17 @@
     force_rebuild: true
     ---
     """),
+    "CONFIG_DIR/dirty_prefer_system_check.sh": dedent("""\
+    package: dirty_prefer_system_check
+    version: v1
+    prefer_system: .*
+    prefer_system_check: |
+      pwd > HEREE
+      exit 0
+    ---
+    """),
 }
 
-
 class MockReader:
     def __init__(self, url, dist=None):
         self._contents = RECIPES[url]
@@ -80,6 +89,9 @@ def __call__(self):
 
 def getPackageListWithDefaults(packages, force_rebuild=()):
     specs = {}   # getPackageList will mutate this
+    def performPreferCheckWithTempDir(pkg, cmd):
+      with tempfile.TemporaryDirectory(prefix=f"bits_prefer_check_{pkg['package']}_") as temp_dir:
+        return getstatusoutput(cmd, cwd=temp_dir)
     return_values = getPackageList(
         packages=packages,
         specs=specs,
@@ -93,8 +105,8 @@ def getPackageListWithDefaults(packages, force_rebuild=()):
         disable=[],
         defaults="release",
         # Mock recipes just run "echo" or ":", so this is safe.
-        performPreferCheck=lambda spec, cmd: getstatusoutput(cmd),
-        performRequirementCheck=lambda spec, cmd: getstatusoutput(cmd),
+        performPreferCheck=performPreferCheckWithTempDir,
+        performRequirementCheck=performPreferCheckWithTempDir,
         performValidateDefaults=lambda spec: (True, "", ["release"]),
         overrides={"defaults-release": {}},
         taps={},
@@ -168,6 +180,14 @@ def side_effect(msg, *args, **kwargs):
             getPackageListWithDefaults(["missing-spec"])
         self.assertTrue(warning_exists)
 
+    def test_dirty_system_check(self) -> None:
+        """Check that prefer_system_check runs in isolation and doesn't create files in cwd."""
+        def fake_exists(n):
+            return n in RECIPES.keys()
+        with patch.object(os.path, "exists", fake_exists):
+            getPackageListWithDefaults(["dirty_prefer_system_check"])
+            # can't use os.path.exists() ourselves, as we just mocked it
+            self.assertFalse("HEREE" in os.listdir())
 
 
 @mock.patch("bits_helpers.utilities.getRecipeReader", new=MockReader)
@@ -190,5 +210,7 @@ def test_force_rebuild_command_line(self) -> None:
         self.assertTrue(specs["defaults-release"]["force_rebuild"])
 
 
+
+
 if __name__ == '__main__':
     unittest.main()