feat(git): add explicit Blobless opt-in to BareCloneOptions and CloneOptions#6014
Conversation
Restore --filter=blob:none for bare clones with a graceful fallback for Git servers that do not support partial clones. When the clone fails with a "promisor remote" error, retry without the filter so the operation still succeeds on non-GitHub servers.
…o.go Re-enable filtering option for cloning repositories and handle errors related to promisor remotes.
✅ Deploy Preview for docs-kargo-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi team — could a maintainer take a look at this PR when you get a chance? The branch is up to date with |
|
If we'd prefer a more explicit approach, here's an idea: Rather than auto-detecting server support reactively, we could expose a subscriptions:
git:
- repoURL: https://github.com/org/repo
branch: main
blobless: truesteps:
- uses: git-clone
config:
repoURL: https://github.com/org/repo
blobless: true
checkout:
- branch: main
path: ./src
sparse: [some/path]This would pass |
|
Thank you @EronWright for the thoughtful alternative design suggestion! Your proposal to add an explicit The current approach (reactive fallback) has the advantage of being transparent to existing users — repos already using I think the explicit opt-in approach aligns well with issue #5725's intent. I'm happy to pivot to that design if the team prefers it. It would involve:
Would love to hear the team's preference — should I update this PR to implement the explicit opt-in approach? |
|
Given that this was initially backed out in a hurry because of the unanticipated problems it caused many users, the bar for re-enabling this without being an explicit opt-in is going to be very high. If it's an explicit opt-in, it's a much easier thing to entertain -- and sooner rather than later. It even leaves us with the option of declaring the feature beta. |
|
Thank you @krancour and @EronWright for the clear direction! I'll update this PR to implement the explicit opt-in approach — adding a This way users who want blobless clones can opt in explicitly, it can be marked beta if needed, and there's no risk of surprising anyone who didn't ask for it. I'll push the updated implementation shortly. |
…eCloneOptions Per maintainer feedback, replace the reactive error-string-matching Filter field with an explicit Blobless bool field. When true, passes --filter=blob:none unconditionally without retry logic.
…neOptions Replaced Filter option with Blobless for blobless cloning. Updated clone logic to use Blobless when specified.
Update: Refactored to explicit opt-in (per maintainer feedback)Per @krancour and @EronWright's feedback, this PR has been refactored away from the reactive error-string-matching retry approach. The new design uses an explicit Changes in this update:
The callers that previously set |
Summary
Fixes #5725
PR #5677 temporarily disabled
--filter=blob:noneon all clone operations because some non-GitHub Git servers responded with a "could not fetch from promisor remote" error. This PR restores the filter with a graceful fallback: if a clone with--filterfails because the server does not support partial clones, Kargo automatically retries the clone without the filter, so the operation still succeeds.Changes
pkg/controller/git/bare_repo.go: Restoreoptsparameter (was discarded as_), re-enable--filterwhenopts.Filter != "", and add astrings.Contains(err, "promisor remote")retry-without-filter fallback.pkg/controller/git/repo.go: Same fix — re-enable--filterin the regular (non-bare) clone path and add the same fallback. Also adds"strings"to the import block.Behaviour
--filter=blob:none; significantly faster for large repos with sparse checkout--filter→ succeeds as beforeThe fallback only triggers when
opts.Filteris set (i.e. when the caller opted into blobless clone). Plain clones are unaffected.