Allow users to specify root env dir (spack#32836)

* Allow users to specify root env dir

Environments managed by spack have some advantages over anonymous Environments
but they are tucked away inside spack's directory tree. This PR gives
users the ability to specify where the environments should live.

See spack#32823

This is also taken as an opportunity to ensure that all references are to "managed environments",
rather than "named environments". Prior to this PR some references to the latter persisted.

Co-authored-by: Tom Scogland <scogland1@llnl.gov>
Co-authored-by: Tamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>
Co-authored-by: Gregory Becker <becker33@llnl.gov>

bin/spack-tmpconfig

@@ -72,6 +72,7 @@ config:
     root: $TMP_DIR/install
   misc_cache: $$user_cache_path/cache
   source_cache: $$user_cache_path/source
+  environments_root: $TMP_DIR/envs
 EOF
 cat >"$SPACK_USER_CONFIG_PATH/bootstrap.yaml" <<EOF
 bootstrap:

etc/spack/defaults/config.yaml

@@ -81,6 +81,10 @@ config:
   source_cache: $spack/var/spack/cache
 
 
+  ## Directory where spack managed environments are created and stored
+  # environments_root: $spack/var/spack/environments
+
+
   # Cache directory for miscellaneous files, like the package index.
   # This can be purged with `spack clean --misc-cache`
   misc_cache: $user_cache_path/cache

lib/spack/docs/environments.rst

@@ -58,9 +58,9 @@ Using Environments
 Here we follow a typical use case of creating, concretizing,
 installing and loading an environment.
 
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-Creating a named Environment
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Creating a managed Environment
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 An environment is created by:
 
@@ -72,7 +72,8 @@ Spack then creates the directory ``var/spack/environments/myenv``.
 
 .. note::
 
-   All named environments are stored in the ``var/spack/environments`` folder.
+   All managed environments by default are stored in the ``var/spack/environments`` folder.
+   This location can be changed by setting the ``environments_root`` variable in ``config.yaml``.
 
 In the ``var/spack/environments/myenv`` directory, Spack creates the
 file ``spack.yaml`` and the hidden directory ``.spack-env``.

lib/spack/spack/cmd/env.py

@@ -165,7 +165,7 @@ def env_activate(args):
         short_name = os.path.basename(env_path)
         ev.Environment(env).write(regenerate=False)
 
-    # Named environment
+    # Managed environment
     elif ev.exists(env_name_or_dir) and not args.dir:
         env_path = ev.root(env_name_or_dir)
         short_name = env_name_or_dir

lib/spack/spack/cmd/location.py

@@ -95,7 +95,7 @@ def location(parser, args):
             spack.cmd.require_active_env("location -e")
             path = ev.active_environment().path
         else:
-            # Get named environment path
+            # Get path of requested environment
             if not ev.exists(args.location_env):
                 tty.die("no such environment: '%s'" % args.location_env)
             path = ev.root(args.location_env)

lib/spack/spack/environment/environment.py

@@ -62,8 +62,8 @@
 _active_environment = None
 
 
-#: path where environments are stored in the spack tree
-env_path = os.path.join(spack.paths.var_path, "environments")
+#: default path where environments are stored in the spack tree
+default_env_path = os.path.join(spack.paths.var_path, "environments")
 
 
 #: Name of the input yaml file for an environment
@@ -78,6 +78,26 @@
 env_subdir_name = ".spack-env"
 
 
+def env_root_path():
+    """Override default root path if the user specified it"""
+    return spack.util.path.canonicalize_path(
+        spack.config.get("config:environments_root", default=default_env_path)
+    )
+
+
+def check_disallowed_env_config_mods(scopes):
+    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
+
+
 def default_manifest_yaml():
     """default spack.yaml file to put in new environments"""
     return """\
@@ -214,7 +234,7 @@ def active_environment():
 
 def _root(name):
     """Non-validating version of root(), to be used internally."""
-    return os.path.join(env_path, name)
+    return os.path.join(env_root_path(), name)
 
 
 def root(name):
@@ -249,10 +269,12 @@ def read(name):
 
 
 def create(name, init_file=None, with_view=None, keep_relative=False):
-    """Create a named environment in Spack."""
+    """Create a managed environment in Spack."""
+    if not os.path.isdir(env_root_path()):
+        fs.mkdirp(env_root_path())
     validate_env_name(name)
     if exists(name):
-        raise SpackEnvironmentError("'%s': environment already exists" % name)
+        raise SpackEnvironmentError("'%s': environment already exists at %s" % (name, root(name)))
     return Environment(root(name), init_file, with_view, keep_relative)
 
 
@@ -266,10 +288,10 @@ def all_environment_names():
     """List the names of environments that currently exist."""
     # just return empty if the env path does not exist.  A read-only
     # operation like list should not try to create a directory.
-    if not os.path.exists(env_path):
+    if not os.path.exists(env_root_path()):
         return []
 
-    candidates = sorted(os.listdir(env_path))
+    candidates = sorted(os.listdir(env_root_path()))
     names = []
     for candidate in candidates:
         yaml_path = os.path.join(_root(candidate), manifest_name)
@@ -279,7 +301,7 @@ def all_environment_names():
 
 
 def all_environments():
-    """Generator for all named Environments."""
+    """Generator for all managed Environments."""
     for name in all_environment_names():
         yield read(name)
 
@@ -859,14 +881,14 @@ def clear(self, re_read=False):
     @property
     def internal(self):
         """Whether this environment is managed by Spack."""
-        return self.path.startswith(env_path)
+        return self.path.startswith(env_root_path())
 
     @property
     def name(self):
         """Human-readable representation of the environment.
 
         This is the path for directory environments, and just the name
-        for named environments.
+        for managed environments.
         """
         if self.internal:
             return os.path.basename(self.path)
@@ -1044,7 +1066,9 @@ def env_file_config_scope(self):
 
     def config_scopes(self):
         """A list of all configuration scopes for this environment."""
-        return self.included_config_scopes() + [self.env_file_config_scope()]
+        return check_disallowed_env_config_mods(
+            self.included_config_scopes() + [self.env_file_config_scope()]
+        )
 
     def destroy(self):
         """Remove this environment from Spack entirely."""

lib/spack/spack/main.py

@@ -459,7 +459,7 @@ def make_argument_parser(**kwargs):
         dest="env_dir",
         metavar="DIR",
         action="store",
-        help="run with an environment directory (ignore named environments)",
+        help="run with an environment directory (ignore managed environments)",
     )
     env_group.add_argument(
         "-E",

lib/spack/spack/schema/config.py

@@ -67,6 +67,7 @@
             "license_dir": {"type": "string"},
             "source_cache": {"type": "string"},
             "misc_cache": {"type": "string"},
+            "environments_root": {"type": "string"},
             "connect_timeout": {"type": "integer", "minimum": 0},
             "verify_ssl": {"type": "boolean"},
             "suppress_gpg_warnings": {"type": "boolean"},

lib/spack/spack/test/cmd/concretize.py

@@ -9,8 +9,7 @@
 import spack.environment as ev
 from spack.main import SpackCommand
 
-# everything here uses the mock_env_path
-pytestmark = pytest.mark.usefixtures("mutable_mock_env_path", "config", "mutable_mock_repo")
+pytestmark = pytest.mark.usefixtures("config", "mutable_mock_repo")
 
 env = SpackCommand("env")
 add = SpackCommand("add")
@@ -21,7 +20,7 @@
 
 
 @pytest.mark.parametrize("unify", unification_strategies)
-def test_concretize_all_test_dependencies(unify):
+def test_concretize_all_test_dependencies(unify, mutable_mock_env_path):
     """Check all test dependencies are concretized."""
     env("create", "test")
 
@@ -33,7 +32,7 @@ def test_concretize_all_test_dependencies(unify):
 
 
 @pytest.mark.parametrize("unify", unification_strategies)
-def test_concretize_root_test_dependencies_not_recursive(unify):
+def test_concretize_root_test_dependencies_not_recursive(unify, mutable_mock_env_path):
     """Check that test dependencies are not concretized recursively."""
     env("create", "test")
 
@@ -45,7 +44,7 @@ def test_concretize_root_test_dependencies_not_recursive(unify):
 
 
 @pytest.mark.parametrize("unify", unification_strategies)
-def test_concretize_root_test_dependencies_are_concretized(unify):
+def test_concretize_root_test_dependencies_are_concretized(unify, mutable_mock_env_path):
     """Check that root test dependencies are concretized."""
     env("create", "test")
 

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

@@ -3222,3 +3222,20 @@ def test_relative_view_path_on_command_line_is_made_absolute(tmpdir, config):
         env("create", "--with-view", "view", "--dir", "env")
         environment = ev.Environment(os.path.join(".", "env"))
         assert os.path.samefile("view", environment.default_view.root)
+
+
+def test_environment_created_in_users_location(mutable_config, tmpdir):
+    """Test that an environment is created in a location based on the config"""
+    spack.config.set("config:environments_root", str(tmpdir.join("envs")))
+    env_dir = spack.config.get("config:environments_root")
+
+    assert tmpdir.strpath in env_dir
+    assert not os.path.isdir(env_dir)
+
+    dir_name = "user_env"
+    env("create", dir_name)
+    out = env("list")
+
+    assert dir_name in out
+    assert env_dir in ev.root(dir_name)
+    assert os.path.isdir(os.path.join(env_dir, dir_name))

lib/spack/spack/test/cmd/uninstall.py

@@ -225,7 +225,7 @@ class TestUninstallFromEnv(object):
     concretize = SpackCommand("concretize")
     find = SpackCommand("find")
 
-    @pytest.fixture
+    @pytest.fixture(scope="function")
     def environment_setup(
         self, mutable_mock_env_path, config, mock_packages, mutable_database, install_mockery
     ):
@@ -244,6 +244,9 @@ def environment_setup(
             TestUninstallFromEnv.add("diamond-link-bottom")
             TestUninstallFromEnv.concretize()
             install("--fake")
+        yield "environment_setup"
+        TestUninstallFromEnv.env("rm", "e1", "-y")
+        TestUninstallFromEnv.env("rm", "e2", "-y")
 
     def test_basic_env_sanity(self, environment_setup):
         for env_name in ["e1", "e2"]:

lib/spack/spack/test/conftest.py

@@ -1535,14 +1535,14 @@ def get_rev():
     yield t
 
 
-@pytest.fixture()
-def mutable_mock_env_path(tmpdir_factory):
+@pytest.fixture(scope="function")
+def mutable_mock_env_path(tmpdir_factory, mutable_config):
     """Fixture for mocking the internal spack environments directory."""
-    saved_path = ev.environment.env_path
+    saved_path = ev.environment.default_env_path
     mock_path = tmpdir_factory.mktemp("mock-env-path")
-    ev.environment.env_path = str(mock_path)
+    ev.environment.default_env_path = str(mock_path)
     yield mock_path
-    ev.environment.env_path = saved_path
+    ev.environment.default_env_path = saved_path
 
 
 @pytest.fixture()

lib/spack/spack/test/env.py

@@ -143,3 +143,21 @@ def test_user_view_path_is_not_canonicalized_in_yaml(tmpdir, config):
         snd = ev.Environment(env_path)
         assert snd.yaml["spack"]["view"] == view
         assert os.path.samefile(snd.default_view.root, absolute_view)
+
+
+def test_environment_cant_modify_environments_root(tmpdir):
+    filename = str(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)