targets: print a warning message before downgrading (#13513)

* Make package preferences a soft failure for targets, instead of a hard failure.
* Added unit tests for preferences expressed via packages.yaml

2 files changed, 57 insertions, 25 deletions

diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py
index a31529e0ea..417afed35b 100644
--- a/lib/spack/spack/concretize.py
+++ b/lib/spack/spack/concretize.py
@@ -281,25 +281,7 @@ class Concretizer(object):
             else:
                 # To get default platform, consider package prefs
                 if PackagePrefs.has_preferred_targets(spec.name):
-                    target_prefs = PackagePrefs(spec.name, 'target')
-                    target_specs = [spack.spec.Spec('target=%s' % tname)
-                                    for tname in cpu.targets]
-
-                    def tspec_filter(s):
-                        # Filter target specs by whether the architecture
-                        # family is the current machine type. This ensures
-                        # we only consider x86_64 targets when on an
-                        # x86_64 machine, etc. This may need to change to
-                        # enable setting cross compiling as a default
-                        target = cpu.targets[str(s.architecture.target)]
-                        arch_family_name = target.family.name
-                        return arch_family_name == platform.machine()
-
-                    # Sort filtered targets by package prefs
-                    target_specs = list(filter(tspec_filter, target_specs))
-                    target_specs.sort(key=target_prefs)
-
-                    new_target = target_specs[0].architecture.target
+                    new_target = self.target_from_package_preferences(spec)
                 else:
                     new_target = new_plat.target('default_target')
 
@@ -310,6 +292,33 @@ class Concretizer(object):
         spec.architecture = new_arch
         return spec_changed
 
+    def target_from_package_preferences(self, spec):
+        """Returns the preferred target from the package preferences if
+        there's any.
+
+        Args:
+            spec: abstract spec to be concretized
+        """
+        target_prefs = PackagePrefs(spec.name, 'target')
+        target_specs = [spack.spec.Spec('target=%s' % tname)
+                        for tname in cpu.targets]
+
+        def tspec_filter(s):
+            # Filter target specs by whether the architecture
+            # family is the current machine type. This ensures
+            # we only consider x86_64 targets when on an
+            # x86_64 machine, etc. This may need to change to
+            # enable setting cross compiling as a default
+            target = cpu.targets[str(s.architecture.target)]
+            arch_family_name = target.family.name
+            return arch_family_name == platform.machine()
+
+        # Sort filtered targets by package prefs
+        target_specs = list(filter(tspec_filter, target_specs))
+        target_specs.sort(key=target_prefs)
+        new_target = target_specs[0].architecture.target
+        return new_target
+
     def concretize_variants(self, spec):
         """If the spec already has variants filled in, return.  Otherwise, add
            the user preferences from packages.yaml or the default variants from
@@ -526,7 +535,12 @@ class Concretizer(object):
         current_platform = spack.architecture.get_platform(
             spec.architecture.platform
         )
-        if current_target != current_platform.target('default_target') or \
+
+        default_target = current_platform.target('default_target')
+        if PackagePrefs.has_preferred_targets(spec.name):
+            default_target = self.target_from_package_preferences(spec)
+
+        if current_target != default_target or \
             (self.abstract_spec.architecture is not None and
              self.abstract_spec.architecture.target is not None):
             return False
@@ -544,6 +558,11 @@ class Concretizer(object):
                     continue
 
                 if candidate is not None:
+                    msg = ('{0.name}@{0.version} cannot build optimized '
+                           'binaries for "{1}". Using best target possible: '
+                           '"{2}"')
+                    msg = msg.format(spec.compiler, current_target, candidate)
+                    tty.warn(msg)
                     spec.architecture.target = candidate
                     return True
             else:
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index 592b515dbf..e0774909f4 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -11,6 +11,7 @@ import spack.concretize
 import spack.repo
 
 from spack.concretize import find_spec, NoValidVersionError
+from spack.package_prefs import PackagePrefs
 from spack.spec import Spec, CompilerSpec
 from spack.spec import ConflictsInSpecError, SpecError
 from spack.version import ver
@@ -83,13 +84,25 @@ def spec(request):
 
 
 @pytest.fixture(params=[
-    'haswell', 'broadwell', 'skylake', 'icelake'
+    # Mocking the host detection
+    'haswell', 'broadwell', 'skylake', 'icelake',
+    # Using preferred targets from packages.yaml
+    'icelake-preference', 'cannonlake-preference'
 ])
 def current_host(request, monkeypatch):
-    target = llnl.util.cpu.targets[request.param]
-    monkeypatch.setattr(llnl.util.cpu, 'host', lambda: target)
-    monkeypatch.setattr(spack.platforms.test.Test, 'default', request.param)
-    return target
+    # is_preference is not empty if we want to supply the
+    # preferred target via packages.yaml
+    cpu, _, is_preference = request.param.partition('-')
+    target = llnl.util.cpu.targets[cpu]
+    if not is_preference:
+        monkeypatch.setattr(llnl.util.cpu, 'host', lambda: target)
+        monkeypatch.setattr(spack.platforms.test.Test, 'default', cpu)
+        yield target
+    else:
+        # There's a cache that needs to be cleared for unit tests
+        PackagePrefs._packages_config_cache = None
+        with spack.config.override('packages:all', {'target': [cpu]}):
+            yield target
 
 
 @pytest.mark.usefixtures('config', 'mock_packages')