fix(cache): survive an unusable cache, and add asimov doctor - #123
Merged
Conversation
The cache under ~/.cache/asimov is an optimisation, but nothing treated it as optional. A bare `cat` on an unreadable state file failed under `set -Eeu -o pipefail` and aborted the run immediately, printing nothing but "Permission denied" — no context, no fix, exit 1. The cause was Asimov itself. ensure_cache_dir chowned the cache directory to the console user when running as root, but the state files were created *after* that chown, so every `sudo asimov` left root-owned files behind and broke the next run as the user. Guard every cache read and write behind cache_readable/cache_writable, warn once naming the reset command, and carry on without the cache. Create the state files before the chown so appends — which never change an existing file's owner — keep them owned by the user who has to read them next. Closes #122
Checks the install rather than the projects: which asimov the shell actually runs, whether a schedule is installed and loaded, whether the cache is readable and writable, whether the config parses, and whether tmutil can read exclusions at all. Exits 1 if it finds anything. Two rules keep it safe to run part-way through a migration. It never writes — fixes are printed, not applied. And it never executes another asimov binary it finds: v0.3.0 parses no arguments at all, so running it to ask its version would start a real scan, so versions are read out of the file instead. The test helper now strips any real asimov install from PATH, since doctor inspects PATH and would otherwise report a different result per machine. Refs #122
v0.3.0 is the version most people have, and it parses no arguments at all: `asimov --version` and `asimov doctor` are both ignored and it goes straight to scanning. So the upgrade notes lead with a version check that reads the file instead of running it, and put `asimov doctor` last, after the new binary is in place. Also documents the three leftovers that outlive a v0.3.0 install — the LaunchAgent, the old cellar, and a root-owned cache — and that Asimov is a one-shot scan, not a daemon, so an empty `ps aux | grep asimov` after `brew services start` is expected. Refs #122
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.
Closes #122.
@ajoslin103 migrated from
0.3.0to0.10.0and hit three bumps. Two of them are our bugs; the third is a documentation gap.The crash
asimovaborted with nothing butcat: ~/.cache/asimov/excluded: Permission denied. Reproduced exactly:The cache is an optimisation, but nothing treated it as optional: a bare
caton an unreadable state file fails underset -Eeu -o pipefailand takes the run with it.The cause was Asimov itself.
ensure_cache_dirchowned the cache directory to the console user when running as root, but the state files were created after that chown — so everysudo asimovleft root-owned files behind and broke the next run as the user.Both halves are fixed. Reads and writes go through
cache_readable/cache_writable, which warn once, name the reset command, and continue without the cache. The state files are created before the chown, and appending never changes an existing file's owner.One thing this caught: the full-scan branch pipes
find | tee "$ASIMOV_PATH_CACHE" | exclude_paths_from_stdin, so a failingteetakes the pipeline down underpipefail. It now falls back to the untee'd pipeline.asimov doctorChecks the install rather than the projects, and exits
1if it finds anything:Two rules keep it safe to run part-way through a migration:
asimovbinary it finds. v0.3.0 parses no arguments at all, so running it to ask its version would start a real scan. Versions are read out of the file withgrep. A test asserts the stub binary is never executed.Docs
UPGRADING.mdgains a v0.3.0 section — the version most people have. Because v0.3.0 ignores--versionand--helptoo, it opens with a version check that reads the file rather than running it, and putsasimov doctorlast, after the new binary is in place. It also covers the three leftovers that outlive a v0.3.0 install (the LaunchAgent, the old cellar, the root-owned cache) and notes that Asimov is a one-shot scan, not a daemon — an emptyps aux | grep asimovafterbrew services startis expected, not a failure.Tests
219 passing, up from 188; shellcheck clean. Written test-first — every behaviour here was red against the pre-fix script before it was green.
tests/cache.bats— 7 tests for unreadable/unwritable state, warn-once, and the--full-scantee pathtests/doctor.bats— 31 tests, including one asserting doctor never executes a binary it findstests/bin/launchctl— new mock, driven byASIMOV_TEST_LAUNCHCTL_LOADEDtests/test_helper.bash— now strips any realasimovinstall fromPATH, sincedoctorinspectsPATHand would otherwise report a different result on every machinescripts/test.sh—doctor.batsadded to the file listTests that depend on
chmodskip when running as root, where they'd be meaningless.