[carry] Automatically detect default git branch #2228
Conversation
Instead of just assuming that the default branch is master, use ls-remote to find out. Also removed tests that didn't specifiy a branch but required authentication, because those will fail now that the repo is actually checked. Signed-off-by: Levi Harrison <[email protected]>
It is my suspecion that the tests were failing on previous commits because of the lack of authentication and other stuff like that available in gitidentifier as compared to gitsource Signed-off-by: Levi Harrison <[email protected]>
Unfortunately, further test cases will have to be removed because gitindentifier will now leave the branch blank instead of filling it in Signed-off-by: Levi Harrison <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
| func getDefaultBranch(ctx context.Context, gitDir, workDir, sshAuthSock, knownHosts string, auth []string, remoteURL string) (string, error) { | ||
| buf, err := gitWithinDir(ctx, gitDir, workDir, sshAuthSock, knownHosts, auth, "ls-remote", "--symref", remoteURL, "HEAD") | ||
| if err != nil { | ||
| return "", errors.Wrapf(err, "error fetching default branch for repository %s", redactCredentials(remoteURL)) |
There was a problem hiding this comment.
Can we fallback to "master" on error?
There was a problem hiding this comment.
Do you know a case that wouldn't work? The problem is that this is the first network request via git so there is actually a good chance that this will legitimately fail and I wouldn't want to hide the error.
There was a problem hiding this comment.
When was (EDIT: no later than 2007: git/git@7c2c6ee)ls-remote introduced?
Some git server implementation may not support ls-remote?
There was a problem hiding this comment.
We already had a dependency on ls-remote before to get the sha for the ref.
| // getDefaultBranch gets the default branch of a repository using ls-remote | ||
| func getDefaultBranch(ctx context.Context, gitDir, workDir, sshAuthSock, knownHosts string, auth []string, remoteURL string) (string, error) { | ||
| buf, err := gitWithinDir(ctx, gitDir, workDir, sshAuthSock, knownHosts, auth, "ls-remote", "--symref", remoteURL, "HEAD") | ||
| if err != nil { |
There was a problem hiding this comment.
I think it would be helpful to include a hint on how you would set the branch being used (add #branchname to URL). If greenlit, I can make a PR for the hint in error messages.
carry #2011
fix moby/moby#41914
@LeviHarrison