Skip to content

cache: add seek()/tell() to SyncFile, use SaveFile in _write_files_cache, fixes #9390#9392

Merged
ThomasWaldmann merged 1 commit intoborgbackup:masterfrom
mr-raj12:fix-syncfile-seek-tell
Feb 28, 2026
Merged

cache: add seek()/tell() to SyncFile, use SaveFile in _write_files_cache, fixes #9390#9392
ThomasWaldmann merged 1 commit intoborgbackup:masterfrom
mr-raj12:fix-syncfile-seek-tell

Conversation

@mr-raj12
Copy link
Contributor

Description

  • _write_files_cache() in cache.py has a TODO noting that SaveFile couldn't be used because SyncFile lacks .seek() and .tell()
  • Without SaveFile, the files cache is written directly to the final path — a crash mid-write can leave a corrupted file
  • SaveFile provides atomic writes (temp file + os.replace), but its __enter__ returns a SyncFile, and IntegrityCheckedFile needs .seek()/.tell() on its backing fd for hash finalization (hash_length calls seek(0, SEEK_END) + tell())
  • Fix: add the missing methods to SyncFile, make close() idempotent (needed because the IntegrityCheckedFile → hasher → FileLikeWrapper exit chain closes the SyncFile before SaveFile.__exit__ tries to close it again), then refactor _write_files_cache to use SaveFile

Fixes #9390

Changes

File Change
src/borg/platform/base.py Add seek() and tell() to SyncFile, delegating to self.f; make close() idempotent with if self.f.closed: return guard
src/borg/cache.py Refactor _write_files_cache to wrap writes in SaveFile + IntegrityCheckedFile(override_fd=...) for atomic updates; remove TODO comment
src/borg/testsuite/platform/all_test.py Add tests for seek/tell, idempotent close, and SaveFile + IntegrityCheckedFile integration
src/borg/testsuite/crypto/file_integrity_test.py Add test for IntegrityCheckedFile with SyncFile as override_fd

Checklist

  • PR is against master
  • New code has tests
  • Tests pass
  • Commit messages are clean and reference related issues
  • Code formatted with black

@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.50%. Comparing base (0b05b44) to head (2357071).
⚠️ Report is 12 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/borg/cache.py 86.36% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9392      +/-   ##
==========================================
+ Coverage   76.47%   76.50%   +0.03%     
==========================================
  Files          85       85              
  Lines       14803    14818      +15     
  Branches     2213     2214       +1     
==========================================
+ Hits        11321    11337      +16     
  Misses       2803     2803              
+ Partials      679      678       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good change, found some minor things.

@mr-raj12 mr-raj12 force-pushed the fix-syncfile-seek-tell branch from 6c1017b to c095cc8 Compare February 28, 2026 00:37
@ThomasWaldmann ThomasWaldmann added this to the 2.0.0b21 milestone Feb 28, 2026
@mr-raj12 mr-raj12 force-pushed the fix-syncfile-seek-tell branch from c095cc8 to 2357071 Compare February 28, 2026 08:04
Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ThomasWaldmann ThomasWaldmann merged commit f3ac2e0 into borgbackup:master Feb 28, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cache: SyncFile lacks .seek()/.tell(), preventing SaveFile usage in _write_files_cache

2 participants