Skip to content

mz468: refs/pull is not a valid branchname#509

Open
mzihlmann wants to merge 4 commits intomainfrom
mz468-fix-git-clone
Open

mz468: refs/pull is not a valid branchname#509
mzihlmann wants to merge 4 commits intomainfrom
mz468-fix-git-clone

Conversation

@mzihlmann
Copy link
Collaborator

@mzihlmann mzihlmann commented Feb 13, 2026

Fixes #468

Description

Github exposes refs/pull references as branchnames during clone, but that is a github-ism. Other git servers do not need to do that. Only refs/heads are branch names. We therefore change the logic to handle all non-branch references via a three stage approach:

clone default + fetch ref + checkout commit

We added an exception for github/gitlab to keep behaviour efficient and compatible

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Adds integration tests if needed.

See the contribution guide for more details.

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

Release Notes

Describe any changes here so maintainer can include it in the release notes, or delete this block.

Examples of user facing changes:
- kaniko adds a new flag `--registry-repo` to override registry

@mzihlmann mzihlmann force-pushed the mz468-fix-git-clone branch 6 times, most recently from ac812c7 to 834df31 Compare February 15, 2026 11:11
Github exposes refs/pull references as branchnames during clone, but that is a github-ism.
Other git servers do not need to do that. Only refs/heads are branch names.
We therefore change the logic to handle all non-branch references via a three stage approach:

    clone default + fetch branch + checkout branch

We might consider adding an exception for github to keep behaviour compatible
@mzihlmann mzihlmann marked this pull request as ready for review February 15, 2026 11:51
@mzihlmann mzihlmann requested review from 0hlov3, BobDu, babs and nejch February 15, 2026 11:57
@mzihlmann
Copy link
Collaborator Author

I noticed that the integration tests for git checkout are a bit lacking. we should add more integration tests, both for execution and planning. I think we should do as buildkit does and switch away from git-go to regular C git, as go-git will always be different to the main implementation of git and we already do include foreign binaries with our inclusion of tini.

@mzihlmann
Copy link
Collaborator Author

mzihlmann commented Feb 15, 2026

imo fetching context should not be part of kaniko executor, it should be a separate command entirely. The only advantage that integration could bring is if we could do deferred fetching, like for s3, when we only fetch resources if we have a cache miss and need to download. But then the same could be achieved by s3-fuse filesystems.
If fetching is a separate command, not only can the user use git as is, they can use whatever cloud provider tool as is, but then also kaniko can drop like 50% of its dependencies.

but that is out of this scope and more of a discussion for kaniko v2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sometimes git references are not fetched correctly

1 participant