Avoid best-effort expansion of stacks#40792
Conversation
spackbot-app
bot
added
commands
core
alalazo
force-pushed
the
bugfix/matrix-expansion-should-be-consistent
branch
from
fc2df49 to
625b64d
Compare
|
I don't think we should remove best-effort expansion, because people are actually using it. If I look at the original issue, it seems like to me that the env should emit a warning (or maybe an error? I don't really care) if a variant couldn't apply to any spec in the matrix, but I wouldn't remove best-effort expansion. That said, I agree that the current way we handle this is somewhat inconsistent. In particular I think we could adopt the change in this PR if we still had a way to do best-effort. Something like: spack:
specs:
- zstd
- prefer: +shrdWhere |
For what is worth, best-effort expansion is only used with |
There are a couple of things I want to note. The behavior is not only inconsistent across different unification modes, it's just wrong, and the issue is not only about variants but about everything: spack:
specs:
- matrix:
- [zlib]
- ["^trilinos", "~shrd"]
concretizer:
unify: false
view: falseHere we concretize a The corresponding lockfile is: spack.lock
{
"_meta": {
"file-type": "spack-lockfile",
"lockfile-version": 5,
"specfile-version": 4
},
"spack": {
"version": "0.22.0.dev0",
"type": "git",
"commit": "84999b699682f8505908e7c5f7e767f9d3be6c79"
},
"roots": [
{
"hash": "qnzaxw3i5na6hu4fr23buxys7ht74w2v",
"spec": "zlib ^trilinos"
},
{
"hash": "qnzaxw3i5na6hu4fr23buxys7ht74w2v",
"spec": "zlib~shrd"
}
],
"concrete_specs": {
"qnzaxw3i5na6hu4fr23buxys7ht74w2v": {
"name": "zlib",
"version": "1.3",
"arch": {
"platform": "linux",
"platform_os": "ubuntu20.04",
"target": {
"name": "icelake",
"vendor": "GenuineIntel",
"features": [
"adx",
"aes",
"avx",
"avx2",
"avx512_bitalg",
"avx512_vbmi2",
"avx512_vnni",
"avx512_vpopcntdq",
"avx512bw",
"avx512cd",
"avx512dq",
"avx512f",
"avx512ifma",
"avx512vbmi",
"avx512vl",
"bmi1",
"bmi2",
"clflushopt",
"clwb",
"f16c",
"fma",
"gfni",
"mmx",
"movbe",
"pclmulqdq",
"popcnt",
"rdpid",
"rdrand",
"rdseed",
"sha_ni",
"sse",
"sse2",
"sse4_1",
"sse4_2",
"ssse3",
"vaes",
"vpclmulqdq",
"xsavec",
"xsaveopt"
],
"generation": 0,
"parents": [
"cascadelake",
"cannonlake"
]
}
},
"compiler": {
"name": "gcc",
"version": "9.4.0"
},
"namespace": "builtin",
"parameters": {
"build_system": "makefile",
"optimize": true,
"pic": true,
"shared": true,
"cflags": [],
"cppflags": [],
"cxxflags": [],
"fflags": [],
"ldflags": [],
"ldlibs": []
},
"package_hash": "u7vqvwmacj5j7zngg2evhytlxmzhad35mxlxna6tmr4bjyeisgsa====",
"dependencies": [
{
"name": "gmake",
"hash": "li5apiwbiexfsusfyv73rualbl6q35wt",
"parameters": {
"deptypes": [
"build"
],
"virtuals": []
}
}
],
"hash": "qnzaxw3i5na6hu4fr23buxys7ht74w2v"
},
"li5apiwbiexfsusfyv73rualbl6q35wt": {
"name": "gmake",
"version": "4.4.1",
"arch": {
"platform": "linux",
"platform_os": "ubuntu20.04",
"target": {
"name": "icelake",
"vendor": "GenuineIntel",
"features": [
"adx",
"aes",
"avx",
"avx2",
"avx512_bitalg",
"avx512_vbmi2",
"avx512_vnni",
"avx512_vpopcntdq",
"avx512bw",
"avx512cd",
"avx512dq",
"avx512f",
"avx512ifma",
"avx512vbmi",
"avx512vl",
"bmi1",
"bmi2",
"clflushopt",
"clwb",
"f16c",
"fma",
"gfni",
"mmx",
"movbe",
"pclmulqdq",
"popcnt",
"rdpid",
"rdrand",
"rdseed",
"sha_ni",
"sse",
"sse2",
"sse4_1",
"sse4_2",
"ssse3",
"vaes",
"vpclmulqdq",
"xsavec",
"xsaveopt"
],
"generation": 0,
"parents": [
"cascadelake",
"cannonlake"
]
}
},
"compiler": {
"name": "gcc",
"version": "9.4.0"
},
"namespace": "builtin",
"parameters": {
"build_system": "generic",
"guile": false,
"cflags": [],
"cppflags": [],
"cxxflags": [],
"fflags": [],
"ldflags": [],
"ldlibs": []
},
"package_hash": "ghstvqlc3r7kgiikwx24xhcxdxcqdk5viinrzgm2mssqigfonika====",
"hash": "li5apiwbiexfsusfyv73rualbl6q35wt"
}
}
}
Neither of the roots satisfies the concrete spec they point to, which is in my opinion a severe breakage of the invariants that are supposed to hold for concretization. |
alalazo
force-pushed
the
bugfix/matrix-expansion-should-be-consistent
branch
from
625b64d to
98d787e
Compare
|
@tgamblin We have a |
|
Ping @tgamblin |
alalazo
force-pushed
the
bugfix/matrix-expansion-should-be-consistent
branch
from
98d787e to
23da877
Compare
alalazo
force-pushed
the
bugfix/matrix-expansion-should-be-consistent
branch
from
23da877 to
60b45f7
Compare
fixes spack#40791 Currently stacks behave differently if used in unify:false environments, which leads to inconsistencies during concretization. For instance, we might have two abstract user specs that do not intersect with each other map to the same concrete spec in the environment. This is clearly wrong. This PR removes the best effort expansion, so that user specs are always applied strictly.
alalazo
force-pushed
the
bugfix/matrix-expansion-should-be-consistent
branch
from
60b45f7 to
b9b28dd
Compare
|
Since #45215 it doesn't make much sense to keep best-effort expansion. It was never supported by the new concretizer. |
3 participants
fixes #40791
Currently, stacks behave differently if used in unify:false environments, which leads to inconsistencies during concretization.
For instance, we might have two abstract user specs that do not intersect with each other map to the same concrete spec in the environment. This is clearly wrong.
This PR removes the best effort expansion, so that user specs are always applied strictly.