Fix force_rebuild setting when specified by recipe (#823)

Previously, alibuild would always overwrite the `force_rebuild:` YAML key with
the value of the `--force-rebuild` command-line flag, so the former would have
no effect.

Add unit test to verify fixed behaviour

alibuild_helpers/utilities.py

@@ -506,7 +506,8 @@ def getPackageList(packages, specs, configDir, preferSystem, noSystem,
     spec["tag"] = spec.get("tag", spec["version"])
     spec["version"] = spec["version"].replace("/", "_")
     spec["recipe"] = recipe.strip("\n")
-    spec["force_rebuild"] = spec["package"] in force_rebuild
+    if spec["package"] in force_rebuild:
+      spec["force_rebuild"] = True
     specs[spec["package"]] = spec
     packages += spec["requires"]
   return (systemPackages, ownPackages, failedRequirements, validDefaults)

tests/test_system_replacement.py → tests/test_packagelist.py

@@ -14,6 +14,7 @@
     "CONFIG_DIR/defaults-release.sh": dedent("""\
     package: defaults-release
     version: v1
+    force_rebuild: false
     ---
     """),
     "CONFIG_DIR/disable.sh": dedent("""\
@@ -63,6 +64,12 @@
         : magic sentinel command
     ---
     """),
+    "CONFIG_DIR/force-rebuild.sh": dedent("""\
+    package: force-rebuild
+    version: v1
+    force_rebuild: true
+    ---
+    """),
 }
 
 
@@ -75,7 +82,7 @@ def __call__(self):
         return self._contents
 
 
-def getPackageListWithDefaults(packages):
+def getPackageListWithDefaults(packages, force_rebuild=()):
     specs = {}   # getPackageList will mutate this
     return_values = getPackageList(
         packages=packages,
@@ -89,12 +96,14 @@ def getPackageListWithDefaults(packages):
         architecture="ARCH",
         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),
         performValidateDefaults=lambda spec: (True, "", ["release"]),
         overrides={"defaults-release": {}},
         taps={},
         log=lambda *_: None,
+        force_rebuild=force_rebuild,
     )
     return (specs, *return_values)
 
@@ -157,5 +166,24 @@ def test_missing_replacement_spec(self, mock_dieOnError):
                                         "spec for missing-spec: missing_tag")
 
 
+@mock.patch("alibuild_helpers.utilities.getRecipeReader", new=MockReader)
+class ForceRebuildTestCase(unittest.TestCase):
+    """Test that force_rebuild keys are applied properly."""
+
+    def test_force_rebuild_recipe(self):
+        """If the recipe specifies force_rebuild, it must be applied."""
+        specs, _, _, _, _ = getPackageListWithDefaults(["force-rebuild"])
+        self.assertTrue(specs["force-rebuild"]["force_rebuild"])
+        self.assertFalse(specs["defaults-release"]["force_rebuild"])
+
+    def test_force_rebuild_command_line(self):
+        """The --force-rebuild option must take precedence, if given."""
+        specs, _, _, _, _ = getPackageListWithDefaults(
+            ["force-rebuild"], force_rebuild=["defaults-release", "force-rebuild"],
+        )
+        self.assertTrue(specs["force-rebuild"]["force_rebuild"])
+        self.assertTrue(specs["defaults-release"]["force_rebuild"])
+
+
 if __name__ == '__main__':
     unittest.main()