mirror of
https://github.com/go-gitea/gitea.git
synced 2026-11-09 17:37:20 -06:00
fix(actions): run pull_request workflows on the merge commit (#40076)
`pull_request` workflow runs checked out the PR head, so a PR could pass CI and still break the base branch once merged. Gitea now maintains `refs/pull/<index>/merge`, a test merge of the head into the base branch, for open pull requests. It is updated on creation and mergeability checks and removed on close or merge. - `pull_request` runs read their workflows from the test merge and expose it as `github.ref`, `github.sha` and `github.event.pull_request.merge_commit_sha`. Commit statuses, the API's `head_sha`, `workflow_run` payloads and `paths` filters stay on the head. - Runs fall back to the head when the merge conflicts, the pull request is closed or git is older than 2.40. - Hidden refs keep each run's test merge (`refs/gitea/run/<id>`) and merged commits (`refs/gitea/pull/<index>/merged`) from `git gc`, for reruns and after the base branch is deleted or force-pushed. Fixes: https://github.com/go-gitea/gitea/issues/27039 Fixes: https://github.com/go-gitea/gitea/issues/13134 Fixes: https://github.com/go-gitea/gitea/issues/30972 Docs: https://gitea.com/gitea/docs/pulls/617 --------- Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
@@ -13,6 +13,7 @@ import (
|
||||
git_model "gitea.dev/models/git"
|
||||
actions_module "gitea.dev/modules/actions"
|
||||
"gitea.dev/modules/container"
|
||||
"gitea.dev/modules/git"
|
||||
"gitea.dev/modules/log"
|
||||
"gitea.dev/modules/setting"
|
||||
"gitea.dev/modules/storage"
|
||||
@@ -259,6 +260,14 @@ func DeleteRun(ctx context.Context, run *actions_model.ActionRun) error {
|
||||
return err
|
||||
}
|
||||
|
||||
if run.TriggerEvent == actions_module.GithubEventPullRequest {
|
||||
if err := run.LoadRepo(ctx); err != nil {
|
||||
log.Error("LoadRepo for run %d: %v", run.ID, err)
|
||||
} else if err := git.RemoveRef(ctx, run.Repo, git.RunRefNameFromRunID(run.ID).String()); err != nil {
|
||||
log.Error("RemoveRef for run %d: %v", run.ID, err)
|
||||
}
|
||||
}
|
||||
|
||||
actions_model.UpdateRepoRunsNumbers(ctx, repoID)
|
||||
|
||||
// Delete files on storage
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
repo_model "gitea.dev/models/repo"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/container"
|
||||
"gitea.dev/modules/git"
|
||||
"gitea.dev/modules/optional"
|
||||
"gitea.dev/modules/setting"
|
||||
"gitea.dev/modules/test"
|
||||
@@ -29,7 +30,7 @@ func insertCleanupRun(t *testing.T, index int64, status actions_model.Status, cr
|
||||
run := &actions_model.ActionRun{
|
||||
Title: "cleanup-run", RepoID: repo.ID, OwnerID: repo.OwnerID, WorkflowID: "test.yaml", Index: index,
|
||||
TriggerUserID: 1, Ref: "refs/heads/main",
|
||||
CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", Event: "push", TriggerEvent: "push",
|
||||
CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", Event: "pull_request", TriggerEvent: "pull_request",
|
||||
Status: status,
|
||||
}
|
||||
require.NoError(t, db.Insert(t.Context(), run))
|
||||
@@ -68,8 +69,12 @@ func TestCleanupOldRuns(t *testing.T) {
|
||||
oldRunning := insertCleanupRun(t, 2005, actions_model.StatusRunning, old)
|
||||
oldCanceled := insertCleanupRun(t, 2006, actions_model.StatusCancelled, old)
|
||||
oldSkipped := insertCleanupRun(t, 2007, actions_model.StatusSkipped, old)
|
||||
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
|
||||
runRef := git.RunRefNameFromRunID(oldSuccess.ID).String()
|
||||
require.NoError(t, git.UpdateRef(t.Context(), repo, runRef, "refs/heads/master"))
|
||||
|
||||
require.NoError(t, CleanupOldRuns(t.Context()))
|
||||
assert.False(t, git.IsReferenceExist(t.Context(), repo, runRef))
|
||||
|
||||
unittest.AssertNotExistsBean(t, &actions_model.ActionRun{ID: oldSuccess.ID})
|
||||
unittest.AssertNotExistsBean(t, &actions_model.ActionRun{ID: oldFailure.ID})
|
||||
|
||||
@@ -47,13 +47,10 @@ func GenerateGiteaContext(ctx context.Context, run *actions_model.ActionRun, att
|
||||
if pullPayload, err := run.GetPullRequestEventPayload(); err == nil && pullPayload.PullRequest != nil && pullPayload.PullRequest.Base != nil && pullPayload.PullRequest.Head != nil {
|
||||
baseRef = pullPayload.PullRequest.Base.Ref
|
||||
headRef = pullPayload.PullRequest.Head.Ref
|
||||
|
||||
// if the TriggerEvent is pull_request_target, ref and sha need to be set according to the base of pull request
|
||||
// In GitHub's documentation, ref should be the branch or tag that triggered workflow. But when the TriggerEvent is pull_request_target,
|
||||
// the ref will be the base branch.
|
||||
if run.TriggerEvent == actions_module.GithubEventPullRequestTarget {
|
||||
ref = git.BranchPrefix + pullPayload.PullRequest.Base.Ref
|
||||
sha = pullPayload.PullRequest.Base.Sha
|
||||
ref, sha = git.BranchPrefix+pullPayload.PullRequest.Base.Ref, pullPayload.PullRequest.Base.Sha
|
||||
} else if run.TriggerEvent == actions_module.GithubEventPullRequest && !pullPayload.PullRequest.HasMerged && pullPayload.PullRequest.MergedCommitID != nil {
|
||||
ref, sha = git.MergeRefNameFromPullIndex(pullPayload.PullRequest.Index).String(), *pullPayload.PullRequest.MergedCommitID
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -420,6 +420,20 @@ func TestGenerateGiteaContext(t *testing.T) {
|
||||
run.TriggerEvent, run.Ref = string(actions_module.GithubEventPullRequest), "refs/pull/1/head"
|
||||
assert.Equal(t, "branch", GenerateGiteaContext(t.Context(), run, nil, nil)["ref_type"])
|
||||
|
||||
payload.PullRequest.Index, payload.PullRequest.MergedCommitID = 1, new("merge-sha")
|
||||
payloadBytes, err = json.Marshal(payload)
|
||||
require.NoError(t, err)
|
||||
run.EventPayload, run.CommitSHA = string(payloadBytes), "head-sha"
|
||||
giteaCtx = GenerateGiteaContext(t.Context(), run, nil, nil)
|
||||
assert.Equal(t, "refs/pull/1/merge", giteaCtx["ref"])
|
||||
assert.Equal(t, "merge-sha", giteaCtx["sha"])
|
||||
|
||||
payload.PullRequest.HasMerged = true
|
||||
payloadBytes, err = json.Marshal(payload)
|
||||
require.NoError(t, err)
|
||||
run.EventPayload = string(payloadBytes)
|
||||
assert.Equal(t, "head-sha", GenerateGiteaContext(t.Context(), run, nil, nil)["sha"])
|
||||
|
||||
defer test.MockVariableValue(&setting.Actions.ArtifactRetentionDays, 7)()
|
||||
assert.Equal(t, "7", GenerateGiteaContext(t.Context(), run, nil, nil)["retention_days"])
|
||||
setting.Actions.ArtifactRetentionDays = 0
|
||||
|
||||
@@ -29,6 +29,7 @@ import (
|
||||
"gitea.dev/modules/util"
|
||||
webhook_module "gitea.dev/modules/webhook"
|
||||
"gitea.dev/services/convert"
|
||||
pull_service "gitea.dev/services/pull"
|
||||
)
|
||||
|
||||
type methodCtxKeyType struct{}
|
||||
@@ -71,6 +72,7 @@ type notifyInput struct {
|
||||
PullRequest *issues_model.PullRequest
|
||||
|
||||
encodedPayload string // cached by EncodedPayload
|
||||
mergeCommitID string
|
||||
}
|
||||
|
||||
func newNotifyInput(repo *repo_model.Repository, doer *user_model.User, event webhook_module.HookEventType) *notifyInput {
|
||||
@@ -195,10 +197,23 @@ func notify(ctx context.Context, input *notifyInput) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
workflowCommit := commit
|
||||
var baseCommit *git.Commit
|
||||
if input.PullRequest != nil {
|
||||
baseCommit, err = gitRepo.GetCommit(ctx, git.BranchPrefix+input.PullRequest.BaseBranch)
|
||||
if err != nil {
|
||||
return fmt.Errorf("gitRepo.GetCommit: %w", err)
|
||||
}
|
||||
workflowCommit, err = preparePullRequestMerge(ctx, input, gitRepo, commit, baseCommit)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
var detectedWorkflows []*actions_module.DetectedWorkflow
|
||||
var filteredWorkflows []*actions_module.DetectedWorkflow
|
||||
actionsConfig := input.Repo.MustGetUnit(ctx, unit_model.TypeActions).ActionsConfig()
|
||||
workflows, schedules, filtered, invalid, err := actions_module.DetectWorkflows(ctx, gitRepo, commit,
|
||||
workflows, schedules, filtered, invalid, err := actions_module.DetectWorkflows(ctx, gitRepo, workflowCommit,
|
||||
input.Event,
|
||||
input.Payload,
|
||||
shouldDetectSchedules,
|
||||
@@ -209,7 +224,7 @@ func notify(ctx context.Context, input *notifyInput) error {
|
||||
|
||||
log.Trace("repo %s with commit %s event %s find %d workflows and %d schedules",
|
||||
input.Repo.FullName(),
|
||||
commit.ID,
|
||||
workflowCommit.ID,
|
||||
input.Event,
|
||||
len(workflows),
|
||||
len(schedules),
|
||||
@@ -239,11 +254,6 @@ func notify(ctx context.Context, input *notifyInput) error {
|
||||
|
||||
if input.PullRequest != nil {
|
||||
// detect pull_request_target workflows
|
||||
baseRef := git.BranchPrefix + input.PullRequest.BaseBranch
|
||||
baseCommit, err := gitRepo.GetCommit(ctx, baseRef)
|
||||
if err != nil {
|
||||
return fmt.Errorf("gitRepo.GetCommit: %w", err)
|
||||
}
|
||||
baseWorkflows, _, baseFiltered, _, err := actions_module.DetectWorkflows(ctx, gitRepo, baseCommit, input.Event, input.Payload, false)
|
||||
if err != nil {
|
||||
return fmt.Errorf("DetectWorkflows: %w", err)
|
||||
@@ -291,6 +301,35 @@ func notify(ctx context.Context, input *notifyInput) error {
|
||||
return detectAndHandleScopedWorkflows(ctx, input, ref, gitRepo, commit)
|
||||
}
|
||||
|
||||
// preparePullRequestMerge returns the commit to read workflows from, the test merge for open pull requests
|
||||
func preparePullRequestMerge(ctx context.Context, input *notifyInput, gitRepo *git.Repository, headCommit, baseCommit *git.Commit) (*git.Commit, error) {
|
||||
payload, ok := input.Payload.(*api.PullRequestPayload)
|
||||
if !ok || payload.PullRequest == nil || input.PullRequest.Issue.IsClosed || !input.Event.IsPullRequest() {
|
||||
return headCommit, nil
|
||||
}
|
||||
_, entries, err := actions_module.ListWorkflows(ctx, gitRepo, headCommit)
|
||||
if err == nil && len(entries) == 0 {
|
||||
_, entries, err = actions_module.ListWorkflows(ctx, gitRepo, baseCommit)
|
||||
}
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if len(entries) == 0 {
|
||||
if sources, err := actions_model.GetEffectiveScopedWorkflowSources(ctx, input.Repo.OwnerID); err != nil || len(sources) == 0 {
|
||||
return headCommit, err
|
||||
}
|
||||
}
|
||||
mergeCommitID, err := pull_service.UpdateMergeRef(ctx, input.PullRequest, baseCommit.ID.String(), headCommit.ID.String())
|
||||
if err != nil {
|
||||
log.Error("UpdateMergeRef for %-v: %v", input.PullRequest, err)
|
||||
}
|
||||
if mergeCommitID == "" {
|
||||
return headCommit, nil
|
||||
}
|
||||
payload.PullRequest.MergedCommitID, input.mergeCommitID = &mergeCommitID, mergeCommitID
|
||||
return gitRepo.GetCommit(ctx, mergeCommitID)
|
||||
}
|
||||
|
||||
func skipWorkflows(ctx context.Context, input *notifyInput, commit *git.Commit) bool {
|
||||
// skip workflow runs with a configured skip-ci string in commit message or pr title if the event is push or pull_request(_sync)
|
||||
// https://docs.github.com/en/actions/managing-workflow-runs/skipping-workflow-runs
|
||||
@@ -434,6 +473,11 @@ func buildApproveAndInsertRun(
|
||||
}
|
||||
return fmt.Errorf("PrepareRunAndInsert: %w", err)
|
||||
}
|
||||
if input.mergeCommitID != "" && run.TriggerEvent == actions_module.GithubEventPullRequest {
|
||||
if err := git.UpdateRef(ctx, input.Repo, git.RunRefNameFromRunID(run.ID).String(), input.mergeCommitID); err != nil {
|
||||
log.Error("UpdateRef for run %d: %v", run.ID, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -175,8 +175,10 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, issue *issues_model
|
||||
if err := issue.PullRequest.LoadBaseRepo(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := git.RemoveRef(ctx, issue.PullRequest.BaseRepo, issue.PullRequest.GetGitHeadRefName()); err != nil {
|
||||
return err
|
||||
for _, ref := range []git.RefName{git.RefNameFromPullIndex(issue.Index), git.MergeRefNameFromPullIndex(issue.Index), git.MergedRefNameFromPullIndex(issue.Index)} {
|
||||
if err := git.RemoveRef(ctx, issue.PullRequest.BaseRepo, ref.String()); err != nil {
|
||||
log.Error("RemoveRef %s: %v", ref, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -862,6 +862,12 @@ func MarkAsMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommi
|
||||
} else if !ok {
|
||||
return false, err
|
||||
}
|
||||
if err := git.UpdateRef(ctx, pr.Issue.Repo, git.MergedRefNameFromPullIndex(pr.Index).String(), pr.MergedCommitID); err != nil {
|
||||
log.Error("UpdateRef for merged head: %v", err)
|
||||
}
|
||||
if err := git.RemoveRef(ctx, pr.Issue.Repo, git.MergeRefNameFromPullIndex(pr.Index).String()); err != nil {
|
||||
log.Error("RemoveRef for test merge: %v", err)
|
||||
}
|
||||
|
||||
err = markAsMergedPostProcess(ctx, pr.ID, merger, wasAutoMerged)
|
||||
if err != nil {
|
||||
|
||||
72
services/pull/merge_ref.go
Normal file
72
services/pull/merge_ref.go
Normal file
@@ -0,0 +1,72 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package pull
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"strings"
|
||||
|
||||
issues_model "gitea.dev/models/issues"
|
||||
user_model "gitea.dev/models/user"
|
||||
"gitea.dev/modules/git"
|
||||
"gitea.dev/modules/git/gitcmd"
|
||||
"gitea.dev/modules/log"
|
||||
notify_service "gitea.dev/services/notify"
|
||||
)
|
||||
|
||||
func init() {
|
||||
notify_service.RegisterNotifier(&mergeRefNotifier{})
|
||||
}
|
||||
|
||||
type mergeRefNotifier struct {
|
||||
notify_service.NullNotifier
|
||||
}
|
||||
|
||||
func (*mergeRefNotifier) IssueChangeStatus(ctx context.Context, _doer *user_model.User, _commitID string, issue *issues_model.Issue, _comment *issues_model.Comment, isClosed bool) {
|
||||
if !issue.IsPull {
|
||||
return
|
||||
}
|
||||
if err := issue.LoadPullRequest(ctx); err != nil {
|
||||
log.Error("LoadPullRequest: %v", err)
|
||||
} else if !isClosed {
|
||||
StartPullRequestCheckImmediately(ctx, issue.PullRequest)
|
||||
} else if err := git.RemoveRef(ctx, issue.Repo, git.MergeRefNameFromPullIndex(issue.Index).String()); err != nil {
|
||||
log.Error("RemoveRef: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// UpdateMergeRef refreshes refs/pull/<index>/merge and returns its commit, empty when closed, conflicting or git lacks merge-tree
|
||||
func UpdateMergeRef(ctx context.Context, pr *issues_model.PullRequest, baseCommitID, headCommitID string) (string, error) {
|
||||
if !git.DefaultFeatures().SupportGitMergeTree {
|
||||
return "", nil
|
||||
}
|
||||
treeID, conflicted, _, err := git.MergeTree(ctx, pr.BaseRepo, baseCommitID, headCommitID, "")
|
||||
if err != nil || conflicted {
|
||||
return "", err
|
||||
}
|
||||
return updateMergeRef(ctx, pr, baseCommitID, headCommitID, treeID)
|
||||
}
|
||||
|
||||
func updateMergeRef(ctx context.Context, pr *issues_model.PullRequest, baseCommitID, headCommitID, treeID string) (string, error) {
|
||||
if err := pr.LoadIssue(ctx); err != nil || pr.Issue.IsClosed {
|
||||
return "", err
|
||||
}
|
||||
timestamp, _, err := gitcmd.NewCommand("log", "-1", "--no-walk=sorted", "--format=%ct").
|
||||
AddDynamicArguments(baseCommitID, headCommitID).WithRepo(pr.BaseRepo).RunStdString(ctx)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
date := "@" + strings.TrimSpace(timestamp) + " +0000" // the newer parent's date, so equal inputs give the same commit
|
||||
stdout, _, err := gitcmd.NewCommand("commit-tree", "--no-gpg-sign", "-p").AddDynamicArguments(baseCommitID).
|
||||
AddArguments("-p").AddDynamicArguments(headCommitID).
|
||||
AddOptionValues("-m", "Merge "+headCommitID+" into "+baseCommitID).
|
||||
AddDynamicArguments(treeID).WithRepo(pr.BaseRepo).
|
||||
WithEnv(append(os.Environ(), "GIT_AUTHOR_DATE="+date, "GIT_COMMITTER_DATE="+date)).RunStdString(ctx)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
commitID := strings.TrimSpace(stdout)
|
||||
return commitID, git.UpdateRef(ctx, pr.BaseRepo, git.MergeRefNameFromPullIndex(pr.Index).String(), commitID)
|
||||
}
|
||||
@@ -22,6 +22,11 @@ func checkConflictsMergeTree(ctx context.Context, pr *issues_model.PullRequest,
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("MergeTree: %w", err)
|
||||
}
|
||||
if pr.Index > 0 && !conflict {
|
||||
if _, err := updateMergeRef(ctx, pr, baseCommitID, pr.HeadCommitID, treeHash); err != nil {
|
||||
log.Error("updateMergeRef for %-v: %v", pr, err)
|
||||
}
|
||||
}
|
||||
if conflict {
|
||||
pr.Status = issues_model.PullRequestStatusConflict
|
||||
// sometimes git merge-tree will detect conflicts but not list any conflicted files
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
issues_model "gitea.dev/models/issues"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/git"
|
||||
"gitea.dev/modules/git/gitcmd"
|
||||
"gitea.dev/modules/git/gitrepo"
|
||||
|
||||
@@ -46,6 +47,21 @@ func TestPullRequestMergeable(t *testing.T) {
|
||||
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2})
|
||||
t.Run("NoConflict-MergeTree", func(t *testing.T) {
|
||||
testPullRequestMergeCheck(t, checkPullRequestMergeableByMergeTree, pr, issues_model.PullRequestStatusMergeable, nil, nil)
|
||||
mergeRef := git.MergeRefNameFromPullIndex(pr.Index).String()
|
||||
mergeCommitID, err := git.GetFullCommitID(t.Context(), pr.BaseRepo, mergeRef)
|
||||
require.NoError(t, err)
|
||||
baseCommitID, err := git.GetFullCommitID(t.Context(), pr.BaseRepo, mergeRef+"^1")
|
||||
require.NoError(t, err)
|
||||
t.Setenv("GIT_COMMITTER_DATE", "2002-01-01T00:00:00Z")
|
||||
updatedCommitID, err := UpdateMergeRef(t.Context(), pr, baseCommitID, pr.HeadCommitID)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, mergeCommitID, updatedCommitID)
|
||||
|
||||
require.NoError(t, git.RemoveRef(t.Context(), pr.BaseRepo, mergeRef))
|
||||
pr.Issue.IsClosed = true
|
||||
testPullRequestMergeCheck(t, checkPullRequestMergeableByMergeTree, pr, issues_model.PullRequestStatusMergeable, nil, nil)
|
||||
assert.False(t, git.IsReferenceExist(t.Context(), pr.BaseRepo, mergeRef))
|
||||
pr.Issue.IsClosed = false
|
||||
})
|
||||
t.Run("NoConflict-TmpRepo", func(t *testing.T) {
|
||||
testPullRequestMergeCheck(t, checkPullRequestMergeableByTmpRepo, pr, issues_model.PullRequestStatusMergeable, nil, nil)
|
||||
|
||||
@@ -161,6 +161,11 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error {
|
||||
}
|
||||
return err
|
||||
}
|
||||
if baseCommitID, err := git.GetFullCommitID(ctx, pr.BaseRepo, git.BranchPrefix+pr.BaseBranch); err != nil {
|
||||
log.Error("GetFullCommitID for %-v: %v", pr, err)
|
||||
} else if _, err := UpdateMergeRef(ctx, pr, baseCommitID, pr.HeadCommitID); err != nil {
|
||||
log.Error("UpdateMergeRef for %-v: %v", pr, err)
|
||||
}
|
||||
|
||||
issue_service.ReviewRequestNotify(ctx, issue, issue.Poster, reviewNotifiers)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user