environment.py: fix excessive re-reads (#43746)

lib/spack/spack/config.py

@@ -1562,8 +1562,9 @@ def ensure_latest_format_fn(section: str) -> Callable[[YamlConfigDict], bool]:
 def use_configuration(
     *scopes_or_paths: Union[ConfigScope, str]
 ) -> Generator[Configuration, None, None]:
-    """Use the configuration scopes passed as arguments within the
-    context manager.
+    """Use the configuration scopes passed as arguments within the context manager.
+
+    This function invalidates caches, and is therefore very slow.
 
     Args:
         *scopes_or_paths: scope objects or paths to be used

lib/spack/spack/environment/environment.py

@@ -106,17 +106,16 @@ def environment_name(path: Union[str, pathlib.Path]) -> str:
         return path_str
 
 
-def check_disallowed_env_config_mods(scopes):
+def ensure_no_disallowed_env_config_mods(scopes: List[spack.config.ConfigScope]) -> None:
     for scope in scopes:
-        with spack.config.use_configuration(scope):
-            if spack.config.get("config:environments_root"):
-                raise SpackEnvironmentError(
-                    "Spack environments are prohibited from modifying 'config:environments_root' "
-                    "because it can make the definition of the environment ill-posed. Please "
-                    "remove from your environment and place it in a permanent scope such as "
-                    "defaults, system, site, etc."
-                )
-    return scopes
+        config = scope.get_section("config")
+        if config and "environments_root" in config["config"]:
+            raise SpackEnvironmentError(
+                "Spack environments are prohibited from modifying 'config:environments_root' "
+                "because it can make the definition of the environment ill-posed. Please "
+                "remove from your environment and place it in a permanent scope such as "
+                "defaults, system, site, etc."
+            )
 
 
 def default_manifest_yaml():
@@ -2463,6 +2462,10 @@ def __init__(self, manifest_dir: Union[pathlib.Path, str]) -> None:
         self.scope_name = f"env:{environment_name(self.manifest_dir)}"
         self.config_stage_dir = os.path.join(env_subdir_path(manifest_dir), "config")
 
+        #: Configuration scopes associated with this environment. Note that these are not
+        #: invalidated by a re-read of the manifest file.
+        self._config_scopes: Optional[List[spack.config.ConfigScope]] = None
+
         if not self.manifest_file.exists():
             msg = f"cannot find '{manifest_name}' in {self.manifest_dir}"
             raise SpackEnvironmentError(msg)
@@ -2808,16 +2811,19 @@ def included_config_scopes(self) -> List[spack.config.ConfigScope]:
 
     @property
     def env_config_scopes(self) -> List[spack.config.ConfigScope]:
-        """A list of all configuration scopes for the environment manifest.
-
-        Returns:  All configuration scopes associated with the environment
-        """
-        config_name = self.scope_name
-        env_scope = spack.config.SingleFileScope(
-            config_name, str(self.manifest_file), spack.schema.env.schema, [TOP_LEVEL_KEY]
-        )
-
-        return check_disallowed_env_config_mods(self.included_config_scopes + [env_scope])
+        """A list of all configuration scopes for the environment manifest. On the first call this
+        instantiates all the scopes, on subsequent calls it returns the cached list."""
+        if self._config_scopes is not None:
+            return self._config_scopes
+        scopes: List[spack.config.ConfigScope] = [
+            *self.included_config_scopes,
+            spack.config.SingleFileScope(
+                self.scope_name, str(self.manifest_file), spack.schema.env.schema, [TOP_LEVEL_KEY]
+            ),
+        ]
+        ensure_no_disallowed_env_config_mods(scopes)
+        self._config_scopes = scopes
+        return scopes
 
     def prepare_config_scope(self) -> None:
         """Add the manifest's scopes to the global configuration search path."""

lib/spack/spack/test/cmd/env.py

@@ -858,8 +858,7 @@ def test_with_config_bad_include_activate(environment_from_manifest, tmpdir):
 """
     )
 
-    e = ev.Environment(env_root)
-    with e:
+    with ev.Environment(env_root) as e:
         e.concretize()
 
     # we've created an environment with some included config files (which do
@@ -869,7 +868,7 @@ def test_with_config_bad_include_activate(environment_from_manifest, tmpdir):
     os.remove(abs_include_path)
     os.remove(include1)
     with pytest.raises(spack.config.ConfigFileError) as exc:
-        ev.activate(e)
+        ev.activate(ev.Environment(env_root))
 
     err = exc.value.message
     assert "missing include" in err
@@ -1063,8 +1062,7 @@ def test_config_change_new(mutable_mock_env_path, tmp_path, mock_packages, mutab
 """
     )
 
-    e = ev.Environment(tmp_path)
-    with e:
+    with ev.Environment(tmp_path):
         config("change", "packages:mpich:require:~debug")
         with pytest.raises(spack.solver.asp.UnsatisfiableSpecError):
             spack.spec.Spec("mpich+debug").concretized()
@@ -1081,7 +1079,7 @@ def test_config_change_new(mutable_mock_env_path, tmp_path, mock_packages, mutab
       require: "@3.0.3"
 """
     )
-    with e:
+    with ev.Environment(tmp_path):
         assert spack.spec.Spec("mpich").concretized().satisfies("@3.0.3")
         with pytest.raises(spack.config.ConfigError, match="not a list"):
             config("change", "packages:mpich:require:~debug")