Skip to content

Set Partial Clone flags on sparse-checkout#4044

Merged
isaacsu merged 25 commits into
mainfrom
fix/set-clone-flags-on-sparsecheckout
Jul 15, 2026
Merged

Set Partial Clone flags on sparse-checkout#4044
isaacsu merged 25 commits into
mainfrom
fix/set-clone-flags-on-sparsecheckout

Conversation

@lizrabuya

@lizrabuya lizrabuya commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Description

When BUILDKITE_GIT_SPARSE_CHECKOUT_PATHS is set, the agent now injects --sparse and --filter=blob:none into git clone and --filter=blob:none into git fetch, so only the paths listed in BUILDKITE_GIT_SPARSE_CHECKOUT_PATHS are materialised and blobs outside the sparse set aren't downloaded up front.

Each auto-injected flag respects a user-supplied equivalent — the agent won't add --sparse if the user's clone flags already contain it, and won't add --filter=blob:none if the user has --filter=… in either BUILDKITE_GIT_CLONE_FLAGS or BUILDKITE_GIT_FETCH_FLAGS (git takes the last --filter on the command line, so stacking ours after would silently override theirs; on fetch, git clone --filter=X also stores the filter in remote.origin.partialclonefilter and subsequent fetches inherit it automatically, so re-passing our own filter would override the user's choice for that fetch).

The whole flow is gated on the local git actually supporting sparse-checkout set --cone (>= 2.27). On older git, the agent falls back to a plain full checkout — no partial clone, no --sparse, no --cone attempt.

The sparse-checkout decision is resolved once at the top of defaultCheckoutPhase via planSparseCheckout, which runs and validates git --version at most once per checkout and only when sparse paths are configured.

Changes

  • add a new sparseCheckoutPlan struct and planSparseCheckout function to perform git version check only once before clone starts
  • version floor bumped from 2.26 → 2.27 (single-call sparse-checkout set --cone landed in 2.27; 2.26 only accepted --cone on init).
  • In defaultCheckoutPhase, when sparsePlan.supported:
    • Append --sparse to gitCloneFlags unless the user already supplied it.
    • Append --filter=blob:none to gitCloneFlags unless the user already
      supplied a --filter (checked via hasPartialCloneFilter).
  • fetchSource now takes an autoAddBloblessFilter bool and prepends --filter=blob:none to fetch flags when set. The caller computes this as sparsePlan.supported && no --filter in clone flags && no --filter in fetch flags, so a user-supplied filter on either side is respected.
  • Add hasPartialCloneFilter and hasSparseCheckoutFlag helpers in internal/job/git.go.
  • Added unit and integration tests, including:
    • TestCheckingOutLocalGitProjectWithSparseCheckoutAutoAddsBlobNoneFilter
      — verifies both auto-adds happen when the user supplies neither.
    • TestCheckingOutLocalGitProjectWithSparseCheckoutPreservesUserFilter
      — verifies a user-supplied --filter=tree:0 on clone is preserved and no --filter=blob:none is prepended to fetch.
    • TestCheckingOutLocalGitProjectWithSparseCheckoutFallsBackOnOldGit
      — verifies the git < 2.27 fallback: no auto-added flags, no sparse-checkout set --cone, warning appears.

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go tool gofumpt -extra -w .)

Disclosures / Credits

Claude code drafted all the tests and helped diagnose build failures. I applied and reviewed all changes.

@omehegan omehegan added the bug label Jul 2, 2026
@lizrabuya lizrabuya added feature New user-facing feature! and removed bug labels Jul 3, 2026
@lizrabuya lizrabuya marked this pull request as ready for review July 3, 2026 05:35
@lizrabuya lizrabuya requested review from a team as code owners July 3, 2026 05:35

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I found one checkout-path issue that looks like it prevents the new auto-filter behavior from meeting the sparse-checkout intent when users haven't also supplied clone flags such as --sparse.

Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 2623, then answer my questions about the findings.

Comment thread internal/job/checkout.go
@lizrabuya lizrabuya marked this pull request as draft July 3, 2026 06:24
@lizrabuya lizrabuya marked this pull request as ready for review July 6, 2026 05:24
@lizrabuya

Copy link
Copy Markdown
Contributor Author

@buildsworth-bk , please re-review

@buildsworth-bk-app buildsworth-bk-app Bot dismissed their stale review July 6, 2026 05:29

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The previous clone-path concern is fixed and I dismissed that stale review. I found one remaining issue where the new fetch-path filter can still override a user-supplied clone filter.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 2898, then answer my questions about the findings.

Comment thread internal/job/checkout.go Outdated

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The previous checkout concern is addressed. I left one non-blocking note from this pass; this touches core checkout behavior, so I'm leaving a comment review rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 2897, then answer my questions about the findings.

Comment thread internal/job/integration/checkout_integration_test.go Outdated

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The clone-path concern is resolved, but the fetch-filter concern in the open thread is still unresolved. I don’t have any new findings beyond that.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 2899, then answer my questions about the findings.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The stale test note is addressed, but the fetch-filter concern in the open thread is still unresolved. I don't have any new findings beyond that.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 2909, then answer my questions about the findings.

@buildsworth-bk-app buildsworth-bk-app Bot dismissed stale reviews from themself July 6, 2026 06:09

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The fetch-filter concern is fixed, and I don't have any new findings. This still touches core checkout behavior, so I'm leaving a comment rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 2930, then answer my questions about the findings.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't have any new findings. This touches core checkout behavior, so I'm leaving a comment rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 3189, then answer my questions about the findings.

@isaacsu isaacsu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've raised a PR with suggestions: #4067

Feel free to incorporate all/some/none 😀

Co-authored-by: Josh Deprez <jd@buildkite.com>

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I found one compile blocker in the helper update.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 3803, then answer my questions about the findings.

Comment thread internal/job/git.go Outdated
@buildsworth-bk-app buildsworth-bk-app Bot dismissed their stale review July 12, 2026 12:10

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The previous import blocker is fixed. I found one remaining lint blocker from the sparse helper being left unused.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 4416, then answer my questions about the findings.

Comment thread internal/job/git.go Outdated
@buildsworth-bk-app buildsworth-bk-app Bot dismissed their stale review July 12, 2026 12:15

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The previous lint blocker is fixed, and I don't have any new findings. This still touches core checkout behavior, so I'm leaving a comment rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 4417, then answer my questions about the findings.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't have any new findings. This still touches core checkout behavior, so I'm leaving a comment rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 4418, then answer my questions about the findings.

@isaacsu isaacsu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one more, then I think we should be good to go.

Comment thread internal/job/checkout.go Outdated

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't have any new findings. This still touches core checkout behavior, so I'm leaving a comment rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 4972, then answer my questions about the findings.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I found one failing integration-test expectation from the version-check order change.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 5024, then answer my questions about the findings.

Comment thread internal/job/integration/checkout_integration_test.go Outdated
@buildsworth-bk-app buildsworth-bk-app Bot dismissed their stale review July 15, 2026 01:19

Previous concerns have been addressed.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The previous test-order blocker is fixed, and I don't have any new findings. This still touches core checkout behavior, so I'm leaving a comment rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 5085, then answer my questions about the findings.

@buildsworth-bk-app buildsworth-bk-app Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't have any new findings. This still touches core checkout behavior, so I'm leaving a comment rather than approving.

Want to dig deeper?

The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:

Download the buildsworth logs from build 5188, then answer my questions about the findings.

@isaacsu isaacsu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work @lizrabuya 🥳

@isaacsu isaacsu merged commit 2d4779d into main Jul 15, 2026
4 checks passed
@isaacsu isaacsu deleted the fix/set-clone-flags-on-sparsecheckout branch July 15, 2026 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New user-facing feature!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants