diff options
| -rw-r--r-- | lib/spack/spack/test/util/file_cache.py | 14 | ||||
| -rw-r--r-- | lib/spack/spack/util/file_cache.py | 9 |
2 files changed, 19 insertions, 4 deletions
diff --git a/lib/spack/spack/test/util/file_cache.py b/lib/spack/spack/test/util/file_cache.py index 106a411b69..815b21fb38 100644 --- a/lib/spack/spack/test/util/file_cache.py +++ b/lib/spack/spack/test/util/file_cache.py @@ -55,9 +55,12 @@ def test_write_and_remove_cache_file(file_cache): file_cache.remove('test.yaml') - # After removal both the file and the lock file should not exist + # After removal the file should not exist assert not os.path.exists(file_cache.cache_path('test.yaml')) - assert not os.path.exists(file_cache._lock_path('test.yaml')) + + # Whether the lock file exists is more of an implementation detail, on Linux they + # continue to exist, on Windows they don't. + # assert os.path.exists(file_cache._lock_path('test.yaml')) @pytest.mark.skipif(sys.platform == 'win32', @@ -94,3 +97,10 @@ def test_cache_write_readonly_cache_fails(file_cache): with pytest.raises(CacheError, match='Insufficient permissions to write'): file_cache.write_transaction(filename) + + +@pytest.mark.regression('31475') +def test_delete_is_idempotent(file_cache): + """Deleting a non-existent key should be idempotent, to simplify life when + running delete with multiple processes""" + file_cache.remove('test.yaml') diff --git a/lib/spack/spack/util/file_cache.py b/lib/spack/spack/util/file_cache.py index cf5ed8310e..611f428881 100644 --- a/lib/spack/spack/util/file_cache.py +++ b/lib/spack/spack/util/file_cache.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import errno import os import shutil @@ -170,13 +171,17 @@ class FileCache(object): return sinfo.st_mtime def remove(self, key): + file = self.cache_path(key) lock = self._get_lock(key) try: lock.acquire_write() - os.unlink(self.cache_path(key)) + os.unlink(file) + except OSError as e: + # File not found is OK, so remove is idempotent. + if e.errno != errno.ENOENT: + raise finally: lock.release_write() - lock.cleanup() class CacheError(SpackError): |