fix(atomic_io): quarantine falls back to exclusive-create copy when os.link is unavailable - #966
Open
kvandre12-commits wants to merge 1 commit into
Conversation
…s.link is unavailable
Android/Termux (bionic) does not expose os.link, so the hard-link-based
quarantine_file() raised AttributeError instead of quarantining a corrupt
config -- breaking config-corruption recovery on that platform (and masking
nine test failures behind a pytest WindowsPath INTERNALERROR).
quarantine_file() now creates the backup via _hardlink_or_copy(): it prefers
os.link where available (atomic, shares the inode) and otherwise falls back to
an exclusive-create (O_CREAT|O_EXCL) copy. The fallback preserves the
guarantees that matter -- but not perfect hard-link identity:
- no-overwrite: O_EXCL raises FileExistsError like os.link (retry loop);
- byte integrity: bounded os.read chunks; every os.write looped to completion;
- permissions: source permission bits are copied;
- rollback / no-data-loss: fsync before return; on ANY failure the partial
destination is removed, fds are closed, and the error re-raised with the
source untouched.
It does NOT reproduce inode identity, link count, ownership, or timestamps.
EACCES is surfaced (a real restriction), never treated as "unsupported"; only
ENOSYS/EPERM/EOPNOTSUPP/EMLINK/EXDEV trigger the fallback. The existing
retry/unlink/rollback loop is unchanged.
Adds tests/test_atomic_io_quarantine_fallback.py (name/content preservation,
no-overwrite, partial-failure rollback for read/write/fsync/close, short
writes, EACCES surfacing, symlink handling, os.link preferred where supported)
and makes the two existing os.link-hardcoded collision tests backend-agnostic.
Contributor
Author
|
CI passed Ruff and Windows encoding. The macOS test job had one failure in The identical head and base passed the full macOS suite in fork qualification PR #10: https://github.com/kvandre12-commits/code_puppy/actions/runs/36147090556 A rerun may clear the timeout when convenient. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On Android/Termux (bionic),
os.linkis not available (hasattr(os, 'link') == False).atomic_io.quarantine_file()usesos.linkto set a confirmed-corrupt config aside before recovery, so on those platforms a corrupt config crashes withAttributeError: module 'os' has no attribute 'link'instead of being quarantined and recovered —load_config/mutate_configraise instead of self-healing.Fix
quarantine_file()now creates the backup via a small_hardlink_or_copy()helper:os.linkwhere available (atomic, shares the inode, refuses to overwrite) — unchanged on Linux/macOS/Windows.os.open(..., O_CREAT | O_EXCL)) only whenos.linkis missing (AttributeError) or unsupported by the filesystem (ENOSYS/EPERM/EOPNOTSUPP/EMLINK/EXDEV).EACCESis surfaced, not treated as 'unsupported'.The fallback preserves the guarantees that matter (but not hard-link identity):
O_EXCLraisesFileExistsErrorexactly likeos.link, so the existing retry loop still picks a fresh collision-resistant name and never clobbers a prior backup;os.readin bounded chunks (memory never balloons); everyos.writeis looped until the whole buffer lands;dstisfsynced before return; on any failure the partial destination is removed, fds are closed, and the error re-raised with the source untouched.Inode identity, link count, ownership and timestamps are NOT reproduced (only data + mode). The retry/unlink/rollback loop is otherwise unchanged.
Tests
New
tests/test_atomic_io_quarantine_fallback.py(forcesos.linkabsent so it runs everywhere): name/content preservation + original vacated; never overwrites an existing backup; partial-failure rollback for injected read/write/fsync/close and source-unlink failures (source always intact, partial dest removed);os.writeshort-write looping;EACCESsurfaced (not downgraded); symlink source; andos.linkstill preferred where available. Existing collision tests were made backend-agnostic.Verification
main(this fork): ruff + macOS/Python 3.13 + Windows-encoding all green — https://github.com/kvandre12-commits/code_puppy/actions/runs/36147090556