environment: handle view root at existing directory better (#45263)

- remove empty dir if exists at view root
- error better if non-empty dir

Co-authored-by: Harmen Stoppels <me@harmenstoppels.nl>

2 files changed, 43 insertions, 0 deletions

diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index e0002fd0ee..65a08ff7fa 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -5,6 +5,7 @@
 import collections
 import collections.abc
 import contextlib
+import errno
 import os
 import pathlib
 import re
@@ -788,6 +789,23 @@ class ViewDescriptor:
         root_dirname = os.path.dirname(self.root)
         tmp_symlink_name = os.path.join(root_dirname, "._view_link")
 
+        # Remove self.root if is it an empty dir, since we need a symlink there. Note that rmdir
+        # fails if self.root is a symlink.
+        try:
+            os.rmdir(self.root)
+        except (FileNotFoundError, NotADirectoryError):
+            pass
+        except OSError as e:
+            if e.errno == errno.ENOTEMPTY:
+                msg = "it is a non-empty directory"
+            elif e.errno == errno.EACCES:
+                msg = "of insufficient permissions"
+            else:
+                raise
+            raise SpackEnvironmentViewError(
+                f"The environment view in {self.root} cannot not be created because {msg}."
+            ) from e
+
         # Create a new view
         try:
             fs.mkdirp(new_root)
diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py
index c09393376a..46be9ca66c 100644
--- a/lib/spack/spack/test/env.py
+++ b/lib/spack/spack/test/env.py
@@ -841,3 +841,28 @@ def test_root_version_weights_for_old_versions(mutable_mock_env_path, mock_packa
 
     assert bowtie.satisfies("@=1.3.0")
     assert gcc.satisfies("@=1.0")
+
+
+def test_env_view_on_empty_dir_is_fine(tmp_path, config, mock_packages, temporary_store):
+    """Tests that creating a view pointing to an empty dir is not an error."""
+    view_dir = tmp_path / "view"
+    view_dir.mkdir()
+    env = ev.create_in_dir(tmp_path, with_view="view")
+    env.add("mpileaks")
+    env.concretize()
+    env.install_all(fake=True)
+    env.regenerate_views()
+    assert view_dir.is_symlink()
+
+
+def test_env_view_on_non_empty_dir_errors(tmp_path, config, mock_packages, temporary_store):
+    """Tests that creating a view pointing to a non-empty dir errors."""
+    view_dir = tmp_path / "view"
+    view_dir.mkdir()
+    (view_dir / "file").write_text("")
+    env = ev.create_in_dir(tmp_path, with_view="view")
+    env.add("mpileaks")
+    env.concretize()
+    env.install_all(fake=True)
+    with pytest.raises(ev.SpackEnvironmentError, match="because it is a non-empty dir"):
+        env.regenerate_views()