-
-
Notifications
You must be signed in to change notification settings - Fork 836
Description
Description:
_write_files_cache() in src/borg/cache.py (line 585) has a TODO:
# TODO: use something like SaveFile here, but that didn't work due to SyncFile missing .seek().
with IntegrityCheckedFile(path=str(self.path / self.files_cache_name()), write=True) as fd:This was introduced in d27b7a7 (Aug 2024, "cache: remove transactions, load files/chunks cache on demand", part of PR #8332). When refactoring _write_files_cache, as to wrap the write in SaveFile for atomic writes (temp file + os.replace), but couldn't because SaveFile returns a SyncFile, and SyncFile lacks .seek() and .tell().
Why .seek() is needed
IntegrityCheckedFile wraps its backing fd in a FileHashingWrapper (src/borg/crypto/file_integrity.py:15). The wrapper's hash_length() (line 101) calls self.seek(0, io.SEEK_END) to finalize the integrity hash. Without .seek() on the backing fd, IntegrityCheckedFile can't be used with SyncFile.
Current state
- SyncFile (src/borg/platform/base.py:138) exposes write(), sync(), close() but not .seek() or .tell()
- IntegrityCheckedFile already supports override_fd (line 131) and correctly skips closing it, so the plumbing for external fd injection exists
- Without SaveFile, the files cache is written directly — a crash mid-write can leave a corrupted file
Proposed fix
- Add .seek() and .tell() to SyncFile, delegating to self.f
- Refactor _write_files_cache to use SaveFile with IntegrityCheckedFile(override_fd=...)
Affected code
- src/borg/platform/base.py:138 — SyncFile class (add .seek(), .tell())
- src/borg/cache.py:585 — _write_files_cache() (use SaveFile)
- src/borg/crypto/file_integrity.py:101 — hash_length() (the .seek() caller)