ensure the staging dir exists for spack stage -p <PATH> (#23963)

* ensure that the stage root exists for `spack stage -p <PATH>`

* add test to verify `spack stage -p <PATH>` works!

* move out shared tmp staging path setup to a fixture to fix the test

lib/spack/spack/cmd/stage.py

@@ -3,12 +3,15 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import os
+
 import llnl.util.tty as tty
 
 import spack.environment as ev
 import spack.repo
 import spack.cmd
 import spack.cmd.common.arguments as arguments
+import spack.stage
 
 description = "expand downloaded archive in preparation for install"
 section = "build"
@@ -24,6 +27,12 @@ def setup_parser(subparser):
 
 
 def stage(parser, args):
+    # We temporarily modify the working directory when setting up a stage, so we need to
+    # convert this to an absolute path here in order for it to remain valid later.
+    custom_path = os.path.abspath(args.path) if args.path else None
+    if custom_path:
+        spack.stage.create_stage_root(custom_path)
+
     if not args.specs:
         env = ev.get_env(args, 'stage')
         if env:
@@ -44,12 +53,12 @@ def stage(parser, args):
     specs = spack.cmd.parse_specs(args.specs, concretize=False)
 
     # prevent multiple specs from extracting in the same folder
-    if len(specs) > 1 and args.path:
+    if len(specs) > 1 and custom_path:
         tty.die("`--path` requires a single spec, but multiple were provided")
 
     for spec in specs:
         spec = spack.cmd.matching_spec_from_env(spec)
         package = spack.repo.get(spec)
-        if args.path:
+        if custom_path:
-            package.path = args.path
+            package.path = custom_path
         package.do_stage()

lib/spack/spack/stage.py

@@ -44,7 +44,8 @@
 stage_prefix = 'spack-stage-'
 
 
-def _create_stage_root(path):
+def create_stage_root(path):
+    # type: (str) -> None
     """Create the stage root directory and ensure appropriate access perms."""
     assert path.startswith(os.path.sep) and len(path.strip()) > 1
 
@@ -99,6 +100,15 @@ def _create_stage_root(path):
             tty.warn("Expected user {0} to own {1}, but it is owned by {2}"
                      .format(user_uid, p, p_stat.st_uid))
 
+    spack_src_subdir = os.path.join(path, _source_path_subdir)
+    # When staging into a user-specified directory with `spack stage -p <PATH>`, we need
+    # to ensure the `spack-src` subdirectory exists, as we can't rely on it being
+    # created automatically by spack. It's not clear why this is the case for `spack
+    # stage -p`, but since `mkdirp()` is idempotent, this should not change the behavior
+    # for any other code paths.
+    if not os.path.isdir(spack_src_subdir):
+        mkdirp(spack_src_subdir, mode=stat.S_IRWXU)
+
 
 def _first_accessible_path(paths):
     """Find the first path that is accessible, creating it if necessary."""
@@ -110,7 +120,7 @@ def _first_accessible_path(paths):
                     return path
             else:
                 # Now create the stage root with the proper group/perms.
-                _create_stage_root(path)
+                create_stage_root(path)
                 return path
 
         except OSError as e:

lib/spack/spack/test/cmd/stage.py

@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import os
 import pytest
 from spack.main import SpackCommand
 import spack.environment as ev
@@ -31,27 +32,30 @@ def fake_stage(pkg, mirror_only=False):
     assert len(expected) == 0
 
 
-def test_stage_path(monkeypatch):
+@pytest.fixture(scope='function')
-    """Verify that --path only works with single specs."""
+def check_stage_path(monkeypatch, tmpdir):
+    expected_path = os.path.join(str(tmpdir), 'x')
 
     def fake_stage(pkg, mirror_only=False):
-        assert pkg.path == 'x'
+        assert pkg.path == expected_path
+        assert os.path.isdir(expected_path), expected_path
 
     monkeypatch.setattr(spack.package.PackageBase, 'do_stage', fake_stage)
 
-    stage('--path=x', 'trivial-install-test-package')
+    return expected_path
 
 
-def test_stage_path_errors_multiple_specs(monkeypatch):
+def test_stage_path(check_stage_path):
     """Verify that --path only works with single specs."""
+    stage('--path={0}'.format(check_stage_path), 'trivial-install-test-package')
 
-    def fake_stage(pkg, mirror_only=False):
-        pass
-
-    monkeypatch.setattr(spack.package.PackageBase, 'do_stage', fake_stage)
 
+def test_stage_path_errors_multiple_specs(check_stage_path):
+    """Verify that --path only works with single specs."""
     with pytest.raises(spack.main.SpackCommandError):
-        stage('--path=x', 'trivial-install-test-package', 'mpileaks')
+        stage('--path={0}'.format(check_stage_path),
+              'trivial-install-test-package',
+              'mpileaks')
 
 
 def test_stage_with_env_outside_env(mutable_mock_env_path, monkeypatch):

lib/spack/spack/test/stage.py

@@ -691,14 +691,14 @@ def test_first_accessible_path(self, tmpdir):
         shutil.rmtree(str(name))
 
     def test_create_stage_root(self, tmpdir, no_path_access):
-        """Test _create_stage_root permissions."""
+        """Test create_stage_root permissions."""
         test_dir = tmpdir.join('path')
         test_path = str(test_dir)
 
         try:
             if getpass.getuser() in str(test_path).split(os.sep):
                 # Simply ensure directory created if tmpdir includes user
-                spack.stage._create_stage_root(test_path)
+                spack.stage.create_stage_root(test_path)
                 assert os.path.exists(test_path)
 
                 p_stat = os.stat(test_path)
@@ -706,7 +706,7 @@ def test_create_stage_root(self, tmpdir, no_path_access):
             else:
                 # Ensure an OS Error is raised on created, non-user directory
                 with pytest.raises(OSError) as exc_info:
-                    spack.stage._create_stage_root(test_path)
+                    spack.stage.create_stage_root(test_path)
 
                 assert exc_info.value.errno == errno.EACCES
         finally:
@@ -748,10 +748,10 @@ def _stat(path):
         #
         #  with monkeypatch.context() as m:
         #      m.setattr(os, 'stat', _stat)
-        #      spack.stage._create_stage_root(user_path)
+        #      spack.stage.create_stage_root(user_path)
         #      assert os.stat(user_path).st_uid != os.getuid()
         monkeypatch.setattr(os, 'stat', _stat)
-        spack.stage._create_stage_root(user_path)
+        spack.stage.create_stage_root(user_path)
 
         # The following check depends on the patched os.stat as a poor
         # substitute for confirming the generated warnings.