`spack develop`: stage build artifacts in same root as non-dev builds (#41373)

Currently (outside of this PR) when you `spack develop` a path, this path is treated as the staging
directory (this means that for example all build artifacts are placed in the develop path).

This PR creates a separate staging directory for all `spack develop`ed builds. It looks like

```
# the stage root
/the-stage-root-for-all-spack-builds/
    spack-stage-<hash>
        # Spack packages inheriting CMakePackage put their build artifacts here
        spack-build-<hash>/
```

Unlike non-develop builds, there is no `spack-src` directory, `source_path` is the provided `dev_path`.
Instead, separately, in the `dev_path`, we have:

```
/dev/path/for/foo/
    build-{arch}-<hash> -> /the-stage-root-for-all-spack-builds/spack-stage-<hash>/
```

The main benefit of this is that build artifacts for out-of-source builds that are relative to
`Stage.path` are easily identified (and you can delete them with `spack clean`).

Other behavior added here:

- [x] A symlink is made from the `dev_path` to the stage directory. This symlink name incorporates
    spec details, so that multiple Spack environments that develop the same path will not conflict
    with one another

- [x] `spack cd` and `spack location` have added a `-c` shorthand for `--source-dir`

Spack builds can still change the develop path (in particular to keep track of applied patches), 
and for in-source builds, this doesn't change much (although logs would not be written into 
the develop path). Packages inheriting from `CMakePackage` should get this benefit
automatically though.

10 files changed, 297 insertions, 127 deletions

diff --git a/lib/spack/spack/cmd/location.py b/lib/spack/spack/cmd/location.py
index d2bac9b9ba..d774784622 100644
--- a/lib/spack/spack/cmd/location.py
+++ b/lib/spack/spack/cmd/location.py
@@ -53,6 +53,7 @@ def setup_parser(subparser):
         "-S", "--stages", action="store_true", help="top level stage directory"
     )
     directories.add_argument(
+        "-c",
         "--source-dir",
         action="store_true",
         help="source directory for a spec (requires it to be staged first)",
diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py
index 9e9f4728e1..2f8e0ae003 100644
--- a/lib/spack/spack/package_base.py
+++ b/lib/spack/spack/package_base.py
@@ -64,7 +64,7 @@ from spack.install_test import (
     install_test_root,
 )
 from spack.installer import InstallError, PackageInstaller
-from spack.stage import DIYStage, ResourceStage, Stage, StageComposite, compute_stage_name
+from spack.stage import DevelopStage, ResourceStage, Stage, StageComposite, compute_stage_name
 from spack.util.executable import ProcessError, which
 from spack.util.package_hash import package_hash
 from spack.version import GitVersion, StandardVersion
@@ -1075,7 +1075,12 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
         # If it's a dev package (not transitively), use a DIY stage object
         dev_path_var = self.spec.variants.get("dev_path", None)
         if dev_path_var:
-            return DIYStage(dev_path_var.value)
+            dev_path = dev_path_var.value
+            link_format = spack.config.get("config:develop_stage_link")
+            if not link_format:
+                link_format = "build-{arch}-{hash:7}"
+            stage_link = self.spec.format_path(link_format)
+            return DevelopStage(compute_stage_name(self.spec), dev_path, stage_link)
 
         # To fetch the current version
         source_stage = self._make_root_stage(self.fetcher)
@@ -1407,7 +1412,7 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
             return
 
         checksum = spack.config.get("config:checksum")
-        fetch = self.stage.managed_by_spack
+        fetch = self.stage.needs_fetching
         if (
             checksum
             and fetch
@@ -1480,9 +1485,6 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
         if self.has_code:
             self.do_fetch(mirror_only)
             self.stage.expand_archive()
-
-            if not os.listdir(self.stage.path):
-                raise spack.error.FetchError("Archive was empty for %s" % self.name)
         else:
             # Support for post-install hooks requires a stage.source_path
             fsys.mkdirp(self.stage.source_path)
@@ -1516,7 +1518,7 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
         # If we encounter an archive that failed to patch, restage it
         # so that we can apply all the patches again.
         if os.path.isfile(bad_file):
-            if self.stage.managed_by_spack:
+            if self.stage.requires_patch_success:
                 tty.debug("Patching failed last time. Restaging.")
                 self.stage.restage()
             else:
@@ -1537,6 +1539,8 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
             tty.msg("No patches needed for {0}".format(self.name))
             return
 
+        errors = []
+
         # Apply all the patches for specs that match this one
         patched = False
         for patch in patches:
@@ -1546,12 +1550,16 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
                 tty.msg("Applied patch {0}".format(patch.path_or_url))
                 patched = True
             except spack.error.SpackError as e:
-                tty.debug(e)
-
                 # Touch bad file if anything goes wrong.
-                tty.msg("Patch %s failed." % patch.path_or_url)
                 fsys.touch(bad_file)
-                raise
+                error_msg = f"Patch {patch.path_or_url} failed."
+                if self.stage.requires_patch_success:
+                    tty.msg(error_msg)
+                    raise
+                else:
+                    tty.debug(error_msg)
+                    tty.debug(e)
+                    errors.append(e)
 
         if has_patch_fun:
             try:
@@ -1569,24 +1577,29 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta):
                     # printed a message for each patch.
                     tty.msg("No patches needed for {0}".format(self.name))
             except spack.error.SpackError as e:
-                tty.debug(e)
-
                 # Touch bad file if anything goes wrong.
-                tty.msg("patch() function failed for {0}".format(self.name))
                 fsys.touch(bad_file)
-                raise
-
-        # Get rid of any old failed file -- patches have either succeeded
-        # or are not needed.  This is mostly defensive -- it's needed
-        # if the restage() method doesn't clean *everything* (e.g., for a repo)
-        if os.path.isfile(bad_file):
-            os.remove(bad_file)
-
-        # touch good or no patches file so that we skip next time.
-        if patched:
-            fsys.touch(good_file)
-        else:
-            fsys.touch(no_patches_file)
+                error_msg = f"patch() function failed for {self.name}"
+                if self.stage.requires_patch_success:
+                    tty.msg(error_msg)
+                    raise
+                else:
+                    tty.debug(error_msg)
+                    tty.debug(e)
+                    errors.append(e)
+
+        if not errors:
+            # Get rid of any old failed file -- patches have either succeeded
+            # or are not needed.  This is mostly defensive -- it's needed
+            # if we didn't restage
+            if os.path.isfile(bad_file):
+                os.remove(bad_file)
+
+            # touch good or no patches file so that we skip next time.
+            if patched:
+                fsys.touch(good_file)
+            else:
+                fsys.touch(no_patches_file)
 
     @classmethod
     def all_patches(cls):
diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py
index 2a6499058d..b7fca09938 100644
--- a/lib/spack/spack/schema/config.py
+++ b/lib/spack/spack/schema/config.py
@@ -63,6 +63,7 @@ properties: Dict[str, Any] = {
                 "oneOf": [{"type": "string"}, {"type": "array", "items": {"type": "string"}}]
             },
             "stage_name": {"type": "string"},
+            "develop_stage_link": {"type": "string"},
             "test_stage": {"type": "string"},
             "extensions": {"type": "array", "items": {"type": "string"}},
             "template_dirs": {"type": "array", "items": {"type": "string"}},
diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py
index b756255bd6..46848c0016 100644
--- a/lib/spack/spack/stage.py
+++ b/lib/spack/spack/stage.py
@@ -208,7 +208,103 @@ def _mirror_roots():
     ]
 
 
-class Stage:
+class LockableStagingDir:
+    """A directory whose lifetime can be managed with a context
+    manager (but persists if the user requests it). Instances can have
+    a specified name and if they do, then for all instances that have
+    the same name, only one can enter the context manager at a time.
+    """
+
+    def __init__(self, name, path, keep, lock):
+        # 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())
+
+        # 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(get_stage_root(), self.name)
+
+        # Flag to decide whether to delete the stage folder on exit or not
+        self.keep = keep
+
+        # File lock for the stage directory.  We use one file for all
+        # stage locks. See spack.database.Database.prefix_locker.lock for
+        # details on this approach.
+        self._lock = None
+        self._use_locks = lock
+
+        # When stages are reused, we need to know whether to re-create
+        # it.  This marks whether it has been created/destroyed.
+        self.created = False
+
+    def _get_lock(self):
+        if not self._lock:
+            sha1 = hashlib.sha1(self.name.encode("utf-8")).digest()
+            lock_id = prefix_bits(sha1, bit_length(sys.maxsize))
+            stage_lock_path = os.path.join(get_stage_root(), ".lock")
+            self._lock = spack.util.lock.Lock(
+                stage_lock_path, start=lock_id, length=1, desc=self.name
+            )
+        return self._lock
+
+    def __enter__(self):
+        """
+        Entering a stage context will create the stage directory
+
+        Returns:
+            self
+        """
+        if self._use_locks:
+            self._get_lock().acquire_write(timeout=60)
+        self.create()
+        return self
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        """
+        Exiting from a stage context will delete the stage directory unless:
+        - it was explicitly requested not to do so
+        - an exception has been raised
+
+        Args:
+            exc_type: exception type
+            exc_val: exception value
+            exc_tb: exception traceback
+
+        Returns:
+            Boolean
+        """
+        # Delete when there are no exceptions, unless asked to keep.
+        if exc_type is None and not self.keep:
+            self.destroy()
+
+        if self._use_locks:
+            self._get_lock().release_write()
+
+    def create(self):
+        """
+        Ensures the top-level (config:build_stage) directory exists.
+        """
+        # User has full permissions and group has only read permissions
+        if not os.path.exists(self.path):
+            mkdirp(self.path, mode=stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP)
+        elif not os.path.isdir(self.path):
+            os.remove(self.path)
+            mkdirp(self.path, mode=stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP)
+
+        # Make sure we can actually do something with the stage we made.
+        ensure_access(self.path)
+        self.created = True
+
+    def destroy(self):
+        raise NotImplementedError(f"{self.__class__.__name__} is abstract")
+
+
+class Stage(LockableStagingDir):
     """Manages a temporary stage directory for building.
 
     A Stage object is a context manager that handles a directory where
@@ -251,7 +347,8 @@ class Stage:
     """
 
     #: Most staging is managed by Spack. DIYStage is one exception.
-    managed_by_spack = True
+    needs_fetching = True
+    requires_patch_success = True
 
     def __init__(
         self,
@@ -297,6 +394,8 @@ class Stage:
               The search function that provides the fetch strategy
               instance.
         """
+        super().__init__(name, path, keep, lock)
+
         # TODO: fetch/stage coupling needs to be reworked -- the logic
         # TODO: here is convoluted and not modular enough.
         if isinstance(url_or_fetch_strategy, str):
@@ -314,72 +413,8 @@ class Stage:
 
         self.srcdir = None
 
-        # 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_paths = mirror_paths
 
-        # 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(get_stage_root(), self.name)
-
-        # Flag to decide whether to delete the stage folder on exit or not
-        self.keep = keep
-
-        # File lock for the stage directory.  We use one file for all
-        # stage locks. See spack.database.Database.prefix_locker.lock for
-        # details on this approach.
-        self._lock = None
-        if lock:
-            sha1 = hashlib.sha1(self.name.encode("utf-8")).digest()
-            lock_id = prefix_bits(sha1, bit_length(sys.maxsize))
-            stage_lock_path = os.path.join(get_stage_root(), ".lock")
-            self._lock = spack.util.lock.Lock(
-                stage_lock_path, start=lock_id, length=1, desc=self.name
-            )
-
-        # When stages are reused, we need to know whether to re-create
-        # it.  This marks whether it has been created/destroyed.
-        self.created = False
-
-    def __enter__(self):
-        """
-        Entering a stage context will create the stage directory
-
-        Returns:
-            self
-        """
-        if self._lock is not None:
-            self._lock.acquire_write(timeout=60)
-        self.create()
-        return self
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        """
-        Exiting from a stage context will delete the stage directory unless:
-        - it was explicitly requested not to do so
-        - an exception has been raised
-
-        Args:
-            exc_type: exception type
-            exc_val: exception value
-            exc_tb: exception traceback
-
-        Returns:
-            Boolean
-        """
-        # Delete when there are no exceptions, unless asked to keep.
-        if exc_type is None and not self.keep:
-            self.destroy()
-
-        if self._lock is not None:
-            self._lock.release_write()
-
     @property
     def expected_archive_files(self):
         """Possible archive file paths."""
@@ -631,21 +666,6 @@ class Stage:
         """
         self.fetcher.reset()
 
-    def create(self):
-        """
-        Ensures the top-level (config:build_stage) directory exists.
-        """
-        # User has full permissions and group has only read permissions
-        if not os.path.exists(self.path):
-            mkdirp(self.path, mode=stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP)
-        elif not os.path.isdir(self.path):
-            os.remove(self.path)
-            mkdirp(self.path, mode=stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP)
-
-        # Make sure we can actually do something with the stage we made.
-        ensure_access(self.path)
-        self.created = True
-
     def destroy(self):
         """Removes this stage directory."""
         remove_linked_tree(self.path)
@@ -752,7 +772,8 @@ class StageComposite(pattern.Composite):
                 "cache_mirror",
                 "steal_source",
                 "disable_mirrors",
-                "managed_by_spack",
+                "needs_fetching",
+                "requires_patch_success",
             ]
         )
 
@@ -808,8 +829,8 @@ class DIYStage:
     directory naming convention.
     """
 
-    """DIY staging is, by definition, not managed by Spack."""
-    managed_by_spack = False
+    needs_fetching = False
+    requires_patch_success = False
 
     def __init__(self, path):
         if path is None:
@@ -857,6 +878,65 @@ class DIYStage:
         tty.debug("Sources for DIY stages are not cached")
 
 
+class DevelopStage(LockableStagingDir):
+    needs_fetching = False
+    requires_patch_success = False
+
+    def __init__(self, name, dev_path, reference_link):
+        super().__init__(name=name, path=None, keep=False, lock=True)
+        self.dev_path = dev_path
+        self.source_path = dev_path
+
+        # The path of a link that will point to this stage
+        if os.path.isabs(reference_link):
+            link_path = reference_link
+        else:
+            link_path = os.path.join(self.source_path, reference_link)
+        if not os.path.isdir(os.path.dirname(link_path)):
+            raise StageError(f"The directory containing {link_path} must exist")
+        self.reference_link = link_path
+
+    @property
+    def archive_file(self):
+        return None
+
+    def fetch(self, *args, **kwargs):
+        tty.debug("No fetching needed for develop stage.")
+
+    def check(self):
+        tty.debug("No checksum needed for develop stage.")
+
+    def expand_archive(self):
+        tty.debug("No expansion needed for develop stage.")
+
+    @property
+    def expanded(self):
+        """Returns True since the source_path must exist."""
+        return True
+
+    def create(self):
+        super().create()
+        try:
+            llnl.util.symlink.symlink(self.path, self.reference_link)
+        except (llnl.util.symlink.AlreadyExistsError, FileExistsError):
+            pass
+
+    def destroy(self):
+        # Destroy all files, but do not follow symlinks
+        try:
+            shutil.rmtree(self.path)
+        except FileNotFoundError:
+            pass
+        self.created = False
+
+    def restage(self):
+        self.destroy()
+        self.create()
+
+    def cache_local(self):
+        tty.debug("Sources for Develop stages are not cached")
+
+
 def ensure_access(file):
     """Ensure we can access a directory and die with an error if we can't."""
     if not can_access(file):
diff --git a/lib/spack/spack/test/cmd/dev_build.py b/lib/spack/spack/test/cmd/dev_build.py
index 0a717fa747..3e6631cf51 100644
--- a/lib/spack/spack/test/cmd/dev_build.py
+++ b/lib/spack/spack/test/cmd/dev_build.py
@@ -41,6 +41,7 @@ def test_dev_build_basics(tmpdir, install_mockery):
     assert os.path.exists(str(tmpdir))
 
 
+@pytest.mark.disable_clean_stage_check
 def test_dev_build_before(tmpdir, install_mockery):
     spec = spack.spec.Spec(f"dev-build-test-install@0.0.0 dev_path={tmpdir}").concretized()
 
@@ -57,6 +58,7 @@ def test_dev_build_before(tmpdir, install_mockery):
     assert not os.path.exists(spec.prefix)
 
 
+@pytest.mark.disable_clean_stage_check
 def test_dev_build_until(tmpdir, install_mockery):
     spec = spack.spec.Spec(f"dev-build-test-install@0.0.0 dev_path={tmpdir}").concretized()
 
@@ -74,6 +76,7 @@ def test_dev_build_until(tmpdir, install_mockery):
     assert not spack.store.STORE.db.query(spec, installed=True)
 
 
+@pytest.mark.disable_clean_stage_check
 def test_dev_build_until_last_phase(tmpdir, install_mockery):
     # Test that we ignore the last_phase argument if it is already last
     spec = spack.spec.Spec(f"dev-build-test-install@0.0.0 dev_path={tmpdir}").concretized()
@@ -93,6 +96,7 @@ def test_dev_build_until_last_phase(tmpdir, install_mockery):
     assert os.path.exists(str(tmpdir))
 
 
+@pytest.mark.disable_clean_stage_check
 def test_dev_build_before_until(tmpdir, install_mockery, capsys):
     spec = spack.spec.Spec(f"dev-build-test-install@0.0.0 dev_path={tmpdir}").concretized()
 
@@ -130,6 +134,7 @@ def mock_module_noop(*args):
     pass
 
 
+@pytest.mark.disable_clean_stage_check
 def test_dev_build_drop_in(tmpdir, mock_packages, monkeypatch, install_mockery, working_env):
     monkeypatch.setattr(os, "execvp", print_spack_cc)
     monkeypatch.setattr(spack.build_environment, "module", mock_module_noop)
diff --git a/lib/spack/spack/test/cmd/develop.py b/lib/spack/spack/test/cmd/develop.py
index 7d01853fd4..b090f0d21b 100644
--- a/lib/spack/spack/test/cmd/develop.py
+++ b/lib/spack/spack/test/cmd/develop.py
@@ -14,6 +14,7 @@ import spack.environment as ev
 import spack.spec
 from spack.main import SpackCommand
 
+add = SpackCommand("add")
 develop = SpackCommand("develop")
 env = SpackCommand("env")
 
@@ -192,14 +193,16 @@ def test_develop_full_git_repo(
     finally:
         spec.package.do_clean()
 
-    # Now use "spack develop": look at the resulting stage directory and make
+    # Now use "spack develop": look at the resulting dev_path and make
     # sure the git repo pulled includes the full branch history (or rather,
     # more than just one commit).
     env("create", "test")
-    with ev.read("test"):
+    with ev.read("test") as e:
+        add("git-test-commit")
         develop("git-test-commit@1.2")
 
-        location = SpackCommand("location")
-        develop_stage_dir = location("git-test-commit").strip()
-        commits = _git_commit_list(develop_stage_dir)
+        e.concretize()
+        spec = e.all_specs()[0]
+        develop_dir = spec.variants["dev_path"].value
+        commits = _git_commit_list(develop_dir)
         assert len(commits) > 1
diff --git a/lib/spack/spack/test/patch.py b/lib/spack/spack/test/patch.py
index c8e9e8ece1..3710c58303 100644
--- a/lib/spack/spack/test/patch.py
+++ b/lib/spack/spack/test/patch.py
@@ -268,7 +268,7 @@ def trigger_bad_patch(pkg):
 
 
 def test_patch_failure_develop_spec_exits_gracefully(
-    mock_packages, config, install_mockery, mock_fetch, tmpdir
+    mock_packages, config, install_mockery, mock_fetch, tmpdir, mock_stage
 ):
     """
     ensure that a failing patch does not trigger exceptions
diff --git a/lib/spack/spack/test/stage.py b/lib/spack/spack/test/stage.py
index 024d195daa..37e8fd4559 100644
--- a/lib/spack/spack/test/stage.py
+++ b/lib/spack/spack/test/stage.py
@@ -22,7 +22,7 @@ import spack.stage
 import spack.util.executable
 import spack.util.url as url_util
 from spack.resource import Resource
-from spack.stage import DIYStage, ResourceStage, Stage, StageComposite
+from spack.stage import DevelopStage, DIYStage, ResourceStage, Stage, StageComposite
 from spack.util.path import canonicalize_path
 
 # The following values are used for common fetch and stage mocking fixtures:
@@ -145,7 +145,7 @@ def check_destroy(stage, stage_name):
     assert not os.path.exists(stage_path)
 
     # tmp stage needs to remove tmp dir too.
-    if not stage.managed_by_spack:
+    if not isinstance(stage, DIYStage):
         target = os.path.realpath(stage_path)
         assert not os.path.exists(target)
 
@@ -857,6 +857,73 @@ class TestStage:
             _file.read() == _readme_contents
 
 
+def _create_files_from_tree(base, tree):
+    for name, content in tree.items():
+        sub_base = os.path.join(base, name)
+        if isinstance(content, dict):
+            os.mkdir(sub_base)
+            _create_files_from_tree(sub_base, content)
+        else:
+            assert (content is None) or (isinstance(content, str))
+            with open(sub_base, "w") as f:
+                if content:
+                    f.write(content)
+
+
+def _create_tree_from_dir_recursive(path):
+    if os.path.islink(path):
+        return os.readlink(path)
+    elif os.path.isdir(path):
+        tree = {}
+        for name in os.listdir(path):
+            sub_path = os.path.join(path, name)
+            tree[name] = _create_tree_from_dir_recursive(sub_path)
+        return tree
+    else:
+        with open(path, "r") as f:
+            content = f.read() or None
+        return content
+
+
+@pytest.fixture
+def develop_path(tmpdir):
+    dir_structure = {"a1": {"b1": None, "b2": "b1content"}, "a2": None}
+    srcdir = str(tmpdir.join("test-src"))
+    os.mkdir(srcdir)
+    _create_files_from_tree(srcdir, dir_structure)
+    yield dir_structure, srcdir
+
+
+class TestDevelopStage:
+    def test_sanity_check_develop_path(self, develop_path):
+        _, srcdir = develop_path
+        with open(os.path.join(srcdir, "a1", "b2")) as f:
+            assert f.read() == "b1content"
+
+        assert os.path.exists(os.path.join(srcdir, "a2"))
+
+    def test_develop_stage(self, develop_path, tmp_build_stage_dir):
+        """Check that (a) develop stages update the given
+        `dev_path` with a symlink that points to the stage dir and
+        (b) that destroying the stage does not destroy `dev_path`
+        """
+        devtree, srcdir = develop_path
+        stage = DevelopStage("test-stage", srcdir, reference_link="link-to-stage")
+        stage.create()
+        srctree1 = _create_tree_from_dir_recursive(stage.source_path)
+        assert os.path.samefile(srctree1["link-to-stage"], stage.path)
+        del srctree1["link-to-stage"]
+        assert srctree1 == devtree
+
+        stage.destroy()
+        # Make sure destroying the stage doesn't change anything
+        # about the path
+        assert not os.path.exists(stage.path)
+        srctree2 = _create_tree_from_dir_recursive(srcdir)
+        del srctree2["link-to-stage"]  # Note the symlink persists but is broken
+        assert srctree2 == devtree
+
+
 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
diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash
index bdc2907d42..90d17c5f48 100755
--- a/share/spack/spack-completion.bash
+++ b/share/spack/spack-completion.bash
@@ -668,7 +668,7 @@ _spack_buildcache_rebuild_index() {
 _spack_cd() {
     if $list_options
     then
-        SPACK_COMPREPLY="-h --help -m --module-dir -r --spack-root -i --install-dir -p --package-dir -P --packages -s --stage-dir -S --stages --source-dir -b --build-dir -e --env --first"
+        SPACK_COMPREPLY="-h --help -m --module-dir -r --spack-root -i --install-dir -p --package-dir -P --packages -s --stage-dir -S --stages -c --source-dir -b --build-dir -e --env --first"
     else
         _all_packages
     fi
@@ -1347,7 +1347,7 @@ _spack_load() {
 _spack_location() {
     if $list_options
     then
-        SPACK_COMPREPLY="-h --help -m --module-dir -r --spack-root -i --install-dir -p --package-dir -P --packages -s --stage-dir -S --stages --source-dir -b --build-dir -e --env --first"
+        SPACK_COMPREPLY="-h --help -m --module-dir -r --spack-root -i --install-dir -p --package-dir -P --packages -s --stage-dir -S --stages -c --source-dir -b --build-dir -e --env --first"
     else
         _all_packages
     fi
diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish
index 600213454b..84e1a4721e 100755
--- a/share/spack/spack-completion.fish
+++ b/share/spack/spack-completion.fish
@@ -880,7 +880,7 @@ complete -c spack -n '__fish_spack_using_command buildcache rebuild-index' -s k
 complete -c spack -n '__fish_spack_using_command buildcache rebuild-index' -s k -l keys -d 'if provided, key index will be updated as well as package index'
 
 # spack cd
-set -g __fish_spack_optspecs_spack_cd h/help m/module-dir r/spack-root i/install-dir p/package-dir P/packages s/stage-dir S/stages source-dir b/build-dir e/env= first
+set -g __fish_spack_optspecs_spack_cd h/help m/module-dir r/spack-root i/install-dir p/package-dir P/packages s/stage-dir S/stages c/source-dir b/build-dir e/env= first
 complete -c spack -n '__fish_spack_using_command_pos_remainder 0 cd' -f -k -a '(__fish_spack_specs)'
 complete -c spack -n '__fish_spack_using_command cd' -s h -l help -f -a help
 complete -c spack -n '__fish_spack_using_command cd' -s h -l help -d 'show this help message and exit'
@@ -898,8 +898,8 @@ complete -c spack -n '__fish_spack_using_command cd' -s s -l stage-dir -f -a sta
 complete -c spack -n '__fish_spack_using_command cd' -s s -l stage-dir -d 'stage directory for a spec'
 complete -c spack -n '__fish_spack_using_command cd' -s S -l stages -f -a stages
 complete -c spack -n '__fish_spack_using_command cd' -s S -l stages -d 'top level stage directory'
-complete -c spack -n '__fish_spack_using_command cd' -l source-dir -f -a source_dir
-complete -c spack -n '__fish_spack_using_command cd' -l source-dir -d 'source directory for a spec (requires it to be staged first)'
+complete -c spack -n '__fish_spack_using_command cd' -s c -l source-dir -f -a source_dir
+complete -c spack -n '__fish_spack_using_command cd' -s c -l source-dir -d 'source directory for a spec (requires it to be staged first)'
 complete -c spack -n '__fish_spack_using_command cd' -s b -l build-dir -f -a build_dir
 complete -c spack -n '__fish_spack_using_command cd' -s b -l build-dir -d 'build directory for a spec (requires it to be staged first)'
 complete -c spack -n '__fish_spack_using_command cd' -s e -l env -r -f -a location_env
@@ -2089,7 +2089,7 @@ complete -c spack -n '__fish_spack_using_command load' -l list -f -a list
 complete -c spack -n '__fish_spack_using_command load' -l list -d 'show loaded packages: same as `spack find --loaded`'
 
 # spack location
-set -g __fish_spack_optspecs_spack_location h/help m/module-dir r/spack-root i/install-dir p/package-dir P/packages s/stage-dir S/stages source-dir b/build-dir e/env= first
+set -g __fish_spack_optspecs_spack_location h/help m/module-dir r/spack-root i/install-dir p/package-dir P/packages s/stage-dir S/stages c/source-dir b/build-dir e/env= first
 complete -c spack -n '__fish_spack_using_command_pos_remainder 0 location' -f -k -a '(__fish_spack_specs)'
 complete -c spack -n '__fish_spack_using_command location' -s h -l help -f -a help
 complete -c spack -n '__fish_spack_using_command location' -s h -l help -d 'show this help message and exit'
@@ -2107,8 +2107,8 @@ complete -c spack -n '__fish_spack_using_command location' -s s -l stage-dir -f
 complete -c spack -n '__fish_spack_using_command location' -s s -l stage-dir -d 'stage directory for a spec'
 complete -c spack -n '__fish_spack_using_command location' -s S -l stages -f -a stages
 complete -c spack -n '__fish_spack_using_command location' -s S -l stages -d 'top level stage directory'
-complete -c spack -n '__fish_spack_using_command location' -l source-dir -f -a source_dir
-complete -c spack -n '__fish_spack_using_command location' -l source-dir -d 'source directory for a spec (requires it to be staged first)'
+complete -c spack -n '__fish_spack_using_command location' -s c -l source-dir -f -a source_dir
+complete -c spack -n '__fish_spack_using_command location' -s c -l source-dir -d 'source directory for a spec (requires it to be staged first)'
 complete -c spack -n '__fish_spack_using_command location' -s b -l build-dir -f -a build_dir
 complete -c spack -n '__fish_spack_using_command location' -s b -l build-dir -d 'build directory for a spec (requires it to be staged first)'
 complete -c spack -n '__fish_spack_using_command location' -s e -l env -r -f -a location_env