Error when an anonymous spec is required for a virtual package (#49385)
When requiring a constraint on a virtual package, it makes little sense to use anonymous specs, and our documentation shows no example of requirements on virtual packages starting with `^`. Right now, due to how `^` is implemented in the solver, writing: ```yaml mpi: require: "^openmpi" ``` is equivalent to the more correct form: ```yaml mpi: require: "openmpi" ``` but the situation will change when `%` will shift its meaning to be a direct dependency. To avoid later errors that are both unclear, and quite slow to get to the user, this commit makes anonymous specs under virtual requirements an error, and shows a clear error message pointing to the file and line where the spec needs to be changed. Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
This commit is contained in:
parent
4cb4634c74
commit
d352b71df0
@ -165,7 +165,8 @@ def _rules_from_requirements(
|
|||||||
|
|
||||||
# validate specs from YAML first, and fail with line numbers if parsing fails.
|
# validate specs from YAML first, and fail with line numbers if parsing fails.
|
||||||
constraints = [
|
constraints = [
|
||||||
parse_spec_from_yaml_string(constraint) for constraint in constraints
|
parse_spec_from_yaml_string(constraint, named=kind == RequirementKind.VIRTUAL)
|
||||||
|
for constraint in constraints
|
||||||
]
|
]
|
||||||
when_str = requirement.get("when")
|
when_str = requirement.get("when")
|
||||||
when = parse_spec_from_yaml_string(when_str) if when_str else spack.spec.Spec()
|
when = parse_spec_from_yaml_string(when_str) if when_str else spack.spec.Spec()
|
||||||
@ -203,7 +204,7 @@ def reject_requirement_constraint(
|
|||||||
s.validate_or_raise()
|
s.validate_or_raise()
|
||||||
except spack.error.SpackError as e:
|
except spack.error.SpackError as e:
|
||||||
tty.debug(
|
tty.debug(
|
||||||
f"[SETUP] Rejecting the default '{constraint}' requirement "
|
f"[{__name__}] Rejecting the default '{constraint}' requirement "
|
||||||
f"on '{pkg_name}': {str(e)}",
|
f"on '{pkg_name}': {str(e)}",
|
||||||
level=2,
|
level=2,
|
||||||
)
|
)
|
||||||
@ -211,21 +212,37 @@ def reject_requirement_constraint(
|
|||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
def parse_spec_from_yaml_string(string: str) -> spack.spec.Spec:
|
def parse_spec_from_yaml_string(string: str, *, named: bool = False) -> spack.spec.Spec:
|
||||||
"""Parse a spec from YAML and add file/line info to errors, if it's available.
|
"""Parse a spec from YAML and add file/line info to errors, if it's available.
|
||||||
|
|
||||||
Parse a ``Spec`` from the supplied string, but also intercept any syntax errors and
|
Parse a ``Spec`` from the supplied string, but also intercept any syntax errors and
|
||||||
add file/line information for debugging using file/line annotations from the string.
|
add file/line information for debugging using file/line annotations from the string.
|
||||||
|
|
||||||
Arguments:
|
Args:
|
||||||
string: a string representing a ``Spec`` from config YAML.
|
string: a string representing a ``Spec`` from config YAML.
|
||||||
|
named: if True, the spec must have a name
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
return spack.spec.Spec(string)
|
result = spack.spec.Spec(string)
|
||||||
except spack.error.SpecSyntaxError as e:
|
except spack.error.SpecSyntaxError as e:
|
||||||
mark = get_mark_from_yaml_data(string)
|
mark = get_mark_from_yaml_data(string)
|
||||||
if mark:
|
if mark:
|
||||||
msg = f"{mark.name}:{mark.line + 1}: {str(e)}"
|
msg = f"{mark.name}:{mark.line + 1}: {str(e)}"
|
||||||
raise spack.error.SpecSyntaxError(msg) from e
|
raise spack.error.SpecSyntaxError(msg) from e
|
||||||
raise e
|
raise e
|
||||||
|
|
||||||
|
if named is True and not result.name:
|
||||||
|
msg = f"expected a named spec, but got '{string}' instead"
|
||||||
|
mark = get_mark_from_yaml_data(string)
|
||||||
|
|
||||||
|
# Add a hint in case it's dependencies
|
||||||
|
deps = result.dependencies()
|
||||||
|
if len(deps) == 1:
|
||||||
|
msg = f"{msg}. Did you mean '{deps[0]}'?"
|
||||||
|
|
||||||
|
if mark:
|
||||||
|
msg = f"{mark.name}:{mark.line + 1}: {msg}"
|
||||||
|
|
||||||
|
raise spack.error.SpackError(msg)
|
||||||
|
|
||||||
|
return result
|
||||||
|
@ -1125,3 +1125,46 @@ def test_strong_preferences_higher_priority_than_reuse(concretize_scope, mock_pa
|
|||||||
)
|
)
|
||||||
ascent = result.specs[0]
|
ascent = result.specs[0]
|
||||||
assert ascent["adios2"].dag_hash() == reused_spec.dag_hash(), ascent
|
assert ascent["adios2"].dag_hash() == reused_spec.dag_hash(), ascent
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"packages_yaml,err_match",
|
||||||
|
[
|
||||||
|
(
|
||||||
|
"""
|
||||||
|
packages:
|
||||||
|
mpi:
|
||||||
|
require:
|
||||||
|
- "+bzip2"
|
||||||
|
""",
|
||||||
|
"expected a named spec",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"""
|
||||||
|
packages:
|
||||||
|
mpi:
|
||||||
|
require:
|
||||||
|
- one_of: ["+bzip2", openmpi]
|
||||||
|
""",
|
||||||
|
"expected a named spec",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"""
|
||||||
|
packages:
|
||||||
|
mpi:
|
||||||
|
require:
|
||||||
|
- "^mpich"
|
||||||
|
""",
|
||||||
|
"Did you mean",
|
||||||
|
),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_anonymous_spec_cannot_be_used_in_virtual_requirements(
|
||||||
|
packages_yaml, err_match, concretize_scope, mock_packages
|
||||||
|
):
|
||||||
|
"""Tests that using anonymous specs in requirements for virtual packages raises an
|
||||||
|
appropriate error message.
|
||||||
|
"""
|
||||||
|
update_packages_config(packages_yaml)
|
||||||
|
with pytest.raises(spack.error.SpackError, match=err_match):
|
||||||
|
spack.concretize.concretize_one("mpileaks")
|
||||||
|
@ -25,7 +25,7 @@ spack:
|
|||||||
# Minimize LLVM
|
# Minimize LLVM
|
||||||
variants: ~lldb~lld~libomptarget~polly~gold libunwind=none compiler-rt=none
|
variants: ~lldb~lld~libomptarget~polly~gold libunwind=none compiler-rt=none
|
||||||
libllvm:
|
libllvm:
|
||||||
require: ["^llvm"]
|
require: ["llvm"]
|
||||||
visit:
|
visit:
|
||||||
require: ["@3.4.1"]
|
require: ["@3.4.1"]
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user