[ci_runner] Revert use merge commit sha from webhook event (buildbudd…

…y-io#6888)

enterprise/server/cmd/ci_runner/main.go

@@ -172,9 +172,8 @@ var (
 	gitFetchFilters = flag.Slice("git_fetch_filters", []string{}, "Filters to apply to `git fetch` commands.")
 	gitFetchDepth   = flag.Int("git_fetch_depth", smartFetchDepth, "Depth to use for `git fetch` commands.")
 	// Flags to configure merge-with-base behavior
-	targetRepoURL  = flag.String("target_repo_url", "", "If different from pushed_repo_url, indicates a fork (`pushed_repo_url`) is being merged into this repo.")
-	targetBranch   = flag.String("target_branch", "", "If different from pushed_branch, pushed_branch should be merged into this branch in the target repo.")
-	mergeCommitSHA = flag.String("merge_commit_sha", "", "If set and merge-with-base is enabled, checkout this commit sha directly instead of manually fetching the target repo and merging to generate it.")
+	targetRepoURL = flag.String("target_repo_url", "", "If different from pushed_repo_url, indicates a fork (`pushed_repo_url`) is being merged into this repo.")
+	targetBranch  = flag.String("target_branch", "", "If different from pushed_branch, pushed_branch should be merged into this branch in the target repo.")
 
 	shutdownAndExit = flag.Bool("shutdown_and_exit", false, "If set, runs bazel shutdown with the configured bazel_command, and exits. No other commands are run.")
 
@@ -1785,9 +1784,6 @@ func (ws *workspace) sync(ctx context.Context) error {
 func (ws *workspace) shouldMergeBranches(actionTriggers *config.Triggers) bool {
 	return *triggerEvent == webhook_data.EventName.PullRequest &&
 		actionTriggers.GetPullRequestTrigger().GetMergeWithBase() &&
-		// If the merge commit SHA already exists, we don't need to re-merge the branches
-		// to generate it
-		*mergeCommitSHA == "" &&
 		ws.hasMultipleBranches()
 }
 
@@ -1805,12 +1801,6 @@ func (ws *workspace) fetchPushedRef(ctx context.Context) error {
 		fetchDepth = *gitFetchDepth
 	}
 
-	// If the merge commit sha was pre-generated and passed as an argument,
-	// we should fetch it from the target repo.
-	if *mergeCommitSHA != "" {
-		return ws.fetch(ctx, *targetRepoURL, []string{*mergeCommitSHA}, fetchDepth)
-	}
-
 	refToFetch := *commitSHA
 	if refToFetch == "" {
 		refToFetch = *pushedBranch
@@ -1855,10 +1845,6 @@ func (ws *workspace) checkoutRef(ctx context.Context) error {
 	if checkoutRef == "" {
 		checkoutRef = fmt.Sprintf("%s/%s", gitRemoteName(*pushedRepoURL), *pushedBranch)
 	}
-	if *mergeCommitSHA != "" {
-		checkoutRef = *mergeCommitSHA
-		writeCommandSummary(ws.log, fmt.Sprintf("Checking out merge commit sha %s...", checkoutRef))
-	}
 
 	if checkoutLocalBranchName != "" {
 		// Create the local branch if it doesn't already exist, then update it to point to the checkout ref

enterprise/server/test/integration/ci_runner/ci_runner_test.go

@@ -680,60 +680,6 @@ func TestCIRunner_Merge_FetchesCompleteGitHistory(t *testing.T) {
 	checkRunnerResult(t, result)
 }
 
-func TestCIRunner_Merge_MergeCommitSHA(t *testing.T) {
-	wsPath := testfs.MakeTempDir(t)
-	repoPath, _ := makeGitRepo(t, workspaceContentsWithRunScript)
-
-	// Create a feature branch
-	testshell.Run(t, repoPath, `
-		git checkout -B feature
-	`)
-	modifiedWorkflowConfig := `
-actions:
-  - name: "Print args"
-    triggers:
-      pull_request: { branches: [ master ] }
-      push: { branches: [ master ] }
-    bazel_commands:
-      - run //:print_args -- "Switcheroo!"
-`
-	_ = testgit.CommitFiles(t, repoPath, map[string]string{"buildbuddy.yaml": modifiedWorkflowConfig})
-
-	// Create another branch to generate the merge commit sha
-	testshell.Run(t, repoPath, `
-		git checkout -B merge_branch
-		git merge master
-	`)
-	mergeCommitSHA := strings.TrimSpace(testshell.Run(t, repoPath, `git rev-parse HEAD`))
-
-	baselineRunnerFlags := []string{
-		"--workflow_id=test-workflow",
-		"--action_name=Print args",
-		"--trigger_event=pull_request",
-		"--pushed_repo_url=file://" + repoPath,
-		"--pushed_branch=feature",
-		"--merge_commit_sha=" + mergeCommitSHA,
-		"--target_repo_url=file://" + repoPath,
-		"--target_branch=master",
-		// Disable clean checkout fallback for this test since we expect to sync
-		// without errors.
-		"--fallback_to_clean_checkout=false",
-	}
-	// Start the app so the runner can use it as the BES backend.
-	app := buildbuddy.Run(t)
-	baselineRunnerFlags = append(baselineRunnerFlags, app.BESBazelFlags()...)
-
-	result := invokeRunner(t, baselineRunnerFlags, []string{}, wsPath)
-	checkRunnerResult(t, result)
-
-	// Runner should not fetch the target branch or merge the branches manually
-	// because the merge commit sha was set
-	runnerInvocation := singleInvocation(t, app, result)
-	assert.NotContains(t, runnerInvocation.ConsoleBuffer, "git merge")
-	// Output should contain modified print statement from feature branch
-	assert.Contains(t, result.Output, "args: {{ Switcheroo! }}")
-}
-
 func TestCIRunner_PullRequest_FailedSync_CanRecoverAndRunCommand(t *testing.T) {
 	wsPath := testfs.MakeTempDir(t)
 

enterprise/server/webhooks/bitbucket/bitbucket.go

@@ -58,7 +58,6 @@ func (*bitbucketGitProvider) ParseWebhookData(r *http.Request) (*interfaces.Webh
 			TargetBranch:  branch,
 			SHA:           v["Push.Changes.0.New.Target.Hash"],
 		}, nil
-		// TODO(Maggie): Is it possible to get the merge commit?
 	case "pullrequest:created", "pullrequest:updated":
 		payload := &PullRequestEventPayload{}
 		if err := unmarshalBody(r, payload); err != nil {

enterprise/server/webhooks/github/github.go

@@ -200,18 +200,6 @@ func parsePullRequestOrReview(event interface{}) (*interfaces.WebhookData, error
 		return nil, err
 	}
 
-	// Sometimes the mergeCommitSHA will not be set on the webhook data
-	// if the background job computing it has not completed yet. In this
-	// case, leave it empty.
-	mergeCommitSHAValue, err := fieldgetter.ExtractValues(
-		event,
-		"PullRequest.MergeCommitSHA",
-	)
-	mergeCommitSHA := ""
-	if err == nil {
-		mergeCommitSHA = mergeCommitSHAValue["PullRequest.MergeCommitSHA"]
-	}
-
 	isTargetRepoPublic := v["PullRequest.Base.Repo.Private"] == "false"
 	return &interfaces.WebhookData{
 		EventName:               webhook_data.EventName.PullRequest,
@@ -222,7 +210,6 @@ func parsePullRequestOrReview(event interface{}) (*interfaces.WebhookData, error
 		TargetRepoDefaultBranch: v["PullRequest.Base.Repo.DefaultBranch"],
 		IsTargetRepoPublic:      isTargetRepoPublic,
 		TargetBranch:            v["PullRequest.Base.Ref"],
-		MergeCommitSHA:          mergeCommitSHA,
 		PullRequestAuthor:       v["PullRequest.User.Login"],
 	}, nil
 }

enterprise/server/webhooks/github/github_test.go

@@ -79,7 +79,6 @@ func TestParseRequest_ValidPullRequestReviewEvent_Success(t *testing.T) {
 		PullRequestNumber:       1,
 		PullRequestAuthor:       "test2",
 		PullRequestApprover:     "test",
-		MergeCommitSHA:          "70897fde472746bcbb7641e05de39d9d3b97b4d9",
 	}, data)
 }
 

server/interfaces/interfaces.go

@@ -755,15 +755,6 @@ type WebhookData struct {
 	// Ex: "main"
 	TargetBranch string
 
-	// MergeCommitSHA is the SHA generated by merging the pushed branch into the
-	// target branch. This behavior is used to guarantee that CI runs on up-to-date
-	// refs. It will be empty for non-pull-request events.The merge commit SHA is
-	// generated by a background job, so can be empty for pull-request events if
-	// the background job has not completed, or if the webhook provider does not
-	// provide a generated merge commit. In this case, it should be manually
-	// generated by merging the pushed and target branches.
-	MergeCommitSHA string
-
 	// IsTargetRepoPublic reflects whether the target repo is publicly visible via
 	// the git provider.
 	IsTargetRepoPublic bool