develop: canonicalize dev paths and base relative paths on env.path (#30075)

Allow environment variables and spack-specific path substitution variables (e.g. `$spack`) to be
used in the paths associated with develop specs, while maintaining the ability to keep those
paths relative to the environment rather than the working directory.

lib/spack/spack/cmd/develop.py

@@ -9,6 +9,7 @@
 
 import spack.cmd
 import spack.cmd.common.arguments as arguments
+import spack.util.path
 from spack.error import SpackError
 
 description = "add a spec to an environment's dev-build information"
@@ -52,7 +53,7 @@ def develop(parser, args):
         # download all dev specs
         for name, entry in env.dev_specs.items():
             path = entry.get("path", name)
-            abspath = path if os.path.isabs(path) else os.path.join(env.path, path)
+            abspath = spack.util.path.canonicalize_path(path, default_wd=env.path)
 
             if os.path.exists(abspath):
                 msg = "Skipping developer download of %s" % entry["spec"]
@@ -79,11 +80,7 @@ def develop(parser, args):
 
     # default path is relative path to spec.name
     path = args.path or spec.name
-
+    abspath = spack.util.path.canonicalize_path(path, default_wd=env.path)
-    # get absolute path to check
-    abspath = path
-    if not os.path.isabs(abspath):
-        abspath = os.path.join(env.path, path)
 
     # clone default: only if the path doesn't exist
     clone = args.clone

lib/spack/spack/concretize.py

@@ -38,6 +38,7 @@
 import spack.spec
 import spack.target
 import spack.tengine
+import spack.util.path
 import spack.variant as vt
 from spack.config import config
 from spack.package_prefs import PackagePrefs, is_spec_buildable, spec_externals
@@ -91,7 +92,7 @@ def concretize_develop(self, spec):
         if not dev_info:
             return False
 
-        path = os.path.normpath(os.path.join(env.path, dev_info["path"]))
+        path = spack.util.path.canonicalize_path(dev_info["path"], default_wd=env.path)
 
         if "dev_path" in spec.variants:
             assert spec.variants["dev_path"].value == path

lib/spack/spack/environment/environment.py

@@ -1217,7 +1217,7 @@ def develop(self, spec, path, clone=False):
 
         if clone:
             # "steal" the source code via staging API
-            abspath = os.path.normpath(os.path.join(self.path, path))
+            abspath = spack.util.path.canonicalize_path(path, default_wd=self.path)
 
             # Stage, at the moment, requires a concrete Spec, since it needs the
             # dag_hash for the stage dir name. Below though we ask for a stage

lib/spack/spack/solver/asp.py

@@ -47,6 +47,7 @@
 import spack.repo
 import spack.spec
 import spack.store
+import spack.util.path
 import spack.util.timer
 import spack.variant
 import spack.version
@@ -2286,7 +2287,7 @@ def _develop_specs_from_env(spec, env):
     if not dev_info:
         return
 
-    path = os.path.normpath(os.path.join(env.path, dev_info["path"]))
+    path = spack.util.path.canonicalize_path(dev_info["path"], default_wd=env.path)
 
     if "dev_path" in spec.variants:
         error_msg = (

lib/spack/spack/test/cmd/develop.py

@@ -102,3 +102,47 @@ def test_develop_update_spec(self):
             develop("mpich@2.0")
             self.check_develop(e, spack.spec.Spec("mpich@2.0"))
             assert len(e.dev_specs) == 1
+
+    def test_develop_canonicalize_path(self, monkeypatch, config):
+        env("create", "test")
+        with ev.read("test") as e:
+            path = "../$user"
+            abspath = spack.util.path.canonicalize_path(path, e.path)
+
+            def check_path(stage, dest):
+                assert dest == abspath
+
+            monkeypatch.setattr(spack.stage.Stage, "steal_source", check_path)
+
+            develop("-p", path, "mpich@1.0")
+            self.check_develop(e, spack.spec.Spec("mpich@1.0"), path)
+
+            # Check modifications actually worked
+            assert spack.spec.Spec("mpich@1.0").concretized().satisfies("dev_path=%s" % abspath)
+
+    def test_develop_canonicalize_path_no_args(self, monkeypatch, config):
+        env("create", "test")
+        with ev.read("test") as e:
+            path = "$user"
+            abspath = spack.util.path.canonicalize_path(path, e.path)
+
+            def check_path(stage, dest):
+                assert dest == abspath
+
+            monkeypatch.setattr(spack.stage.Stage, "steal_source", check_path)
+
+            # Defensive check to ensure canonicalization failures don't pollute FS
+            assert abspath.startswith(e.path)
+
+            # Create path to allow develop to modify env
+            fs.mkdirp(abspath)
+            develop("--no-clone", "-p", path, "mpich@1.0")
+
+            # Remove path to ensure develop with no args runs staging code
+            os.rmdir(abspath)
+
+            develop()
+            self.check_develop(e, spack.spec.Spec("mpich@1.0"), path)
+
+            # Check modifications actually worked
+            assert spack.spec.Spec("mpich@1.0").concretized().satisfies("dev_path=%s" % abspath)

lib/spack/spack/util/path.py

@@ -314,9 +314,13 @@ def add_padding(path, length):
     return os.path.join(path, padding)
 
 
-def canonicalize_path(path):
+def canonicalize_path(path, default_wd=None):
     """Same as substitute_path_variables, but also take absolute path.
 
+    If the string is a yaml object with file annotations, make absolute paths
+    relative to that file's directory.
+    Otherwise, use ``default_wd`` if specified, otherwise ``os.getcwd()``
+
     Arguments:
         path (str): path being converted as needed
 
@@ -335,8 +339,9 @@ def canonicalize_path(path):
         if filename:
             path = os.path.join(filename, path)
         else:
-            path = os.path.abspath(path)
+            base = default_wd or os.getcwd()
-            tty.debug("Using current working directory as base for abspath")
+            path = os.path.join(base, path)
+            tty.debug("Using working directory %s as base for abspath" % base)
 
     return os.path.normpath(path)