From ea2d43b4a6752da25c1f4c7234acf47b6d539695 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 18 Jul 2024 07:18:14 +0200 Subject: [PATCH] Do not initialize previous store state in "use_store" (#45268) The "use_store" context manager is used to swap the value of a global variable (spack.store.STORE), while keeping another global variable consistent (spack.config.CONFIG). When doing that it tries to evaluate the previous value of the store, if that was not done already. This is wrong, since the configuration might be in an "intermediate" state that was never meant to trigger side effects. Remove that operation, and add a unit test to prevent regressions. --- lib/spack/spack/store.py | 1 - lib/spack/spack/test/bootstrap.py | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/store.py b/lib/spack/spack/store.py index dc2d5de54b..8509d728aa 100644 --- a/lib/spack/spack/store.py +++ b/lib/spack/spack/store.py @@ -371,7 +371,6 @@ def use_store( data.update(extra_data) # Swap the store with the one just constructed and return it - ensure_singleton_created() spack.config.CONFIG.push_scope( spack.config.InternalConfigScope(name=scope_name, data={"config": {"install_tree": data}}) ) diff --git a/lib/spack/spack/test/bootstrap.py b/lib/spack/spack/test/bootstrap.py index 5f11e2d381..f0d11d4124 100644 --- a/lib/spack/spack/test/bootstrap.py +++ b/lib/spack/spack/test/bootstrap.py @@ -228,3 +228,25 @@ def test_source_is_disabled(mutable_config): spack.config.add("bootstrap:trusted:{0}:{1}".format(conf["name"], False)) with pytest.raises(ValueError): spack.bootstrap.core.source_is_enabled_or_raise(conf) + + +@pytest.mark.regression("45247") +def test_use_store_does_not_try_writing_outside_root(tmp_path, monkeypatch, mutable_config): + """Tests that when we use the 'use_store' context manager, there is no attempt at creating + a Store outside the given root. + """ + initial_store = mutable_config.get("config:install_tree:root") + user_store = tmp_path / "store" + + fn = spack.store.Store.__init__ + + def _checked_init(self, root, *args, **kwargs): + fn(self, root, *args, **kwargs) + assert self.root == str(user_store) + + monkeypatch.setattr(spack.store.Store, "__init__", _checked_init) + + spack.store.reinitialize() + with spack.store.use_store(user_store): + assert spack.config.CONFIG.get("config:install_tree:root") == str(user_store) + assert spack.config.CONFIG.get("config:install_tree:root") == initial_store