feat: adding support for commit refs in CAS#6010
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR extends the CAS system to support cloning Git repositories by full or abbreviated commit SHA via ChangesCAS Commit-SHA Reference Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
2c10fbd to
d634f65
Compare
d634f65 to
26e2414
Compare
3a8acae to
b04a540
Compare
26e2414 to
7b1bb75
Compare
3b7e072 to
a659c3e
Compare
b04a540 to
1b39d0c
Compare
a659c3e to
f75cf82
Compare
f75cf82 to
4255f05
Compare
4255f05 to
3f73aa2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/content/docs/03-features/07-caching/04-cas.mdx`:
- Around line 224-227: The steps currently imply two network fetches for
commit-SHA cold-clones; update the wording so Step 1 ("Terragrunt resolves the
Git reference to a commit hash" and mention of "git ls-remote") is strictly
resolution-focused (note that ls-remote only resolves named refs and will not
fetch SHA objects), and move all fetching behavior into Step 3 (the description
that opens the bare repo under cas/store/git/, takes the per-URL lock, and
fetches—shallow for branch/tag refs, full history for commit SHAs) so that Step
1 only detects the ref type and Step 3 performs the actual network fetch; keep
references to "commit SHA", "git ls-remote", and "cas/store/git/" to locate the
lines to edit.
In `@internal/cas/gitstore.go`:
- Around line 210-212: The fallback fetch only pulls branch refs, so commits
reachable only via tags can still be missing; update the two
session.runner.Fetch calls to fetch tags as well (e.g. add
"+refs/tags/*:refs/tags/*" or use a broader refspec like "+refs/*:refs/*") so
tag-only commits are fetched and no longer return ErrNoMatchingReference; modify
the refspec strings passed to session.runner.Fetch accordingly in gitstore.go
(both occurrences of session.runner.Fetch).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b2b4358-3e2f-4384-a2f3-deba371a9c9f
📒 Files selected for processing (14)
docs/src/content/docs/03-features/07-caching/04-cas.mdxdocs/src/data/changelog/v1.0.5/cas-commit-refs.mdxinternal/cas/cas.gointernal/cas/cas_test.gointernal/cas/commitref_test.gointernal/cas/gitstore.gointernal/cas/gitstore_test.gointernal/cas/stacks.gointernal/cas/testserver_test.gointernal/git/errors.gointernal/git/git.gointernal/git/server.gotest/fixtures/download/remote-commit-ref/terragrunt.hcltest/integration_download_test.go
| 1. Terragrunt resolves the Git reference to a commit hash. Branch and tag refs resolve via `git ls-remote`. `ls-remote` lists named refs and does not resolve commit SHAs, so for SHA refs Terragrunt fetches the full history of every branch into the central Git store and resolves the SHA locally. | ||
| 2. The tree related to the commit hash is not found in the CAS | ||
| 3. Terragrunt opens the bare repository for the remote URL under `cas/store/git/` (initializing it on first use), takes a per-URL lock, and fetches the requested ref. Subsequent misses against the same URL reuse the existing pack files and only transfer new objects. | ||
| 3. Terragrunt opens the bare repository for the remote URL under `cas/store/git/` (initializing it on first use), takes a per-URL lock, and fetches the requested ref (shallow for branch/tag refs, full history for commit SHAs). Subsequent misses against the same URL reuse the existing pack files and only transfer new objects. | ||
| 4. All blobs and trees required to reproduce the repository are extracted from the bare repository |
There was a problem hiding this comment.
Clarify cold-clone step sequencing to avoid implying two SHA fetches.
Line 224 and Line 227 both describe fetching for commit-SHA refs; the current phrasing can read as two network fetches in one cold-clone path. Consider making Step 1 resolution-focused and keeping fetch details in Step 3.
✏️ Suggested doc tweak
-1. Terragrunt resolves the Git reference to a commit hash. Branch and tag refs resolve via `git ls-remote`. `ls-remote` lists named refs and does not resolve commit SHAs, so for SHA refs Terragrunt fetches the full history of every branch into the central Git store and resolves the SHA locally.
+1. Terragrunt resolves the Git reference to a commit hash. Branch and tag refs resolve via `git ls-remote`. For commit-SHA refs, resolution is performed locally from the central Git store (because `ls-remote` does not resolve commit SHAs).
-3. Terragrunt opens the bare repository for the remote URL under `cas/store/git/` (initializing it on first use), takes a per-URL lock, and fetches the requested ref (shallow for branch/tag refs, full history for commit SHAs). Subsequent misses against the same URL reuse the existing pack files and only transfer new objects.
+3. Terragrunt opens the bare repository for the remote URL under `cas/store/git/` (initializing it on first use), takes a per-URL lock, and performs the fetch needed for the ref kind (shallow for branch/tag refs, full history for commit SHAs). Subsequent misses against the same URL reuse existing pack files and only transfer new objects.As per coding guidelines: "Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. Terragrunt resolves the Git reference to a commit hash. Branch and tag refs resolve via `git ls-remote`. `ls-remote` lists named refs and does not resolve commit SHAs, so for SHA refs Terragrunt fetches the full history of every branch into the central Git store and resolves the SHA locally. | |
| 2. The tree related to the commit hash is not found in the CAS | |
| 3. Terragrunt opens the bare repository for the remote URL under `cas/store/git/` (initializing it on first use), takes a per-URL lock, and fetches the requested ref. Subsequent misses against the same URL reuse the existing pack files and only transfer new objects. | |
| 3. Terragrunt opens the bare repository for the remote URL under `cas/store/git/` (initializing it on first use), takes a per-URL lock, and fetches the requested ref (shallow for branch/tag refs, full history for commit SHAs). Subsequent misses against the same URL reuse the existing pack files and only transfer new objects. | |
| 4. All blobs and trees required to reproduce the repository are extracted from the bare repository | |
| 1. Terragrunt resolves the Git reference to a commit hash. Branch and tag refs resolve via `git ls-remote`. For commit-SHA refs, resolution is performed locally from the central Git store (because `ls-remote` does not resolve commit SHAs). | |
| 2. The tree related to the commit hash is not found in the CAS | |
| 3. Terragrunt opens the bare repository for the remote URL under `cas/store/git/` (initializing it on first use), takes a per-URL lock, and performs the fetch needed for the ref kind (shallow for branch/tag refs, full history for commit SHAs). Subsequent misses against the same URL reuse existing pack files and only transfer new objects. | |
| 4. All blobs and trees required to reproduce the repository are extracted from the bare repository |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/content/docs/03-features/07-caching/04-cas.mdx` around lines 224 -
227, The steps currently imply two network fetches for commit-SHA cold-clones;
update the wording so Step 1 ("Terragrunt resolves the Git reference to a commit
hash" and mention of "git ls-remote") is strictly resolution-focused (note that
ls-remote only resolves named refs and will not fetch SHA objects), and move all
fetching behavior into Step 3 (the description that opens the bare repo under
cas/store/git/, takes the per-URL lock, and fetches—shallow for branch/tag refs,
full history for commit SHAs) so that Step 1 only detects the ref type and Step
3 performs the actual network fetch; keep references to "commit SHA", "git
ls-remote", and "cas/store/git/" to locate the lines to edit.
| if err := session.runner.Fetch(ctx, url, "+refs/heads/*:refs/heads/*", 0); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Commit fallback fetch misses tag-only commits
The cache-miss path fetches only branch refs. A commit reachable only via tags can still fail resolution and be returned as ErrNoMatchingReference, even though it exists remotely.
💡 Suggested fix
- if err := session.runner.Fetch(ctx, url, "+refs/heads/*:refs/heads/*", 0); err != nil {
+ if err := session.runner.Fetch(ctx, url, "+refs/heads/*:refs/heads/*", 0); err != nil {
+ return nil, err
+ }
+
+ if err := session.runner.Fetch(ctx, url, "+refs/tags/*:refs/tags/*", 0); err != nil {
return nil, err
}- if err := session.runner.Fetch(ctx, url, "+refs/heads/*:refs/heads/*", 0); err != nil {
+ if err := session.runner.Fetch(ctx, url, "+refs/heads/*:refs/heads/*", 0); err != nil {
+ return nil, err
+ }
+
+ if err := session.runner.Fetch(ctx, url, "+refs/tags/*:refs/tags/*", 0); err != nil {
return nil, err
}Also applies to: 253-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/cas/gitstore.go` around lines 210 - 212, The fallback fetch only
pulls branch refs, so commits reachable only via tags can still be missing;
update the two session.runner.Fetch calls to fetch tags as well (e.g. add
"+refs/tags/*:refs/tags/*" or use a broader refspec like "+refs/*:refs/*") so
tag-only commits are fetched and no longer return ErrNoMatchingReference; modify
the refspec strings passed to session.runner.Fetch accordingly in gitstore.go
(both occurrences of session.runner.Fetch).
Description
Right now, the CAS only supports branch/tag refs. This updates the implementation to also support commit refs.
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Summary by CodeRabbit
New Features
Documentation
Tests