Skip to content

Commit e4d4f6c

Browse files
alalazoalalazo
authored
Do not attach dependency types to % by default (spack#50565)
Following spack#49808 this PR removes the default dependency type attached to the % sigil, so that ^ and % have a symmetric behavior. Modifications: * Modify Spec.satisfies to account for different deptypes attached to % * Modify the solver to avoid assuming % refers to a "build only" deptype * Allow solving %[deptypes=link] mpich etc. Signed-off-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>
1 parent 3f33c71 commit e4d4f6c

11 files changed

Lines changed: 226 additions & 105 deletions

File tree

‎lib/spack/spack/directives.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ def _depends_on(
296296
dependency = deps_by_name.get(spec.name)
297297

298298
edges = spec.edges_to_dependencies()
299-
if edges and not all(x.depflag == dt.BUILD for x in edges):
299+
if edges and not all(x.direct for x in edges):
300300
raise DirectiveError(
301301
f"the '^' sigil cannot be used in 'depends_on' directives. Please reformulate "
302302
f"the directive below as multiple directives:\n\n"

‎lib/spack/spack/solver/asp.py‎

Lines changed: 39 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -298,24 +298,6 @@ def _remove(spec: spack.spec.Spec, facts: List[AspFunction]) -> List[AspFunction
298298
return _remove
299299

300300

301-
def remove_build_deps(spec: spack.spec.Spec, facts: List[AspFunction]) -> List[AspFunction]:
302-
build_deps = {x.args[2]: x.args[1] for x in facts if x.args[0] == "depends_on"}
303-
result = []
304-
for x in facts:
305-
current_name = x.args[1]
306-
if current_name in build_deps:
307-
x.name = "build_requirement"
308-
result.append(fn.attr("direct_dependency", build_deps[current_name], x))
309-
continue
310-
311-
if x.args[0] == "depends_on":
312-
continue
313-
314-
result.append(x)
315-
316-
return result
317-
318-
319301
def all_libcs() -> Set[spack.spec.Spec]:
320302
"""Return a set of all libc specs targeted by any configured compiler. If none, fall back to
321303
libc determined from the current Python process if dynamically linked."""
@@ -1030,7 +1012,7 @@ def _get_cause_tree(
10301012
"""
10311013
seen.add(cause)
10321014
parents = [c for e, c in condition_causes if e == cause and c not in seen]
1033-
local = "required because %s " % conditions[cause[0]]
1015+
local = f"required because {conditions[cause[0]]} "
10341016

10351017
return [indent + local] + [
10361018
c
@@ -1084,7 +1066,7 @@ def handle_error(self, msg, *args):
10841066
# Spec(...) with the string representation of the spec mentioned
10851067
specs_to_construct = re.findall(r"Spec\(([^)]*)\)", msg)
10861068
for spec_str in specs_to_construct:
1087-
msg = msg.replace("Spec(%s)" % spec_str, str(spack.spec.Spec(spec_str)))
1069+
msg = msg.replace(f"Spec({spec_str})", str(spack.spec.Spec(spec_str)))
10881070

10891071
for cause in set(causes):
10901072
for c in self.get_cause_tree(cause):
@@ -1183,7 +1165,7 @@ def solve(self, setup, specs, reuse=None, output=None, control=None, allow_depre
11831165
if sys.platform == "win32":
11841166
tty.debug("Ensuring basic dependencies {win-sdk, wgl} available")
11851167
spack.bootstrap.core.ensure_winsdk_external_or_raise()
1186-
control_files = ["concretize.lp", "heuristic.lp", "display.lp"]
1168+
control_files = ["concretize.lp", "heuristic.lp", "display.lp", "direct_dependency.lp"]
11871169
if not setup.concretize_everything:
11881170
control_files.append("when_possible.lp")
11891171
if using_libc_compatibility():
@@ -1457,6 +1439,7 @@ def __init__(self, *, source: Optional[str] = None):
14571439
# (which means it isn't important to keep track of the source
14581440
# in that case).
14591441
self.source = "none" if source is None else source
1442+
self.wrap_node_requirement: Optional[bool] = None
14601443

14611444

14621445
class ConditionIdContext(SourceContext):
@@ -1491,17 +1474,24 @@ def __init__(self):
14911474
# transformation applied to facts from the imposed spec. Defaults
14921475
# to removing "node" and "virtual_node" facts.
14931476
self.transform_imposed = None
1477+
# Whether to wrap direct dependency facts as node requirements,
1478+
# imposed by the parent. If None, the default is used, which is:
1479+
# - wrap head of rules
1480+
# - do not wrap body of rules
1481+
self.wrap_node_requirement: Optional[bool] = None
14941482

14951483
def requirement_context(self) -> ConditionIdContext:
14961484
ctxt = ConditionIdContext()
14971485
ctxt.source = self.source
14981486
ctxt.transform = self.transform_required
1487+
ctxt.wrap_node_requirement = self.wrap_node_requirement
14991488
return ctxt
15001489

15011490
def impose_context(self) -> ConditionIdContext:
15021491
ctxt = ConditionIdContext()
15031492
ctxt.source = self.source
15041493
ctxt.transform = self.transform_imposed
1494+
ctxt.wrap_node_requirement = self.wrap_node_requirement
15051495
return ctxt
15061496

15071497

@@ -1625,7 +1615,7 @@ def target_ranges(self, spec, single_target_fn):
16251615

16261616
def conflict_rules(self, pkg):
16271617
for when_spec, conflict_specs in pkg.conflicts.items():
1628-
when_spec_msg = "conflict constraint %s" % str(when_spec)
1618+
when_spec_msg = f"conflict constraint {str(when_spec)}"
16291619
when_spec_id = self.condition(when_spec, required_name=pkg.name, msg=when_spec_msg)
16301620

16311621
for conflict_spec, conflict_msg in conflict_specs:
@@ -2221,8 +2211,9 @@ def emit_facts_from_requirement_rules(self, rules: List[RequirementRule]):
22212211
context.source = ConstraintOrigin.append_type_suffix(
22222212
pkg_name, ConstraintOrigin.REQUIRE
22232213
)
2214+
context.wrap_node_requirement = True
22242215
if not virtual:
2225-
context.transform_required = remove_build_deps
2216+
context.transform_required = remove_facts("depends_on")
22262217
context.transform_imposed = remove_facts(
22272218
"node", "virtual_node", "depends_on"
22282219
)
@@ -2669,22 +2660,29 @@ def _spec_clauses(
26692660
###
26702661
# Dependency expressed with "^"
26712662
###
2672-
if not dspec.depflag == dt.BUILD:
2663+
if not dspec.direct:
26732664
edge_clauses.extend(dependency_clauses)
26742665
continue
26752666

26762667
###
26772668
# Direct dependencies expressed with "%"
26782669
###
2679-
edge_clauses.append(fn.attr("depends_on", spec.name, dep.name, "build"))
2680-
# Body of a rule
2681-
if body is True:
2670+
for dependency_type in dt.flag_to_tuple(dspec.depflag):
2671+
edge_clauses.append(
2672+
fn.attr("depends_on", spec.name, dep.name, dependency_type)
2673+
)
2674+
2675+
# By default, wrap head of rules, unless the context says otherwise
2676+
wrap_node_requirement = body is False
2677+
if context and context.wrap_node_requirement is not None:
2678+
wrap_node_requirement = context.wrap_node_requirement
2679+
2680+
if not wrap_node_requirement:
26822681
edge_clauses.extend(dependency_clauses)
26832682
continue
26842683

2685-
# Head of a rule
26862684
for clause in dependency_clauses:
2687-
clause.name = "build_requirement"
2685+
clause.name = "node_requirement"
26882686
edge_clauses.append(fn.attr("direct_dependency", spec.name, clause))
26892687

26902688
clauses.extend(edge_clauses)
@@ -2944,7 +2942,7 @@ def versions_for(v):
29442942
elif isinstance(v, vn.VersionList):
29452943
return sum((versions_for(e) for e in v), [])
29462944
else:
2947-
raise TypeError("expected version type, found: %s" % type(v))
2945+
raise TypeError(f"expected version type, found: {type(v)}")
29482946

29492947
# define a set of synthetic possible versions for virtuals, so
29502948
# that `version_satisfies(Package, Constraint, Version)` has the
@@ -3217,7 +3215,7 @@ def setup(
32173215

32183216
self.gen.h1("Package Constraints")
32193217
for pkg in sorted(self.pkgs):
3220-
self.gen.h2("Package rules: %s" % pkg)
3218+
self.gen.h2(f"Package rules: {pkg}")
32213219
self.pkg_rules(pkg, tests=self.tests)
32223220
self.preferred_variants(pkg)
32233221

@@ -3228,7 +3226,7 @@ def setup(
32283226
self.gen.h1("Develop specs")
32293227
# Inject dev_path from environment
32303228
for ds in dev_specs:
3231-
self.condition(spack.spec.Spec(ds.name), ds, msg="%s is a develop spec" % ds.name)
3229+
self.condition(spack.spec.Spec(ds.name), ds, msg=f"{ds.name} is a develop spec")
32323230
self.trigger_rules()
32333231
self.effect_rules()
32343232

@@ -3266,7 +3264,7 @@ def visit(node):
32663264
arg = ast_sym(ast_sym(term.atom).arguments[0])
32673265
symbol = AspFunction(name)(arg.string)
32683266
self.assumptions.append((parse_term(str(symbol)), True))
3269-
self.gen.asp_problem.append(f"{{ {symbol} }}.\n")
3267+
self.gen.asp_problem.append(f"{symbol}.\n")
32703268

32713269
path = os.path.join(parent_dir, "concretize.lp")
32723270
parse_files([path], visit)
@@ -3291,9 +3289,10 @@ def define_runtime_constraints(self) -> List[spack.spec.Spec]:
32913289
# FIXME (compiler as nodes): think of using isinstance(compiler_cls, WrappedCompiler)
32923290
# Add a dependency on the compiler wrapper
32933291
for language in ("c", "cxx", "fortran"):
3292+
compiler_str = f"{compiler.name}@{compiler.versions}"
32943293
recorder("*").depends_on(
32953294
"compiler-wrapper",
3296-
when=f"%[virtuals={language}] {compiler.name}@{compiler.versions}",
3295+
when=f"%[deptypes=build virtuals={language}] {compiler_str}",
32973296
type="build",
32983297
description=f"Add the compiler wrapper when using {compiler} for {language}",
32993298
)
@@ -3313,13 +3312,13 @@ def define_runtime_constraints(self) -> List[spack.spec.Spec]:
33133312
if current_libc:
33143313
recorder("*").depends_on(
33153314
"libc",
3316-
when=f"%{compiler.name}@{compiler.versions}",
3315+
when=f"%[deptypes=build] {compiler_str}",
33173316
type="link",
33183317
description=f"Add libc when using {compiler}",
33193318
)
33203319
recorder("*").depends_on(
33213320
f"{current_libc.name}@={current_libc.version}",
3322-
when=f"%{compiler.name}@{compiler.versions}",
3321+
when=f"%[deptypes=build] {compiler_str}",
33233322
type="link",
33243323
description=f"Libc is {current_libc} when using {compiler}",
33253324
)
@@ -3329,7 +3328,7 @@ def define_runtime_constraints(self) -> List[spack.spec.Spec]:
33293328

33303329
def literal_specs(self, specs):
33313330
for spec in sorted(specs):
3332-
self.gen.h2("Spec: %s" % str(spec))
3331+
self.gen.h2(f"Spec: {str(spec)}")
33333332
condition_id = next(self._id_counter)
33343333
trigger_id = next(self._id_counter)
33353334

@@ -3755,7 +3754,7 @@ def default_flags(self, spec: "spack.spec.Spec"):
37553754
self.reset()
37563755
return
37573756

3758-
when_spec = spack.spec.Spec(f"^[deptypes=build] {spec}")
3757+
when_spec = spack.spec.Spec(f"%[deptypes=build] {spec}")
37593758
body_str, node_variable = self.rule_body_from(when_spec)
37603759

37613760
node_placeholder = "XXX"
@@ -4038,7 +4037,7 @@ def _order_index(flag_group):
40384037
extend_flag_list(ordered_flags, cmd_flags)
40394038

40404039
compiler_flags = spec.compiler_flags.get(flag_type, [])
4041-
msg = "%s does not equal %s" % (set(compiler_flags), set(ordered_flags))
4040+
msg = f"{set(compiler_flags)} does not equal {set(ordered_flags)}"
40424041
assert set(compiler_flags) == set(ordered_flags), msg
40434042

40444043
spec.compiler_flags.update({flag_type: ordered_flags})
@@ -4097,7 +4096,7 @@ def build_specs(self, function_tuples):
40974096

40984097
# print out unknown actions so we can display them for debugging
40994098
if not action:
4100-
msg = 'UNKNOWN SYMBOL: attr("%s", %s)' % (name, ", ".join(str(a) for a in args))
4099+
msg = f'UNKNOWN SYMBOL: attr("{name}", {", ".join(str(a) for a in args)})'
41014100
tty.debug(msg)
41024101
continue
41034102

0 commit comments

Comments
 (0)