Assign priorities to configuration scopes #48420
Conversation
alalazo
added
bugfix
spackbot-app
bot
added
core
Up to now configuration behaved like a stack, where scopes could be only be pushed at the top. This commit allows to assign priorities to scopes, and ensures that scopes of lower priorities are always "below" scopes of higher priorities. When scopes have the same priority, what matters is the insertion order.
The function is only used in a fixture, and it can be substituted with already existing code.
alalazo
force-pushed
the
bugfix/config-priority-based-order
branch
from
2b17111 to
8c0cc36
Compare
|
Failure in pipelines is unrelated, and due to system errors: |
lib/spack/llnl/util/lang.py
Outdated
| if key in self._data: | ||
| self.remove(key) | ||
|
|
||
| self._priorities.append((priority, self._new_id(), key)) |
There was a problem hiding this comment.
sort/sorted is stable, so no need for _new_id().
There was a problem hiding this comment.
We need that It's useful to be defensive when iterating (in case a new function scrambles the order of _priorities).
EDIT: I changed that in 87752f1 This is more similar to my first implementation, which I modified by adding _new_id() out of possibly an abundance of caution - motivated by the numerous bugs we had with config scopes order. If we have a second thought, and want to get back to having an explicit index, we can drop the commit.
There was a problem hiding this comment.
I've looked over the code and the concept seems sound to me. The idea of stack like behavior within a given priority is a more robust design. It also seems truer to our description of how configs will work with the users. I'm still mulling a bit about the defaults selected but not to the point of wanting to hold up a merge. Thanks @alalazo for taking the time to shore up this design.
|
I don't see any code change in bootstrap 🤔 what about bootstrapping to /tmp instead of ~/.spack/bootstrap? |
|
@haampie There shouldn't be any change to bootstrap. The bootstrap store is specified in another place, so if you want to modify it from the command line you should: $ spack -c bootstrap:root:/tmp spec zlibeven though the simpler way to do it is: $ spack bootstrap root /tmpAnyhow, I'm not aiming at changing any bootstrap semantic in this PR. |
fixes #48414
Up to now configuration behaved like a stack, where scopes could be only be pushed at the top. This PR allows to assign priorities to scopes, and ensures that scopes of lower priorities are always "below" scopes of higher priorities.
When scopes have the same priority, what matters is the insertion order.
Modifications:
Configurationconstructor f1d0371Configuration.pop_scope61dd2f2