features: Remove stage symlinks (#12072)

Fixes #11163

The goal of this work is to simplify stage directory structures by eliminating use of symbolic links. This means, among other things, that` $spack/var/spack/stage` will no longer be the core staging directory. Instead, the first accessible `config:build_stage` path will be used.

Spack will no longer automatically append `spack-stage` (or the like) to configured build stage directories so the onus of distinguishing the directory from other work -- so the other work is not automatically removed with a `spack clean` operation -- falls on the user.

17 files changed, 417 insertions, 315 deletions

diff --git a/etc/spack/defaults/config.yaml b/etc/spack/defaults/config.yaml
index 37db99f4c9..33a37bbdfa 100644
--- a/etc/spack/defaults/config.yaml
+++ b/etc/spack/defaults/config.yaml
@@ -37,18 +37,28 @@ config:
 
   # Temporary locations Spack can try to use for builds.
   #
-  # Spack will use the first one it finds that exists and is writable.
-  # You can use $tempdir to refer to the system default temp directory
-  # (as returned by tempfile.gettempdir()).
+  # Recommended options are given below.
   #
-  # A value of $spack/var/spack/stage indicates that Spack should run
-  # builds directly inside its install directory without staging them in
+  # Builds can be faster in temporary directories on some (e.g., HPC) systems.
+  # Specifying `$tempdir` will ensure use of the system default temporary
+  # directory (as returned by `tempfile.gettempdir()`).  Spack will append
+  # `spack-stage` and, if the username is not already in the path, the value
+  # of `$user` to the path.  The latter is used to avoid conflicts between
+  # users in shared temporary spaces.
+  #
+  # Another option that prevents conflicts and potential permission issues is
+  # to specify `~/.spack/stage`, which ensures each user builds in their home
+  # directory.
+  #
+  # A more traditional path uses the value of `$spack/var/spack/stage`, which
+  # builds directly inside Spack's instance without staging them in a
   # temporary space.
   #
-  # The build stage can be purged with `spack clean --stage`.
+  # The build stage can be purged with `spack clean --stage` and
+  # `spack clean -a`, so it is important that the specified directory uniquely
+  # identifies Spack staging to avoid accidentally wiping out non-Spack work.
   build_stage:
-    - $tempdir
-    - $spack/var/spack/stage
+    - $tempdir/spack-stage
 
 
   # Cache directory for already downloaded source tarballs and archived
diff --git a/lib/spack/docs/config_yaml.rst b/lib/spack/docs/config_yaml.rst
index 7e2759349b..ef3ad1d03a 100644
--- a/lib/spack/docs/config_yaml.rst
+++ b/lib/spack/docs/config_yaml.rst
@@ -85,38 +85,39 @@ See :ref:`modules` for details.
 
 Spack is designed to run out of a user home directory, and on many
 systems the home directory is a (slow) network file system.  On most systems,
-building in a temporary file system results in faster builds than building
-in the home directory.  Usually, there is also more space available in
-the temporary location than in the home directory. So, Spack tries to
-create build stages in temporary space.
+building in a temporary file system is faster.  Usually, there is also more
+space available in the temporary location than in the home directory.  If the
+username is not already in the path, Spack will also append the value of
+`$user` to the path.
+
+.. warning:: We highly recommend appending `spack-stage` to `$tempdir` in order
+   to ensure `spack clean` does not delete everything in `$tempdir`.
 
 By default, Spack's ``build_stage`` is configured like this:
 
 .. code-block:: yaml
 
    build_stage:
-    - $tempdir
-    - $spack/var/spack/stage
+    - $tempdir/spack-stage
 
-This is an ordered list of paths that Spack should search when trying to
+This can be an ordered list of paths that Spack should search when trying to
 find a temporary directory for the build stage.  The list is searched in
 order, and Spack will use the first directory to which it has write access.
-See :ref:`config-file-variables` for more on ``$tempdir`` and ``$spack``.
+Specifying `~/.spack/stage` first will ensure each user builds in their home
+directory.  The historic Spack stage path `$spack/var/spack/stage` will build
+directly inside the Spack instance.  See :ref:`config-file-variables` for more
+on ``$tempdir`` and ``$spack``.
 
 When Spack builds a package, it creates a temporary directory within the
-``build_stage``, and it creates a symbolic link to that directory in
-``$spack/var/spack/stage``. This is used to track the temporary
-directory.  After the package is successfully installed, Spack deletes
+``build_stage``.  After the package is successfully installed, Spack deletes
 the temporary directory it used to build.  Unsuccessful builds are not
 deleted, but you can manually purge them with :ref:`spack clean --stage
 <cmd-spack-clean>`.
 
 .. note::
 
-   The last item in the list is ``$spack/var/spack/stage``.  If this is the
-   only writable directory in the ``build_stage`` list, Spack will build
-   *directly* in ``$spack/var/spack/stage`` and will not link to temporary
-   space.
+   The build will fail if there is no writable directory in the ``build_stage``
+   list.
 
 --------------------
 ``source_cache``
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index e58f657aea..19feb1686f 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -347,12 +347,22 @@ def copy_tree(src, dest, symlinks=True, ignore=None, _permissions=False):
     else:
         tty.debug('Copying {0} to {1}'.format(src, dest))
 
-    mkdirp(dest)
+    abs_src = os.path.abspath(src)
+    if not abs_src.endswith(os.path.sep):
+        abs_src += os.path.sep
+    abs_dest = os.path.abspath(dest)
+    if not abs_dest.endswith(os.path.sep):
+        abs_dest += os.path.sep
+
+    # Stop early to avoid unnecessary recursion if being asked to copy from a
+    # parent directory.
+    if abs_dest.startswith(abs_src):
+        raise ValueError('Cannot copy ancestor directory {0} into {1}'.
+                         format(abs_src, abs_dest))
 
-    src = os.path.abspath(src)
-    dest = os.path.abspath(dest)
+    mkdirp(dest)
 
-    for s, d in traverse_tree(src, dest, order='pre',
+    for s, d in traverse_tree(abs_src, abs_dest, order='pre',
                               follow_symlinks=not symlinks,
                               ignore=ignore,
                               follow_nonexisting=True):
@@ -361,7 +371,7 @@ def copy_tree(src, dest, symlinks=True, ignore=None, _permissions=False):
             if symlinks:
                 target = os.readlink(s)
                 if os.path.isabs(target):
-                    new_target = re.sub(src, dest, target)
+                    new_target = re.sub(abs_src, abs_dest, target)
                     if new_target != target:
                         tty.debug("Redirecting link {0} to {1}"
                                   .format(target, new_target))
diff --git a/lib/spack/spack/cmd/location.py b/lib/spack/spack/cmd/location.py
index 38d1eab174..d6716233f9 100644
--- a/lib/spack/spack/cmd/location.py
+++ b/lib/spack/spack/cmd/location.py
@@ -14,6 +14,7 @@ import spack.cmd
 import spack.environment
 import spack.paths
 import spack.repo
+import spack.stage
 
 description = "print out locations of packages and spack directories"
 section = "basic"
@@ -76,7 +77,7 @@ def location(parser, args):
         print(spack.repo.path.first_repo().root)
 
     elif args.stages:
-        print(spack.paths.stage_path)
+        print(spack.stage.get_stage_root())
 
     else:
         specs = spack.cmd.parse_specs(args.spec)
diff --git a/lib/spack/spack/compilers/clang.py b/lib/spack/spack/compilers/clang.py
index e4b109588b..e53332363b 100644
--- a/lib/spack/spack/compilers/clang.py
+++ b/lib/spack/spack/compilers/clang.py
@@ -12,6 +12,7 @@ import llnl.util.lang
 import llnl.util.tty as tty
 
 import spack.paths
+import spack.stage
 from spack.compiler import Compiler, UnsupportedCompilerFlag
 from spack.util.executable import Executable
 from spack.version import ver
@@ -279,7 +280,7 @@ class Clang(Compiler):
             raise OSError(msg)
 
         real_root = os.path.dirname(os.path.dirname(real_root))
-        developer_root = os.path.join(spack.paths.stage_path,
+        developer_root = os.path.join(spack.stage.get_stage_root(),
                                       'xcode-select',
                                       self.name,
                                       str(self.version))
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 8114ec21b4..c73b5047f5 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -101,6 +101,7 @@ config_defaults = {
         'checksum': True,
         'dirty': False,
         'build_jobs': min(16, multiprocessing.cpu_count()),
+        'build_stage': '$tempdir/spack-stage',
     }
 }
 
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 07d2c28679..5f3cfef6f0 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1769,9 +1769,7 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)):
                 else:
                     partial = True
 
-        stage_is_managed_in_spack = self.stage.path.startswith(
-            spack.paths.stage_path)
-        if restage and stage_is_managed_in_spack:
+        if restage and self.stage.managed_by_spack:
             self.stage.destroy()
             self.stage.create()
 
diff --git a/lib/spack/spack/patch.py b/lib/spack/spack/patch.py
index 09aa48d9da..cb012af7ff 100644
--- a/lib/spack/spack/patch.py
+++ b/lib/spack/spack/patch.py
@@ -192,9 +192,8 @@ class UrlPatch(Patch):
             fetch_digest = self.archive_sha256
 
         fetcher = fs.URLFetchStrategy(self.url, fetch_digest)
-        mirror = os.path.join(
-            os.path.dirname(stage.mirror_path),
-            os.path.basename(self.url))
+        mirror = os.path.join(os.path.dirname(stage.mirror_path),
+                              os.path.basename(self.url))
 
         self.stage = spack.stage.Stage(fetcher, mirror_path=mirror)
         self.stage.create()
diff --git a/lib/spack/spack/paths.py b/lib/spack/spack/paths.py
index 2f7ccf3b5d..d91b01d87c 100644
--- a/lib/spack/spack/paths.py
+++ b/lib/spack/spack/paths.py
@@ -38,7 +38,6 @@ operating_system_path = os.path.join(module_path, 'operating_systems')
 test_path             = os.path.join(module_path, "test")
 hooks_path            = os.path.join(module_path, "hooks")
 var_path              = os.path.join(prefix, "var", "spack")
-stage_path            = os.path.join(var_path, "stage")
 repos_path            = os.path.join(var_path, "repos")
 share_path            = os.path.join(prefix, "share", "spack")
 
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index 6c52afb529..aa9920ca58 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import grp
 import os
 import stat
 import sys
@@ -16,7 +17,7 @@ from six.moves.urllib.parse import urljoin
 
 import llnl.util.tty as tty
 from llnl.util.filesystem import mkdirp, can_access, install, install_tree
-from llnl.util.filesystem import remove_if_dead_link, remove_linked_tree
+from llnl.util.filesystem import remove_linked_tree
 
 import spack.paths
 import spack.caches
@@ -28,45 +29,54 @@ import spack.util.pattern as pattern
 import spack.util.path as sup
 from spack.util.crypto import prefix_bits, bit_length
 
+
+# The well-known stage source subdirectory name.
 _source_path_subdir = 'spack-src'
+
+# The temporary stage name prefix.
 _stage_prefix = 'spack-stage-'
 
 
 def _first_accessible_path(paths):
-    """Find a tmp dir that exists that we can access."""
+    """Find the first path that is accessible, creating it if necessary."""
     for path in paths:
         try:
-            # try to create the path if it doesn't exist.
+            # Ensure the user has access, creating the directory if necessary.
             path = sup.canonicalize_path(path)
-            mkdirp(path)
-
-            # ensure accessible
-            if not can_access(path):
-                continue
-
-            # return it if successful.
-            return path
+            if os.path.exists(path):
+                if can_access(path):
+                    return path
+            else:
+                # The path doesn't exist so create it and adjust permissions
+                # and group as needed (e.g., shared ``$tempdir``).
+                prefix = os.path.sep
+                parts = path.strip(os.path.sep).split(os.path.sep)
+                for part in parts:
+                    prefix = os.path.join(prefix, part)
+                    if not os.path.exists(prefix):
+                        break
+                parent = os.path.dirname(prefix)
+                group = grp.getgrgid(os.stat(parent).st_gid)[0]
+                mkdirp(path, group=group, default_perms='parents')
+
+                if can_access(path):
+                    return path
 
         except OSError as e:
-            tty.debug('OSError while checking temporary path %s: %s' % (
+            tty.debug('OSError while checking stage path %s: %s' % (
                       path, str(e)))
-            continue
 
     return None
 
 
-# cached temporary root
-_tmp_root = None
-_use_tmp_stage = True
+# Cached stage path root
+_stage_root = None
 
 
-def get_tmp_root():
-    global _tmp_root, _use_tmp_stage
+def get_stage_root():
+    global _stage_root
 
-    if not _use_tmp_stage:
-        return None
-
-    if _tmp_root is None:
+    if _stage_root is None:
         candidates = spack.config.get('config:build_stage')
         if isinstance(candidates, string_types):
             candidates = [candidates]
@@ -75,23 +85,16 @@ def get_tmp_root():
         if not path:
             raise StageError("No accessible stage paths in:", candidates)
 
-        # Return None to indicate we're using a local staging area.
-        if path == sup.canonicalize_path(spack.paths.stage_path):
-            _use_tmp_stage = False
-            return None
-
         # Ensure that any temp path is unique per user, so users don't
         # fight over shared temporary space.
         user = getpass.getuser()
         if user not in path:
-            path = os.path.join(path, user, 'spack-stage')
-        else:
-            path = os.path.join(path, 'spack-stage')
+            path = os.path.join(path, user)
+            mkdirp(path)
 
-        mkdirp(path)
-        _tmp_root = path
+        _stage_root = path
 
-    return _tmp_root
+    return _stage_root
 
 
 class Stage(object):
@@ -139,6 +142,9 @@ class Stage(object):
     """Shared dict of all stage locks."""
     stage_locks = {}
 
+    """Most staging is managed by Spack.  DIYStage is one exception."""
+    managed_by_spack = True
+
     def __init__(
             self, url_or_fetch_strategy,
             name=None, mirror_path=None, keep=False, path=None, lock=True,
@@ -165,6 +171,17 @@ class Stage(object):
                  is deleted on exit when no exceptions are raised.
                  Pass True to keep the stage intact even if no
                  exceptions are raised.
+
+            path
+                 If provided, the stage path to use for associated builds.
+
+            lock
+                 True if the stage directory file lock is to be used, False
+                 otherwise.
+
+            search_fn
+                 The search function that provides the fetch strategy
+                 instance.
         """
         # TODO: fetch/stage coupling needs to be reworked -- the logic
         # TODO: here is convoluted and not modular enough.
@@ -184,20 +201,19 @@ class Stage(object):
 
         self.srcdir = None
 
-        # TODO : this uses a protected member of tempfile, but seemed the only
-        # TODO : way to get a temporary name besides, the temporary link name
-        # TODO : won't be the same as the temporary stage area in tmp_root
+        # TODO: This uses a protected member of tempfile, but seemed the only
+        # TODO: way to get a temporary name.  It won't be the same as the
+        # TODO: temporary stage area in _stage_root.
         self.name = name
         if name is None:
             self.name = _stage_prefix + next(tempfile._get_candidate_names())
         self.mirror_path = mirror_path
 
-        # Try to construct here a temporary name for the stage directory
-        # If this is a named stage, then construct a named path.
+        # Use the provided path or construct an optionally named stage path.
         if path is not None:
             self.path = path
         else:
-            self.path = os.path.join(spack.paths.stage_path, self.name)
+            self.path = os.path.join(get_stage_root(), self.name)
 
         # Flag to decide whether to delete the stage folder on exit or not
         self.keep = keep
@@ -210,7 +226,7 @@ class Stage(object):
             if self.name not in Stage.stage_locks:
                 sha1 = hashlib.sha1(self.name.encode('utf-8')).digest()
                 lock_id = prefix_bits(sha1, bit_length(sys.maxsize))
-                stage_lock_path = os.path.join(spack.paths.stage_path, '.lock')
+                stage_lock_path = os.path.join(get_stage_root(), '.lock')
 
                 Stage.stage_locks[self.name] = spack.util.lock.Lock(
                     stage_lock_path, lock_id, 1)
@@ -254,44 +270,6 @@ class Stage(object):
         if self._lock is not None:
             self._lock.release_write()
 
-    def _need_to_create_path(self):
-        """Makes sure nothing weird has happened since the last time we
-           looked at path.  Returns True if path already exists and is ok.
-           Returns False if path needs to be created."""
-        # Path doesn't exist yet.  Will need to create it.
-        if not os.path.exists(self.path):
-            return True
-
-        # Path exists but points at something else.  Blow it away.
-        if not os.path.isdir(self.path):
-            os.unlink(self.path)
-            return True
-
-        # Path looks ok, but need to check the target of the link.
-        if os.path.islink(self.path):
-            tmp_root = get_tmp_root()
-            if tmp_root is not None:
-                real_path = os.path.realpath(self.path)
-                real_tmp = os.path.realpath(tmp_root)
-
-                # If we're using a tmp dir, it's a link, and it points at the
-                # right spot, then keep it.
-                if (real_path.startswith(real_tmp) and
-                        os.path.exists(real_path)):
-                    return False
-                else:
-                    # otherwise, just unlink it and start over.
-                    os.unlink(self.path)
-                    return True
-
-            else:
-                # If we're not tmp mode, then it's a link and we want a
-                # directory.
-                os.unlink(self.path)
-                return True
-
-        return False
-
     @property
     def expected_archive_files(self):
         """Possible archive file paths."""
@@ -455,30 +433,18 @@ class Stage(object):
         self.fetcher.reset()
 
     def create(self):
-        """Creates the stage directory.
-
-        If get_tmp_root() is None, the stage directory is created
-        directly under spack.paths.stage_path, otherwise this will attempt to
-        create a stage in a temporary directory and link it into
-        spack.paths.stage_path.
-
         """
-        # Create the top-level stage directory
-        mkdirp(spack.paths.stage_path)
-        remove_if_dead_link(self.path)
-
-        # If a tmp_root exists then create a directory there and then link it
-        # in the stage area, otherwise create the stage directory in self.path
-        if self._need_to_create_path():
-            tmp_root = get_tmp_root()
-            if tmp_root is not None:
-                # tempfile.mkdtemp already sets mode 0700
-                tmp_dir = tempfile.mkdtemp('', _stage_prefix, tmp_root)
-                tty.debug('link %s -> %s' % (self.path, tmp_dir))
-                os.symlink(tmp_dir, self.path)
-            else:
-                # emulate file permissions for tempfile.mkdtemp
-                mkdirp(self.path, mode=stat.S_IRWXU)
+        Ensures the top-level (config:build_stage) directory exists.
+        """
+        # Emulate file permissions for tempfile.mkdtemp.
+        if not os.path.exists(self.path):
+            print("TLD: %s does not exist, creating it" % self.path)
+            mkdirp(self.path, mode=stat.S_IRWXU)
+        elif not os.path.isdir(self.path):
+            print("TLD: %s is not a directory, replacing it" % self.path)
+            os.remove(self.path)
+            mkdirp(self.path, mode=stat.S_IRWXU)
+
         # Make sure we can actually do something with the stage we made.
         ensure_access(self.path)
         self.created = True
@@ -562,7 +528,7 @@ class ResourceStage(Stage):
 
 @pattern.composite(method_list=[
     'fetch', 'create', 'created', 'check', 'expand_archive', 'restage',
-    'destroy', 'cache_local'])
+    'destroy', 'cache_local', 'managed_by_spack'])
 class StageComposite:
     """Composite for Stage type objects. The first item in this composite is
     considered to be the root package, and operations that return a value are
@@ -612,6 +578,9 @@ class DIYStage(object):
     directory naming convention.
     """
 
+    """DIY staging is, by definition, not managed by Spack."""
+    managed_by_spack = False
+
     def __init__(self, path):
         if path is None:
             raise ValueError("Cannot construct DIYStage without a path.")
@@ -659,7 +628,7 @@ class DIYStage(object):
         tty.msg("Sources for DIY stages are not cached")
 
 
-def ensure_access(file=spack.paths.stage_path):
+def ensure_access(file):
     """Ensure we can access a directory and die with an error if we can't."""
     if not can_access(file):
         tty.die("Insufficient permissions for %s" % file)
@@ -667,9 +636,10 @@ def ensure_access(file=spack.paths.stage_path):
 
 def purge():
     """Remove all build directories in the top-level stage path."""
-    if os.path.isdir(spack.paths.stage_path):
-        for stage_dir in os.listdir(spack.paths.stage_path):
-            stage_path = os.path.join(spack.paths.stage_path, stage_dir)
+    root = get_stage_root()
+    if os.path.isdir(root):
+        for stage_dir in os.listdir(root):
+            stage_path = os.path.join(root, stage_dir)
             remove_linked_tree(stage_path)
 
 
diff --git a/lib/spack/spack/test/cmd/location.py b/lib/spack/spack/test/cmd/location.py
index 6dbc4d94d4..f48cdb75e7 100644
--- a/lib/spack/spack/test/cmd/location.py
+++ b/lib/spack/spack/test/cmd/location.py
@@ -11,6 +11,7 @@ from llnl.util.filesystem import mkdirp
 import spack.environment as ev
 from spack.main import SpackCommand, SpackCommandError
 import spack.paths
+import spack.stage
 
 
 # Everything here uses (or can use) the mock config and database.
@@ -128,4 +129,4 @@ def test_location_stage_dir(mock_spec):
 @pytest.mark.db
 def test_location_stages(mock_spec):
     """Tests spack location --stages."""
-    assert location('--stages').strip() == spack.paths.stage_path
+    assert location('--stages').strip() == spack.stage.get_stage_root()
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index 53493eda91..f8c8e2a8bc 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -121,14 +121,40 @@ def reset_compiler_cache():
     spack.compilers._compiler_cache = {}
 
 
-@pytest.fixture(scope='session', autouse=True)
-def mock_stage(tmpdir_factory):
-    """Mocks up a fake stage directory for use by tests."""
-    stage_path = spack.paths.stage_path
-    new_stage = str(tmpdir_factory.mktemp('mock_stage'))
-    spack.paths.stage_path = new_stage
-    yield new_stage
-    spack.paths.stage_path = stage_path
+@pytest.fixture
+def clear_stage_root(monkeypatch):
+    """Ensure spack.stage._stage_root is not set at test start."""
+    monkeypatch.setattr(spack.stage, '_stage_root', None)
+    yield
+
+
+@pytest.fixture(scope='function', autouse=True)
+def mock_stage(clear_stage_root, tmpdir_factory, request):
+    """Establish the temporary build_stage for the mock archive."""
+    # Workaround to skip mock_stage for 'nomockstage' test cases
+    if 'nomockstage' not in request.keywords:
+        new_stage = tmpdir_factory.mktemp('mock-stage')
+        new_stage_path = str(new_stage)
+
+        # Set test_stage_path as the default directory to use for test stages.
+        current = spack.config.get('config:build_stage')
+        spack.config.set('config',
+                         {'build_stage': new_stage_path}, scope='user')
+
+        # Ensure the source directory exists
+        source_path = new_stage.join(spack.stage._source_path_subdir)
+        source_path.ensure(dir=True)
+
+        yield new_stage_path
+
+        spack.config.set('config', {'build_stage': current}, scope='user')
+
+        # Clean up the test stage directory
+        if os.path.isdir(new_stage_path):
+            shutil.rmtree(new_stage_path)
+    else:
+        # Must yield a path to avoid a TypeError on test teardown
+        yield str(tmpdir_factory)
 
 
 @pytest.fixture(scope='session')
@@ -138,7 +164,7 @@ def ignore_stage_files():
     Used to track which leftover files in the stage have been seen.
     """
     # to start with, ignore the .lock file at the stage root.
-    return set(['.lock'])
+    return set(['.lock', spack.stage._source_path_subdir])
 
 
 def remove_whatever_it_is(path):
@@ -173,17 +199,19 @@ def check_for_leftover_stage_files(request, mock_stage, ignore_stage_files):
 
     to tests that need it.
     """
+    stage_path = mock_stage
+
     yield
 
     files_in_stage = set()
-    if os.path.exists(spack.paths.stage_path):
-        files_in_stage = set(
-            os.listdir(spack.paths.stage_path)) - ignore_stage_files
+    if os.path.exists(stage_path):
+        stage_files = os.listdir(stage_path)
+        files_in_stage = set(stage_files) - ignore_stage_files
 
     if 'disable_clean_stage_check' in request.keywords:
         # clean up after tests that are expected to be dirty
         for f in files_in_stage:
-            path = os.path.join(spack.paths.stage_path, f)
+            path = os.path.join(stage_path, f)
             remove_whatever_it_is(path)
     else:
         ignore_stage_files |= files_in_stage
diff --git a/lib/spack/spack/test/data/config.yaml b/lib/spack/spack/test/data/config.yaml
index 2fcd10ad97..501001ff00 100644
--- a/lib/spack/spack/test/data/config.yaml
+++ b/lib/spack/spack/test/data/config.yaml
@@ -5,7 +5,7 @@ config:
   - $spack/lib/spack/spack/test/data/templates
   - $spack/lib/spack/spack/test/data/templates_again
   build_stage:
-  - $tempdir
+  - $tempdir/spack-stage-test
   - /nfs/tmp2/$user
   - $spack/var/spack/stage
   source_cache: $spack/var/spack/cache
diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py
index 4fc360ec4e..f70844978e 100644
--- a/lib/spack/spack/test/llnl/util/filesystem.py
+++ b/lib/spack/spack/test/llnl/util/filesystem.py
@@ -107,6 +107,21 @@ class TestCopyTree:
 
             assert os.path.exists('dest/sub/directory/a/b/2')
 
+    def test_parent_dir(self, stage):
+        """Test copying to from a parent directory."""
+
+        # Make sure we get the right error if we try to copy a parent into
+        # a descendent directory.
+        with pytest.raises(ValueError, matches="Cannot copy"):
+            with fs.working_dir(str(stage)):
+                fs.copy_tree('source', 'source/sub/directory')
+
+        # Only point with this check is to make sure we don't try to perform
+        # the copy.
+        with pytest.raises(IOError, matches="No such file or directory"):
+            with fs.working_dir(str(stage)):
+                fs.copy_tree('foo/ba', 'foo/bar')
+
     def test_symlinks_true(self, stage):
         """Test copying with symlink preservation."""
 
diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py
index 5ea3ba65bc..c3df33dc8b 100644
--- a/lib/spack/spack/test/patch.py
+++ b/lib/spack/spack/test/patch.py
@@ -32,10 +32,10 @@ url2_archive_sha256 = 'abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcda
 
 
 @pytest.fixture()
-def mock_stage(tmpdir, monkeypatch):
-    # don't disrupt the spack install directory with tests.
-    mock_path = str(tmpdir)
-    monkeypatch.setattr(spack.paths, 'stage_path', mock_path)
+def mock_patch_stage(tmpdir_factory, monkeypatch):
+    # Don't disrupt the spack install directory with tests.
+    mock_path = str(tmpdir_factory.mktemp('mock-patch-stage'))
+    monkeypatch.setattr(spack.stage, '_stage_root', mock_path)
     return mock_path
 
 
@@ -52,7 +52,7 @@ data_path = os.path.join(spack.paths.test_path, 'data', 'patch')
      '252c0af58be3d90e5dc5e0d16658434c9efa5d20a5df6c10bf72c2d77f780866',
      None)
 ])
-def test_url_patch(mock_stage, filename, sha256, archive_sha256):
+def test_url_patch(mock_patch_stage, filename, sha256, archive_sha256):
     # Make a patch object
     url = 'file://' + filename
     pkg = spack.repo.get('patch')
@@ -61,13 +61,9 @@ def test_url_patch(mock_stage, filename, sha256, archive_sha256):
 
     # make a stage
     with Stage(url) as stage:  # TODO: url isn't used; maybe refactor Stage
-        # TODO: there is probably a better way to mock this.
-        stage.mirror_path = mock_stage  # don't disrupt the spack install
-
-        # Fake a source path and ensure the directory exists
-        with working_dir(stage.path):
-            mkdirp(spack.stage._source_path_subdir)
+        stage.mirror_path = mock_patch_stage
 
+        mkdirp(stage.source_path)
         with working_dir(stage.source_path):
             # write a file to be patched
             with open('foo.txt', 'w') as f:
diff --git a/lib/spack/spack/test/stage.py b/lib/spack/spack/test/stage.py
index 9f5e7363f5..b8b217032a 100644
--- a/lib/spack/spack/test/stage.py
+++ b/lib/spack/spack/test/stage.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 """Test that the Stage class works correctly."""
+import grp
 import os
 import collections
 import shutil
@@ -12,7 +13,7 @@ import getpass
 
 import pytest
 
-from llnl.util.filesystem import working_dir
+from llnl.util.filesystem import mkdirp, working_dir
 
 import spack.paths
 import spack.stage
@@ -20,6 +21,7 @@ import spack.util.executable
 
 from spack.resource import Resource
 from spack.stage import Stage, StageComposite, ResourceStage, DIYStage
+from spack.util.path import canonicalize_path
 
 # The following values are used for common fetch and stage mocking fixtures:
 _archive_base = 'test-files'
@@ -38,6 +40,10 @@ _include_readme = 1
 _include_hidden = 2
 _include_extra = 3
 
+# Some standard unix directory that does NOT include the username
+_non_user_root = os.path.join(os.path.sep, 'opt')
+
+
 # Mock fetch directories are expected to appear as follows:
 #
 # TMPDIR/
@@ -133,7 +139,7 @@ def check_destroy(stage, stage_name):
     assert not os.path.exists(stage_path)
 
     # tmp stage needs to remove tmp dir too.
-    if spack.stage._use_tmp_stage:
+    if not stage.managed_by_spack:
         target = os.path.realpath(stage_path)
         assert not os.path.exists(target)
 
@@ -145,153 +151,102 @@ def check_setup(stage, stage_name, archive):
     # Ensure stage was created in the spack stage directory
     assert os.path.isdir(stage_path)
 
-    if spack.stage.get_tmp_root():
-        # Check that the stage dir is really a symlink.
-        assert os.path.islink(stage_path)
-
-        # Make sure it points to a valid directory
-        target = os.path.realpath(stage_path)
-        assert os.path.isdir(target)
-        assert not os.path.islink(target)
-
-        # Make sure the directory is in the place we asked it to
-        # be (see setUp, tearDown, and use_tmp)
-        assert target.startswith(str(archive.test_tmp_dir))
+    # Make sure it points to a valid directory
+    target = os.path.realpath(stage_path)
+    assert os.path.isdir(target)
+    assert not os.path.islink(target)
 
-    else:
-        # Make sure the stage path is NOT a link for a non-tmp stage
-        assert not os.path.islink(stage_path)
+    # Make sure the directory is in the place we asked it to
+    # be (see setUp, tearDown, and use_tmp)
+    assert target.startswith(str(archive.stage_path))
 
 
 def get_stage_path(stage, stage_name):
     """Figure out where a stage should be living. This depends on
     whether it's named.
     """
+    stage_path = spack.stage.get_stage_root()
     if stage_name is not None:
         # If it is a named stage, we know where the stage should be
-        return os.path.join(spack.paths.stage_path, stage_name)
+        return os.path.join(stage_path, stage_name)
     else:
         # If it's unnamed, ensure that we ran mkdtemp in the right spot.
         assert stage.path is not None
-        assert stage.path.startswith(spack.paths.stage_path)
+        assert stage.path.startswith(stage_path)
         return stage.path
 
 
 @pytest.fixture
-def no_path_for_stage(monkeypatch):
-    """Ensure there is no accessible path for staging."""
-    def _no_stage_path(paths):
-        return None
-
-    monkeypatch.setattr(spack.stage, '_first_accessible_path', _no_stage_path)
-    yield
-
-
-@pytest.fixture
-def no_tmp_root_stage(monkeypatch):
-    """
-    Disable use of a temporary root for staging.
-
-    Note that it can be important for other tests that the previous settings be
-    restored when the test case is over.
-    """
-    monkeypatch.setattr(spack.stage, '_tmp_root', None)
-    monkeypatch.setattr(spack.stage, '_use_tmp_stage', False)
-    yield
-
-
-@pytest.fixture
-def non_user_path_for_stage(config):
-    """
-    Use a non-user path for staging.
-
-    Note that it can be important for other tests that the previous settings be
-    restored when the test case is over.
-    """
+def bad_stage_path():
+    """Temporarily ensure there is no accessible path for staging."""
     current = spack.config.get('config:build_stage')
-    spack.config.set('config', {'build_stage': ['/var/spack/non-path']},
-                     scope='user')
+    spack.config.set('config', {'build_stage': '/no/such/path'}, scope='user')
     yield
     spack.config.set('config', {'build_stage': current}, scope='user')
 
 
 @pytest.fixture
-def stage_path_for_stage(config):
-    """
-    Use the basic stage_path for staging.
+def non_user_path_for_stage(monkeypatch):
+    """Temporarily use a Linux-standard non-user path for staging. """
+    def _can_access(path, perms):
+        return True
 
-    Note that it can be important for other tests that the previous settings be
-    restored when the test case is over.
-    """
     current = spack.config.get('config:build_stage')
-    spack.config.set('config',
-                     {'build_stage': spack.paths.stage_path}, scope='user')
+    spack.config.set('config', {'build_stage': [_non_user_root]}, scope='user')
+    monkeypatch.setattr(os, 'access', _can_access)
     yield
     spack.config.set('config', {'build_stage': current}, scope='user')
 
 
 @pytest.fixture
-def tmp_path_for_stage(tmpdir, config):
+def instance_path_for_stage():
     """
-    Use a built-in, temporary, test directory for staging.
+    Temporarily use the "traditional" spack instance stage path for staging.
 
     Note that it can be important for other tests that the previous settings be
     restored when the test case is over.
     """
     current = spack.config.get('config:build_stage')
-    spack.config.set('config', {'build_stage': [str(tmpdir)]}, scope='user')
+    base = canonicalize_path(os.path.join('$spack', 'test-stage'))
+    mkdirp(base)
+    path = tempfile.mkdtemp(dir=base)
+    spack.config.set('config', {'build_stage': path}, scope='user')
     yield
     spack.config.set('config', {'build_stage': current}, scope='user')
+    shutil.rmtree(base)
 
 
 @pytest.fixture
-def tmp_root_stage(monkeypatch):
-    """
-    Enable use of a temporary root for staging.
-
-    Note that it can be important for other tests that the previous settings be
-    restored when the test case is over.
-    """
-    monkeypatch.setattr(spack.stage, '_tmp_root', None)
-    monkeypatch.setattr(spack.stage, '_use_tmp_stage', True)
-    yield
-
-
-@pytest.fixture
-def tmpdir_for_stage(config, mock_stage_archive):
+def tmp_path_for_stage(tmpdir):
     """
-    Use the mock_stage_archive's temporary directory for staging.
+    Use a temporary test directory for staging.
 
     Note that it can be important for other tests that the previous settings be
     restored when the test case is over.
     """
-    archive = mock_stage_archive()
     current = spack.config.get('config:build_stage')
-    spack.config.set(
-        'config',
-        {'build_stage': [str(archive.test_tmp_dir)]},
-        scope='user')
-    yield
+    spack.config.set('config', {'build_stage': [str(tmpdir)]}, scope='user')
+    yield tmpdir
     spack.config.set('config', {'build_stage': current}, scope='user')
 
 
 @pytest.fixture
-def tmp_build_stage_dir(tmpdir, config):
+def tmp_build_stage_dir(tmpdir):
     """Establish the temporary build_stage for the mock archive."""
-    test_tmp_path = tmpdir.join('tmp')
+    test_stage_path = tmpdir.join('stage')
 
-    # Set test_tmp_path as the default test directory to use for stages.
+    # Set test_stage_path as the default directory to use for test stages.
     current = spack.config.get('config:build_stage')
     spack.config.set('config',
-                     {'build_stage': [str(test_tmp_path)]}, scope='user')
+                     {'build_stage': [str(test_stage_path)]}, scope='user')
 
-    yield (tmpdir, test_tmp_path)
+    yield (tmpdir, test_stage_path)
 
     spack.config.set('config', {'build_stage': current}, scope='user')
 
 
 @pytest.fixture
-def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request):
+def mock_stage_archive(clear_stage_root, tmp_build_stage_dir, request):
     """
     Create the directories and files for the staged mock archive.
 
@@ -301,7 +256,7 @@ def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request):
     # Mock up a stage area that looks like this:
     #
     # tmpdir/                test_files_dir
-    #     tmp/               test_tmp_path (where stage should be)
+    #     stage/             test_stage_path (where stage should be)
     #     <_archive_base>/   archive_dir_path
     #         <_readme_fn>   Optional test_readme (contains _readme_contents)
     #     <_extra_fn>        Optional extra file (contains _extra_contents)
@@ -309,8 +264,8 @@ def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request):
     #     <_archive_fn>      archive_url = file:///path/to/<_archive_fn>
     #
     def create_stage_archive(expected_file_list=[_include_readme]):
-        tmpdir, test_tmp_path = tmp_build_stage_dir
-        test_tmp_path.ensure(dir=True)
+        tmpdir, test_stage_path = tmp_build_stage_dir
+        test_stage_path.ensure(dir=True)
 
         # Create the archive directory and associated file
         archive_dir = tmpdir.join(_archive_base)
@@ -349,10 +304,10 @@ def mock_stage_archive(tmp_build_stage_dir, tmp_root_stage, request):
             tar(*tar_args)
 
         Archive = collections.namedtuple(
-            'Archive', ['url', 'tmpdir', 'test_tmp_dir', 'archive_dir']
+            'Archive', ['url', 'tmpdir', 'stage_path', 'archive_dir']
         )
         return Archive(url=archive_url, tmpdir=tmpdir,
-                       test_tmp_dir=test_tmp_path, archive_dir=archive_dir)
+                       stage_path=test_stage_path, archive_dir=archive_dir)
 
     return create_stage_archive
 
@@ -368,22 +323,33 @@ def mock_noexpand_resource(tmpdir):
 @pytest.fixture
 def mock_expand_resource(tmpdir):
     """Sets up an expandable resource in tmpdir prior to staging."""
-    resource_dir = tmpdir.join('resource-expand')
+    # Mock up an expandable resource:
+    #
+    # tmpdir/                    test_files_dir
+    #     resource-expand/       resource source dir
+    #         resource-file.txt  resource contents (contains 'test content')
+    #     resource.tar.gz        archive of resource content
+    #
+    subdir = 'resource-expand'
+    resource_dir = tmpdir.join(subdir)
+    resource_dir.ensure(dir=True)
+
     archive_name = 'resource.tar.gz'
     archive = tmpdir.join(archive_name)
     archive_url = 'file://' + str(archive)
-    test_file = resource_dir.join('resource-file.txt')
-    resource_dir.ensure(dir=True)
+
+    filename = 'resource-file.txt'
+    test_file = resource_dir.join(filename)
     test_file.write('test content\n')
 
     with tmpdir.as_cwd():
         tar = spack.util.executable.which('tar', required=True)
-        tar('czf', str(archive_name), 'resource-expand')
+        tar('czf', str(archive_name), subdir)
 
     MockResource = collections.namedtuple(
         'MockResource', ['url', 'files'])
 
-    return MockResource(archive_url, ['resource-file.txt'])
+    return MockResource(archive_url, [filename])
 
 
 @pytest.fixture
@@ -404,7 +370,7 @@ def composite_stage_with_expanding_resource(
     resource_stage = ResourceStage(
         test_resource_fetcher, root_stage, test_resource)
     composite_stage.append(resource_stage)
-    return composite_stage, root_stage, resource_stage
+    return composite_stage, root_stage, resource_stage, mock_expand_resource
 
 
 @pytest.fixture
@@ -445,7 +411,6 @@ class TestStage(object):
 
     stage_name = 'spack-test-stage'
 
-    @pytest.mark.usefixtures('tmpdir_for_stage')
     def test_setup_and_destroy_name_with_tmp(self, mock_stage_archive):
         archive = mock_stage_archive()
         with Stage(archive.url, name=self.stage_name) as stage:
@@ -458,14 +423,12 @@ class TestStage(object):
             check_setup(stage, self.stage_name, archive)
         check_destroy(stage, self.stage_name)
 
-    @pytest.mark.usefixtures('tmpdir_for_stage')
     def test_setup_and_destroy_no_name_with_tmp(self, mock_stage_archive):
         archive = mock_stage_archive()
         with Stage(archive.url) as stage:
             check_setup(stage, None, archive)
         check_destroy(stage, None)
 
-    @pytest.mark.usefixtures('tmpdir_for_stage')
     def test_noexpand_stage_file(
             self, mock_stage_archive, mock_noexpand_resource):
         """When creating a stage with a nonexpanding URL, the 'archive_file'
@@ -479,7 +442,6 @@ class TestStage(object):
             assert os.path.exists(stage.archive_file)
 
     @pytest.mark.disable_clean_stage_check
-    @pytest.mark.usefixtures('tmpdir_for_stage')
     def test_composite_stage_with_noexpand_resource(
             self, mock_stage_archive, mock_noexpand_resource):
         archive = mock_stage_archive()
@@ -505,12 +467,10 @@ class TestStage(object):
             os.path.join(composite_stage.source_path, resource_dst_name))
 
     @pytest.mark.disable_clean_stage_check
-    @pytest.mark.usefixtures('tmpdir_for_stage')
     def test_composite_stage_with_expand_resource(
-            self, mock_stage_archive, mock_expand_resource,
-            composite_stage_with_expanding_resource):
+            self, composite_stage_with_expanding_resource):
 
-        composite_stage, root_stage, resource_stage = (
+        composite_stage, root_stage, resource_stage, mock_resource = (
             composite_stage_with_expanding_resource)
 
         composite_stage.create()
@@ -519,23 +479,24 @@ class TestStage(object):
 
         assert composite_stage.expanded  # Archive is expanded
 
-        for fname in mock_expand_resource.files:
+        for fname in mock_resource.files:
             file_path = os.path.join(
                 root_stage.source_path, 'resource-dir', fname)
             assert os.path.exists(file_path)
 
+        # Perform a little cleanup
+        shutil.rmtree(root_stage.path)
+
     @pytest.mark.disable_clean_stage_check
-    @pytest.mark.usefixtures('tmpdir_for_stage')
     def test_composite_stage_with_expand_resource_default_placement(
-            self, mock_stage_archive, mock_expand_resource,
-            composite_stage_with_expanding_resource):
+            self, composite_stage_with_expanding_resource):
         """For a resource which refers to a compressed archive which expands
         to a directory, check that by default the resource is placed in
         the source_path of the root stage with the name of the decompressed
         directory.
         """
 
-        composite_stage, root_stage, resource_stage = (
+        composite_stage, root_stage, resource_stage, mock_resource = (
             composite_stage_with_expanding_resource)
 
         resource_stage.resource.placement = None
@@ -544,11 +505,14 @@ class TestStage(object):
         composite_stage.fetch()
         composite_stage.expand_archive()
 
-        for fname in mock_expand_resource.files:
+        for fname in mock_resource.files:
             file_path = os.path.join(
                 root_stage.source_path, 'resource-expand', fname)
             assert os.path.exists(file_path)
 
+        # Perform a little cleanup
+        shutil.rmtree(root_stage.path)
+
     def test_setup_and_destroy_no_name_without_tmp(self, mock_stage_archive):
         archive = mock_stage_archive()
         with Stage(archive.url) as stage:
@@ -710,42 +674,108 @@ class TestStage(object):
         assert source_path.endswith(spack.stage._source_path_subdir)
         assert not os.path.exists(source_path)
 
-    def test_first_accessible_path_error(self):
-        """Test _first_accessible_path handling of an OSError."""
-        with tempfile.NamedTemporaryFile() as _file:
-            assert spack.stage._first_accessible_path([_file.name]) is None
+    def test_first_accessible_path(self, tmpdir):
+        """Test _first_accessible_path names."""
+        spack_dir = tmpdir.join('spack-test-fap')
+        name = str(spack_dir)
+        files = [os.path.join(os.path.sep, 'no', 'such', 'path'), name]
+
+        # Ensure the tmpdir path is returned since the user should have access
+        path = spack.stage._first_accessible_path(files)
+        assert path == name
+        assert os.path.isdir(path)
+
+        # Ensure an existing path is returned
+        spack_subdir = spack_dir.join('existing').ensure(dir=True)
+        subdir = str(spack_subdir)
+        path = spack.stage._first_accessible_path([subdir])
+        assert path == subdir
+
+        # Cleanup
+        shutil.rmtree(str(name))
 
-    def test_get_tmp_root_no_use(self, no_tmp_root_stage):
-        """Ensure not using tmp root results in no path."""
-        assert spack.stage.get_tmp_root() is None
+    def test_first_accessible_perms(self, tmpdir):
+        """Test _first_accessible_path permissions."""
+        name = str(tmpdir.join('first', 'path'))
+        path = spack.stage._first_accessible_path([name])
 
-    def test_get_tmp_root_no_stage_path(self, tmp_root_stage,
-                                        no_path_for_stage):
-        """Ensure using tmp root with no stage path raises StageError."""
+        # Ensure the non-existent path was created
+        assert path == name
+        assert os.path.isdir(path)
+
+        # Ensure the non-existent subdirectories have their parent's perms
+        prefix = str(tmpdir)
+        status = os.stat(prefix)
+        group = grp.getgrgid(status.st_gid)[0]
+        parts = path[len(prefix):].split(os.path.sep)
+        for part in parts:
+            prefix = os.path.join(prefix, part)
+            prefix_status = os.stat(prefix)
+            assert group == grp.getgrgid(os.stat(prefix).st_gid)[0]
+            assert status.st_mode == prefix_status.st_mode
+
+        # Cleanup
+        shutil.rmtree(os.path.dirname(name))
+
+    def test_get_stage_root_bad_path(self, clear_stage_root, bad_stage_path):
+        """Ensure an invalid stage path root raises a StageError."""
         with pytest.raises(spack.stage.StageError,
                            match="No accessible stage paths in"):
-            spack.stage.get_tmp_root()
-
-    def test_get_tmp_root_non_user_path(self, tmp_root_stage,
-                                        non_user_path_for_stage):
-        """Ensure build_stage of tmp root with non-user path includes user."""
-        path = spack.stage.get_tmp_root()
-        assert path.endswith(os.path.join(getpass.getuser(), 'spack-stage'))
-
-    def test_get_tmp_root_use(self, tmp_root_stage, tmp_path_for_stage):
-        """Ensure build_stage of tmp root provides has right ancestors."""
-        path = spack.stage.get_tmp_root()
-        shutil.rmtree(path)
-        assert path.rfind('test_get_tmp_root_use') > 0
-        assert path.endswith('spack-stage')
-
-    def test_get_tmp_root_stage_path(self, tmp_root_stage,
-                                     stage_path_for_stage):
-        """
-        Ensure build_stage of tmp root with stage_path means use local path.
-        """
-        assert spack.stage.get_tmp_root() is None
-        assert not spack.stage._use_tmp_stage
+            assert spack.stage.get_stage_root() is None
+
+        # Make sure the cached stage path values are unchanged.
+        assert spack.stage._stage_root is None
+
+    def test_get_stage_root_non_user_path(self, clear_stage_root,
+                                          non_user_path_for_stage):
+        """Ensure a non-user stage root includes the username."""
+        # The challenge here is whether the user has access to the standard
+        # non-user path.  If not, the path should still appear in the error.
+        try:
+            path = spack.stage.get_stage_root()
+            assert getpass.getuser() in path.split(os.path.sep)
+
+            # Make sure the cached stage path values are changed appropriately.
+            assert spack.stage._stage_root == path
+        except OSError as e:
+            expected = os.path.join(_non_user_root, getpass.getuser())
+            assert expected in str(e)
+
+            # Make sure the cached stage path values are unchanged.
+            assert spack.stage._stage_root is None
+
+    def test_get_stage_root_tmp(self, clear_stage_root, tmp_path_for_stage):
+        """Ensure a temp path stage root is a suitable temp path."""
+        assert spack.stage._stage_root is None
+
+        tmpdir = tmp_path_for_stage
+        path = spack.stage.get_stage_root()
+        assert path == str(tmpdir)
+        assert 'test_get_stage_root_tmp' in path
+
+        # Make sure the cached stage path values are changed appropriately.
+        assert spack.stage._stage_root == path
+
+        # Add then purge a couple of directories
+        dir1 = tmpdir.join('dir1')
+        dir1.ensure(dir=True)
+        dir2 = tmpdir.join('dir2')
+        dir2.ensure(dir=True)
+
+        spack.stage.purge()
+        assert not os.path.exists(str(dir1))
+        assert not os.path.exists(str(dir2))
+
+    def test_get_stage_root_in_spack(self, clear_stage_root,
+                                     instance_path_for_stage):
+        """Ensure an instance path stage root is a suitable path."""
+        assert spack.stage._stage_root is None
+
+        path = spack.stage.get_stage_root()
+        assert 'spack' in path.split(os.path.sep)
+
+        # Make sure the cached stage path values are changed appropriately.
+        assert spack.stage._stage_root == path
 
     def test_stage_constructor_no_fetcher(self):
         """Ensure Stage constructor with no URL or fetch strategy fails."""
@@ -814,3 +844,41 @@ class TestStage(object):
         assert os.path.isfile(readmefn)
         with open(readmefn) as _file:
             _file.read() == _readme_contents
+
+
+@pytest.fixture
+def tmp_build_stage_nondir(tmpdir):
+    """Establish the temporary build_stage pointing to non-directory."""
+    test_stage_path = tmpdir.join('stage', 'afile')
+    test_stage_path.ensure(dir=False)
+
+    # Set test_stage_path as the default directory to use for test stages.
+    current = spack.config.get('config:build_stage')
+    stage_dir = os.path.dirname(str(test_stage_path))
+    spack.config.set('config', {'build_stage': [stage_dir]}, scope='user')
+
+    yield test_stage_path
+
+    spack.config.set('config', {'build_stage': current}, scope='user')
+
+
+def test_stage_create_replace_path(tmp_build_stage_dir):
+    """Ensure stage creation replaces a non-directory path."""
+    _, test_stage_path = tmp_build_stage_dir
+    test_stage_path.ensure(dir=True)
+
+    nondir = test_stage_path.join('afile')
+    nondir.ensure(dir=False)
+    path = str(nondir)
+
+    stage = Stage(path, name='')
+    stage.create()  # Should ensure the path is converted to a dir
+
+    assert os.path.isdir(stage.path)
+
+
+def test_cannot_access():
+    """Ensure can_access dies with the expected error."""
+    with pytest.raises(SystemExit, matches='Insufficient permissions'):
+        # It's far more portable to use a non-existent filename.
+        spack.stage.ensure_access('/no/such/file')
diff --git a/lib/spack/spack/test/util/spack_lock_wrapper.py b/lib/spack/spack/test/util/spack_lock_wrapper.py
index 1a0e41c7f6..c64af82e37 100644
--- a/lib/spack/spack/test/util/spack_lock_wrapper.py
+++ b/lib/spack/spack/test/util/spack_lock_wrapper.py
@@ -38,6 +38,8 @@ def test_disable_locking(tmpdir):
     assert old_value == spack.config.get('config:locks')
 
 
+# "Disable" mock_stage fixture to avoid subdir permissions issues on cleanup.
+@pytest.mark.nomockstage
 def test_lock_checks_user(tmpdir):
     """Ensure lock checks work with a self-owned, self-group repo."""
     uid = os.getuid()
@@ -70,6 +72,8 @@ def test_lock_checks_user(tmpdir):
     lk.check_lock_safety(path)
 
 
+# "Disable" mock_stage fixture to avoid subdir permissions issues on cleanup.
+@pytest.mark.nomockstage
 def test_lock_checks_group(tmpdir):
     """Ensure lock checks work with a self-owned, non-self-group repo."""
     uid = os.getuid()