diff --git a/bitrise.yml b/bitrise.yml index b650aad7..76c98be5 100644 --- a/bitrise.yml +++ b/bitrise.yml @@ -635,7 +635,7 @@ workflows: - pull_request_merge_branch: "" - branch_dest: "master" - branch: "master" - - commit: "" + - commit: "c6810e6" - tag: "" - clone_depth: "1" - manual_merge: "no" diff --git a/gitclone/checkout.go b/gitclone/checkout.go index f3112c48..e730ea4e 100644 --- a/gitclone/checkout.go +++ b/gitclone/checkout.go @@ -156,7 +156,12 @@ func createCheckoutStrategy(checkoutMethod CheckoutMethod, cfg Config, patch pat return nil, fmt.Errorf("merging PR (automatic) failed, there is no Pull Request branch and could not download diff file: %v", err) } - params, err := NewPRDiffFileParams(cfg.BranchDest) + prManualMergeParam, forkPRManualMergeParam, err := createManualMergeParams(cfg) + if err != nil { + return nil, err + } + + params, err := NewPRDiffFileParams(cfg.BranchDest, prManualMergeParam, forkPRManualMergeParam) if err != nil { return nil, err } @@ -228,3 +233,12 @@ func selectFallbacks(checkoutStrategy CheckoutMethod, fetchOpts fetchOptions) fa return nil } } + +func createManualMergeParams(cfg Config) (prManualMergeParam *PRManualMergeParams, forkPRManualMergeParam *ForkPRManualMergeParams, err error) { + if isFork(cfg.RepositoryURL, cfg.PRRepositoryURL) { + forkPRManualMergeParam, err = NewForkPRManualMergeParams(cfg.Branch, cfg.PRRepositoryURL, cfg.BranchDest) + } else { + prManualMergeParam, err = NewPRManualMergeParams(cfg.Branch, cfg.Commit, cfg.BranchDest) + } + return +} diff --git a/gitclone/checkout_helper.go b/gitclone/checkout_helper.go index 7c0e75ab..ba30be28 100644 --- a/gitclone/checkout_helper.go +++ b/gitclone/checkout_helper.go @@ -10,6 +10,17 @@ import ( "github.com/bitrise-io/go-utils/log" ) +type fetchParams struct { + branch string + remote string + options fetchOptions +} + +type mergeParams struct { + arg string + fallback fallbackRetry +} + type fetchOptions struct { // Sets '--tags' flag // From https://git-scm.com/docs/fetch-options/2.29.0#Documentation/fetch-options.txt---allTags: @@ -94,7 +105,7 @@ 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", defaultRemoteName, branch) + remoteBranch := fmt.Sprintf("%s/%s", originRemoteName, branch) if err := runner.Run(gitCmd.Merge(remoteBranch)); err != nil { return newStepError( "update_branch_failed", @@ -123,6 +134,15 @@ func mergeWithCustomRetry(gitCmd git.Git, arg string, retry fallbackRetry) error return nil } +func fetchAndMerge(gitCmd git.Git, fetchParam fetchParams, mergeParam mergeParams) error { + headBranchRef := branchRefPrefix + fetchParam.branch + if err := fetch(gitCmd, fetchParam.remote, headBranchRef, fetchParam.options); err != nil { + return nil + } + + return mergeWithCustomRetry(gitCmd, mergeParam.arg, mergeParam.fallback) +} + func detachHead(gitCmd git.Git) error { if err := runner.Run(gitCmd.Checkout("--detach")); err != nil { return newStepError( diff --git a/gitclone/checkout_method_pr_auto_diff.go b/gitclone/checkout_method_pr_auto_diff.go index 3ff07d63..aa0d4991 100644 --- a/gitclone/checkout_method_pr_auto_diff.go +++ b/gitclone/checkout_method_pr_auto_diff.go @@ -9,23 +9,32 @@ import ( "github.com/bitrise-io/go-steputils/input" "github.com/bitrise-io/go-utils/command/git" + "github.com/bitrise-io/go-utils/log" "github.com/bitrise-steplib/bitrise-step-export-universal-apk/filedownloader" ) // // PRDiffFileParams are parameters to check out a Merge/Pull Request (when a diff file is available) type PRDiffFileParams struct { - BaseBranch string + BaseBranch string + PRManualMergeParam *PRManualMergeParams + ForkPRManualMergeParam *ForkPRManualMergeParams } // NewPRDiffFileParams validates and returns a new PRDiffFileParams -func NewPRDiffFileParams(baseBranch string) (*PRDiffFileParams, error) { +func NewPRDiffFileParams( + baseBranch string, + prManualMergeParam *PRManualMergeParams, + forkPRManualMergeParam *ForkPRManualMergeParams, +) (*PRDiffFileParams, error) { if strings.TrimSpace(baseBranch) == "" { return nil, NewParameterValidationError("PR diff file based checkout strategy can not be used: no base branch specified") } return &PRDiffFileParams{ - BaseBranch: baseBranch, + BaseBranch: baseBranch, + PRManualMergeParam: prManualMergeParam, + ForkPRManualMergeParam: forkPRManualMergeParam, }, nil } @@ -37,7 +46,7 @@ type checkoutPRDiffFile struct { func (c checkoutPRDiffFile) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error { baseBranchRef := branchRefPrefix + c.params.BaseBranch - if err := fetch(gitCmd, defaultRemoteName, baseBranchRef, fetchOptions); err != nil { + if err := fetch(gitCmd, originRemoteName, baseBranchRef, fetchOptions); err != nil { return err } @@ -46,7 +55,16 @@ func (c checkoutPRDiffFile) do(gitCmd git.Git, fetchOptions fetchOptions, fallba } if err := runner.Run(gitCmd.Apply(c.patchFile)); err != nil { - return fmt.Errorf("could not apply patch (%s): %v", c.patchFile, err) + log.Warnf("Could not apply patch (%s): %v", c.patchFile, err) + log.Warnf("Falling back to manual merge...") + + return fallbackToManualMergeOnApplyError( + gitCmd, + c.params.PRManualMergeParam, + c.params.ForkPRManualMergeParam, + fetchOptions, + fallback, + ) } return detachHead(gitCmd) @@ -72,3 +90,44 @@ func (defaultPatchSource) getDiffPath(buildURL, apiToken string) (string, error) fileProvider := input.NewFileProvider(filedownloader.New(http.DefaultClient)) return fileProvider.LocalPath(diffURL) } + +func fallbackToManualMergeOnApplyError( + gitCmd git.Git, + prManualMergeParam *PRManualMergeParams, + forkPRManualMergeParam *ForkPRManualMergeParams, + fetchOptions fetchOptions, + fallback fallbackRetry, +) error { + var fetchParam fetchParams + var mergeParam mergeParams + + if prManualMergeParam != nil { + fetchParam = fetchParams{ + branch: prManualMergeParam.HeadBranch, + remote: originRemoteName, + options: fetchOptions, + } + + mergeParam = mergeParams{ + arg: prManualMergeParam.Commit, + fallback: fallback, + } + } else if forkPRManualMergeParam != nil { + if err := runner.Run(gitCmd.RemoteAdd(forkRemoteName, forkPRManualMergeParam.HeadRepoURL)); err != nil { + return fmt.Errorf("adding remote fork repository failed (%s): %v", forkPRManualMergeParam.HeadRepoURL, err) + } + + fetchParam = fetchParams{ + branch: forkPRManualMergeParam.HeadBranch, + remote: forkRemoteName, + options: fetchOptions, + } + + mergeParam = mergeParams{ + arg: fmt.Sprintf("%s/%s", forkRemoteName, forkPRManualMergeParam.HeadBranch), + fallback: fallback, + } + } + + return fetchAndMerge(gitCmd, fetchParam, mergeParam) +} diff --git a/gitclone/checkout_method_pr_auto_merge_branch.go b/gitclone/checkout_method_pr_auto_merge_branch.go index 3d30165b..34daebfc 100644 --- a/gitclone/checkout_method_pr_auto_merge_branch.go +++ b/gitclone/checkout_method_pr_auto_merge_branch.go @@ -38,13 +38,13 @@ func (c checkoutPRMergeBranch) do(gitCmd git.Git, fetchOpts fetchOptions, fallba // Check out initial branch (fetchInitialBranch part1) // `git "fetch" "origin" "refs/heads/master"` baseBranchRef := branchRefPrefix + c.params.BaseBranch - if err := fetch(gitCmd, defaultRemoteName, baseBranchRef, fetchOpts); err != nil { + if err := fetch(gitCmd, originRemoteName, baseBranchRef, fetchOpts); err != nil { return err } // `git "fetch" "origin" "refs/pull/7/head:pull/7"` headBranchRef := fetchArg(c.params.MergeBranch) - if err := fetch(gitCmd, defaultRemoteName, headBranchRef, fetchOpts); err != nil { + if err := fetch(gitCmd, originRemoteName, headBranchRef, fetchOpts); err != nil { return err } @@ -54,7 +54,7 @@ func (c checkoutPRMergeBranch) do(gitCmd git.Git, fetchOpts fetchOptions, fallba if err := checkoutWithCustomRetry(gitCmd, c.params.BaseBranch, nil); err != nil { return err } - remoteBaseBranch := fmt.Sprintf("%s/%s", defaultRemoteName, c.params.BaseBranch) + remoteBaseBranch := fmt.Sprintf("%s/%s", originRemoteName, c.params.BaseBranch) if err := runner.Run(gitCmd.Merge(remoteBaseBranch)); err != nil { return err } diff --git a/gitclone/checkout_method_pr_manual.go b/gitclone/checkout_method_pr_manual.go index 8ffeba80..cfc1e022 100644 --- a/gitclone/checkout_method_pr_manual.go +++ b/gitclone/checkout_method_pr_manual.go @@ -44,7 +44,7 @@ type checkoutPRManualMerge struct { func (c checkoutPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error { // Fetch and checkout base (target) branch baseBranchRef := branchRefPrefix + c.params.BaseBranch - if err := fetchInitialBranch(gitCmd, defaultRemoteName, baseBranchRef, fetchOptions); err != nil { + if err := fetchInitialBranch(gitCmd, originRemoteName, baseBranchRef, fetchOptions); err != nil { return err } @@ -56,7 +56,7 @@ func (c checkoutPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions, fal // Fetch and merge headBranchRef := branchRefPrefix + c.params.HeadBranch - if err := fetch(gitCmd, defaultRemoteName, headBranchRef, fetchOptions); err != nil { + if err := fetch(gitCmd, originRemoteName, headBranchRef, fetchOptions); err != nil { return nil } @@ -103,7 +103,7 @@ type checkoutForkPRManualMerge struct { func (c checkoutForkPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error { // Fetch and checkout base branch baseBranchRef := branchRefPrefix + c.params.BaseBranch - if err := fetchInitialBranch(gitCmd, defaultRemoteName, baseBranchRef, fetchOptions); err != nil { + if err := fetchInitialBranch(gitCmd, originRemoteName, baseBranchRef, fetchOptions); err != nil { return err } diff --git a/gitclone/checkout_method_simple.go b/gitclone/checkout_method_simple.go index 75e8750a..f2035765 100644 --- a/gitclone/checkout_method_simple.go +++ b/gitclone/checkout_method_simple.go @@ -44,7 +44,7 @@ func (c checkoutCommit) do(gitCmd git.Git, fetchOptions fetchOptions, fallback f branchRefParam = branchRefPrefix + c.params.Branch } - if err := fetch(gitCmd, defaultRemoteName, branchRefParam, fetchOptions); err != nil { + if err := fetch(gitCmd, originRemoteName, branchRefParam, fetchOptions); err != nil { return err } @@ -79,7 +79,7 @@ type checkoutBranch struct { func (c checkoutBranch) do(gitCmd git.Git, fetchOptions fetchOptions, _ fallbackRetry) error { branchRef := branchRefPrefix + c.params.Branch - if err := fetchInitialBranch(gitCmd, defaultRemoteName, branchRef, fetchOptions); err != nil { + if err := fetchInitialBranch(gitCmd, originRemoteName, branchRef, fetchOptions); err != nil { return err } @@ -116,7 +116,7 @@ func (c checkoutTag) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fall branchRefParam = branchRefPrefix + c.params.Branch } - if err := fetch(gitCmd, defaultRemoteName, branchRefParam, fetchOptions); err != nil { + if err := fetch(gitCmd, originRemoteName, branchRefParam, fetchOptions); err != nil { return err } diff --git a/gitclone/git.go b/gitclone/git.go index 46717a8f..b648d8ac 100644 --- a/gitclone/git.go +++ b/gitclone/git.go @@ -157,7 +157,7 @@ func handleCheckoutError(callback getAvailableBranches, tag string, err error, s // We were checking out a branch (not tag or commit) if branch != "" { branchesByRemote, branchesErr := callback() - branches := branchesByRemote[defaultRemoteName] + branches := branchesByRemote[originRemoteName] // There was no error grabbing the available branches // And the current branch is not present in the list if branchesErr == nil && !sliceutil.IsStringInSlice(branch, branches) { diff --git a/gitclone/git_test.go b/gitclone/git_test.go index 476fdb5a..fc18fad7 100644 --- a/gitclone/git_test.go +++ b/gitclone/git_test.go @@ -141,7 +141,7 @@ func Test_handleCheckoutError(t *testing.T) { args: args{ callback: func() (map[string][]string, error) { return map[string][]string{ - defaultRemoteName: {"master", "develop"}, + originRemoteName: {"master", "develop"}, }, nil }, tag: "checkout_failed", @@ -192,7 +192,7 @@ func Test_handleCheckoutError(t *testing.T) { args: args{ callback: func() (map[string][]string, error) { return map[string][]string{ - defaultRemoteName: {"master", "develop", "test"}, + originRemoteName: {"master", "develop", "test"}, }, nil }, tag: "checkout_failed", diff --git a/gitclone/gitclone.go b/gitclone/gitclone.go index 4b9589cb..6813d44b 100644 --- a/gitclone/gitclone.go +++ b/gitclone/gitclone.go @@ -32,8 +32,9 @@ type Config struct { const ( trimEnding = "..." - defaultRemoteName = "origin" + originRemoteName = "origin" updateSubmodelFailedTag = "update_submodule_failed" + forkRemoteName = "fork" ) func printLogAndExportEnv(gitCmd git.Git, format, env string, maxEnvLength int) error { @@ -130,7 +131,7 @@ func Execute(cfg Config) error { ) } if !originPresent { - if err := runner.Run(gitCmd.RemoteAdd(defaultRemoteName, cfg.RepositoryURL)); err != nil { + if err := runner.Run(gitCmd.RemoteAdd(originRemoteName, cfg.RepositoryURL)); err != nil { return newStepError( "add_remote_failed", fmt.Errorf("adding remote repository failed (%s): %v", cfg.RepositoryURL, err), diff --git a/gitclone/gitclone_test.go b/gitclone/gitclone_test.go index 78e9e549..e21ffc68 100644 --- a/gitclone/gitclone_test.go +++ b/gitclone/gitclone_test.go @@ -284,6 +284,7 @@ var testCases = [...]struct { BranchDest: "master", PRID: 7, ManualMerge: false, + Commit: "76a934ae", CloneDepth: 1, }, patchSource: MockPatchSource{"diff_path", nil}, @@ -295,6 +296,54 @@ var testCases = [...]struct { `git "checkout" "--detach"`, }, }, + { + name: "PR - no fork - auto merge - diff file: fallback to manual merge if unable to apply patch", + cfg: Config{ + RepositoryURL: "https://github.com/bitrise-io/git-clone-test.git", + Branch: "test/commit-messages", + BranchDest: "master", + PRID: 7, + ManualMerge: false, + Commit: "76a934ae", + CloneDepth: 1, + }, + patchSource: MockPatchSource{"diff_path", nil}, + mockRunner: givenMockRunner(). + GivenRunFailsForCommand(`git "apply" "--index" "diff_path"`, 1). + GivenRunWithRetrySucceeds(). + GivenRunSucceeds(), + wantCmds: []string{ + `git "fetch" "--depth=1" "origin" "refs/heads/master"`, + `git "checkout" "master"`, + `git "apply" "--index" "diff_path"`, + `git "fetch" "--depth=1" "origin" "refs/heads/test/commit-messages"`, + `git "merge" "76a934ae"`, + }, + }, + { + name: "PR - fork - auto merge - diff file: fallback to manual merge if unable to apply patch", + cfg: Config{ + RepositoryURL: "https://github.com/bitrise-io/git-clone-test.git", + PRRepositoryURL: "git@github.com:bitrise-io/other-repo.git", + Branch: "test/commit-messages", + BranchDest: "master", + Commit: "76a934ae", + ManualMerge: true, + }, + patchSource: MockPatchSource{"diff_path", nil}, + mockRunner: givenMockRunner(). + GivenRunFailsForCommand(`git "apply" "--index" "diff_path"`, 1). + GivenRunWithRetrySucceeds(). + GivenRunSucceeds(), + wantCmds: []string{ + `git "fetch" "origin" "refs/heads/master"`, + `git "checkout" "master"`, + `git "apply" "--index" "diff_path"`, + `git "remote" "add" "fork" "git@github.com:bitrise-io/other-repo.git"`, + `git "fetch" "fork" "refs/heads/test/commit-messages"`, + `git "merge" "fork/test/commit-messages"`, + }, + }, // ** Errors ** {