Force-checkout remote branch to fix unrelated histories #228
Conversation
ofalvai
changed the title
| @@ -99,31 +99,21 @@ func checkoutWithCustomRetry(gitCmd git.Git, arg string, retry fallbackRetry) er | |||
| return nil | |||
| } | |||
|
|
|||
| func fetchInitialBranch(gitCmd git.Git, remote string, branchRef string, fetchTraits fetchOptions) error { | |||
| return fmt.Errorf("%v: %w", wErr, errors.New("please make sure the branch still exists")) | ||
| } | ||
|
|
||
| if err := checkoutWithCustomRetry(gitCmd, branch, nil); err != nil { | ||
| return handleCheckoutError( |
There was a problem hiding this comment.
The custom retry here adds nicer error message (a list of existing branch names), but if a branch doesn't exist, I think it fails in the fetch step above.
I don't have a strong opinion on this, but I don't like this abstraction and I can easily say goodbye to this.
I was also thinking about adding -B to this checkout wrapper, but it only works in the context of checking out branches, while the wrapper is used for tags and commits as well.
There was a problem hiding this comment.
I have vague memories about this error handling, I think the project scanner uses it. Probably the returned branches are propagated to the UI.
I suggest checking if it is in use by the scanner, or leaving it here.
There was a problem hiding this comment.
"Unfortunately" I found references to it even in the frontend code, so this is still used: https://github.com/bitrise-io/bitrise-website/blob/2815eaa08f3d1d31bd782535dffa56c69a549dae/app/javascript/pages/AddNewApp/stages/ProjectConfig/ProjectConfig.types.ts#L11
gitclone/checkout_helper.go
Outdated
| // -B: create the branch if it doesn't exist, reset if it does | ||
| // The latter is important in persistent environments because shallow-fetching only fetches 1 commit, | ||
| // so the next run would see unrelated histories after shallow-fetching another single commit. | ||
| out, err := runner.RunForOutput(command.New("git", "checkout", "-B", branch, remoteBranch)) |
There was a problem hiding this comment.
Shouldn't we use gitCmd.Checkout("-B", branch, remoteBranch) instead of a raw command? The reason is that gitCmd is initialised with a workadir, the command here only works if pwd = repo root.
(gitCmd.Checkout accepts a single argumentum, we might update to a variadic function)
| return fmt.Errorf("%v: %w", wErr, errors.New("please make sure the branch still exists")) | ||
| } | ||
|
|
||
| if err := checkoutWithCustomRetry(gitCmd, branch, nil); err != nil { | ||
| return handleCheckoutError( |
There was a problem hiding this comment.
I have vague memories about this error handling, I think the project scanner uses it. Probably the returned branches are propagated to the UI.
I suggest checking if it is in use by the scanner, or leaving it here.
Checklist
step.ymlandREADME.mdis updated with the changes (if needed)Version
Requires a MAJOR/MINOR/PATCH version update
Context
Git clones can fail in persistent environments (bare-metal runners) when the build is triggered by a push event. Imagine this sequence:
origin/main(because it's not a PR build) with--depth=1(default step setting).origin/mainorigin/mainagain with--depth=1, but when it tries to checkout and merge the local branch, it fails with therefusing to merge unrelated historieserror.An actual step log:
Changes
Branch checkout was implemented using a
fetch+checkout+merge remote localsequence. This doesn't work in persistent envs because there is a leftover local branch from the previous run with a single commit (because of shallow fetching). The new remote-tracking branch also has a single commit, and there is no common ancestor of these two commits, so the merge command fails.This PR changes the implementation of our branch checkout so that any previous local branch state is overwritten, sort of like a
git pull --force.The end result remains the same:
Investigation details
Decisions