Unified cache under sys.prefix - #5400
Merged
Merged
Conversation
pbrubeck
changed the title
pbrubeck
commented
Aug 27, 2026
pbrubeck
commented
Aug 27, 2026
connorjward
requested changes
Aug 27, 2026
pbrubeck
force-pushed
the
pbrubeck/unify-cache-dirs
branch
2 times, most recently
from
August 28, 2026 10:35
a478fee to
20a77ee
Compare
connorjward
requested changes
Aug 28, 2026
connorjward
previously approved these changes
Aug 28, 2026
setup_cache_dirs() used $VIRTUAL_ENV, which is unset for a bare venv/bin/python invocation and can point somewhere unwritable (e.g. a system-wide install). Use sys.prefix instead, falling back to the user's home directory when it is not writable. Introduce FIREDRAKE_CACHE_DIR as the single root that PYOP2_CACHE_DIR, FIREDRAKE_TSFC_KERNEL_CACHE_DIR and loopy's pytools-backed persistent cache (via XDG_CACHE_HOME) are all derived from, so the three caches live under one directory and one variable overrides all of them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
connorjward asked to minimise environment mutation, so setup_cache_dirs() no longer sets FIREDRAKE_CACHE_DIR itself when it falls back to sys.prefix or the home directory -- it only ever reads it. The derived-cache-dir tests move into test_0init.py alongside the other cache-dir test, and gain a fixture that clears the cache env vars after each test too, since setup_cache_dirs() writes os.environ directly and monkeypatch's own undo stack never sees those writes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRubJuLapd9jZw6iBPUxrL
Update the caching section for the sys.prefix-based default and the new XDG_CACHE_HOME/loopy cache, and add guidance on running with an isolated FIREDRAKE_CACHE_DIR plus the risk that firedrake-clean cannot see caches left behind under a FIREDRAKE_CACHE_DIR that isn't set in the environment it's run under. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRubJuLapd9jZw6iBPUxrL
monkeypatch.delenv() already snapshots each variable's pre-test value and restores it automatically at teardown, regardless of what setup_cache_dirs() writes to os.environ directly in between -- the restore is unconditional, not a diff against the current value. The second delenv() pass after yield was dead weight. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRubJuLapd9jZw6iBPUxrL
os.access(path, os.W_OK) reports writable for root regardless of the file mode, so a chmod(0o555) directory doesn't actually simulate an unwritable prefix when the suite runs as root (as CI does), and setup_cache_dirs() picked the "unwritable" prefix instead of falling back to home. Mock os.access directly for the path under test instead of relying on real permission bits, which is deterministic for any caller including root. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JRubJuLapd9jZw6iBPUxrL
pbrubeck
force-pushed
the
pbrubeck/unify-cache-dirs
branch
from
August 28, 2026 12:33
03e69d1 to
51d604e
Compare
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
2 participants
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.
David asked, on #5346, for the
sys.prefixwritability check and theFIREDRAKE_CACHE_DIRunification to be split into their own PR:Summary
setup_cache_dirs()now checkssys.prefixfor writability instead of reading$VIRTUAL_ENV(which is unset for a barevenv/bin/pythoninvocation, and can point somewhere unwritable), falling back toPath.home()when it is not writable.FIREDRAKE_CACHE_DIRas a single root directory.PYOP2_CACHE_DIR,FIREDRAKE_TSFC_KERNEL_CACHE_DIR, and loopy's pytools-backed persistent cache (viaXDG_CACHE_HOME) are all derived from it, so setting one variable moves all three caches together.