concretizer: added logic for preferred variants

If preferred variants are present, they'll
set the default value of a variant. Otherwise
the default value is what is encoded
in package.py

3 files changed, 58 insertions, 20 deletions

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 261b469709..b16a6d14ab 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -819,12 +819,16 @@ class SpackSolverSetup(object):
             if single_value:
                 self.gen.fact(fn.variant_single_value(pkg.name, name))
                 self.gen.fact(
-                    fn.variant_default_value(pkg.name, name, variant.default))
+                    fn.variant_default_value_from_package_py(
+                        pkg.name, name, variant.default)
+                )
             else:
                 defaults = variant.default.split(',')
                 for val in sorted(defaults):
                     self.gen.fact(
-                        fn.variant_default_value(pkg.name, name, val))
+                        fn.variant_default_value_from_package_py(
+                            pkg.name, name, val)
+                    )
 
             values = variant.values
             if values is None:
@@ -947,16 +951,16 @@ class SpackSolverSetup(object):
                 self.gen.fact(fn.external_only(pkg_name))
 
             # Read a list of all the specs for this package
-            externals = data['externals']
+            externals = data.get('externals', [])
             external_specs = [spack.spec.Spec(x['spec']) for x in externals]
 
             # Compute versions with appropriate weights
             external_versions = [
-                (x.version, id) for id, x in enumerate(external_specs)
+                (x.version, idx) for idx, x in enumerate(external_specs)
             ]
             external_versions = [
-                (v, -(w + 1), id)
-                for w, (v, id) in enumerate(sorted(external_versions))
+                (v, -(w + 1), idx)
+                for w, (v, idx) in enumerate(sorted(external_versions))
             ]
             for version, weight, id in external_versions:
                 self.gen.fact(fn.external_version_declared(
@@ -993,6 +997,26 @@ class SpackSolverSetup(object):
             self.gen.out.write(external_rule)
             self.gen.control.add("base", [], external_rule)
 
+    def concretization_preferences(self, pkg_name):
+        """Facts on concretization preferences, as read from packages.yaml"""
+        preferences = spack.package_prefs.PackagePrefs
+        preferred_variants = preferences.preferred_variants(pkg_name)
+        if not preferred_variants:
+            return
+
+        self.gen.h2('Concretization preferences {0}'.format(pkg_name))
+        for variant_name in sorted(preferred_variants):
+            variant = preferred_variants[variant_name]
+            values = variant.value
+
+            if not isinstance(values, tuple):
+                values = (values,)
+
+            for value in values:
+                self.gen.fact(fn.variant_default_value_from_packages_yaml(
+                    pkg_name, variant.name, value
+                ))
+
     def flag_defaults(self):
         self.gen.h2("Compiler flag defaults")
 
@@ -1370,6 +1394,7 @@ class SpackSolverSetup(object):
         for pkg in sorted(pkgs):
             self.gen.h2('Package: %s' % pkg)
             self.pkg_rules(pkg)
+            self.concretization_preferences(pkg)
 
         self.gen.h1('Spec Constraints')
         for spec in sorted(specs):
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 0da95b1907..be1bd0ccc6 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -163,7 +163,16 @@ variant_not_default(Package, Variant, Value, 0)
     variant_default_value(Package, Variant, Value),
     node(Package).
 
-% suppress wranings about this atom being unset.  It's only set if some
+% The default value for a variant in a package is what is written
+% in the package.py file, unless some preference is set in packages.yaml
+variant_default_value(Package, Variant, Value)
+ :- variant_default_value_from_package_py(Package, Variant, Value),
+    not variant_default_value_from_packages_yaml(Package, Variant, _).
+
+variant_default_value(Package, Variant, Value)
+ :- variant_default_value_from_packages_yaml(Package, Variant, Value).
+
+% suppress warnings about this atom being unset.  It's only set if some
 % spec or some package sets it, and without this, clingo will give
 % warnings like 'info: atom does not occur in any rule head'.
 #defined variant/2.
@@ -171,6 +180,7 @@ variant_not_default(Package, Variant, Value, 0)
 #defined variant_single_value/2.
 #defined variant_default_value/3.
 #defined variant_possible_value/3.
+#defined variant_default_value_from_packages_yaml/3.
 
 %-----------------------------------------------------------------------------
 % Platform semantics
diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py
index 7d2a1bd3fe..4d9cc9412b 100644
--- a/lib/spack/spack/test/concretize_preferences.py
+++ b/lib/spack/spack/test/concretize_preferences.py
@@ -70,19 +70,22 @@ def assert_variant_values(spec, **variants):
 
 @pytest.mark.usefixtures('concretize_scope', 'mock_packages')
 class TestConcretizePreferences(object):
-    def test_preferred_variants(self):
-        """Test preferred variants are applied correctly
-        """
-        update_packages('mpileaks', 'variants', '~debug~opt+shared+static')
-        assert_variant_values(
-            'mpileaks', debug=False, opt=False, shared=True, static=True
-        )
-        update_packages(
-            'mpileaks', 'variants', ['+debug', '+opt', '~shared', '-static']
-        )
-        assert_variant_values(
-            'mpileaks', debug=True, opt=True, shared=False, static=False
-        )
+    @pytest.mark.parametrize('package_name,variant_value,expected_results', [
+        ('mpileaks', '~debug~opt+shared+static',
+         {'debug': False, 'opt': False, 'shared': True, 'static': True}),
+        # Check that using a list of variants instead of a single string works
+        ('mpileaks', ['~debug', '~opt', '+shared', '+static'],
+         {'debug': False, 'opt': False, 'shared': True, 'static': True}),
+        # Use different values for the variants and check them again
+        ('mpileaks', ['+debug', '+opt', '~shared', '-static'],
+         {'debug': True, 'opt': True, 'shared': False, 'static': False}),
+    ])
+    def test_preferred_variants(
+            self, package_name, variant_value, expected_results
+    ):
+        """Test preferred variants are applied correctly"""
+        update_packages(package_name, 'variants', variant_value)
+        assert_variant_values(package_name, **expected_results)
 
     def test_preferred_variants_from_wildcard(self):
         """