Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force-checkout remote branch to fix unrelated histories #228

Merged
merged 7 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions gitclone/checkout_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,19 @@ func checkoutWithCustomRetry(gitCmd git.Git, arg string, retry fallbackRetry) er
return nil
}

func fetchInitialBranch(gitCmd git.Git, remote string, branchRef string, fetchTraits fetchOptions) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Naming is hard 😵‍💫

func forceCheckoutRemoteBranch(gitCmd git.Git, remote string, branchRef string, fetchTraits fetchOptions) error {
branch := strings.TrimPrefix(branchRef, refsHeadsPrefix)
if err := fetch(gitCmd, remote, branchRef, fetchTraits); err != nil {
wErr := fmt.Errorf("failed to fetch branch (%s): %w", branchRef, err)
wErr := fmt.Errorf("fetch branch %s: %w", branchRef, err)
return fmt.Errorf("%v: %w", wErr, errors.New("please make sure the branch still exists"))
}

if err := checkoutWithCustomRetry(gitCmd, branch, nil); err != nil {

remoteBranch := fmt.Sprintf("%s/%s", remote, branch)
// -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.
err := runner.Run(gitCmd.Checkout("-B", branch, remoteBranch))
if err != nil {
return handleCheckoutError(
Copy link
Contributor Author

@ofalvai ofalvai Dec 11, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted handleCheckoutError so the above should keep working.
checkoutWithCustomRetry is still removed, but because it was called with a nil retry function, it wasn't doing anything interesting.

listBranches(gitCmd),
checkoutFailedTag,
Expand All @@ -116,16 +121,6 @@ func fetchInitialBranch(gitCmd git.Git, remote string, branchRef string, fetchTr
)
}

// Update branch: 'git fetch' followed by a 'git merge' is the same as 'git pull'.
remoteBranch := fmt.Sprintf("%s/%s", remote, branch)
if err := runner.Run(gitCmd.Merge(remoteBranch)); err != nil {
return newStepError(
"update_branch_failed",
fmt.Errorf("updating branch (%s) failed: %w", branch, err),
"Updating branch failed",
)
}

return nil
}

Expand Down
2 changes: 1 addition & 1 deletion gitclone/checkout_method_pr_manual.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type checkoutPRManualMerge struct {
func (c checkoutPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error {
// Fetch and checkout destinations branch
destBranchRef := refsHeadsPrefix + c.params.DestinationBranch
if err := fetchInitialBranch(gitCmd, originRemoteName, destBranchRef, fetchOptions); err != nil {
if err := forceCheckoutRemoteBranch(gitCmd, originRemoteName, destBranchRef, fetchOptions); err != nil {
return fmt.Errorf("failed to fetch base branch: %w", err)
}

Expand Down
2 changes: 1 addition & 1 deletion gitclone/checkout_method_simple.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type checkoutBranch struct {
}

func (c checkoutBranch) do(gitCmd git.Git, fetchOptions fetchOptions, _ fallbackRetry) error {
if err := fetchInitialBranch(gitCmd, originRemoteName, c.localRef(), fetchOptions); err != nil {
if err := forceCheckoutRemoteBranch(gitCmd, originRemoteName, c.localRef(), fetchOptions); err != nil {
return err
}

Expand Down
16 changes: 6 additions & 10 deletions gitclone/gitclone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ func Test_checkoutState(t *testing.T) {
},
wantCmds: []string{
`git "fetch" "--jobs=10" "--depth=1" "--no-tags" "--no-recurse-submodules" "origin" "refs/heads/hcnarb"`,
`git "checkout" "hcnarb"`,
`git "merge" "origin/hcnarb"`,
`git "checkout" "-B" "hcnarb" "origin/hcnarb"`,
},
},
{
Expand Down Expand Up @@ -204,7 +203,7 @@ func Test_checkoutState(t *testing.T) {
`git "fetch" "--jobs=10"`,
`git "branch" "-r"`,
},
wantErr: fmt.Errorf("failed to fetch base branch: failed to fetch branch (refs/heads/master): dummy_cmd_error: please make sure the branch still exists"),
wantErr: fmt.Errorf("failed to fetch base branch: fetch branch refs/heads/master: dummy_cmd_error: please make sure the branch still exists"),
},
{
name: "PR - fork - no merge ref - diff file available",
Expand Down Expand Up @@ -245,8 +244,7 @@ func Test_checkoutState(t *testing.T) {
`git "checkout" "master"`,
`git "apply" "--index" "diff_path"`,
`git "fetch" "--jobs=10" "--depth=1" "--no-tags" "--no-recurse-submodules" "origin" "refs/heads/master"`,
`git "checkout" "master"`,
`git "merge" "origin/master"`,
`git "checkout" "-B" "master" "origin/master"`,
`git "log" "-1" "--format=%H"`,
`git "fetch" "--jobs=10" "--depth=1" "--no-tags" "--no-recurse-submodules" "origin" "refs/heads/test/commit-messages"`,
`git "merge" "76a934ae"`,
Expand All @@ -273,8 +271,7 @@ func Test_checkoutState(t *testing.T) {
`git "checkout" "master"`,
`git "apply" "--index" "diff_path"`,
`git "fetch" "--jobs=10" "--depth=1" "--no-tags" "--no-recurse-submodules" "origin" "refs/heads/master"`,
`git "checkout" "master"`,
`git "merge" "origin/master"`,
`git "checkout" "-B" "master" "origin/master"`,
`git "log" "-1" "--format=%H"`,
`git "remote" "add" "fork" "git@github.com:bitrise-io/other-repo.git"`,
`git "fetch" "--jobs=10" "--depth=1" "--no-tags" "--no-recurse-submodules" "fork" "refs/heads/test/commit-messages"`,
Expand Down Expand Up @@ -375,7 +372,7 @@ func Test_checkoutState(t *testing.T) {
},
wantErr: newStepErrorWithBranchRecommendations(
fetchFailedTag,
fmt.Errorf("failed to fetch branch (refs/heads/fake): %w: please make sure the branch still exists", errors.New(rawCmdError)),
fmt.Errorf("fetch branch refs/heads/fake: %w: please make sure the branch still exists", errors.New(rawCmdError)),
"Fetching repository has failed",
"fake",
[]string{"master"},
Expand Down Expand Up @@ -466,8 +463,7 @@ func Test_checkoutState(t *testing.T) {
},
wantCmds: []string{
`git "fetch" "--jobs=10" "--depth=1" "--filter=tree:0" "--no-tags" "--no-recurse-submodules" "origin" "refs/heads/hcnarb"`,
`git "checkout" "hcnarb"`,
`git "merge" "origin/hcnarb"`,
`git "checkout" "-B" "hcnarb" "origin/hcnarb"`,
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion gitclone/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func Test_gitOutputs(t *testing.T) {
gitCmd, err := git.New(t.TempDir())
if err != nil {
t.Fatalf(err.Error())
t.Fatal(err)
}
type args struct {
gitRef string
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/bitrise-io/envman v0.0.0-20210517135508-b2b4fe89eac5
github.com/bitrise-io/go-steputils v1.0.5
github.com/bitrise-io/go-steputils/v2 v2.0.0-alpha.15
github.com/bitrise-io/go-utils v1.0.13
github.com/bitrise-io/go-utils v1.0.14
github.com/bitrise-io/go-utils/v2 v2.0.0-alpha.15
github.com/bitrise-steplib/steps-authenticate-host-with-netrc v0.0.0-20230711084209-91fcd09b2017
github.com/hashicorp/go-retryablehttp v0.7.7
Expand Down
13 changes: 8 additions & 5 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ github.com/bitrise-io/go-utils v0.0.0-20210517140706-aa64fd88ca49/go.mod h1:DRx7
github.com/bitrise-io/go-utils v1.0.1/go.mod h1:ZY1DI+fEpZuFpO9szgDeICM4QbqoWVt0RSY3tRI1heY=
github.com/bitrise-io/go-utils v1.0.13 h1:1QENhTS/JlKH9F7+/nB+TtbTcor6jGrE6cQ4CJWfp5U=
github.com/bitrise-io/go-utils v1.0.13/go.mod h1:ZY1DI+fEpZuFpO9szgDeICM4QbqoWVt0RSY3tRI1heY=
github.com/bitrise-io/go-utils v1.0.14-0.20241213163208-7104b0101841 h1:UbDuwrjY1ONky1Kk5i6vZhpgymoHZdn8bygsJ2XYEO4=
github.com/bitrise-io/go-utils v1.0.14-0.20241213163208-7104b0101841/go.mod h1:ZY1DI+fEpZuFpO9szgDeICM4QbqoWVt0RSY3tRI1heY=
github.com/bitrise-io/go-utils v1.0.14 h1:hrBriQh47qvy4p3pLkc7gEw9qCWEJYESTJ19ggHRpYs=
github.com/bitrise-io/go-utils v1.0.14/go.mod h1:ZY1DI+fEpZuFpO9szgDeICM4QbqoWVt0RSY3tRI1heY=
github.com/bitrise-io/go-utils/v2 v2.0.0-alpha.15 h1:ERQb+OOa+eKMWb+HByyPd5ugz6TeaJPnQ3xHgyaR7hA=
github.com/bitrise-io/go-utils/v2 v2.0.0-alpha.15/go.mod h1:Laih4ji980SQkRgdnMCH0g4u2GZI/5nnbqmYT9UfKFQ=
github.com/bitrise-io/go-xcode v0.0.0-20210517092111-792daa927657/go.mod h1:OexOTlMlXuf88bsFsaw5KxmIYrYDsHkTh01vbhGNpus=
Expand All @@ -42,6 +46,7 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsr
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
github.com/fullsailor/pkcs7 v0.0.0-20190404230743-d7302db945fa/go.mod h1:KnogPXtdwXqoenmZCw6S+25EAm2MkxbG0deNDu4cbSA=
github.com/gofrs/uuid v4.3.1+incompatible h1:0/KbAdpx3UXAx1kEOWHJeOkpbgRFGHVgv+CFIY7dBJI=
github.com/gofrs/uuid v4.3.1+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
Expand All @@ -50,12 +55,10 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI=
github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
github.com/hashicorp/go-hclog v1.6.3 h1:Qr2kF+eVWjTiYmU7Y31tYlP1h0q/X3Nl3tPGdaB11/k=
github.com/hashicorp/go-retryablehttp v0.6.6/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
github.com/hashicorp/go-retryablehttp v0.7.0/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
github.com/hashicorp/go-retryablehttp v0.7.1 h1:sUiuQAnLlbvmExtFQs72iFW/HXeUn8Z1aJLQ4LJJbTQ=
github.com/hashicorp/go-retryablehttp v0.7.1/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
github.com/hashicorp/go-retryablehttp v0.7.7 h1:C8hUCYzor8PIfXHa4UrZkU4VvK8o9ISHxT2Q8+VepXU=
github.com/hashicorp/go-retryablehttp v0.7.7/go.mod h1:pkQpWZeYWskR+D1tR2O5OcBFOxfA7DoAO6xtkuQnHTk=
github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
Expand All @@ -69,6 +72,8 @@ github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfn
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
Expand Down Expand Up @@ -123,8 +128,6 @@ golang.org/x/sys v0.0.0-20210511113859-b0526f3d8744/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20210514084401-e8d321eab015/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211205182925-97ca703d548d/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y=
golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
Expand Down
7 changes: 4 additions & 3 deletions vendor/github.com/bitrise-io/go-utils/command/git/commands.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ github.com/bitrise-io/go-steputils/step
## explicit; go 1.17
github.com/bitrise-io/go-steputils/v2/export
github.com/bitrise-io/go-steputils/v2/stepconf
# github.com/bitrise-io/go-utils v1.0.13
# github.com/bitrise-io/go-utils v1.0.14
## explicit; go 1.13
github.com/bitrise-io/go-utils/colorstring
github.com/bitrise-io/go-utils/command
Expand Down