From 390112fc7631b02a7d72b6920e44bd984bbccc6f 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: [PATCH] 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(-)

diff --git a/lib/spack/spack/test/util/file_cache.py b/lib/spack/spack/test/util/file_cache.py
index 024ce329b31947..201eb46cba346c 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 file not found error
+    with pytest.raises((IOError, OSError), 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 759d150045ab02..dc89afdde73a65 100644
--- a/lib/spack/spack/util/file_cache.py
+++ b/lib/spack/spack/util/file_cache.py
@@ -144,8 +144,7 @@ def __exit__(cm, type, value, traceback):
                 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)