summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorJonathon Anderson <17242663+blue42u@users.noreply.github.com>2023-01-10 06:36:12 -0600
committerGitHub <noreply@github.com>2023-01-10 13:36:12 +0100
commit6879c35d1c7cdb479d578b219b3cdd4c0139d950 (patch)
treed5ef1003f9a4ecb5b19b596a7bcdb4fce1dfc09b /lib
parentf1b8bc97f0414d335344852f744c5a1186738f32 (diff)
downloadspack-6879c35d1c7cdb479d578b219b3cdd4c0139d950.tar.gz
spack-6879c35d1c7cdb479d578b219b3cdd4c0139d950.tar.bz2
spack-6879c35d1c7cdb479d578b219b3cdd4c0139d950.tar.xz
spack-6879c35d1c7cdb479d578b219b3cdd4c0139d950.zip
FileCache: Delete the new cache file on exception (#34623)
The code in FileCache for write_transaction attempts to delete the temporary file when an exception occurs under the context by calling shutil.rmtree. However, rmtree only operates on directories while the rest of FileCache uses normal files. This causes an empty file to be written to the cache key when unneeded. Use os.remove instead which operates on normal files. Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/test/util/file_cache.py21
-rw-r--r--lib/spack/spack/util/file_cache.py3
2 files changed, 22 insertions, 2 deletions
diff --git a/lib/spack/spack/test/util/file_cache.py b/lib/spack/spack/test/util/file_cache.py
index 024ce329b3..1b28cd760d 100644
--- a/lib/spack/spack/test/util/file_cache.py
+++ b/lib/spack/spack/test/util/file_cache.py
@@ -32,6 +32,27 @@ def test_write_and_read_cache_file(file_cache):
assert text == "foobar\n"
+@pytest.mark.skipif(sys.platform == "win32", reason="Locks not supported on Windows")
+def test_failed_write_and_read_cache_file(file_cache):
+ """Test failing to write then attempting to read a cached file."""
+ with pytest.raises(RuntimeError, match=r"^foobar$"):
+ with file_cache.write_transaction("test.yaml") as (old, new):
+ assert old is None
+ assert new is not None
+ raise RuntimeError("foobar")
+
+ # Cache dir should have exactly one (lock) file
+ assert os.listdir(file_cache.root) == [".test.yaml.lock"]
+
+ # File does not exist
+ assert not file_cache.init_entry("test.yaml")
+
+ # Attempting to read will cause a FileNotFoundError
+ with pytest.raises(FileNotFoundError, match=r"test\.yaml"):
+ with file_cache.read_transaction("test.yaml"):
+ pass
+
+
def test_write_and_remove_cache_file(file_cache):
"""Test two write transactions on a cached file. Then try to remove an
entry from it.
diff --git a/lib/spack/spack/util/file_cache.py b/lib/spack/spack/util/file_cache.py
index 759d150045..dc89afdde7 100644
--- a/lib/spack/spack/util/file_cache.py
+++ b/lib/spack/spack/util/file_cache.py
@@ -144,8 +144,7 @@ class FileCache(object):
cm.tmp_file.close()
if value:
- # remove tmp on exception & raise it
- shutil.rmtree(cm.tmp_filename, True)
+ os.remove(cm.tmp_filename)
else:
rename(cm.tmp_filename, cm.orig_filename)