Ensure command_line scope is always last#48255
Merged
psakievich merged 19 commits intodevelopfrom Dec 25, 2024
Merged
Conversation
Move command_line scope to highest precedent after adding environment config scopes
psakievich
added
bug
Contributor
Author
|
Hmmm this fixes |
Member
|
I think what you see is specs from the env itself due to |
Contributor
Author
|
That is not what is going on. I can delete the entire install tree and it will still concretize by pulling in dependencies from the buildcache. Also with $ spack concretize --fresh --force
==> Concretized 1 spec:
- z5ocm6q pmr@master%[email protected]~ipo~rtest~utest build_system=cmake build_type=Release cxxstd=17 dev_path=/scratch/psakiev/sierra/code_repo generator=ninja arch=linux-rhel8-x86_64
[e] 6avuokx ^[email protected]%[email protected]~doc+ncurses+ownlibs build_system=generic build_type=Release patches=dbc3892 arch=linux-rhel8-x86_64
- 4gedwxw ^element@master%[email protected]~ipo~rtest~utest build_system=cmake build_type=Release cxxstd=17 dev_path=/scratch/psakiev/sierra/code_repo generator=ninja arch=linux-rhel8-x86_64
- bn5n4g5 ^framework@master%[email protected]~ipo~rtest~utest build_system=cmake build_type=Release cxxstd=17 dev_path=/scratch/psakiev/sierra/code_repo generator=ninja arch=linux-rhel8-x86_64
- cfwdkxr ^meshtk-field@master%[email protected]~ipo~rtest~utest build_system=cmake build_type=Release cxxstd=17 dev_path=/scratch/psakiev/sierra/code_repo generator=ninja arch=linux-rhel8-x86_64
- lhvedo6 ^mpih@master%[email protected]~ipo~rtest~utest build_system=cmake build_type=Release cxxstd=17 dev_path=/scratch/psakiev/sierra/code_repo generator=ninja arch=linux-rhel8-x86_64
[+] kf2facn ^[email protected]%[email protected] build_system=generic arch=linux-rhel8-x86_64
[e] d7qeg7y ^[email protected]%[email protected] build_system=autotools arch=linux-rhel8-x86_64
- krfjrj3 ^[email protected]%[email protected]+gmock~ipo+pthreads~shared build_system=cmake build_type=Release cxxstd=17 generator=ninja patches=8376c65 arch=linux-rhel8-x86_64
- 3a6zied ^[email protected]%[email protected]~doc+ncurses+ownlibs build_system=generic build_type=Release patches=dbc3892 arch=linux-rhel8-x86_64
- 2pyqcdd ^[email protected]%[email protected]~gssapi~ldap~libidn2~librtmp~libssh~libssh2+nghttp2 build_system=autotools libs=shared,static tls=openssl arch=linux-rhel8-x86_64
- ysq366r ^[email protected]%[email protected] build_system=autotools arch=linux-rhel8-x86_64
- ms2uh2x ^[email protected]%[email protected]~symlinks+termlib abi=none build_system=autotools patches=7a351bc arch=linux-rhel8-x86_64 |
Contributor
Author
|
Also my failed concretizer behavior is |
Contributor
Author
|
More debugging output: $ git -C $SPACK_ROOT diff
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 28588f07b6..0dbfc2af94 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -3092,8 +3092,10 @@ def env_config_scopes(self) -> List[spack.config.ConfigScope]:
def prepare_config_scope(self) -> None:
"""Add the manifest's scopes to the global configuration search path."""
+ spack.config.CONFIG.pop_scope()
for scope in self.env_config_scopes:
spack.config.CONFIG.push_scope(scope)
+ spack.config.CONFIG.push_scope(spack.config.InternalConfigScope("command_line"))
def deactivate_config_scope(self) -> None:
"""Remove any of the manifest's scopes from the global config path."""
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 3fd8b9cc8d..b8a6d58b06 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -4086,6 +4086,8 @@ def solve(
specs = [s.lookup_hash() for s in specs]
reusable_specs = self._check_input_and_extract_concrete_specs(specs)
reusable_specs.extend(self.selector.reusable_specs(specs))
+ print(len(reusable_specs), reusable_specs)
+ assert not reusable_specs
setup = SpackSolverSetup(tests=tests)
output = OutputConfiguration(timers=timers, stats=stats, out=out, setup_only=setup_only)
result, _, _ = self.driver.solve($ spack concretize -fU
343 [[email protected]/lk2cy2n, [email protected]/5p3ti27, [email protected]/e4mtgdx, [email protected]/7bqwpmg, [email protected]/6wx5pf4, [email protected]/kzmv2td, [email protected]/v3xvf3w, [email protected]/d2tqiqf, [email protected]/xulwldz, [email protected]/mzzmjlj, [email protected]/hioitma, [email protected]/ms2pr5d, [email protected]/etxluaf, [email protected]/xh7n7j3, [email protected]/chhxxu2, [email protected]/gnxrk4v, [email protected]/d7qeg7y, [email protected]/kfidcc3, [email protected]/zrebf7f, [email protected]/afru445, [email protected]/q44nyat, [email protected]/aa2bzfb, [email protected]/dejnn43, [email protected]/5mu6grs, [email protected]/hnkhouj, [email protected]/4mx6dde, [email protected]/ecaqhok, [email protected]/fu6azzb, [email protected]/e5osxrx, [email protected]/p5xr2di, [email protected]/bzkfhlp, [email protected]/veuw3jf, parallel@20240822/lrm5wdr, [email protected]/egzmutk, [email protected]/bxuohj7, [email protected]/flsqc7h, [email protected]/qhntmns, [email protected]/sycm5ef, [email protected]/nk5sq3g, [email protected]/27lxu4f, [email protected]/5z4aspl, [email protected]/vklnknu, [email protected]/ee3g7cg, sgm@master/oyuqm32, [email protected]/tqetacy, [email protected]/nqfntio, [email protected]/3litz5x, [email protected]/hfh6gl3, [email protected]/tnrstgl, [email protected]/6mqkolt, [email protected]/2awtznq, [email protected]/7lfld4t, [email protected]/5kzoybw, [email protected]/4qpyeyd, [email protected]/d7qeg7y, [email protected]/kfidcc3, [email protected]/aa2bzfb, [email protected]/hioitma, [email protected]/apkqyva, [email protected]/ysq366r, ca-certificates-mozilla@2023-05-30/ospcymg, [email protected]/7bqwpmg, [email protected]/v3xvf3w, [email protected]/ms2uh2x, [email protected]/fr3xtwd, [email protected]/l4eq6c2, [email protected]/5kzoybw, [email protected]/cpi3f7v, [email protected]/lcv7a2n, [email protected]/2pyqcdd, [email protected]/3a6zied, [email protected]/bfp3djf, [email protected]/n6sz2j4, [email protected]/6mqkolt, [email protected]/cnb5mzf, [email protected]/cewzxlo, [email protected]/nwjemle, [email protected]/dejnn43, [email protected]/4mx6dde, [email protected]/alrwpwh, [email protected]/zdoo5qr, [email protected]/ms2pr5d, [email protected]/5mu6grs, [email protected]/ot7cjvc, openssh@default/imfhvej, [email protected]/yc2bpqx, [email protected]/rn3fc63, [email protected]/wzuwz2c, [email protected]/a4lyx34, [email protected]/wlttkpm, [email protected]/27lxu4f, [email protected]/veuw3jf, [email protected]/xganfy5, [email protected]/6wx5pf4, [email protected]/qhntmns, [email protected]/4qpyeyd, [email protected]/3litz5x, [email protected]/atbismy, [email protected]/jpuborp, [email protected]/z2fcgng, [email protected]/kqq3ly6, [email protected]/px4jdkk, [email protected]/rfbmxfv, [email protected]/2awtznq, [email protected]/kcrksow, [email protected]/35jyata, [email protected]/7kkwcm5, [email protected]/lnajhas, [email protected]/7dgx5mo, [email protected]/ihgjebu, [email protected]/adtwkfz, [email protected]/iido75u, [email protected]/zzxl35w, [email protected]/f7aj6qf, [email protected]/bywc73w, [email protected]/unxz3u4, [email protected]/vuv3b22, [email protected]/3lgln2s, [email protected]/72ttopo, [email protected]/4akjj44, [email protected]/kvb36vu, [email protected]/ggxvfna, [email protected]/7ddcgtu, [email protected]/popntav, [email protected]/7lhbo2f, [email protected]/zqpmy3s, [email protected]/auhz3rw, [email protected]/b2d2pti, [email protected]/fh2zdhm, [email protected]/qmxkyu4, [email protected]/tm52kup, [email protected]/zasyv3v, [email protected]/bih7nhn, [email protected]/g67refr, [email protected]/bz4lxff, [email protected]/ntcgfjj, [email protected]/cgrz2w7, [email protected]/x7kpzhb, [email protected]/dc53vnu, [email protected]/ucmcroi, [email protected]/e252yps, [email protected]/lpsrqfp, [email protected]/dshqpso, [email protected]/4hbamga, [email protected]/e6xpowr, [email protected]/mlscjvw, [email protected]/ext4kbb, [email protected]/bobbco6, [email protected]/xwh6ewh, [email protected]/tyvlyus, [email protected]/5zjaheg, [email protected]/hiwx3qe, [email protected]/3te77zn, [email protected]/zrqkd5s, [email protected]/gzkwfyk, [email protected]/rrinv6u, [email protected]/hy7y7el, [email protected]/q3oe6t7, [email protected]/jplkg5a, [email protected]/t66r5yh, [email protected]/kixvlsm, [email protected]/vrmjinr, [email protected]/yk4e5wq, [email protected]/bndcma3, [email protected]/ddg7hoi, [email protected]/xpowc7i, [email protected]/kxeukvz, [email protected]/zeni4wk, [email protected]/kzyo74z, [email protected]/4j2zoo3, [email protected]/l4keesd, [email protected]/h5j27v7, [email protected]/u4io5hi, [email protected]/xroa6rs, [email protected]/vwglph5, [email protected]/43h5i4a, [email protected]/wf3jrpu, [email protected]/7snqeff, [email protected]/xina5lm, [email protected]/khvormt, [email protected]/ymov3pm, [email protected]/d2tqiqf, [email protected]/hbocy6g, [email protected]/jrmv7v2, [email protected]/zg5hlvb, [email protected]/vh6jarh, [email protected]/6mflemp, [email protected]/rxwudqj, [email protected]/a2of7im, [email protected]/cyos7xd, [email protected]/diqv7el, [email protected]/ouwlyac, [email protected]/h6hmped, [email protected]/qdmutux, [email protected]/n6xgrty, [email protected]/m67qov3, [email protected]/o3bwk44, [email protected]/cewmrbo, [email protected]/qd7g3mo, [email protected]/b4lyd7f, [email protected]/csqnujp, [email protected]/ali4fbz, [email protected]/fhfi472, [email protected]/gockfkx, [email protected]/m3lg4z4, [email protected]/ndrvdqt, [email protected]/phjl6dx, [email protected]/t6paaoh, [email protected]/thl76rc, [email protected]/utiyq7y, [email protected]/cmsgirm, [email protected]/xr7vm5k, [email protected]/di6xanz, [email protected]/shsjj5e, [email protected]/wygitrr, [email protected]/xh7n7j3, [email protected]/f4ubfe3, [email protected]/iu7fqdh, [email protected]/krfjrj3, [email protected]/zrebf7f, [email protected]/6o62aaq, [email protected]/me4ztz6, [email protected]/mfblqwu, [email protected]/noac7ir, [email protected]/xcbgvzf, [email protected]/y6hlxud, [email protected]/yeb3b5v, [email protected]/whxbvb6, [email protected]/ebkknna, [email protected]/mkfnk2a, [email protected]/srx3737, [email protected]/waer7ay, [email protected]/cgom432, [email protected]/76ikd4z, [email protected]/jsinsxt, [email protected]/76lke2u, [email protected]/izdt4wo, [email protected]/aqcv4v3, [email protected]/tpdstou, [email protected]/goymflo, [email protected]/smh2anf, [email protected]/ip7h5fr, [email protected]/jw5bb5m, [email protected]/l5p6ral, [email protected]/nfkruhv, parallel@20220522/e57alz6, parallel@20240822/lifpob6, [email protected]/34yywij, [email protected]/smbnwya, [email protected]/ds7r7dd, [email protected]/axed66v, [email protected]/yuuxix7, [email protected]/fljxvbn, [email protected]/ucnrztv, [email protected]/nk5sq3g, seacas@2024-08-15/2i4mfzk, seacas@2024-08-15/4dp65ak, seacas@2024-08-15/6bivwbd, seacas@2024-08-15/6glnuza, seacas@2024-08-15/gaepasr, seacas@2024-08-15/gix76id, seacas@2024-08-15/hghlngf, seacas@2024-08-15/kblraz7, seacas@2024-08-15/lmi2gnf, seacas@2024-08-15/smti677, seacas@2024-08-15/vffiwfk, seacas@2024-08-15/vrzyamf, seacas@2024-08-15/zf5c6ps, [email protected]/e6pr367, [email protected]/fixupnm, [email protected]/7ydgge3, [email protected]/eisz7lt, [email protected]/ievmvo6, [email protected]/obrvs4y, [email protected]/shgvrb5, [email protected]/wbyv5sb, [email protected]=master/dau62yf, [email protected]/ywu4tgx, sierra-deps@master/25qn4dq, [email protected]/maomjlx, [email protected]/5ckxlda, sierra-deps@master/2aooqmh, [email protected]/kyj2mqu, sierra-deps@master/2qpn6cr, sierra-deps@master/3bs3xw5, [email protected]/exokkfk, sierra-deps@master/3u33fep, [email protected]=master/gpk6eeu, sierra-deps@master/4mbqtoy, [email protected]=master/syk3oiv, sierra-deps@master/4pv2mvi, sierra-deps@master/6nrupkp, [email protected]=master/5vq44w5, sierra-deps@master/7zo3prg, sierra-deps@master/a7n2e4c, sierra-deps@master/bsyr2u2, [email protected]/unfy453, sierra-deps@master/c4molsl, [email protected]/3hmunwa, sierra-deps@master/cjpozjf, sierra-deps@master/csdsfka, [email protected]/ttks2ib, sierra-deps@master/cy7cfi3, [email protected]=master/hppc2ut, sierra-deps@master/eb4zxmn, [email protected]/mvl2rma, sierra-deps@master/eksglpu, sierra-deps@master/esnqeou, [email protected]/swzoxax, sierra-deps@master/euabcvf, [email protected]/ikvkoet, sierra-deps@master/f7w2eai, sierra-deps@master/ftfq444, [email protected]/pxc7yxx, sierra-deps@master/gagfop7, sierra-deps@master/h365p2v, sierra-deps@master/hb5ciss, [email protected]/u7p7zgn, sierra-deps@master/i6s5s7j, [email protected]=master/gxqaazj, sierra-deps@master/jcy5q2k, sierra-deps@master/kicsp4i, [email protected]/j3jzas4, sierra-deps@master/lq24pal, sierra-deps@master/mm4zs7w, [email protected]/4u3qq4i, sierra-deps@master/nfq3ec3, sierra-deps@master/nzk2dm6, sierra-deps@master/oyhag5i, sierra-deps@master/p4md3ve, sierra-deps@master/qdb7wof, sierra-deps@master/qgl6ixm, sierra-deps@master/rxq7e2b, sierra-deps@master/tlguyeg, sierra-deps@master/twjkfea, sierra-deps@master/uazqwyg, sierra-deps@master/vn66mpf, sierra-deps@master/w4qbtcr, sierra-deps@master/w7gm2st, sierra-deps@master/x65uakm, sierra-deps@master/y767wgd, sierra-deps@master/yiyx62q, sierra-deps@master/zcu5gvw]
==> Error:
$ spack solve --fresh
0 []
^CNot sure what the difference is yet. I don't see any additional scope push/pops. Ah I got it. It's my change. When I pop the "command_line" off. |
Contributor
Author
|
This works now with my local tests. Will update description. |
Contributor
Author
|
I think there is a better way by ensuring |
psakievich
changed the title
Contributor
Author
|
@spackbot fix style |
|
Let me see if I can fix that for you! |
|
I was able to run spack style --fix==> Running style checks on spack
selected: import, isort, black, flake8, mypy
==> Modified files
lib/spack/spack/config.py
==> Running import checks
import check requires Python 3.9 or later
import checks were clean
==> Running isort checks
isort checks were clean
==> Running black checks
All done! ✨ 🍰 ✨
1 file left unchanged.
black checks were clean
==> Running flake8 checks
flake8 checks were clean
==> Running mypy checks
lib/spack/spack/version/version_types.py:136: error: Incompatible types in assignment (expression has type "Tuple[Any, ...]", variable has type "Tuple[str]") [assignment]
lib/spack/spack/config.py:440: error: "Dict[str, ConfigScope]" has no attribute "move_to_end" [attr-defined]
lib/spack/spack/variant.py:131: error: Unsupported right operand type for in ("Union[Collection[Any], Callable[..., Any]]") [operator]
Found 3 errors in 3 files (checked 634 source files)
mypy found errors
I wasn't able to make any further changes, but please see the message above for remaining issues you can fix locally! |
|
I was able to run spack style --fix==> Running style checks on spack
selected: import, isort, black, flake8, mypy
==> Modified files
lib/spack/spack/cmd/common/arguments.py
lib/spack/spack/config.py
lib/spack/spack/environment/environment.py
lib/spack/spack/test/config.py
==> Running import checks
import check requires Python 3.9 or later
import checks were clean
==> Running isort checks
isort checks were clean
==> Running black checks
reformatted lib/spack/spack/config.py
All done! ✨ 🍰 ✨
1 file reformatted, 3 files left unchanged.
black checks were clean
==> Running flake8 checks
lib/spack/spack/test/config.py:1455: [F841] local variable 'e' is assigned to but never used
flake8 found errors
==> Running mypy checks
lib/spack/spack/version/version_types.py:136: error: Incompatible types in assignment (expression has type "Tuple[Any, ...]", variable has type "Tuple[str]") [assignment]
lib/spack/spack/variant.py:131: error: Unsupported right operand type for in ("Union[Collection[Any], Callable[..., Any]]") [operator]
Found 2 errors in 2 files (checked 634 source files)
mypy found errors
I've updated the branch with style fixes. |
Contributor
Author
|
I'm not fully satisfied with the state of configs, but I think this PR is now at a point where it fixes a IMHO serious bug. It can be expanded further, but I think we should merge this to fix the bug in the interim. |
alalazo
requested changes
Dec 25, 2024
Member
alalazo
left a comment
alalazo
left a comment
There was a problem hiding this comment.
I added a few suggestions inline, to clean the code from spurious comments, and improve the test. Fine with me to merge as soon as they're in.
If you that's ok with you, I can also take care of pushing them in the branch.
Co-authored-by: Massimiliano Culpo <[email protected]>
Co-authored-by: Massimiliano Culpo <[email protected]>
Co-authored-by: Massimiliano Culpo <[email protected]>
Co-authored-by: Massimiliano Culpo <[email protected]>
Contributor
Author
|
@spackbot fix style |
|
Let me see if I can fix that for you! |
|
I was able to run spack style --fix==> Running style checks on spack
selected: import, isort, black, flake8, mypy
==> Modified files
lib/spack/spack/cmd/common/arguments.py
lib/spack/spack/config.py
lib/spack/spack/environment/environment.py
lib/spack/spack/test/config.py
==> Running import checks
import check requires Python 3.9 or later
import checks were clean
==> Running isort checks
isort checks were clean
==> Running black checks
All done! ✨ 🍰 ✨
4 files left unchanged.
black checks were clean
==> Running flake8 checks
flake8 checks were clean
==> Running mypy checks
lib/spack/spack/version/version_types.py:136: error: Incompatible types in assignment (expression has type "Tuple[Any, ...]", variable has type "Tuple[str]") [assignment]
lib/spack/spack/config.py:436: error: No overload variant of "reversed" matches argument type "dict_values[str, ConfigScope]" [call-overload]
lib/spack/spack/config.py:436: note: Possible overload variants:
lib/spack/spack/config.py:436: note: def [_T] __init__(self, Reversible[_T], /) -> reversed[_T]
lib/spack/spack/config.py:436: note: def [_T] __init__(self, SupportsLenAndGetItem[_T], /) -> reversed[_T]
lib/spack/spack/variant.py:131: error: Unsupported right operand type for in ("Union[Collection[Any], Callable[..., Any]]") [operator]
Found 3 errors in 3 files (checked 634 source files)
mypy found errors
I wasn't able to make any further changes, but please see the message above for remaining issues you can fix locally! |
Contributor
Author
|
@alalazo I merged your changes which all looked good to me. Set it to automerge so when you approve it should go in. |
This fixes an issue with mypy 1.8, and is similar to other "highest*" methods
alalazo
approved these changes
Dec 25, 2024
tdrwenski
pushed a commit
to tdrwenski/spack
that referenced
this pull request
Dec 26, 2024
kshea21
pushed a commit
to kshea21/spack
that referenced
this pull request
Dec 26, 2024
3 tasks
haampie
pushed a commit
that referenced
this pull request
Feb 3, 2025
teaguesterling
pushed a commit
to teaguesterling/spack
that referenced
this pull request
Feb 5, 2025
mrmundt
pushed a commit
to mrmundt/spack
that referenced
this pull request
Feb 17, 2025
haampie
pushed a commit
that referenced
this pull request
Feb 19, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move command_line scope to highest precedent after adding environment config scopes.
What I suspect is that we are calling the
Environment::prepare_config_scopetoo many times. Correcting that is probably a more "proper" fix, but I also worry that it is less reliable than this fix since it is depends on the order of the calling function, where this fix makes sure the final precedent is not wrong no matter how many times this function is called.UPDATED:
I moved the check into the config code itself. It's cleaner and provides better assurances.
Closes #48254