-
Notifications
You must be signed in to change notification settings - Fork 60
Separate cache key for full git clones #690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughRenames and parameterizes CircleCI cache anchors/keys and replaces anchor references; compacts checkout logic in CircleCI macros. Adjusts Azure pipeline/job YAML formatting and comments (disables one typecheck job). Adds Makefile permission toggles around m4. No public API or functional code changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant CI as CircleCI
participant Job as Job
participant Cache as Cache
Dev->>CI: push / open PR
CI->>Job: start job
rect rgb(240,248,255)
note right of Job: restore_cache uses renamed/parameterized keys/anchors
Job->>Cache: restore_cache: e.g. sourcefull-v1full-{{ .Branch }}-{{ .Revision }}\nfallbacks: sourcefull-v1full-{{ .Branch }}-, sourcefull-v1full-
Cache-->>Job: hit / miss
end
Job->>Job: checkout (inline ifelse emits full checkout when macro arg == "full")
Job->>Job: execute steps (build/test)
Job->>Cache: save_cache: matching parameterized key/anchor
Cache-->>Job: saved
Job-->>CI: complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🪛 YAMLlint (1.37.1).circleci/config.yml[warning] 1320-1320: comment not indented like content (comments-indentation) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.circleci/defs.m4(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: codespecs.daikon (quick_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_latest_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_bundled_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (nontxt_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (nonquick_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (misc_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (kvasir_ubuntu_jdk25)
- GitHub Check: build
🔇 Additional comments (1)
.circleci/defs.m4 (1)
18-20: Cache key parameterization looks correct.The addition of
$1to the cache key prefix effectively creates separate cache namespaces for full and shallow clones, which aligns with the PR objective. Whencircleci_boilerplate(full)is invoked (as inmisc_job), keys becomesource-v1full-...; otherwise, they remainsource-v1-....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.azure/Makefile(1 hunks).azure/azure-pipelines.yml(1 hunks).azure/azure-pipelines.yml.m4(1 hunks).azure/defs.m4(1 hunks).azure/jobs.m4(1 hunks).circleci/config.yml(10 hunks)
🔇 Additional comments (15)
.circleci/config.yml (9)
348-350: Consistent v1full keys here as well.Matches the full checkout usage.
370-372: v1full key prefix looks correct.
392-394: Full-clone cache keys updated correctly.
414-416: Consistent with intended full-clone cache separation.
990-992: Rocky misc job uses v1full keys as expected.
1012-1014: v1full cache keys applied correctly.
1034-1036: Correct v1full usage for full checkout.
1056-1058: Cache prefix update aligns with full clone.
1078-1080: Consistent v1full keys; looks good..azure/jobs.m4 (1)
31-38: LGTM! Expanded typecheck coverage.The addition of typecheck jobs for Ubuntu 21 and 25 follows the existing pattern and expands test coverage appropriately. The comment formatting improvement enhances readability.
.azure/defs.m4 (1)
11-146: LGTM! Formatting improvements.The consistent indentation applied across all job definition macros significantly improves readability without changing any functional behavior.
.azure/azure-pipelines.yml.m4 (3)
18-19: LGTM! Improved variable declaration format.The list-based variable declaration format is cleaner and aligns with YAML best practices.
23-41: LGTM! Enhanced canary jobs structure.The reformatted canary_jobs block with improved indentation and explicit dependency list significantly improves readability.
45-59: Verify that disabling the typecheck_onefile_jdk21 job is intentional.The
typecheck_onefile_jdk21job has been commented out. According to the comment, this job was used to "prioritize typechecking a single file, without waiting for other tests." Please confirm that this debugging/development feature is no longer needed..azure/azure-pipelines.yml (1)
775-789: Note: This is a generated file.This file is generated from
.azure/azure-pipelines.yml.m4by the m4 preprocessor. The commented-outtypecheck_onefile_jdk21job reflects the template changes already reviewed. Any modifications should be made to the.m4template file, not directly to this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.circleci/config.yml (1)
326-328: Inconsistent cache key prefix (duplicate of.circleci/defs.m4issue).Line 326 uses
source-v1full-while lines 327-328 usesourcefull-v1full-. This mismatch is generated by the pattern inconsistency in.circleci/defs.m4line 18 and will prevent proper cache restoration.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.circleci/config.yml(11 hunks).circleci/defs.m4(1 hunks)
🔇 Additional comments (3)
.circleci/defs.m4 (2)
21-22: Inline conditional for checkout is correctly structured.The
ifelsemacro correctly produces either a plaincheckoutstep or acheckoutstep withmethod: fullbased on the parameter. The generated YAML structure in.circleci/config.ymlconfirms valid formatting.
24-24: Parameterized cache key reference correctly separates full and shallow clone caches.The change from
*source-cacheto*source$1-cacheproperly references the parameterized anchor, preventing cache collisions between full and shallow clones..circleci/config.yml (1)
28-28: Duplicate anchor declarations remain but are reduced.The
&source-cacheanchor is still declared in multiple jobs throughout the file (lines 28, 48, 68, 88, 108, 129, 149, 169, 189, 209, 230, 249, 268, 287, 306, etc.). While YAML parsers may tolerate this by allowing later definitions to overwrite earlier ones, it technically violates the YAML specification.However, this PR improves the situation by introducing
&sourcefull-cachefor full-clone jobs, reducing the number of duplicate&source-cachedeclarations compared to the previous version.For a complete fix, consider parameterizing anchor names per job in the M4 templates (e.g.,
&source-cache-<job-name>) to ensure each anchor is unique. This would require updating the M4 macro definition in.circleci/defs.m4.
Summary by CodeRabbit
Chores
Tests