Skip to content

Fix wrong behavior with conditional provider and anonymous satisfies#35599

Closed
alalazo wants to merge 2 commits intospack:developfrom
alalazo:bugfix/providers_issue
Closed

Fix wrong behavior with conditional provider and anonymous satisfies#35599
alalazo wants to merge 2 commits intospack:developfrom
alalazo:bugfix/providers_issue

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Feb 21, 2023

fixes #35597

@spackbot-app spackbot-app bot added core PR affects Spack core functionality dependencies new-version tests General test capability(ies) labels Feb 21, 2023
@alalazo alalazo added this to the v0.19.2 milestone Feb 21, 2023
@alalazo alalazo requested review from haampie and tgamblin February 21, 2023 15:17
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Feb 21, 2023

@dev-zero This should fix the issue you reported with intel-oneapi-mkl

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Feb 21, 2023

This also seems wrong:

spack/lib/spack/spack/spec.py

Lines 3548 to 3551 in 1636c89

deps_strict = strict
if self._concrete and not other.name:
# We're dealing with existing specs
deps_strict = True

trying to check if we can remove it easily, and simplify satisfy_dependencies.

@alalazo alalazo mentioned this pull request Feb 21, 2023
Merged
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Feb 21, 2023

Another aspect that doesn't seem to match the strict=True vs. strict=False semantics is:

spack/lib/spack/spack/spec.py

Lines 3837 to 3852 in cddef35

def __contains__(self, spec):
"""True if this spec or some dependency satisfies the spec.
Note: If ``spec`` is anonymous, we ONLY check whether the root
satisfies it, NOT dependencies. This is because most anonymous
specs (e.g., ``@1.2``) don't make sense when applied across an
entire DAG -- we limit them to the root.
"""
spec = self._autospec(spec)
# if anonymous or same name, we only have to look at the root
if not spec.name or spec.name == self.name:
return self.satisfies(spec)
else:
return any(s.satisfies(spec) for s in self.traverse(root=False))

Here I would expect satisfies(..., strict=True) to be used, since without it this is possible:

Spack version 0.20.0.dev0
Python 3.8.10, Linux x86_64
>>> from spack.spec import Spec
>>> s = Spec("cp2k")
>>> t = Spec("[email protected]")
>>> s in t
True
>>> t in s
True

while I would expect:

Spack version 0.20.0.dev0
Python 3.8.10, Linux x86_64
>>> from spack.spec import Spec
>>> s = Spec("cp2k")
>>> t = Spec("[email protected]")
>>> s in t
True
>>> t in s
False

@alalazo alalazo mentioned this pull request Feb 24, 2023
Merged
7 tasks
@alalazo alalazo deleted the bugfix/providers_issue branch March 8, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgamblin tgamblin Awaiting requested review from tgamblin

@haampie haampie Awaiting requested review from haampie

Assignees

No one assigned

Labels

core PR affects Spack core functionality dependencies new-version tests General test capability(ies)

Projects

None yet

Milestone

v0.19.2

Development

Successfully merging this pull request may close these issues.

Spec.__contains__ produces wrong results for intel-oneapi-mkl

1 participant

@alalazo