summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHarmen Stoppels <harmenstoppels@gmail.com>2022-03-24 22:42:33 +0100
committerGitHub <noreply@github.com>2022-03-24 15:42:33 -0600
commite22fbdb6b8e6692989aeccf693c6ff1de0485680 (patch)
tree292e8fbf294446a862709d2771bbabe817501086
parentf8201f4acc53bbb7efcde77a369546181b802d19 (diff)
downloadspack-e22fbdb6b8e6692989aeccf693c6ff1de0485680.tar.gz
spack-e22fbdb6b8e6692989aeccf693c6ff1de0485680.tar.bz2
spack-e22fbdb6b8e6692989aeccf693c6ff1de0485680.tar.xz
spack-e22fbdb6b8e6692989aeccf693c6ff1de0485680.zip
environment.py: ensure view dir does not exist (#29641)
-rw-r--r--lib/spack/spack/environment/__init__.py2
-rw-r--r--lib/spack/spack/environment/environment.py26
-rw-r--r--lib/spack/spack/test/cmd/env.py38
-rw-r--r--lib/spack/spack/test/environment.py35
4 files changed, 101 insertions, 0 deletions
diff --git a/lib/spack/spack/environment/__init__.py b/lib/spack/spack/environment/__init__.py
index 4b475cab08..b710979013 100644
--- a/lib/spack/spack/environment/__init__.py
+++ b/lib/spack/spack/environment/__init__.py
@@ -5,6 +5,7 @@
from .environment import (
Environment,
SpackEnvironmentError,
+ SpackEnvironmentViewError,
activate,
active,
active_environment,
@@ -33,6 +34,7 @@ from .environment import (
__all__ = [
'Environment',
'SpackEnvironmentError',
+ 'SpackEnvironmentViewError',
'activate',
'active',
'active_environment',
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 20f6060265..72df9cbfe3 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -8,6 +8,7 @@ import copy
import os
import re
import shutil
+import stat
import sys
import time
@@ -336,6 +337,29 @@ def _spec_needs_overwrite(spec, changed_dev_specs):
return True
+def _error_on_nonempty_view_dir(new_root):
+ """Defensively error when the target view path already exists and is not an
+ empty directory. This usually happens when the view symlink was removed, but
+ not the directory it points to. In those cases, it's better to just error when
+ the new view dir is non-empty, since it indicates the user removed part but not
+ all of the view, and it likely in an inconsistent state."""
+ # Check if the target path lexists
+ try:
+ st = os.lstat(new_root)
+ except (IOError, OSError):
+ return
+
+ # Empty directories are fine
+ if stat.S_ISDIR(st.st_mode) and len(os.listdir(new_root)) == 0:
+ return
+
+ # Anything else is an error
+ raise SpackEnvironmentViewError(
+ "Failed to generate environment view, because the target {} already "
+ "exists or is not empty. To update the view, remove this path, and run "
+ "`spack env view regenerate`".format(new_root))
+
+
class ViewDescriptor(object):
def __init__(self, base_path, root, projections={}, select=[], exclude=[],
link=default_view_link, link_type='symlink'):
@@ -519,6 +543,8 @@ class ViewDescriptor(object):
tty.debug("View at %s does not need regeneration." % self.root)
return
+ _error_on_nonempty_view_dir(new_root)
+
# construct view at new_root
if specs:
tty.msg("Updating view at {0}".format(self.root))
diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py
index 7853cb16f2..7f05e22545 100644
--- a/lib/spack/spack/test/cmd/env.py
+++ b/lib/spack/spack/test/cmd/env.py
@@ -4,6 +4,7 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import glob
import os
+import shutil
import sys
from argparse import Namespace
@@ -2805,3 +2806,40 @@ def test_failed_view_cleanup(tmpdir, mock_stage, mock_fetch, install_mockery):
views_after = os.listdir(all_views)
assert views_before == views_after
assert os.path.samefile(resolved_view, view)
+
+
+def test_environment_view_target_already_exists(
+ tmpdir, mock_stage, mock_fetch, install_mockery
+):
+ """When creating a new view, Spack should check whether
+ the new view dir already exists. If so, it should not be
+ removed or modified."""
+
+ # Create a new environment
+ view = str(tmpdir.join('view'))
+ env('create', '--with-view={0}'.format(view), 'test')
+ with ev.read('test'):
+ add('libelf')
+ install('--fake')
+
+ # Empty the underlying view
+ real_view = os.path.realpath(view)
+ assert os.listdir(real_view) # make sure it had *some* contents
+ shutil.rmtree(real_view)
+
+ # Replace it with something new.
+ os.mkdir(real_view)
+ fs.touch(os.path.join(real_view, 'file'))
+
+ # Remove the symlink so Spack can't know about the "previous root"
+ os.unlink(view)
+
+ # Regenerate the view, which should realize it can't write into the same dir.
+ msg = 'Failed to generate environment view'
+ with ev.read('test'):
+ with pytest.raises(ev.SpackEnvironmentViewError, match=msg):
+ env('view', 'regenerate')
+
+ # Make sure the dir was left untouched.
+ assert not os.path.lexists(view)
+ assert os.listdir(real_view) == ['file']
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index 2040fb90ea..36bd87bc7e 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -3,9 +3,16 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+import os
import pickle
+import pytest
+
from spack.environment import Environment
+from spack.environment.environment import (
+ SpackEnvironmentViewError,
+ _error_on_nonempty_view_dir,
+)
def test_environment_pickle(tmpdir):
@@ -13,3 +20,31 @@ def test_environment_pickle(tmpdir):
obj = pickle.dumps(env1)
env2 = pickle.loads(obj)
assert isinstance(env2, Environment)
+
+
+def test_error_on_nonempty_view_dir(tmpdir):
+ """Error when the target is not an empty dir"""
+ with tmpdir.as_cwd():
+ os.mkdir("empty_dir")
+ os.mkdir("nonempty_dir")
+ with open(os.path.join("nonempty_dir", "file"), "wb"):
+ pass
+ os.symlink("empty_dir", "symlinked_empty_dir")
+ os.symlink("does_not_exist", "broken_link")
+ os.symlink("broken_link", "file")
+
+ # This is OK.
+ _error_on_nonempty_view_dir("empty_dir")
+
+ # This is not OK.
+ with pytest.raises(SpackEnvironmentViewError):
+ _error_on_nonempty_view_dir("nonempty_dir")
+
+ with pytest.raises(SpackEnvironmentViewError):
+ _error_on_nonempty_view_dir("symlinked_empty_dir")
+
+ with pytest.raises(SpackEnvironmentViewError):
+ _error_on_nonempty_view_dir("broken_link")
+
+ with pytest.raises(SpackEnvironmentViewError):
+ _error_on_nonempty_view_dir("file")