summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHarmen Stoppels <harmenstoppels@gmail.com>2021-08-27 21:41:24 +0200
committerGitHub <noreply@github.com>2021-08-27 12:41:24 -0700
commitf5ab3ad82a62585589296ff37ebf7af1aed8303f (patch)
tree5137fad892bb70257e458e166bab11bd7d745c67
parentb5d3c488241e8fec1133aadea73d6bd0a6e244f5 (diff)
downloadspack-f5ab3ad82a62585589296ff37ebf7af1aed8303f.tar.gz
spack-f5ab3ad82a62585589296ff37ebf7af1aed8303f.tar.bz2
spack-f5ab3ad82a62585589296ff37ebf7af1aed8303f.tar.xz
spack-f5ab3ad82a62585589296ff37ebf7af1aed8303f.zip
Fix: --overwrite backs up old install dir, but it gets destroyed anyways (#25583)
* Make sure PackageInstaller does not remove the just-restored install dir after failure in spack install --overwrite * Remove cryptic error message and rethrow actual error
-rw-r--r--lib/spack/llnl/util/filesystem.py7
-rw-r--r--lib/spack/spack/installer.py3
-rw-r--r--lib/spack/spack/test/install.py25
3 files changed, 30 insertions, 5 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index a81425fe96..e44f8c608f 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -692,7 +692,7 @@ def replace_directory_transaction(directory_name, tmp_root=None):
try:
yield tmp_dir
- except (Exception, KeyboardInterrupt, SystemExit) as e:
+ except (Exception, KeyboardInterrupt, SystemExit):
# Delete what was there, before copying back the original content
if os.path.exists(directory_name):
shutil.rmtree(directory_name)
@@ -701,10 +701,7 @@ def replace_directory_transaction(directory_name, tmp_root=None):
dst=os.path.dirname(directory_name)
)
tty.debug('DIRECTORY RECOVERED [{0}]'.format(directory_name))
-
- msg = 'the transactional move of "{0}" failed.'
- msg += '\n ' + str(e)
- raise RuntimeError(msg.format(directory_name))
+ raise
else:
# Otherwise delete the temporary directory
shutil.rmtree(tmp_dir)
diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py
index 8ae2fd410d..eb409caffe 100644
--- a/lib/spack/spack/installer.py
+++ b/lib/spack/spack/installer.py
@@ -1569,6 +1569,9 @@ class PackageInstaller(object):
if os.path.exists(rec.path):
with fs.replace_directory_transaction(
rec.path):
+ # fs transaction will put the old prefix
+ # back on failure, so make sure to keep it.
+ keep_prefix = True
self._install_task(task)
else:
tty.debug("Missing installation to overwrite")
diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py
index f71c77a35b..67d50d2273 100644
--- a/lib/spack/spack/test/install.py
+++ b/lib/spack/spack/test/install.py
@@ -126,6 +126,31 @@ def test_partial_install_delete_prefix_and_stage(install_mockery, mock_fetch):
pkg.remove_prefix = instance_rm_prefix
+@pytest.mark.disable_clean_stage_check
+def test_failing_overwrite_install_should_keep_previous_installation(
+ mock_fetch, install_mockery
+):
+ """
+ Make sure that whenever `spack install --overwrite` fails, spack restores
+ the original install prefix instead of cleaning it.
+ """
+ # Do a successful install
+ spec = Spec('canfail').concretized()
+ pkg = spack.repo.get(spec)
+ pkg.succeed = True
+
+ # Do a failing overwrite install
+ pkg.do_install()
+ pkg.succeed = False
+ kwargs = {'overwrite': [spec.dag_hash()]}
+
+ with pytest.raises(Exception):
+ pkg.do_install(**kwargs)
+
+ assert pkg.installed
+ assert os.path.exists(spec.prefix)
+
+
def test_dont_add_patches_to_installed_package(install_mockery, mock_fetch):
dependency = Spec('dependency-install')
dependency.concretize()