Skip to content

CI - IMPROVEMENT - Pin the CI dependency set per Julia minor version - #395

Merged
logan-nc merged 6 commits into
developfrom
test/ci-pinned-manifest
Aug 15, 2026
Merged

CI - IMPROVEMENT - Pin the CI dependency set per Julia minor version#395
logan-nc merged 6 commits into
developfrom
test/ci-pinned-manifest

Conversation

@matt-pharr

Copy link
Copy Markdown
Collaborator

Essentially this PR adds a pin on the manifest for the CI jobs, so testing will be much much quicker once this is merged. Presently, the packages will all recompile if the dependencies change. However, this manifest pin should allow us to re-run CI on the same versions of all dependencies and therefore skip the precompilation that takes like 15 minutes. @logan-nc I recommend merging this regardless of whether we decide to track the manifest or not and we can always change how this works in the future, but this should make the github actions jobs much faster now so I think we should merge this ASAP.

matt-pharrand others added 2 commits August 15, 2026 16:07
…prefix is cold
A pull request now saves the depot when it restored nothing at all, so a
cold key prefix seeds itself instead of discarding a full compile. Once the
default branch holds an entry, pull requests always prefix-match and skip
the save, which is what keeps per-PR caches from consuming the quota.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@logan-nc

Copy link
Copy Markdown
Collaborator

@claude review this

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@matt-pharr

Copy link
Copy Markdown
CollaboratorAuthor

FYI this most recent commit is just a test to show that the precompile step gets skipped in the gh runner jobs.

@claude

claudeBot commented Aug 15, 2026

Copy link
Copy Markdown

Review: CI - Pin the CI dependency set per Julia minor version

Pure CI/infra change (no src/ or test/ changes) that pins Manifest.toml per Julia minor version and switches from julia-actions/cache@v3 to manual actions/cache/restore|save@v4 steps so PR runs can seed/reuse a shared depot cache. This will be useful for cutting the ~15 min recompile currently paid on every job.

Issues / questions

  1. Cache key doesn't capture the resolved Julia patch/minor version for the floating 1.x matrix entry (.github/workflows/test.yaml, restore/save key:). The key uses ${{ matrix.version }} (the literal string 1.x), while the "Pin the CI dependency set" step correctly re-derives the real running version via julia -e 'print(VERSION.major, ".", VERSION.minor)'. If 1.x silently rolls to a new Julia minor (e.g. 1.12 -> 1.13) before a matching Manifest-v1.13.toml exists, the cache key stays julia-depot-ubuntu-latest-1.x-<hash> (the hash only changes if Project.toml/ci/manifests/Manifest-v*.toml change), so old 1.12 depot contents keep getting restored/merged under the same key rather than the key naturally rotating. Not corruption (Julia's own compiled-cache dirs are self-versioned by version), but the depot can grow stale/bloated across a Julia bump until it ages out on its own (7-day cache LRU). Consider folding the actual resolved julia_version into the key instead of the matrix label.

  2. ignore-no-cache: true on julia-actions/julia-buildpkg@v1 - worth double-checking this is a real, current input for that action version (could not verify the upstream action source in this session). If it isn't recognized, the step still runs but the flag silently becomes a no-op, which is easy to miss.

  3. Minor: project.sha256's staleness check duplicates information Pkg already tracks internally (project_hash embedded in each Manifest-v*.toml, which Pkg.instantiate()/build already uses to warn/re-resolve on mismatch). The sidecar file is defensible since it lets the workflow do the check in plain bash before invoking Julia, but the two mechanisms overlap - if they ever disagree it'll be confusing to debug.

  4. ci/manifests/update.jl resolves a bare copy of Project.toml in a temp dir with no [sources]/path-dependency entries to carry over. Confirmed the project currently has none, so this is fine today, but it's an implicit assumption that would silently break resolution if an unregistered/path dependency is ever added without updating this script.

Positive notes

  • ci/manifests/README.md clearly documents the regeneration workflow, the staleness-warning behavior, and how to add a new Julia version to the matrix - this will reduce the "why is CI slow / why did the manifest go stale" support burden.
  • The depot-save condition (cache-hit != 'true' && (push || cache-matched-key == '')) is a sensible way to avoid burning PR-scoped cache quota while still letting a cold PR seed itself; the accompanying code comment explains the reasoning well.
  • CLAUDE.md was updated with the new "regenerate pins after editing [deps]/[compat]" rule, keeping the instruction file in sync with the new process.

Test coverage

No test coverage change, which is appropriate for a CI-workflow-only PR - there isn't a meaningfully unit-testable surface here beyond running the workflow itself. Per CLAUDE.md, the regression harness is nominally required before every merge to develop; since this PR touches no src/ physics code, no numerical output is expected to change, but I'm flagging its existence per project policy.


Per this repo's CLAUDE.md: this PR must not be merged into develop without a human reviewer's approval - this is a hard project requirement (PARAMOUNT), not a suggestion, regardless of how low-risk the change appears.

@matt-pharr

Copy link
Copy Markdown
CollaboratorAuthor

OK @logan-nc this is ready. As you can see the latest commit finished >10 minutes quicker than the prior one and no longer goes through the whole pre-compiling process.

matt-pharrand others added 2 commits August 15, 2026 17:53
Precompiled .ji files are tied to the exact Julia build, so a patch release
invalidates them all. Cache entries are immutable, so a key built from the
matrix label alone could not rotate: the run would recompile the whole tree,
hit the unchanged key, skip the save as having nothing new to store, and
repeat that on every run indefinitely.
Keying on setup-julia's resolved version rotates it. A version-only fallback
still reuses packages, artifacts and registries across a bump.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@logan-nclogan-nc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The speedup sounds great to me.
Honestly, you are way beyond me with the CI implementations at this point so I definitely defer to you on the best approach. I didn't see anything crazy in my MANDATORY human read through

@logan-nclogan-nc self-assigned this Aug 15, 2026
@logan-nclogan-nc added the test Tests only label Aug 15, 2026
@logan-nc
logan-nc merged commit 97be55d into developAug 15, 2026
8 checks passed
@logan-nc
logan-nc deleted the test/ci-pinned-manifest branch August 15, 2026 23:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testTests only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@matt-pharr@logan-nc