From 6879c35d1c7cdb479d578b219b3cdd4c0139d950 Mon Sep 17 00:00:00 2001
From: Jonathon Anderson <17242663+blue42u@users.noreply.github.com>
Date: Tue, 10 Jan 2023 06:36:12 -0600
Subject: 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>
---
 lib/spack/spack/test/util/file_cache.py | 21 +++++++++++++++++++++
 lib/spack/spack/util/file_cache.py      |  3 +--
 2 files changed, 22 insertions(+), 2 deletions(-)

(limited to 'lib')

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)
-- 
cgit v1.2.3-60-g2f50