repository: keep the store cache out of repo-create and repo-delete, refs #10396

src/borg/archiver/help_cmd.py

@@ -739,6 +739,9 @@ class HelpMixIn:
                 Set it to ``1`` to use ``$BORG_CACHE_DIR/storecache``, or to a directory path to
                 use that directory (it is created if it does not exist). Packs are named by
                 content hash, so one cache directory can safely hold packs of multiple repositories.
+                ``borg repo-delete`` removes nothing from the cache directory: the cached packs of the
+                deleted repository stay there until they are evicted (see BORG_PACK_CACHE_SIZE) or
+                the directory is removed.
                 If it is not set, no such caching happens.
             BORG_PACK_CACHE_SIZE
                 When set to a numeric value, limit the pack cache to that many bytes.

src/borg/repository.py

@@ -964,9 +964,10 @@ def __init__(
         # BORG_STORE_CACHE sets the cache directory ("1" means <cache_dir>/storecache); the
         # directory holds the whole store's cache, currently just the packs/ namespace.
         # BORG_PACK_CACHE_SIZE limits the pack cache size in bytes.
+        # create=True: no cache, Store.create() requires an empty cache directory.
         cache_url = None
         store_cache = os.environ.get("BORG_STORE_CACHE")
-        if store_cache:
+        if store_cache and not create:
             if store_cache == "1":
                 cache_dir = Path(get_cache_dir("storecache"))
             else:
@@ -1055,7 +1056,7 @@ def __enter__(self):
                 self.close(aborting=True)
                 if self.created:
                     # we just created the store, but could not open it: do not leave it behind (see create()).
-                    self.store.destroy()
+                    self._destroy_store()
                 raise
         except StoreBackendError as e:
             if self._location.proto != "ssh":
@@ -1120,7 +1121,7 @@ def create(self):
         except BaseException:
             # do not leave the just created store behind (see above); the original error is what matters.
             try:
-                self.store.destroy()
+                self._destroy_store()
             except Exception as exc:
                 logger.warning("could not remove the incompletely created store: %s", exc)
             raise
@@ -1282,10 +1283,14 @@ def delete_key(self, name):
         except StoreObjectNotFound:
             pass
 
+    def _destroy_store(self):
+        """Destroy the store's backend. The pack cache directory (BORG_STORE_CACHE) is kept."""
+        self.store.backend.destroy()
+
     def destroy(self):
         """Destroy the repository"""
         self.close()
-        self.store.destroy()
+        self._destroy_store()
 
     def open(self, *, exclusive, lock_wait=None, lock=True):
         assert lock_wait is not None

src/borg/testsuite/archiver/repo_create_cmd_test.py

@@ -168,6 +168,22 @@ def failing_save_config(self, key=None):
     assert os.listdir(keys_dir)
 
 
+def test_repo_create_with_a_filled_store_cache(archivers, request, monkeypatch):
+    # BORG_STORE_CACHE is one directory for all repositories: repo-create works while it holds cached packs.
+    archiver = request.getfixturevalue(archivers)
+    cache_dir = os.path.join(archiver.tmpdir, "storecache")
+    monkeypatch.setenv("BORG_STORE_CACHE", cache_dir)
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    create_regular_file(archiver.input_path, "file1", size=1024 * 80)
+    cmd(archiver, "create", "test", "input")
+    assert any(names for _, _, names in os.walk(os.path.join(cache_dir, "packs")))
+    archiver.repository_location += "2"
+    archiver.repository_path += "2"
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input")
+    assert "test" in cmd(archiver, "repo-list")
+
+
 def test_repo_create_writes_an_empty_chunk_index(archivers, request):
     # repo-create stores an empty chunk index (in the key's envelope), so the first use of the repository
     # does not have to build it by listing the packs.

src/borg/testsuite/archiver/repo_delete_cmd_test.py

@@ -4,7 +4,7 @@
 
 from ...constants import *  # NOQA
 from ...helpers import CancelledByUser, Error
-from . import create_regular_file, cmd, generate_archiver_tests, RK_ENCRYPTION
+from . import changedir, create_regular_file, cmd, generate_archiver_tests, RK_ENCRYPTION
 
 pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,binary")  # NOQA
 
@@ -42,6 +42,37 @@ def test_delete_repo_force(archivers, request):
     assert not os.path.exists(archiver.repository_path)
 
 
+def test_delete_repo_keeps_the_store_cache(archivers, request, monkeypatch):
+    # BORG_STORE_CACHE is one directory for all repositories: repo-delete removes nothing from it.
+    archiver = request.getfixturevalue(archivers)
+    cache_dir = os.path.join(archiver.tmpdir, "storecache")
+    monkeypatch.setenv("BORG_STORE_CACHE", cache_dir)
+
+    def cache_files():
+        return sorted(os.path.join(dirpath, name) for dirpath, _, names in os.walk(cache_dir) for name in names)
+
+    create_regular_file(archiver.input_path, "file1", size=1024 * 80)
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input")
+    kept_location, kept_path = archiver.repository_location, archiver.repository_path
+    archiver.repository_location += "2"
+    archiver.repository_path += "2"
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input")
+    os.makedirs(os.path.join(cache_dir, "foreign"))
+    with open(os.path.join(cache_dir, "foreign", "file"), "w") as fd:
+        fd.write("foreign")
+    cached = cache_files()
+    assert len(cached) > 1
+    cmd(archiver, "repo-delete")
+    assert not os.path.exists(archiver.repository_path)
+    assert cache_files() == cached
+    archiver.repository_location, archiver.repository_path = kept_location, kept_path
+    with changedir("output"):
+        cmd(archiver, "extract", "test")
+    assert os.path.exists(os.path.join("output", "input", "file1"))
+
+
 def test_delete_store_without_config(archivers, request):
     # a store without repository config (e.g. the leftover of an interrupted repo-create, or a repository
     # that lost its config) can only be deleted with --force.

src/borg/testsuite/repository_test.py

@@ -804,7 +804,10 @@ def test_gather_many_one_gather_for_many_packs(tmp_path, monkeypatch, variant):
         monkeypatch.setenv("BORG_STORE_CACHE", os.fspath(tmp_path / "storecache"))
     location = Location(f"ssh://__testsuite__/{path}" if variant == "ssh" else path)
     objects = {H(i): fchunk(b"payload-%02d" % i, chunk_id=H(i)) for i in range(5)}
-    with Repository(location, exclusive=True, create=True) as repository:
+    with Repository(location, exclusive=True, create=True):
+        pass
+    with Repository(location, exclusive=True) as repository:
+        assert repository.uses_pack_store_cache == (variant == "storecache")
         repository._pack_writer.max_count = 2  # three packs: {H0,H1} {H2,H3} {H4}
         for chunk_id, chunk in objects.items():
             repository.put(chunk_id, chunk)
@@ -3446,6 +3449,78 @@ def failing_save_config(self, key=None):
     assert os.path.exists(os.path.join(location, "config", "config"))
 
 
+def files_below(path):
+    # relative paths of all files below path.
+    paths = [os.path.join(dirpath, name) for dirpath, _, names in os.walk(path) for name in names]
+    return sorted(os.path.relpath(p, path) for p in paths)
+
+
+def repo_location(path, proto):
+    return Location(os.fspath(path) if proto == "file" else f"ssh://__testsuite__/{os.fspath(path)}")
+
+
+@pytest.fixture()
+def filled_store_cache(tmp_path, monkeypatch):
+    # a BORG_STORE_CACHE directory holding a cached pack of the repository "other" and a file borg did not put there.
+    cache_dir = tmp_path / "storecache"
+    monkeypatch.setenv("BORG_STORE_CACHE", os.fspath(cache_dir))
+    other = os.fspath(tmp_path / "other")
+    with Repository(other, exclusive=True, create=True):
+        pass
+    with Repository(other, exclusive=True) as repository:
+        repository.put(H(0), fchunk(b"other", chunk_id=H(0)))
+        repository.flush()
+    (cache_dir / "foreign").mkdir()
+    (cache_dir / "foreign" / "file").write_text("foreign")
+    files = files_below(cache_dir)
+    assert len(files) == 2 and any(name.startswith("packs") for name in files)
+    return cache_dir
+
+
+@pytest.mark.parametrize("proto", ["file", "ssh"])
+def test_create_with_a_filled_store_cache(tmp_path, filled_store_cache, proto):
+    cached = files_below(filled_store_cache)
+    location = repo_location(tmp_path / "repo", proto)
+    with Repository(location, exclusive=True, create=True) as repository:
+        assert not repository.uses_pack_store_cache
+    assert files_below(filled_store_cache) == cached
+    with Repository(location, exclusive=True) as repository:
+        assert repository.uses_pack_store_cache
+
+
+@pytest.mark.parametrize("proto", ["file", "ssh"])
+def test_destroy_keeps_the_store_cache(tmp_path, filled_store_cache, proto):
+    location = repo_location(tmp_path / "repo", proto)
+    with Repository(location, exclusive=True, create=True):
+        pass
+    with Repository(location, exclusive=True) as repository:
+        repository.put(H(1), fchunk(b"repo", chunk_id=H(1)))
+        repository.flush()
+    cached = files_below(filled_store_cache)
+    assert len(cached) == 3  # the pack of this repository was cached, too
+    with Repository(location, exclusive=True) as repository:
+        repository.destroy()
+    assert not os.path.exists(tmp_path / "repo")
+    assert files_below(filled_store_cache) == cached
+
+
+@pytest.mark.parametrize("failing", ["save_config", "open"])
+def test_create_failure_keeps_the_store_cache(tmp_path, filled_store_cache, monkeypatch, failing):
+    # failing: the Repository method that fails after the store was created.
+    def fail(self, *args, **kwargs):
+        raise OSError("simulated failure")
+
+    cached = files_below(filled_store_cache)
+    location = os.fspath(tmp_path / "repo")
+    with monkeypatch.context() as m:
+        m.setattr(Repository, failing, fail)
+        with pytest.raises(OSError, match="simulated failure"):
+            with Repository(location, exclusive=True, create=True):
+                pass
+    assert not os.path.exists(location)
+    assert files_below(filled_store_cache) == cached
+
+
 def store_damaged_pack(repository, objs, *, listed, flip=(), tail=b"", size=100):
     """Store objs as one pack named by the store hash of their bytes, then damage it.
 
@@ -3628,7 +3703,9 @@ def test_salvage_pack_drops_uncovered_trailing_bytes(salvage_repository, tail):
 def test_salvage_pack_refuses_with_a_pack_store_cache(tmp_path, monkeypatch):
     # with BORG_STORE_CACHE, both loads of a pack can return the same cached copy.
     monkeypatch.setenv("BORG_STORE_CACHE", os.fspath(tmp_path / "cache"))
-    with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True) as repository:
+    with Repository(os.fspath(tmp_path / "repo"), exclusive=True, create=True):
+        pass
+    with Repository(os.fspath(tmp_path / "repo"), exclusive=True) as repository:
         repository.chunks = ChunkIndex()
         repo_objs = plain_repo_objs()
         objs = three_objects(repo_objs)