diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index cc0b3cddf13..164f078748d 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -65,6 +65,11 @@ #: default path where environments are stored in the spack tree default_env_path = os.path.join(spack.paths.var_path, "environments") +# ensure that this base directory exists since we require the root directory to be created +# in our checks +if not os.path.isdir(default_env_path): + os.mkdir(default_env_path) + #: Name of the input yaml file for an environment manifest_name = "spack.yaml" @@ -80,24 +85,21 @@ def env_root_path(): """Override default root path if the user specified it""" - env_path = spack.config.get("config:environments_root", default=default_env_path) - canonical_path = spack.util.path.canonicalize_path(env_path, allow_env=False) - if env_path == default_env_path: - # create the default path if it is missing, if user paths are missing we errors - # and force them to create it themselves to keep spack from adding arbitrary directories - # and to help debug typos - if not os.path.isdir(canonical_path): - os.mkdir(canonical_path) - return canonical_path + return spack.util.path.canonicalize_path( + spack.config.get("config:environments_root", default=default_env_path), allow_env=False + ) def check_disallowed_env_config_mods(scopes): for scope in scopes: with spack.config.override(scope): if spack.config.get("config:environments_root"): - raise SpackEnvironmentError("Need a good message") - else: - spack.config.print_section("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." + ) return scopes diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 1954a93e0f1..091efcea012 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -1463,26 +1463,3 @@ def test_environment_errors_if_root_missing(mutable_config, tmpdir): with pytest.raises(ev.SpackEnvironmentError, match=env_dir): env("create", "test") - - -def test_environment_cant_modify_environments_root(tmpdir): - filename = tmpdir.join("spack.yaml") - with open(filename, "w") as f: - f.write( - """\ -spack: - config: - environments_root: /a/black/hole - view: false - specs: [] -""" - ) - with tmpdir.as_cwd(): - with pytest.raises(ev.SpackEnvironmentError): - # Why does this raise? - e = ev.Environment(tmpdir.strpath) - ev.activate(e) - with pytest.raises(ev.SpackEnvironmentError): - # When this does not? - env("create", "-d", tmpdir.strpath) - env("activate", "-d", tmpdir.strpath) diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 60056e397aa..b83614519ff 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -106,6 +106,24 @@ def test_env_change_spec_in_matrix_raises_error( assert "Cannot directly change specs in matrices" in str(error) +def test_environment_cant_modify_environments_root(tmpdir): + filename = tmpdir.join("spack.yaml") + with open(filename, "w") as f: + f.write( + """\ +spack: + config: + environments_root: /a/black/hole + view: false + specs: [] +""" + ) + with tmpdir.as_cwd(): + with pytest.raises(ev.SpackEnvironmentError): + e = ev.Environment(tmpdir.strpath) + ev.activate(e) + + def test_activate_should_require_an_env(): with pytest.raises(TypeError): ev.activate(env="name")