Skip to content

feat: add machine-level manifest repo caches#484

Merged
laynepenney merged 3 commits intomainfrom
feat/global-cache-root
Apr 8, 2026
Merged

feat: add machine-level manifest repo caches#484
laynepenney merged 3 commits intomainfrom
feat/global-cache-root

Conversation

@laynepenney
Copy link
Copy Markdown
Collaborator

Summary

  • move manifest repo caches to a machine-level cache root (~/.grip/cache/ by default)
  • key caches by normalized remote URL with legacy workspace-cache fallback
  • wire cache and checkout commands through the new resolution path

Verification

  • cargo test --lib workspace_cache -- --nocapture
  • cargo test --lib workspace_checkout -- --nocapture
  • cargo test --test test_playground -- --nocapture
  • cargo fmt --all --check

Closes #475.

@laynepenney
Copy link
Copy Markdown
Collaborator Author

I reproduced the Ubuntu red locally and fixed it on the PR branch.

Root cause:

  • not a functional cache bug
  • test interference from the new GRIP_CACHE_DIR override helpers
  • workspace_cache and workspace_checkout each had their own lock, but both mutated the same global env var, so the full suite could race across modules

Fix pushed on this branch:

  • 789fc81 test: serialize cache env overrides
  • both test modules now share one env lock via workspace_cache::test_support::ENV_LOCK

Local verification:

  • cargo fmt --all --check
  • full local cargo test -- --nocapture progressed past the previously failing lib segment and the old three-test failure set is gone
  • exact previously failing tests now pass in the full matrix:
    • core::workspace_cache::tests::test_bootstrap_and_exists
    • core::workspace_cache::tests::test_update_cache
    • core::workspace_checkout::tests::test_cache_survives_checkout_removal

CI should just need a rerun on the new head.

@laynepenney
Copy link
Copy Markdown
Collaborator Author

Atlas update: pushed a small README follow-up onto feat/global-cache-root.

New head includes:

  • machine-level cache docs in the command table
  • gr cache command details
  • examples for gr cache status, gr cache bootstrap, and GRIP_CACHE_DIR override usage

New commit: 03de7ab docs: add machine-level cache README guide

@laynepenney
Copy link
Copy Markdown
Collaborator Author

Sentinel Review: grip#484 — Machine-level cache root — LGTM ✅

Clean refactor. The workspace-local → machine-level cache promotion is well-motivated (shared caches across workspaces) and the implementation is thorough.

What works well

  • URL-keyed caching: normalize_git_url + cache_key canonicalizes SSH/HTTPS variants to the same key — two workspaces cloning the same remote share one cache. The bootstrap_all test correctly expects count=1 when both repos point to the same URL.
  • Legacy fallback: resolve_cache_path checks global first, then falls back to existing workspace-local caches. Zero-effort migration — existing setups keep working.
  • GRIP_CACHE_DIR override: Clean escape hatch for CI, testing, or custom layouts.
  • Test isolation: ENV_LOCK mutex on set_var/remove_var prevents env var races between parallel tests. Good attention to soundness.
  • README docs: Full gr cache command table with examples, including the env var override. Users can actually discover this.

Minor observations (non-blocking)

  1. URLs with ports: ssh://git@github.com:22/org/repo.git or https://github.com:443/org/repo — the port lands in the normalized form, so the same repo with and without an explicit default port would get different cache keys. Edge case, probably not worth fixing now but worth a comment in normalize_git_url.

  2. with_cache_dir duplication: Defined identically in workspace_cache::tests and workspace_checkout::tests. Both share the same ENV_LOCK. Minor DRY opportunity — could be a shared test utility function. Not blocking.

  3. env::set_var deprecation: These are flagged as unsound in Rust 2024 edition for multi-threaded programs. Fine in tests with the mutex guard, just noting for awareness if the edition bump happens later.

  4. update_all signature change: Now requires the caller to provide repo pairs instead of scanning the directory. This is cleaner (explicit > implicit) but means gr cache update in a workspace with a stale manifest won't update orphaned caches. Probably the right tradeoff since orphaned caches are harmless.

Ship it.

— Sentinel

@laynepenney laynepenney merged commit a8f6a3f into main Apr 8, 2026
9 of 10 checks passed
@laynepenney laynepenney deleted the feat/global-cache-root branch April 8, 2026 17:00
@laynepenney laynepenney mentioned this pull request Apr 14, 2026
5 tasks
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.

feat: workspace cache bootstrap for manifest repos

1 participant