Restore the correct computation of stores in environments (#26560)

Environments push/pop scopes upon activation. If some lazily
evaluated value depending on the current configuration was
computed and cached before the scopes are pushed / popped
there will be an inconsistency in the current state.

This PR fixes the issue for stores, but it would be better
to move away from global state.
This commit is contained in:
Massimiliano Culpo 2021-10-06 20:32:26 +02:00 committed by GitHub
parent fdf76ecb51
commit 98ee00b977
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 66 additions and 2 deletions

View File

@ -126,7 +126,14 @@ def activate(env, use_env_repo=False):
if not isinstance(env, Environment):
raise TypeError("`env` should be of type {0}".format(Environment.__name__))
# Check if we need to reinitialize the store due to pushing the configuration
# below.
store_before_pushing = spack.config.get('config:install_tree')
prepare_config_scope(env)
store_after_pushing = spack.config.get('config:install_tree')
if store_before_pushing != store_after_pushing:
# Hack to store the state of the store before activation
env.store_token = spack.store.reinitialize()
if use_env_repo:
spack.repo.path.put_first(env.repo)
@ -144,6 +151,11 @@ def deactivate():
if not _active_environment:
return
# If we attached a store token on activation, restore the previous state
# and consume the token
if hasattr(_active_environment, 'store_token'):
spack.store.restore(_active_environment.store_token)
delattr(_active_environment, 'store_token')
deactivate_config_scope(_active_environment)
# use _repo so we only remove if a repo was actually constructed

View File

@ -236,17 +236,29 @@ def _store_layout():
def reinitialize():
"""Restore globals to the same state they would have at start-up"""
"""Restore globals to the same state they would have at start-up. Return a token
containing the state of the store before reinitialization.
"""
global store
global root, unpadded_root, db, layout
store = llnl.util.lang.Singleton(_store)
token = store, root, unpadded_root, db, layout
store = llnl.util.lang.Singleton(_store)
root = llnl.util.lang.LazyReference(_store_root)
unpadded_root = llnl.util.lang.LazyReference(_store_unpadded_root)
db = llnl.util.lang.LazyReference(_store_db)
layout = llnl.util.lang.LazyReference(_store_layout)
return token
def restore(token):
"""Restore the environment from a token returned by reinitialize"""
global store
global root, unpadded_root, db, layout
store, root, unpadded_root, db, layout = token
def retrieve_upstream_dbs():
other_spack_instances = spack.config.get('upstreams', {})

View File

@ -118,3 +118,25 @@ def test_config_yaml_is_preserved_during_bootstrap(mutable_config):
with spack.bootstrap.ensure_bootstrap_configuration():
assert spack.config.get('config:test_stage') == expected_dir
assert spack.config.get('config:test_stage') == expected_dir
@pytest.mark.regression('26548')
def test_custom_store_in_environment(mutable_config, tmpdir):
# Test that the custom store in an environment is taken into account
# during bootstrapping
spack_yaml = tmpdir.join('spack.yaml')
spack_yaml.write("""
spack:
specs:
- libelf
config:
install_tree:
root: /tmp/store
""")
with spack.environment.Environment(str(tmpdir)):
assert spack.environment.active_environment()
assert spack.config.get('config:install_tree:root') == '/tmp/store'
# Don't trigger evaluation here
with spack.bootstrap.ensure_bootstrap_configuration():
pass
assert str(spack.store.root) == '/tmp/store'

View File

@ -2663,3 +2663,21 @@ def test_activation_and_deactiviation_ambiguities(method, env, no_env, env_dir,
method(args)
_, err = capsys.readouterr()
assert 'is ambiguous' in err
@pytest.mark.regression('26548')
def test_custom_store_in_environment(mutable_config, tmpdir):
spack_yaml = tmpdir.join('spack.yaml')
spack_yaml.write("""
spack:
specs:
- libelf
config:
install_tree:
root: /tmp/store
""")
current_store_root = str(spack.store.root)
assert str(current_store_root) != '/tmp/store'
with spack.environment.Environment(str(tmpdir)):
assert str(spack.store.root) == '/tmp/store'
assert str(spack.store.root) == current_store_root