diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/environment/environment.py | 34 | ||||
| -rw-r--r-- | lib/spack/spack/test/cmd/env.py | 25 |
2 files changed, 46 insertions, 13 deletions
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 4feecfab53..ea1dc85d4a 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -524,22 +524,30 @@ class ViewDescriptor(object): tty.msg("Updating view at {0}".format(self.root)) view = self.view(new=new_root) - fs.mkdirp(new_root) - view.add_specs(*specs, with_dependencies=False) - # create symlink from tmpname to new_root root_dirname = os.path.dirname(self.root) tmp_symlink_name = os.path.join(root_dirname, '._view_link') - if os.path.exists(tmp_symlink_name): - os.unlink(tmp_symlink_name) - symlink(new_root, tmp_symlink_name) - - # mv symlink atomically over root symlink to old_root - if os.path.exists(self.root) and not os.path.islink(self.root): - msg = "Cannot create view: " - msg += "file already exists and is not a link: %s" % self.root - raise SpackEnvironmentViewError(msg) - rename(tmp_symlink_name, self.root) + + # Create a new view + try: + fs.mkdirp(new_root) + view.add_specs(*specs, with_dependencies=False) + + # create symlink from tmp_symlink_name to new_root + if os.path.exists(tmp_symlink_name): + os.unlink(tmp_symlink_name) + symlink(new_root, tmp_symlink_name) + + # mv symlink atomically over root symlink to old_root + rename(tmp_symlink_name, self.root) + except Exception as e: + # Clean up new view and temporary symlink on any failure. + try: + shutil.rmtree(new_root, ignore_errors=True) + os.unlink(tmp_symlink_name) + except (IOError, OSError): + pass + raise e # Remove the old root when it's in the same folder as the new root. This guards # against removal of an arbitrary path when the original symlink in self.root diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index cf2c947bdc..7315a9d7f1 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2746,3 +2746,28 @@ def test_env_view_fail_if_symlink_points_elsewhere(tmpdir, install_mockery, mock add('libelf') install('--fake') assert os.path.isdir(non_view_dir) + + +def test_failed_view_cleanup(tmpdir, mock_stage, mock_fetch, install_mockery): + """Tests whether Spack cleans up after itself when a view fails to create""" + view = str(tmpdir.join('view')) + with ev.create('env', with_view=view): + add('libelf') + install('--fake') + + # Save the current view directory. + resolved_view = os.path.realpath(view) + all_views = os.path.dirname(resolved_view) + views_before = os.listdir(all_views) + + # Add a spec that results in MergeConflictError's when creating a view + with ev.read('env'): + add('libelf cflags=-O3') + with pytest.raises(llnl.util.link_tree.MergeConflictError): + install('--fake') + + # Make sure there is no broken view in the views directory, and the current + # view is the original view from before the failed regenerate attempt. + views_after = os.listdir(all_views) + assert views_before == views_after + assert os.path.samefile(resolved_view, view) |