Skip to content

Commit

Permalink
Remove spack.repo.PATH.is_virtual call from SpecBuildInterface.(spack…
Browse files Browse the repository at this point in the history
…#48984)

This PR is effectively a breaking change extracted from spack#45189, which removes 
support for spec["mpi"] if spec itself is openmpi / mpich that could provide mpi; 
from the Spec instance we don't have any parent it provides it to, 
hence it's a KeyError.
  • Loading branch information
haampie authored Feb 12, 2025
1 parent 9a7a3d2 commit 03e9723
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 17 deletions.
2 changes: 1 addition & 1 deletion lib/spack/spack/modules/lmod.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def provides(self):
# All the other tokens in the hierarchy must be virtual dependencies
for x in self.hierarchy_tokens:
if self.spec.package.provides(x):
provides[x] = self.spec[x]
provides[x] = self.spec
return provides

@property
Expand Down
27 changes: 14 additions & 13 deletions lib/spack/spack/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -1337,14 +1337,20 @@ class SpecBuildInterface(lang.ObjectWrapper):
"command", default_handler=_command_default_handler, _indirect=True
)

def __init__(self, spec: "Spec", name: str, query_parameters: List[str], _parent: "Spec"):
def __init__(
self,
spec: "Spec",
name: str,
query_parameters: List[str],
_parent: "Spec",
is_virtual: bool,
):
super().__init__(spec)
# Adding new attributes goes after super() call since the ObjectWrapper
# resets __dict__ to behave like the passed object
original_spec = getattr(spec, "wrapped_obj", spec)
self.wrapped_obj = original_spec
self.token = original_spec, name, query_parameters, _parent
is_virtual = spack.repo.PATH.is_virtual(name)
self.token = original_spec, name, query_parameters, _parent, is_virtual
self.last_query = QueryState(
name=name, extra_parameters=query_parameters, isvirtual=is_virtual
)
Expand Down Expand Up @@ -3772,21 +3778,16 @@ def __getitem__(self, name: str):
# Consider runtime dependencies and direct build/test deps before transitive dependencies,
# and prefer matches closest to the root.
try:
child: Spec = next(
e.spec
for e in itertools.chain(
(e for e in order() if e.spec.name == name or name in e.virtuals),
# for historical reasons
(e for e in order() if e.spec.concrete and e.spec.package.provides(name)),
)
)
edge = next((e for e in order() if e.spec.name == name or name in e.virtuals))
except StopIteration:
raise KeyError(f"No spec with name {name} in {self}")

if self._concrete:
return SpecBuildInterface(child, name, query_parameters, _parent=self)
return SpecBuildInterface(
edge.spec, name, query_parameters, _parent=self, is_virtual=name in edge.virtuals
)

return child
return edge.spec

def __contains__(self, spec):
"""True if this spec or some dependency satisfies the spec.
Expand Down
2 changes: 1 addition & 1 deletion lib/spack/spack/test/concretization/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,7 @@ def test_transitive_conditional_virtual_dependency(self, mutable_config):
def test_conditional_provides_or_depends_on(self):
# Check that we can concretize correctly a spec that can either
# provide a virtual or depend on it based on the value of a variant
s = spack.concretize.concretize_one("conditional-provider +disable-v1")
s = spack.concretize.concretize_one("v1-consumer ^conditional-provider +disable-v1")
assert "v1-provider" in s
assert s["v1"].name == "v1-provider"
assert s["v2"].name == "conditional-provider"
Expand Down
4 changes: 2 additions & 2 deletions lib/spack/spack/test/concretization/preferences.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ def test_develop(self):
def test_external_mpi(self):
# make sure this doesn't give us an external first.
spec = spack.concretize.concretize_one("mpi")
assert not spec["mpi"].external
assert not spec.external and spec.package.provides("mpi")

# load config
conf = syaml.load_config(
Expand Down Expand Up @@ -293,7 +293,7 @@ def mock_module(cmd, module):
monkeypatch.setattr(spack.util.module_cmd, "module", mock_module)

spec = spack.concretize.concretize_one("mpi")
assert not spec["mpi"].external
assert not spec.external and spec.package.provides("mpi")

# load config
conf = syaml.load_config(
Expand Down

0 comments on commit 03e9723

Please sign in to comment.