Mover add user to _resolve_paths; remove non-user path test

lib/spack/spack/stage.py

@@ -91,18 +91,29 @@ def _first_accessible_path(paths):
 
 
 def _resolve_paths(candidates):
-    """Resolve paths, removing extra $user from $tempdir if needed."""
+    """
+    Resolve candidate paths and make user-related adjustments.
+
+    Adjustments involve removing extra $user from $tempdir if $tempdir includes
+    $user and appending $user if it is not present in the path.
+    """
     temp_path = sup.canonicalize_path('$tempdir')
-    tmp_has_usr = getpass.getuser() in temp_path.split(os.path.sep)
+    user = getpass.getuser()
+    tmp_has_usr = user in temp_path.split(os.path.sep)
 
     paths = []
     for path in candidates:
-        # First remove the extra `$user` node from a `$tempdir/$user` entry
-        # for hosts that automatically append `$user` to `$tempdir`.
+        # Remove the extra `$user` node from a `$tempdir/$user` entry for
+        # hosts that automatically append `$user` to `$tempdir`.
         if path.startswith(os.path.join('$tempdir', '$user')) and tmp_has_usr:
             path = os.path.join('$tempdir', path[15:])
 
-        paths.append(sup.canonicalize_path(path))
+        # Ensure the path is unique per user.
+        can_path = sup.canonicalize_path(path)
+        if user not in can_path:
+            can_path = os.path.join(can_path, user)
+
+        paths.append(can_path)
 
     return paths
 
@@ -125,14 +136,6 @@ def get_stage_root():
             raise StageError("No accessible stage paths in:",
                              ' '.join(resolved_candidates))
 
-        # Ensure that path is unique per user in an attempt to avoid
-        # conflicts in shared temporary spaces.  Emulate permissions from
-        # `tempfile.mkdtemp`.
-        user = getpass.getuser()
-        if user not in path:
-            path = os.path.join(path, user)
-            mkdirp(path, mode=stat.S_IRWXU)
-
         _stage_root = path
 
     return _stage_root

lib/spack/spack/test/stage.py

@@ -40,9 +40,6 @@
 _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:
 #
@@ -70,16 +67,6 @@
 #
 
 
-@pytest.fixture
-def mock_always_access_path(monkeypatch):
-    """Mock the ability to always access a path (for test coverage)."""
-    def _can_access(path, perms):
-        return True
-
-    monkeypatch.setattr(os, 'access', _can_access)
-    yield
-
-
 @pytest.fixture
 def clear_stage_root(monkeypatch):
     """Ensure spack.stage._stage_root is not set at test start."""
@@ -695,20 +682,34 @@ def test_first_accessible_path(self, tmpdir):
         # Cleanup
         shutil.rmtree(str(name))
 
+    @pytest.mark.nomockstage
     def test_resolve_paths(self):
         """Test _resolve_paths."""
 
         assert spack.stage._resolve_paths([]) == []
 
+        # resolved path without user appends user
         paths = [os.path.join(os.path.sep, 'a', 'b', 'c')]
+        user = getpass.getuser()
+        can_paths = [os.path.join(paths[0], user)]
+        assert spack.stage._resolve_paths(paths) == can_paths
+
+        # resolved path with node including user does not append user
+        paths = [os.path.join(os.path.sep, 'spack-{0}'.format(user), 'stage')]
         assert spack.stage._resolve_paths(paths) == paths
 
+        # resolve paths where user 
         tmp = '$tempdir'
+        can_tmpdir = canonicalize_path(tmp)
+        temp_has_user = user in can_tmpdir.split(os.sep)
         paths = [os.path.join(tmp, 'stage'), os.path.join(tmp, '$user')]
         can_paths = [canonicalize_path(paths[0]), canonicalize_path(tmp)]
         user = getpass.getuser()
-        if user not in can_paths[1].split(os.path.sep):
-            can_paths[1] = os.path.join(can_paths[1], user)
+        if temp_has_user:
+            can_paths[1] = can_tmpdir
+        else:
+            can_paths[0] = os.path.join(can_paths[0], user)
+
         assert spack.stage._resolve_paths(paths) == can_paths
 
     @pytest.mark.nomockstage
@@ -722,28 +723,6 @@ def test_get_stage_root_bad_path(self, clear_stage_root):
         # Make sure the cached stage path values are unchanged.
         assert spack.stage._stage_root is None
 
-    @pytest.mark.nomockstage
-    def test_get_stage_root_non_user_path(self, clear_stage_root,
-                                          mock_always_access_path):
-        """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:
-            with spack.config.override('config:build_stage', _non_user_root):
-                path = spack.stage.get_stage_root()
-
-                assert getpass.getuser() in path.split(os.path.sep)
-
-                # Ensure 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
-
     @pytest.mark.nomockstage
     @pytest.mark.parametrize(
         'path,purged', [('stage-1234567890abcdef1234567890abcdef', True),