Allow setting variables when using system packages (#795)

alibuild_helpers/utilities.py

@@ -12,6 +12,10 @@
   from collections import OrderedDict
 except ImportError:
   from ordereddict import OrderedDict
+try:
+  from shlex import quote  # Python 3.3+
+except ImportError:
+  from pipes import quote  # Python 2.7
 
 from alibuild_helpers.cmd import decode_with_fallback, getoutput
 from alibuild_helpers.git import git
@@ -383,18 +387,50 @@ def getPackageList(packages, specs, configDir, preferSystem, noSystem,
     except TypeError as e:
       dieOnError(True, "Malformed entry prefer_system: %s in %s" % (systemRE, spec["package"]))
     if not noSystem and (preferSystem or systemREMatches):
-      cmd = "REQUESTED_VERSION=%s\n" % resolve_version(spec, defaults, "unavailable", "unavailable")
-      cmd += spec.get("prefer_system_check", "false").strip()
-      if not spec["package"] in testCache:
-        testCache[spec["package"]] = performPreferCheck(spec, cmd.strip())
-
+      requested_version = resolve_version(spec, defaults, "unavailable", "unavailable")
+      cmd = "REQUESTED_VERSION={version}\n{check}".format(
+        version=quote(requested_version),
+        check=spec.get("prefer_system_check", "false"),
+      ).strip()
+      if spec["package"] not in testCache:
+        testCache[spec["package"]] = performPreferCheck(spec, cmd)
       err, output = testCache[spec["package"]]
-
-      if not err:
-        systemPackages.update([spec["package"]])
-        disable.append(spec["package"])
+      if err:
+        # prefer_system_check errored; this means we must build the package ourselves.
+        ownPackages.add(spec["package"])
       else:
-        ownPackages.update([spec["package"]])
+        # prefer_system_check succeeded; this means we should use the system package.
+        match = re.search(r"^alibuild_system_replace:(?P<key>.*)$", output, re.MULTILINE)
+        if not match:
+          # No replacement spec name given. Fall back to old system package
+          # behaviour and just disable the package.
+          systemPackages.add(spec["package"])
+          disable.append(spec["package"])
+        else:
+          # The check printed the name of a replacement; use it.
+          key = match.group("key").strip()
+          try:
+            replacement = spec["prefer_system_replacement_specs"][key]
+          except KeyError:
+            dieOnError(True, "Could not find named replacement spec for "
+                       "%s: %s" % (spec["package"], key))
+          else:
+            # We must keep the package name the same, since it is used to
+            # specify dependencies.
+            replacement["package"] = spec["package"]
+            # The version is required for all specs. What we put there will
+            # influence the package's hash, so allow the user to override it.
+            replacement.setdefault("version", requested_version)
+            spec = replacement
+            recipe = replacement.get("recipe", "")
+            # If there's an explicitly-specified recipe, we're still building
+            # the package. If not, aliBuild will still "build" it, but it's
+            # basically instantaneous, so report to the user that we're taking
+            # it from the system.
+            if recipe:
+              ownPackages.add(spec["package"])
+            else:
+              systemPackages.add(spec["package"])
 
     dieOnError(("system_requirement" in spec) and recipe.strip("\n\t "),
                "System requirements %s cannot have a recipe" % spec["package"])

tests/test_system_replacement.py

@@ -0,0 +1,161 @@
+from __future__ import print_function
+from textwrap import dedent
+import unittest
+try:                  # Python 3
+    from unittest import mock
+except ImportError:   # Python 2
+    import mock
+
+from alibuild_helpers.cmd import getstatusoutput
+from alibuild_helpers.utilities import getPackageList
+
+
+RECIPES = {
+    "CONFIG_DIR/defaults-release.sh": dedent("""\
+    package: defaults-release
+    version: v1
+    ---
+    """),
+    "CONFIG_DIR/disable.sh": dedent("""\
+    package: disable
+    version: v1
+    prefer_system: '.*'
+    prefer_system_check: 'true'
+    ---
+    """),
+    "CONFIG_DIR/with-replacement.sh": dedent("""\
+    package: with-replacement
+    version: v1
+    prefer_system: '.*'
+    prefer_system_check: |
+        echo 'alibuild_system_replace: replacement'
+    prefer_system_replacement_specs:
+        replacement:
+            env:
+                SENTINEL_VAR: magic
+    ---
+    """),
+    "CONFIG_DIR/with-replacement-recipe.sh": dedent("""\
+    package: with-replacement-recipe
+    version: v1
+    prefer_system: '.*'
+    prefer_system_check: |
+        echo 'alibuild_system_replace: replacement'
+    prefer_system_replacement_specs:
+        replacement:
+            recipe: 'true'
+    ---
+    """),
+    "CONFIG_DIR/missing-spec.sh": dedent("""\
+    package: missing-spec
+    version: v1
+    prefer_system: '.*'
+    prefer_system_check: |
+        echo 'alibuild_system_replace: missing_tag'
+    prefer_system_replacement_specs: {}
+    ---
+    """),
+    "CONFIG_DIR/sentinel-command.sh": dedent("""\
+    package: sentinel-command
+    version: v1
+    prefer_system: '.*'
+    prefer_system_check: |
+        : magic sentinel command
+    ---
+    """),
+}
+
+
+class MockReader:
+    def __init__(self, url, dist=None):
+        self._contents = RECIPES[url]
+        self.url = "mock://" + url
+
+    def __call__(self):
+        return self._contents
+
+
+def getPackageListWithDefaults(packages):
+    specs = {}   # getPackageList will mutate this
+    return_values = getPackageList(
+        packages=packages,
+        specs=specs,
+        configDir="CONFIG_DIR",
+        # Make sure getPackageList considers prefer_system_check.
+        # (Even with preferSystem=False + noSystem=False, it is sufficient
+        # if the prefer_system regex matches the architecture.)
+        preferSystem=True,
+        noSystem=False,
+        architecture="ARCH",
+        disable=[],
+        defaults="DEFAULTS",
+        performPreferCheck=lambda spec, cmd: getstatusoutput(cmd),
+        performRequirementCheck=lambda spec, cmd: getstatusoutput(cmd),
+        performValidateDefaults=lambda spec: (True, "", ["DEFAULTS"]),
+        overrides={"defaults-release": {}},
+        taps={},
+        log=lambda *_: None,
+    )
+    return (specs, *return_values)
+
+
+@mock.patch("alibuild_helpers.utilities.getRecipeReader", new=MockReader)
+class ReplacementTestCase(unittest.TestCase):
+    """Test that system package replacements are working."""
+
+    def test_disable(self):
+        """Check that not specifying any replacement disables the package.
+
+        This is was the only available behaviour in previous aliBuild versions
+        and must be preserved for backward compatibility.
+        """
+        specs, systemPkgs, ownPkgs, failedReqs, validDefaults = \
+            getPackageListWithDefaults(["disable"])
+        self.assertIn("disable", systemPkgs)
+        self.assertNotIn("disable", ownPkgs)
+        self.assertNotIn("disable", specs)
+
+    def test_replacement_given(self):
+        """Check that specifying a replacement spec means it is used.
+
+        This also checks that if no recipe is given, we report the package as
+        a system package to the user.
+        """
+        specs, systemPkgs, ownPkgs, failedReqs, validDefaults = \
+            getPackageListWithDefaults(["with-replacement"])
+        self.assertIn("with-replacement", specs)
+        self.assertEqual(specs["with-replacement"]["env"]["SENTINEL_VAR"], "magic")
+        # Make sure nothing is run by default.
+        self.assertEqual(specs["with-replacement"]["recipe"], "")
+        # If the replacement spec has no recipe, report to the user that we're
+        # taking the package from the system.
+        self.assertIn("with-replacement", systemPkgs)
+        self.assertNotIn("with-replacement", ownPkgs)
+
+    def test_replacement_recipe_given(self):
+        """Check that specifying a replacement recipe means it is used.
+
+        Also check that we report to the user that a package will be compiled
+        when a replacement recipe is given.
+        """
+        specs, systemPkgs, ownPkgs, failedReqs, validDefaults = \
+            getPackageListWithDefaults(["with-replacement-recipe"])
+        self.assertIn("with-replacement-recipe", specs)
+        self.assertIn("recipe", specs["with-replacement-recipe"])
+        self.assertEqual("true", specs["with-replacement-recipe"]["recipe"])
+        # If the replacement spec has a recipe, report to the user that we're
+        # compiling the package.
+        self.assertNotIn("with-replacement-recipe", systemPkgs)
+        self.assertIn("with-replacement-recipe", ownPkgs)
+
+    @mock.patch("alibuild_helpers.utilities.dieOnError")
+    def test_missing_replacement_spec(self, mock_dieOnError):
+        """Check an error is thrown when the replacement spec is not found."""
+        specs, systemPkgs, ownPkgs, failedReqs, validDefaults = \
+            getPackageListWithDefaults(["missing-spec"])
+        mock_dieOnError.assert_any_call(True, "Could not find named replacement "
+                                        "spec for missing-spec: missing_tag")
+
+
+if __name__ == '__main__':
+    unittest.main()