Skip to content

Backup: one unreadable file costs that file, not the whole night's archive - #718

Open
chrisjackson-coding wants to merge 1 commit into
davekilleen:mainfrom
chrisjackson-coding:backup-tolerate-unreadable
Open

chrisjackson-coding wants to merge 1 commit into
davekilleen:mainfrom
chrisjackson-coding:backup-tolerate-unreadable

Conversation

@chrisjackson-coding

Copy link
Copy Markdown
Contributor

What was wrong

A scheduled backup on my vault failed three nights out of forty (14 Aug, 4 Sep, 8 Sep), each time on the same file: an image saved by WhatsApp that macOS had stamped with a per-file access list. The launchd job got Operation not permitted on it. On demand the file reads fine.

The defect is in build_artifacts(), which I wrote in #461. The vault goes into the archive through a single tar.add(vault), so the first file the process cannot read raises out of the whole walk and the night stores nothing. The function's own docstring argues the opposite case for the git bundle, that a failure there should "degrade rather than abort the run and store nothing at all", and calls the notes archive "the irreplaceable part". That part was the one with no tolerance.

What this changes

The archive is built one entry at a time. A file the process cannot read is skipped, named in a recorded warning (so the run is not a silent OK, same as the existing link and bundle warnings), and everything else is stored. Only a vault with no readable file at all fails the run, loudly.

The exclusion filter runs per entry exactly as before, excluded directories are not descended, and entries are added in sorted order so two runs of the same vault produce the same archive.

Tests

Two new cases in core/tests/test_backup_vault.py: one unreadable file produces an OK run with a warning naming it and an archive that carries everything else; a vault with nothing readable raises. Both skip under root. The existing 49 cases pass unchanged. All CI gates pass locally against v1.97.13.

Not changed

Why macOS intermittently refuses the read is not something I can name from three data points; the per-file access list (com.apple.macl) on that file is the likely mechanism but this PR does not depend on that being right. It makes the backup honest about a gap instead of losing the night.

…chive

build_artifacts() added the vault through a single tar.add(), so the first
file the scheduled process could not read raised out of it and the day
stored nothing. The function's own docstring already argued the opposite
case for the git bundle, and called the notes archive the irreplaceable
part; that part had no tolerance at all.

The archive is now built one entry at a time. A file the process cannot
read is skipped, named in a recorded warning, and the run still stores
everything else. Only a vault with no readable file at all fails the run.

Seen on a production vault: an image saved by a sandboxed app carrying a
per-file access list refused the launchd job intermittently (three nights
in forty), and each time the whole backup was lost.
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.

1 participant