zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: make _make_when_spec() private to directives.py

Browse Source 
`make_when_spec()` was being used in the solver, but it has semantics that are specific
to parsing when specs from `package.py`. In particular, it returns `None` when the
`when` spec is `False`, and directives are responsible for ignoring that case and not
adding requirements, deps, etc. when there's an actual `False` passed in from
`package.py`.

In `asp.py`, we know that there won't ever be a raw boolean when spec or constraint, so
we know we can parse them without any of the special boolean handling. However, we
should report where in the file the error happened on error, so this adds some parsing
logic to extract the `mark` from YAML and alert the user where the bad parse is.

- [x] refactor `config.py` so that basic `spack_yaml` mark info is in its own method
- [x] refactor `asp.py` so that it uses the smarter YAML parsing routine
- [x] refactor `asp.py` so that YAML input validation for requirements is done up front
This commit is contained in:
Todd Gamblin 2023-12-30 23:05:38 -08:00
parent d2a9e3f871
commit 7994caaeda
4 changed files with 81 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
lib/spack/spack/config.py
View File

@ -1580,6 +1580,49 @@ def _fetch_file(url):
raise ConfigFileError(f"Cannot retrieve configuration (yaml) from {url}")
def get_mark_from_yaml_data(obj):
"""Try to get ``spack.util.spack_yaml`` mark from YAML data.
We try the object, and if that fails we try its first member (if it's a container).
Returns:
mark if one is found, otherwise None.
"""
# mark of object itelf
mark = getattr(obj, "_start_mark", None)
if mark:
return mark
# mark of first member if it is a container
if isinstance(obj, (list, dict)):
first_member = next(iter(obj), None)
if first_member:
mark = getattr(first_member, "_start_mark", None)
return mark
def parse_spec_from_yaml_string(string: str) -> "spack.spec.Spec":
"""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
add file/line information for debugging using file/line annotations from the string.
Arguments:
string: a string representing a ``Spec`` from config YAML.
"""
try:
spec = spack.spec.Spec(string)
return spec
except spack.parser.SpecSyntaxError as e:
mark = spack.config.get_mark_from_yaml_data(string)
if mark:
msg = f"{mark.name}:{mark.line + 1}: {str(e)}"
raise spack.parser.SpecSyntaxError(msg) from e
raise e
class ConfigError(SpackError):
"""Superclass for all Spack config related errors."""
@ -1625,23 +1668,9 @@ def __init__(
def _get_mark(self, validation_error, data):
"""Get the file/line mark fo a validation error from a Spack YAML file."""
def _get_mark_or_first_member_mark(obj):
# mark of object itelf
mark = getattr(obj, "_start_mark", None)
if mark:
return mark
# mark of first member if it is a container
if isinstance(obj, (list, dict)):
first_member = next(iter(obj), None)
if first_member:
mark = getattr(first_member, "_start_mark", None)
if mark:
return mark
# Try various places, starting with instance and parent
for obj in (validation_error.instance, validation_error.parent):
mark = _get_mark_or_first_member_mark(obj)
mark = get_mark_from_yaml_data(obj)
if mark:
return mark

24
lib/spack/spack/directives.py
View File

@ -91,7 +91,7 @@ class OpenMpi(Package):
PatchesType = Optional[Union[Patcher, str, List[Union[Patcher, str]]]]
def make_when_spec(value: WhenType) -> Optional["spack.spec.Spec"]:
def _make_when_spec(value: WhenType) -> Optional["spack.spec.Spec"]:
"""Create a ``Spec`` that indicates when a directive should be applied.
Directives with ``when`` specs, e.g.:
@ -471,7 +471,7 @@ def _depends_on(
type: DepType = dt.DEFAULT_TYPES,
patches: PatchesType = None,
):
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return
@ -547,7 +547,7 @@ def conflicts(conflict_spec: SpecType, when: WhenType = None, msg: Optional[str]
def _execute_conflicts(pkg: "spack.package_base.PackageBase"):
# If when is not specified the conflict always holds
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return
@ -599,7 +599,7 @@ def extends(spec, when=None, type=("build", "run"), patches=None):
"""
def _execute_extends(pkg):
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return
@ -627,7 +627,7 @@ def provides(*specs, when: Optional[str] = None):
def _execute_provides(pkg):
import spack.parser # Avoid circular dependency
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return
@ -685,7 +685,7 @@ def _execute_patch(pkg_or_dep: Union["spack.package_base.PackageBase", Dependenc
"Patches are not allowed in {0}: package has no code.".format(pkg.name)
)
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return
@ -813,7 +813,7 @@ def _raise_default_not_set(pkg):
description = str(description).strip()
def _execute_variant(pkg):
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
when_specs = [when_spec]
if not re.match(spack.spec.IDENTIFIER_RE, name):
@ -855,7 +855,7 @@ def resource(**kwargs):
def _execute_resource(pkg):
when = kwargs.get("when")
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return
@ -921,17 +921,17 @@ def _execute_maintainer(pkg):
def _execute_license(pkg, license_identifier: str, when):
# If when is not specified the license always holds
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return
for other_when_spec in pkg.licenses:
if when_spec.intersects(other_when_spec):
when_message = ""
if when_spec != make_when_spec(None):
if when_spec != _make_when_spec(None):
when_message = f"when {when_spec}"
other_when_message = ""
if other_when_spec != make_when_spec(None):
if other_when_spec != _make_when_spec(None):
other_when_message = f"when {other_when_spec}"
err_msg = (
f"{pkg.name} is specified as being licensed as {license_identifier} "
@ -992,7 +992,7 @@ def _execute_requires(pkg: "spack.package_base.PackageBase"):
)
raise DirectiveError(err_msg)
when_spec = make_when_spec(when)
when_spec = _make_when_spec(when)
if not when_spec:
return

40
lib/spack/spack/solver/asp.py
View File

@ -20,6 +20,7 @@
import spack.config as sc
import spack.deptypes as dt
import spack.parser
import spack.paths as sp
import spack.util.path as sup
@ -862,9 +863,13 @@ def on_model(model):
#: Data class to collect information on a requirement
RequirementRule = collections.namedtuple(
"RequirementRule", ["pkg_name", "policy", "requirements", "condition", "kind", "message"]
)
class RequirementRule(NamedTuple):
pkg_name: str
policy: str
requirements: List["spack.spec.Spec"]
condition: "spack.spec.Spec"
kind: RequirementKind
message: str
class PyclingoDriver:
@ -1352,10 +1357,18 @@ def _rules_from_requirements(
constraints = [constraints]
policy = "one_of"
# validate specs from YAML first, and fail with line numbers if parsing fails.
constraints = [
x
for x in constraints
if not self.reject_requirement_constraint(pkg_name, constraint=x, kind=kind)
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
@ -1367,13 +1380,13 @@ def _rules_from_requirements(
requirements=constraints,
kind=kind,
message=requirement.get("message"),
condition=requirement.get("when"),
condition=when,
)
)
return rules
def reject_requirement_constraint(
self, pkg_name: str, *, constraint: str, kind: RequirementKind
self, pkg_name: str, *, constraint: "spack.spec.Spec", kind: RequirementKind
) -> bool:
"""Returns True if a requirement constraint should be rejected"""
if kind == RequirementKind.DEFAULT:
@ -1740,20 +1753,13 @@ def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]):
virtual = rule.kind == RequirementKind.VIRTUAL
pkg_name, policy, requirement_grp = rule.pkg_name, rule.policy, rule.requirements
requirement_weight = 0
# TODO: don't call make_when_spec here; do it in directives.
main_requirement_condition = spack.directives.make_when_spec(rule.condition)
if main_requirement_condition is False:
continue
# Write explicitly if a requirement is conditional or not
if main_requirement_condition != spack.spec.Spec():
if rule.condition != spack.spec.Spec():
msg = f"condition to activate requirement {requirement_grp_id}"
try:
main_condition_id = self.condition(
main_requirement_condition, name=pkg_name, msg=msg # type: ignore
)
main_condition_id = self.condition(rule.condition, name=pkg_name, msg=msg)
except Exception as e:
if rule.kind != RequirementKind.DEFAULT:
raise RuntimeError(

4
lib/spack/spack/test/directives.py
View File

@ -101,7 +101,7 @@ def test_license_directive(config, mock_packages, package_name, expected_license
def test_duplicate_exact_range_license():
package = namedtuple("package", ["licenses", "name"])
package.licenses = {spack.directives.make_when_spec("+foo"): "Apache-2.0"}
package.licenses = {spack.spec.Spec("+foo"): "Apache-2.0"}
package.name = "test_package"
msg = (
@ -115,7 +115,7 @@ def test_duplicate_exact_range_license():
def test_overlapping_duplicate_licenses():
package = namedtuple("package", ["licenses", "name"])
package.licenses = {spack.directives.make_when_spec("+foo"): "Apache-2.0"}
package.licenses = {spack.spec.Spec("+foo"): "Apache-2.0"}
package.name = "test_package"
msg = (