mirrors
/
forgejo
mirror of
1
Fork 0
Code Issues Releases Activity

Merge pull request 'fix: Run full PR checks on agit push' (#4885) from viceice/forgejo:fix/agit/force-push into forgejo 

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4885
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Otto <otto@codeberg.org>
This commit is contained in:
Otto 2024-08-13 18:23:40 +00:00
parent 69eb8f32cb 2d05e922a2
commit 01affa0908
3 changed files with 63 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
services/agit/agit.go
View File

@ -211,6 +211,8 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
return nil, fmt.Errorf("failed to update the reference of the pull request: %w", err)
}
// TODO: refactor to unify with `pull_service.AddTestPullRequestTask`
// Add the pull request to the merge conflicting checker queue.
pull_service.AddToTaskQueue(ctx, pr)
@ -218,12 +220,19 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
return nil, fmt.Errorf("failed to load the issue of the pull request: %w", err)
}
// Validate pull request.
pull_service.ValidatePullRequest(ctx, pr, oldCommitID, opts.NewCommitIDs[i], pusher)
// TODO: call `InvalidateCodeComments`
// Create and notify about the new commits.
comment, err := pull_service.CreatePushPullComment(ctx, pusher, pr, oldCommitID, opts.NewCommitIDs[i])
if err == nil && comment != nil {
notify_service.PullRequestPushCommits(ctx, pusher, pr, comment)
}
notify_service.PullRequestSynchronized(ctx, pusher, pr)
// this always seems to be false
isForcePush := comment != nil && comment.IsForcePush
results = append(results, private.HookProcReceiveRefResult{

78
services/pull/pull.go
View File

@ -356,43 +356,7 @@ func TestPullRequest(ctx context.Context, doer *user_model.User, repoID, olderTh
}
if err == nil {
for _, pr := range prs {
objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() {
changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID)
if err != nil {
log.Error("checkIfPRContentChanged: %v", err)
}
if changed {
// Mark old reviews as stale if diff to mergebase has changed
if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil {
log.Error("MarkReviewsAsStale: %v", err)
}
// dismiss all approval reviews if protected branch rule item enabled.
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
log.Error("GetFirstMatchProtectedBranchRule: %v", err)
}
if pb != nil && pb.DismissStaleApprovals {
if err := DismissApprovalReviews(ctx, doer, pr); err != nil {
log.Error("DismissApprovalReviews: %v", err)
}
}
}
if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil {
log.Error("MarkReviewsAsNotStale: %v", err)
}
divergence, err := GetDiverging(ctx, pr)
if err != nil {
log.Error("GetDiverging: %v", err)
} else {
err = pr.UpdateCommitDivergence(ctx, divergence.Ahead, divergence.Behind)
if err != nil {
log.Error("UpdateCommitDivergence: %v", err)
}
}
}
ValidatePullRequest(ctx, pr, newCommitID, oldCommitID, doer)
notify_service.PullRequestSynchronized(ctx, doer, pr)
}
}
@ -422,6 +386,46 @@ func TestPullRequest(ctx context.Context, doer *user_model.User, repoID, olderTh
}
}
// Mark old reviews as stale if diff to mergebase has changed.
// Dismiss all approval reviews if protected branch rule item enabled.
// Update commit divergence.
func ValidatePullRequest(ctx context.Context, pr *issues_model.PullRequest, newCommitID, oldCommitID string, doer *user_model.User) {
objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() {
changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID)
if err != nil {
log.Error("checkIfPRContentChanged: %v", err)
}
if changed {
if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil {
log.Error("MarkReviewsAsStale: %v", err)
}
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
if err != nil {
log.Error("GetFirstMatchProtectedBranchRule: %v", err)
}
if pb != nil && pb.DismissStaleApprovals {
if err := DismissApprovalReviews(ctx, doer, pr); err != nil {
log.Error("DismissApprovalReviews: %v", err)
}
}
}
if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil {
log.Error("MarkReviewsAsNotStale: %v", err)
}
divergence, err := GetDiverging(ctx, pr)
if err != nil {
log.Error("GetDiverging: %v", err)
} else {
err = pr.UpdateCommitDivergence(ctx, divergence.Ahead, divergence.Behind)
if err != nil {
log.Error("UpdateCommitDivergence: %v", err)
}
}
}
}
// checkIfPRContentChanged checks if diff to target branch has changed by push
// A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged
func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) {

13
tests/integration/git_test.go
View File

@ -829,6 +829,9 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
if !assert.NotEmpty(t, pr1) {
return
}
assert.Equal(t, 1, pr1.CommitsAhead)
assert.Equal(t, 0, pr1.CommitsBehind)
prMsg, err := doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr1.Index)(t)
require.NoError(t, err)
@ -849,6 +852,8 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
if !assert.NotEmpty(t, pr2) {
return
}
assert.Equal(t, 1, pr2.CommitsAhead)
assert.Equal(t, 0, pr2.CommitsBehind)
prMsg, err = doAPIGetPullRequest(*ctx, ctx.Username, ctx.Reponame, pr2.Index)(t)
require.NoError(t, err)
@ -907,6 +912,14 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, headBranch string
assert.False(t, prMsg.HasMerged)
assert.Equal(t, commit, prMsg.Head.Sha)
pr1 = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{
HeadRepoID: repo.ID,
Flow: issues_model.PullRequestFlowAGit,
Index: pr1.Index,
})
assert.Equal(t, 2, pr1.CommitsAhead)
assert.Equal(t, 0, pr1.CommitsBehind)
_, _, err = git.NewCommand(git.DefaultContext, "push", "origin").AddDynamicArguments("HEAD:refs/for/master/test/" + headBranch).RunStdString(&git.RunOpts{Dir: dstPath})
require.NoError(t, err)