feat: add non-cone mode support for sparse checkout - #4127
Conversation
Native sparse checkout always used cone mode. Add --git-sparse-checkout-no-cone / BUILDKITE_GIT_SPARSE_CHECKOUT_NO_CONE so jobs can pass --no-cone and use gitignore-style patterns, matching the sparse-checkout plugin's no_cone option. Default remains cone mode. Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
…ping - Replace the --git-sparse-checkout-no-cone bool with --git-sparse-checkout-mode cone|no-cone, following the existing --git-mirror-checkout-mode precedent, validated in agent start and bootstrap as well as in the executor. - Require git >= 2.35 for no-cone mode. `sparse-checkout set` only learned --cone/--no-cone in 2.35, and older git silently parses an unrecognised option as another pattern, which would leave the repo in whatever mode it was already in and check out the wrong files. Older git now warns and falls back to a full checkout, and the mode flag is omitted entirely below 2.35 so it can never land in the sparse-checkout file. - Don't reuse non-cone patterns to scope Git LFS. `git lfs fetch --include` has no negation and `git lfs checkout` takes pathspecs, so "!/docs/" silently matches nothing; fetch all LFS objects in no-cone mode instead. - Pass paths after `--` so a pipeline-supplied path can't be parsed as a git option. - Add tests for mode parsing, both version floors, LFS include selection, no-cone with git mirrors, and switching modes across builds in one checkout dir. Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
…ation Addresses the quality review of the previous commit. - Rename pinMode to modeEnforced and warn when it's false. Below git 2.35 the mode option can't be passed, and `clone --sparse` on those versions leaves core.sparseCheckoutCone unset, so cone mode is requested while non-cone interpretation is what actually happens. The build log now says so instead of claiming the requested mode is in force. - Reject an unusable mode before the checkout retry loop, so a typo from job env fails immediately instead of burning the attempt budget with backoff. - Make sparseCheckout.lfsInclude the only place that decides whether the paths can scope Git LFS; the log line is now derived from that decision rather than re-deriving the rule. - Add a SparseCheckoutMode type, following env.CheckoutOverrideMode. AgentConfiguration carries the parsed value; ExecutorConfig keeps a raw string because within-job sources can rewrite it after startup. - Replace versionAtLeast's four positional ints with a gitVer type, and move version handling to gitversion.go so it's findable by the next caller. Name the version floors instead of repeating 27/35 inline. - Tests: cover the version error paths and gitVer comparisons, assert supported gits resolve without warnings, name the table cases, set job env by name instead of by slice index, and add an integration test with a directory named "--stdin" that fails without the `--` separator. Fix gitRepository.Add, which hit the same bug class on that fixture. Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
There was a problem hiding this comment.
This changes the agent's checkout execution and public configuration surface, so I'm leaving it for a human sanity-check. I found one non-blocking issue in the new flag help.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 9340, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
About buildsworth
Model: gpt-5.6-sol with xhigh thinking.
How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.
Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. To let it approve, grant L2 approval by mentioning @buildsworth-bk (see L2 approval grant):
- L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
- L2 — Standard risk (new UI, additive API fields, refactors). Approved only with an L2 grant; otherwise comment-only.
- L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.
Cone mode enables faster pattern matching, not a sparse index (that requires --sparse-index, which we don't pass). Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
There was a problem hiding this comment.
I found one blocking issue in the no-cone Git LFS path; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 9344, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Unscoped `git lfs checkout` recreates skip-worktree files outside the sparse set. Keep fetch unscoped for no-cone, but check out only LFS paths that are not skip-worktree. Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
|
@buildsworth-bk review — the blocking no-cone LFS checkout issue is addressed on the latest commit (fetch stays unscoped; checkout is limited to non-skip-worktree LFS paths). |
zhming0
left a comment
There was a problem hiding this comment.
I focused my review effort on the ~400 LoC implementation code, LGTM.
I recommend extra code comment for the intricacy between non-cone mode and LFS checkout (A table sounds wise).
Document why no-cone leaves FetchInclude empty and scopes checkout via materializedLFSPaths, and key the branch on sparse.noCone(). Co-authored-by: Lachlan Donald <lachlan@ljd.cc>
Adds
--git-sparse-checkout-mode/BUILDKITE_GIT_SPARSE_CHECKOUT_MODE(cone|no-cone, defaultcone) so sparse checkout can use gitignore-style patterns viagit sparse-checkout set --no-cone.--; non-cone fetches all LFS objects but checks out only non-skip-worktree pathsinternal/job/gitversion.goFollow-up: #4130.
Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go tool gofumpt -extra -w .)Disclosures / Credits