environments: don't replace relative view path with absolute path on concretize/install (#34958)

* environments: don't rewrite relative view path, expand path on cli ahead of time

Currently if you have a spack.yaml that specifies a view by relative
path, Spack expands it to an absolute path on `spack -e . install` and
persists that to disk.

This is rather annoying when you have a `spack.yaml` file inside a git
repo, cause you want to use relative paths to make it relocatable, but
you constantly have to undo the changes made to spack.yaml by Spack.

So, as an alternative:

1. Always stick to paths as they are provided in spack.yaml, never
   replace them with a canonicalized version
2. Turn relative paths on the command line into absolute paths before
   storing to spack.yaml. This way you can do `spack env create --dir
   ./env --with-view ./view` and both `./env` and `./view` are resolved
   to the current working dir, as expected (not `./env/view`). This
   corresponds to the old behavior of `spack env create`.

* create --with-view always takes a value

4 files changed, 48 insertions, 4 deletions

diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py
index 16e2838b67..fd8bf2de56 100644
--- a/lib/spack/spack/cmd/env.py
+++ b/lib/spack/spack/cmd/env.py
@@ -291,7 +291,11 @@ def env_create_setup_parser(subparser):
 
 def env_create(args):
     if args.with_view:
-        with_view = args.with_view
+        # Expand relative paths provided on the command line to the current working directory
+        # This way we interpret `spack env create --with-view ./view --dir ./env` as
+        # a view in $PWD/view, not $PWD/env/view. This is different from specifying a relative
+        # path in spack.yaml, which is resolved relative to the environment file.
+        with_view = os.path.abspath(args.with_view)
     elif args.without_view:
         with_view = False
     else:
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 79188c01d9..f81232feec 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -384,7 +384,8 @@ class ViewDescriptor(object):
         link_type="symlink",
     ):
         self.base = base_path
-        self.root = spack.util.path.canonicalize_path(root)
+        self.raw_root = root
+        self.root = spack.util.path.canonicalize_path(root, default_wd=base_path)
         self.projections = projections
         self.select = select
         self.exclude = exclude
@@ -410,7 +411,7 @@ class ViewDescriptor(object):
         )
 
     def to_dict(self):
-        ret = syaml.syaml_dict([("root", self.root)])
+        ret = syaml.syaml_dict([("root", self.raw_root)])
         if self.projections:
             # projections guaranteed to be ordered dict if true-ish
             # for python2.6, may be syaml or ruamel.yaml implementation
@@ -2127,7 +2128,7 @@ class Environment(object):
         # Construct YAML representation of view
         default_name = default_view_name
         if self.views and len(self.views) == 1 and default_name in self.views:
-            path = self.default_view.root
+            path = self.default_view.raw_root
             if self.default_view == ViewDescriptor(self.path, self.view_path_default):
                 view = True
             elif self.default_view == ViewDescriptor(self.path, path):
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index df1de8beeb..19b53c0573 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -3229,3 +3229,10 @@ def test_env_include_packages_url(
 
         cfg = spack.config.get("packages")
         assert "openmpi" in cfg["all"]["providers"]["mpi"]
+
+
+def test_relative_view_path_on_command_line_is_made_absolute(tmpdir, config):
+    with fs.working_dir(str(tmpdir)):
+        env("create", "--with-view", "view", "--dir", "env")
+        environment = ev.Environment(os.path.join(".", "env"))
+        assert os.path.samefile("view", environment.default_view.root)
diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py
index aebad49253..83538e7d71 100644
--- a/lib/spack/spack/test/env.py
+++ b/lib/spack/spack/test/env.py
@@ -4,10 +4,13 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 """Test environment internals without CLI"""
 import io
+import os
 import sys
 
 import pytest
 
+import llnl.util.filesystem as fs
+
 import spack.environment as ev
 import spack.spec
 
@@ -111,3 +114,32 @@ def test_activate_should_require_an_env():
 
     with pytest.raises(TypeError):
         ev.activate(env=None)
+
+
+def test_user_view_path_is_not_canonicalized_in_yaml(tmpdir, config):
+    # When spack.yaml files are checked into version control, we
+    # don't want view: ./relative to get canonicalized on disk.
+
+    # We create a view in <tmpdir>/env_dir
+    env_path = tmpdir.mkdir("env_dir").strpath
+
+    # And use a relative path to specify the view dir
+    view = os.path.join(".", "view")
+
+    # Which should always resolve to the following independent of cwd.
+    absolute_view = os.path.join(env_path, "view")
+
+    # Serialize environment with relative view path
+    with fs.working_dir(str(tmpdir)):
+        fst = ev.Environment(env_path, with_view=view)
+        fst.write()
+
+    # The view link should be created
+    assert os.path.isdir(absolute_view)
+
+    # Deserialize and check if the view path is still relative in yaml
+    # and also check that the getter is pointing to the right dir.
+    with fs.working_dir(str(tmpdir)):
+        snd = ev.Environment(env_path)
+        assert snd.yaml["spack"]["view"] == view
+        assert os.path.samefile(snd.default_view.root, absolute_view)