Add syntactic sugar for "strong preferences" and "conflicts" (#41832)

Currently requirements allow to express "strong preferences" and "conflicts" from
configuration using a convoluted syntax:
```yaml
packages:
  zlib-ng:
    require:
    # conflict on %clang
    - one_of: ["%clang", "@:"]
    # Strong preference for +shared
    - any_of: ["+shared", "@:"]
```
This PR adds syntactic sugar so that the same can be written as:
```yaml
packages:
  zlib-ng:
    conflict:
    - "%clang"
    prefer:
    - "+shared"
```
Preferences written in this way are "stronger" that the ones documented at:
- https://spack.readthedocs.io/en/latest/packages_yaml.html#package-preferences

lib/spack/docs/packages_yaml.rst

@@ -487,6 +487,56 @@ present. For instance with a configuration like:
 
 you will use ``mvapich2~cuda %gcc`` as an ``mpi`` provider.
 
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Conflicts and strong preferences
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If the semantic of requirements is too strong, you can also express "strong preferences" and "conflicts"
+from configuration files:
+
+.. code-block:: yaml
+
+   packages:
+     all:
+       prefer:
+       - '%clang'
+       conflict:
+       - '+shared'
+
+The ``prefer`` and ``conflict`` sections can be used whenever a ``require`` section is allowed.
+The argument is always a list of constraints, and each constraint can be either a simple string,
+or a more complex object:
+
+.. code-block:: yaml
+
+   packages:
+     all:
+       conflict:
+       - spec: '%clang'
+         when: 'target=x86_64_v3'
+         message: 'reason why clang cannot be used'
+
+The ``spec`` attribute is mandatory, while both ``when`` and ``message`` are optional.
+
+.. note::
+
+   Requirements allow for expressing both "strong preferences" and "conflicts".
+   The syntax for doing so, though, may not be immediately clear. For
+   instance, if we want to prevent any package from using ``%clang``, we can set:
+
+   .. code-block:: yaml
+
+      packages:
+        all:
+          require:
+          - one_of: ['%clang', '@:']
+
+   Since only one of the requirements must hold, and ``@:`` is always true, the rule above is
+   equivalent to a conflict. For "strong preferences" we need to substitute the ``one_of`` policy
+   with ``any_of``.
+
+
+
 .. _package-preferences:
 
 -------------------

lib/spack/spack/schema/packages.py

@@ -54,6 +54,24 @@
     ]
 }
 
+prefer_and_conflict = {
+    "type": "array",
+    "items": {
+        "oneOf": [
+            {
+                "type": "object",
+                "additionalProperties": False,
+                "properties": {
+                    "spec": {"type": "string"},
+                    "message": {"type": "string"},
+                    "when": {"type": "string"},
+                },
+            },
+            {"type": "string"},
+        ]
+    },
+}
+
 permissions = {
     "type": "object",
     "additionalProperties": False,
@@ -85,6 +103,8 @@
                 "additionalProperties": False,
                 "properties": {
                     "require": requirements,
+                    "prefer": prefer_and_conflict,
+                    "conflict": prefer_and_conflict,
                     "version": {},  # Here only to warn users on ignored properties
                     "target": {
                         "type": "array",
@@ -133,6 +153,8 @@
                 "additionalProperties": False,
                 "properties": {
                     "require": requirements,
+                    "prefer": prefer_and_conflict,
+                    "conflict": prefer_and_conflict,
                     "version": {
                         "type": "array",
                         "default": [],
@@ -186,7 +208,6 @@
     }
 }
 
-
 #: Full schema with metadata
 schema = {
     "$schema": "http://json-schema.org/draft-07/schema#",

lib/spack/spack/solver/asp.py

@@ -870,7 +870,7 @@ class RequirementRule(NamedTuple):
     requirements: List["spack.spec.Spec"]
     condition: "spack.spec.Spec"
     kind: RequirementKind
-    message: str
+    message: Optional[str]
 
 
 class PyclingoDriver:
@@ -1306,108 +1306,8 @@ def compiler_facts(self):
             self.gen.fact(f)
 
     def package_requirement_rules(self, pkg):
-        rules = self.requirement_rules_from_package_py(pkg)
+        parser = RequirementParser(spack.config.CONFIG)
-        rules.extend(self.requirement_rules_from_packages_yaml(pkg))
+        self.emit_facts_from_requirement_rules(parser.rules(pkg))
-        self.emit_facts_from_requirement_rules(rules)
-
-    def requirement_rules_from_package_py(self, pkg):
-        rules = []
-        for when_spec, requirement_list in pkg.requirements.items():
-            for requirements, policy, message in requirement_list:
-                rules.append(
-                    RequirementRule(
-                        pkg_name=pkg.name,
-                        policy=policy,
-                        requirements=requirements,
-                        kind=RequirementKind.PACKAGE,
-                        condition=when_spec,
-                        message=message,
-                    )
-                )
-        return rules
-
-    def requirement_rules_from_packages_yaml(self, pkg):
-        pkg_name = pkg.name
-        config = spack.config.get("packages")
-        requirements = config.get(pkg_name, {}).get("require", [])
-        kind = RequirementKind.PACKAGE
-        if not requirements:
-            requirements = config.get("all", {}).get("require", [])
-            kind = RequirementKind.DEFAULT
-        return self._rules_from_requirements(pkg_name, requirements, kind=kind)
-
-    def _rules_from_requirements(
-        self, pkg_name: str, requirements, *, kind: RequirementKind
-    ) -> List[RequirementRule]:
-        """Manipulate requirements from packages.yaml, and return a list of tuples
-        with a uniform structure (name, policy, requirements).
-        """
-        if isinstance(requirements, str):
-            requirements = [requirements]
-
-        rules = []
-        for requirement in requirements:
-            # A string is equivalent to a one_of group with a single element
-            if isinstance(requirement, str):
-                requirement = {"one_of": [requirement]}
-
-            for policy in ("spec", "one_of", "any_of"):
-                if policy not in requirement:
-                    continue
-
-                constraints = requirement[policy]
-                # "spec" is for specifying a single spec
-                if policy == "spec":
-                    constraints = [constraints]
-                    policy = "one_of"
-
-                # validate specs from YAML first, and fail with line numbers if parsing fails.
-                constraints = [
-                    sc.parse_spec_from_yaml_string(constraint) for constraint in constraints
-                ]
-                when_str = requirement.get("when")
-                when = sc.parse_spec_from_yaml_string(when_str) if when_str else spack.spec.Spec()
-
-                # filter constraints
-                constraints = [
-                    c
-                    for c in constraints
-                    if not self.reject_requirement_constraint(pkg_name, constraint=c, kind=kind)
-                ]
-                if not constraints:
-                    continue
-
-                rules.append(
-                    RequirementRule(
-                        pkg_name=pkg_name,
-                        policy=policy,
-                        requirements=constraints,
-                        kind=kind,
-                        message=requirement.get("message"),
-                        condition=when,
-                    )
-                )
-        return rules
-
-    def reject_requirement_constraint(
-        self, pkg_name: str, *, constraint: "spack.spec.Spec", kind: RequirementKind
-    ) -> bool:
-        """Returns True if a requirement constraint should be rejected"""
-        if kind == RequirementKind.DEFAULT:
-            # Requirements under all: are applied only if they are satisfiable considering only
-            # package rules, so e.g. variants must exist etc. Otherwise, they are rejected.
-            try:
-                s = spack.spec.Spec(pkg_name)
-                s.constrain(constraint)
-                s.validate_or_raise()
-            except spack.error.SpackError as e:
-                tty.debug(
-                    f"[SETUP] Rejecting the default '{constraint}' requirement "
-                    f"on '{pkg_name}': {str(e)}",
-                    level=2,
-                )
-                return True
-        return False
 
     def pkg_rules(self, pkg, tests):
         pkg = self.pkg_class(pkg)
@@ -1740,13 +1640,10 @@ def provider_requirements(self):
             "Internal Error: possible_virtuals is not populated. Please report to the spack"
             " maintainers"
         )
-        packages_yaml = spack.config.CONFIG.get("packages")
+        parser = RequirementParser(spack.config.CONFIG)
         assert self.possible_virtuals is not None, msg
         for virtual_str in sorted(self.possible_virtuals):
-            requirements = packages_yaml.get(virtual_str, {}).get("require", [])
+            rules = parser.rules_from_virtual(virtual_str)
-            rules = self._rules_from_requirements(
-                virtual_str, requirements, kind=RequirementKind.VIRTUAL
-            )
             if rules:
                 self.emit_facts_from_requirement_rules(rules)
                 self.trigger_rules()
@@ -1786,8 +1683,8 @@ def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]):
                 self.gen.fact(fn.requirement_message(pkg_name, requirement_grp_id, rule.message))
             self.gen.newline()
 
-            for spec_str in requirement_grp:
+            for input_spec in requirement_grp:
-                spec = spack.spec.Spec(spec_str)
+                spec = spack.spec.Spec(input_spec)
                 if not spec.name:
                     spec.name = pkg_name
                 spec.attach_git_version_lookup()
@@ -1807,7 +1704,7 @@ def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]):
                         imposed_spec=spec,
                         name=pkg_name,
                         transform_imposed=transform,
-                        msg=f"{spec_str} is a requirement for package {pkg_name}",
+                        msg=f"{input_spec} is a requirement for package {pkg_name}",
                     )
                 except Exception as e:
                     # Do not raise if the rule comes from the 'all' subsection, since usability
@@ -2884,6 +2781,182 @@ def pkg_class(self, pkg_name: str) -> typing.Type["spack.package_base.PackageBas
         return spack.repo.PATH.get_pkg_class(request)
 
 
+class RequirementParser:
+    """Parses requirements from package.py files and configuration, and returns rules."""
+
+    def __init__(self, configuration):
+        self.config = configuration
+
+    def rules(self, pkg: "spack.package_base.PackageBase") -> List[RequirementRule]:
+        result = []
+        result.extend(self.rules_from_package_py(pkg))
+        result.extend(self.rules_from_require(pkg))
+        result.extend(self.rules_from_prefer(pkg))
+        result.extend(self.rules_from_conflict(pkg))
+        return result
+
+    def rules_from_package_py(self, pkg) -> List[RequirementRule]:
+        rules = []
+        for when_spec, requirement_list in pkg.requirements.items():
+            for requirements, policy, message in requirement_list:
+                rules.append(
+                    RequirementRule(
+                        pkg_name=pkg.name,
+                        policy=policy,
+                        requirements=requirements,
+                        kind=RequirementKind.PACKAGE,
+                        condition=when_spec,
+                        message=message,
+                    )
+                )
+        return rules
+
+    def rules_from_virtual(self, virtual_str: str) -> List[RequirementRule]:
+        requirements = self.config.get("packages", {}).get(virtual_str, {}).get("require", [])
+        return self._rules_from_requirements(
+            virtual_str, requirements, kind=RequirementKind.VIRTUAL
+        )
+
+    def rules_from_require(self, pkg: "spack.package_base.PackageBase") -> List[RequirementRule]:
+        kind, requirements = self._raw_yaml_data(pkg, section="require")
+        return self._rules_from_requirements(pkg.name, requirements, kind=kind)
+
+    def rules_from_prefer(self, pkg: "spack.package_base.PackageBase") -> List[RequirementRule]:
+        result = []
+        kind, preferences = self._raw_yaml_data(pkg, section="prefer")
+        for item in preferences:
+            spec, condition, message = self._parse_prefer_conflict_item(item)
+            result.append(
+                # A strong preference is defined as:
+                #
+                # require:
+                # - any_of: [spec_str, "@:"]
+                RequirementRule(
+                    pkg_name=pkg.name,
+                    policy="any_of",
+                    requirements=[spec, spack.spec.Spec("@:")],
+                    kind=kind,
+                    message=message,
+                    condition=condition,
+                )
+            )
+        return result
+
+    def rules_from_conflict(self, pkg: "spack.package_base.PackageBase") -> List[RequirementRule]:
+        result = []
+        kind, conflicts = self._raw_yaml_data(pkg, section="conflict")
+        for item in conflicts:
+            spec, condition, message = self._parse_prefer_conflict_item(item)
+            result.append(
+                # A conflict is defined as:
+                #
+                # require:
+                # - one_of: [spec_str, "@:"]
+                RequirementRule(
+                    pkg_name=pkg.name,
+                    policy="one_of",
+                    requirements=[spec, spack.spec.Spec("@:")],
+                    kind=kind,
+                    message=message,
+                    condition=condition,
+                )
+            )
+        return result
+
+    def _parse_prefer_conflict_item(self, item):
+        # The item is either a string or an object with at least a "spec" attribute
+        if isinstance(item, str):
+            spec = sc.parse_spec_from_yaml_string(item)
+            condition = spack.spec.Spec()
+            message = None
+        else:
+            spec = sc.parse_spec_from_yaml_string(item["spec"])
+            condition = spack.spec.Spec(item.get("when"))
+            message = item.get("message")
+        return spec, condition, message
+
+    def _raw_yaml_data(self, pkg: "spack.package_base.PackageBase", *, section: str):
+        config = self.config.get("packages")
+        data = config.get(pkg.name, {}).get(section, [])
+        kind = RequirementKind.PACKAGE
+        if not data:
+            data = config.get("all", {}).get(section, [])
+            kind = RequirementKind.DEFAULT
+        return kind, data
+
+    def _rules_from_requirements(
+        self, pkg_name: str, requirements, *, kind: RequirementKind
+    ) -> List[RequirementRule]:
+        """Manipulate requirements from packages.yaml, and return a list of tuples
+        with a uniform structure (name, policy, requirements).
+        """
+        if isinstance(requirements, str):
+            requirements = [requirements]
+
+        rules = []
+        for requirement in requirements:
+            # A string is equivalent to a one_of group with a single element
+            if isinstance(requirement, str):
+                requirement = {"one_of": [requirement]}
+
+            for policy in ("spec", "one_of", "any_of"):
+                if policy not in requirement:
+                    continue
+
+                constraints = requirement[policy]
+                # "spec" is for specifying a single spec
+                if policy == "spec":
+                    constraints = [constraints]
+                    policy = "one_of"
+
+                # validate specs from YAML first, and fail with line numbers if parsing fails.
+                constraints = [
+                    sc.parse_spec_from_yaml_string(constraint) for constraint in constraints
+                ]
+                when_str = requirement.get("when")
+                when = sc.parse_spec_from_yaml_string(when_str) if when_str else spack.spec.Spec()
+
+                constraints = [
+                    x
+                    for x in constraints
+                    if not self.reject_requirement_constraint(pkg_name, constraint=x, kind=kind)
+                ]
+                if not constraints:
+                    continue
+
+                rules.append(
+                    RequirementRule(
+                        pkg_name=pkg_name,
+                        policy=policy,
+                        requirements=constraints,
+                        kind=kind,
+                        message=requirement.get("message"),
+                        condition=when,
+                    )
+                )
+        return rules
+
+    def reject_requirement_constraint(
+        self, pkg_name: str, *, constraint: spack.spec.Spec, kind: RequirementKind
+    ) -> bool:
+        """Returns True if a requirement constraint should be rejected"""
+        if kind == RequirementKind.DEFAULT:
+            # Requirements under all: are applied only if they are satisfiable considering only
+            # package rules, so e.g. variants must exist etc. Otherwise, they are rejected.
+            try:
+                s = spack.spec.Spec(pkg_name)
+                s.constrain(constraint)
+                s.validate_or_raise()
+            except spack.error.SpackError as e:
+                tty.debug(
+                    f"[SETUP] Rejecting the default '{constraint}' requirement "
+                    f"on '{pkg_name}': {str(e)}",
+                    level=2,
+                )
+                return True
+        return False
+
+
 class RuntimePropertyRecorder:
     """An object of this class is injected in callbacks to compilers, to let them declare
     properties of the runtimes they support and of the runtimes they provide, and to add

lib/spack/spack/test/concretize_requirements.py

@@ -1035,3 +1035,101 @@ def test_requiring_package_on_multiple_virtuals(concretize_scope, mock_packages)
     assert s["blas"].name == "intel-parallel-studio"
     assert s["lapack"].name == "intel-parallel-studio"
     assert s["scalapack"].name == "intel-parallel-studio"
+
+
+@pytest.mark.parametrize(
+    "packages_yaml,spec_str,expected,not_expected",
+    [
+        (
+            """
+        packages:
+          all:
+            prefer:
+            - "%clang"
+            compiler: [gcc]
+    """,
+            "multivalue-variant",
+            ["%clang"],
+            ["%gcc"],
+        ),
+        (
+            """
+            packages:
+              all:
+                prefer:
+                - "%clang"
+        """,
+            "multivalue-variant %gcc",
+            ["%gcc"],
+            ["%clang"],
+        ),
+        # Test parsing objects instead of strings
+        (
+            """
+            packages:
+              all:
+                prefer:
+                - spec: "%clang"
+                compiler: [gcc]
+        """,
+            "multivalue-variant",
+            ["%clang"],
+            ["%gcc"],
+        ),
+    ],
+)
+def test_strong_preferences_packages_yaml(
+    packages_yaml, spec_str, expected, not_expected, concretize_scope, mock_packages
+):
+    """Tests that "preferred" specs are stronger than usual preferences, but can be overridden."""
+    update_packages_config(packages_yaml)
+    s = Spec(spec_str).concretized()
+
+    for constraint in expected:
+        assert s.satisfies(constraint), constraint
+
+    for constraint in not_expected:
+        assert not s.satisfies(constraint), constraint
+
+
+@pytest.mark.parametrize(
+    "packages_yaml,spec_str",
+    [
+        (
+            """
+        packages:
+          all:
+            conflict:
+            - "%clang"
+    """,
+            "multivalue-variant %clang",
+        ),
+        # Use an object instead of a string in configuration
+        (
+            """
+        packages:
+          all:
+            conflict:
+            - spec: "%clang"
+              message: "cannot use clang"
+    """,
+            "multivalue-variant %clang",
+        ),
+        (
+            """
+            packages:
+              multivalue-variant:
+                conflict:
+                - spec: "%clang"
+                  when: "@2"
+                  message: "cannot use clang with version 2"
+        """,
+            "multivalue-variant@=2.3 %clang",
+        ),
+    ],
+)
+def test_conflict_packages_yaml(packages_yaml, spec_str, concretize_scope, mock_packages):
+    """Tests conflicts that are specified from configuration files."""
+    update_packages_config(packages_yaml)
+    with pytest.raises(UnsatisfiableSpecError):
+        Spec(spec_str).concretized()

share/spack/gitlab/cloud_pipelines/stacks/e4s-cray-rhel/spack.yaml

@@ -10,8 +10,8 @@ spack:
 
   packages:
     all:
-      require:
+      prefer:
-      - any_of: ["%cce", "@:"]  # cce as a strong preference; not all packages support it
+      - "%cce"
       compiler: [cce]
       providers:
         blas: [cray-libsci]

share/spack/gitlab/cloud_pipelines/stacks/e4s-oneapi/spack.yaml

@@ -8,8 +8,9 @@ spack:
   packages:
     all:
       require:
-      - any_of: ["%oneapi", "@:"]  # oneapi as a strong preference; not all packages support it
       - "target=x86_64_v3"
+      prefer:
+      - "%oneapi"
       providers:
         blas: [openblas]
         mpi: [mpich]