From 98ee00b9773e1ce4a644c431feddb3e66a28ae6a Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Wed, 6 Oct 2021 20:32:26 +0200
Subject: 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.
---
 lib/spack/spack/environment/environment.py | 12 ++++++++++++
 lib/spack/spack/store.py                   | 16 ++++++++++++++--
 lib/spack/spack/test/bootstrap.py          | 22 ++++++++++++++++++++++
 lib/spack/spack/test/cmd/env.py            | 18 ++++++++++++++++++
 4 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 88dd545398..5a271fc6dd 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -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
diff --git a/lib/spack/spack/store.py b/lib/spack/spack/store.py
index c60772e595..b0a418c7e3 100644
--- a/lib/spack/spack/store.py
+++ b/lib/spack/spack/store.py
@@ -236,17 +236,29 @@ layout = llnl.util.lang.LazyReference(_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', {})
diff --git a/lib/spack/spack/test/bootstrap.py b/lib/spack/spack/test/bootstrap.py
index fb84921c7a..99c1a61fd3 100644
--- a/lib/spack/spack/test/bootstrap.py
+++ b/lib/spack/spack/test/bootstrap.py
@@ -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'
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 3d1725a3f0..5c57035527 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -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
-- 
cgit v1.2.3-60-g2f50