tom/forkjo
tom/forkjo
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions 1

Add branch auto deletion for scheduled PRs

Browse source
This commit is contained in:
Tom Neuber 2024-10-21 21:21:50 +02:00
parent e0ca5014af
commit c883a8bcc5
Signed by: tom
GPG key ID: F17EFE4272D89FF6
11 changed files with 88 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
models/forgejo_migrations/migrate.go
View file

@ -82,6 +82,8 @@ var migrations = []*Migration{
NewMigration("Add SSH keypair to `pull_mirror` table", AddSSHKeypairToPushMirror),
// v22 -> v23
NewMigration("Add `legacy` to `web_authn_credential` table", AddLegacyToWebAuthnCredential),
// v23 -> v24
NewMigration("Add `delete_branch_after_merge` to `auto_merge` table", AddDeleteBranchAfterMergeToAutoMerge),
}
// GetCurrentDBVersion returns the current Forgejo database version.

16
models/forgejo_migrations/v23.go Normal file
View file

@ -0,0 +1,16 @@
// Copyright 2024 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package forgejo_migrations //nolint:revive
import "xorm.io/xorm"
// AddDeleteBranchAfterMergeToAutoMerge: add DeleteBranchAfterMerge column, setting existing rows to false
func AddDeleteBranchAfterMergeToAutoMerge(x *xorm.Engine) error {
type AutoMerge struct {
ID int64 `xorm:"pk autoincr"`
DeleteBranchAfterMerge bool `xorm:"NOT NULL DEFAULT false"`
}
return x.Sync(&AutoMerge{})
}

26
models/pull/automerge.go
View file

@ -15,13 +15,14 @@ import (
// AutoMerge represents a pull request scheduled for merging when checks succeed
type AutoMerge struct {
ID int64 `xorm:"pk autoincr"`
PullID int64 `xorm:"UNIQUE"`
DoerID int64 `xorm:"INDEX NOT NULL"`
Doer *user_model.User `xorm:"-"`
MergeStyle repo_model.MergeStyle `xorm:"varchar(30)"`
Message string `xorm:"LONGTEXT"`
CreatedUnix timeutil.TimeStamp `xorm:"created"`
ID int64 `xorm:"pk autoincr"`
PullID int64 `xorm:"UNIQUE"`
DoerID int64 `xorm:"INDEX NOT NULL"`
Doer *user_model.User `xorm:"-"`
MergeStyle repo_model.MergeStyle `xorm:"varchar(30)"`
Message string `xorm:"LONGTEXT"`
DeleteBranchAfterMerge bool `xorm:"NOT NULL DEFAULT false"`
CreatedUnix timeutil.TimeStamp `xorm:"created"`
}
// TableName return database table name for xorm
@ -49,7 +50,7 @@ func IsErrAlreadyScheduledToAutoMerge(err error) bool {
}
// ScheduleAutoMerge schedules a pull request to be merged when all checks succeed
func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, style repo_model.MergeStyle, message string) error {
func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, style repo_model.MergeStyle, message string, deleteBranch bool) error {
// Check if we already have a merge scheduled for that pull request
if exists, _, err := GetScheduledMergeByPullID(ctx, pullID); err != nil {
return err
@ -58,10 +59,11 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64,
}
_, err := db.GetEngine(ctx).Insert(&AutoMerge{
DoerID: doer.ID,
PullID: pullID,
MergeStyle: style,
Message: message,
DoerID: doer.ID,
PullID: pullID,
MergeStyle: style,
Message: message,
DeleteBranchAfterMerge: deleteBranch,
})
return err
}

2
routers/api/v1/repo/pull.go
View file

@ -988,7 +988,7 @@ func MergePullRequest(ctx *context.APIContext) {
}
if form.MergeWhenChecksSucceed {
scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message)
scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, form.DeleteBranchAfterMerge)
if err != nil {
if pull_model.IsErrAlreadyScheduledToAutoMerge(err) {
ctx.Error(http.StatusConflict, "ScheduleAutoMerge", err)

2
routers/private/hook_post_receive_test.go
View file

@ -28,7 +28,7 @@ func TestHandlePullRequestMerging(t *testing.T) {
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
err = pull_model.ScheduleAutoMerge(db.DefaultContext, user1, pr.ID, repo_model.MergeStyleSquash, "squash merge a pr")
err = pull_model.ScheduleAutoMerge(db.DefaultContext, user1, pr.ID, repo_model.MergeStyleSquash, "squash merge a pr", false)
require.NoError(t, err)
autoMerge := unittest.AssertExistsAndLoadBean(t, &pull_model.AutoMerge{PullID: pr.ID})

2
routers/web/repo/pull.go
View file

@ -1302,7 +1302,7 @@ func MergePullRequest(ctx *context.Context) {
// delete all scheduled auto merges
_ = pull_model.DeleteScheduledAutoMerge(ctx, pr.ID)
// schedule auto merge
scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message)
scheduled, err := automerge.ScheduleAutoMerge(ctx, ctx.Doer, pr, repo_model.MergeStyle(form.Do), message, form.DeleteBranchAfterMerge)
if err != nil {
ctx.ServerError("ScheduleAutoMerge", err)
return

16
services/automerge/automerge.go
View file

@ -22,6 +22,7 @@ import (
"code.gitea.io/gitea/modules/queue"
notify_service "code.gitea.io/gitea/services/notify"
pull_service "code.gitea.io/gitea/services/pull"
repo_service "code.gitea.io/gitea/services/repository"
shared_automerge "code.gitea.io/gitea/services/shared/automerge"
)
@ -52,9 +53,9 @@ func handler(items ...string) []string {
}
// ScheduleAutoMerge if schedule is false and no error, pull can be merged directly
func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string) (scheduled bool, err error) {
func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *issues_model.PullRequest, style repo_model.MergeStyle, message string, deleteBranch bool) (scheduled bool, err error) {
err = db.WithTx(ctx, func(ctx context.Context) error {
if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message); err != nil {
if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message, deleteBranch); err != nil {
return err
}
scheduled = true
@ -210,4 +211,15 @@ func handlePullRequestAutoMerge(pullID int64, sha string) {
// on the pull request page. But this should not be finished in a bug fix PR which will be backport to release branch.
return
}
if scheduledPRM.DeleteBranchAfterMerge {
deleted, err := repo_service.DeleteBranchIfUnused(ctx, pr, headGitRepo, doer)
if err != nil {
if deleted {
log.Info("%d branch deleted but got error during comment creation: %v", pr.ID, err)
} else {
log.Error("%d repo_service.DeleteBranchIfUnused: %v", pr.ID, err)
}
}
}
}

31
services/repository/branch.go
View file

@ -26,6 +26,7 @@ import (
"code.gitea.io/gitea/modules/timeutil"
webhook_module "code.gitea.io/gitea/modules/webhook"
notify_service "code.gitea.io/gitea/services/notify"
pull_service "code.gitea.io/gitea/services/pull"
files_service "code.gitea.io/gitea/services/repository/files"
"xorm.io/builder"
@ -475,6 +476,36 @@ func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.R
return nil
}
func DeleteBranchIfUnused(ctx context.Context, pr *issues_model.PullRequest, repo *git.Repository, doer *user_model.User) (bool, error) {
// Don't cleanup when there are other PR's that use this branch as head branch.
exist, err := issues_model.HasUnmergedPullRequestsByHeadInfo(ctx, pr.HeadRepoID, pr.HeadBranch)
if err != nil {
log.Error("HasUnmergedPullRequestsByHeadInfo", err)
return false, err
}
if exist {
log.Info("%d was scheduled to automerge with branch deletion but branch is still used", pr.ID)
return false, nil
}
if err := pull_service.RetargetChildrenOnMerge(ctx, doer, pr); err != nil {
log.Error("RetargetChildrenOnMerge", err)
return false, err
}
if err := DeleteBranch(ctx, doer, pr.HeadRepo, repo, pr.HeadBranch); err != nil {
log.Error("DeleteBranch: %v", err)
return false, err
}
if err := issues_model.AddDeletePRBranchComment(ctx, doer, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
log.Error("AddDeletePRBranchComment: %v", err)
return true, err
}
return true, nil
}
type BranchSyncOptions struct {
RepoID int64
}

2
tests/integration/actions_commit_status_test.go
View file

@ -34,7 +34,7 @@ func TestActionsAutomerge(t *testing.T) {
assert.False(t, pr.HasMerged, "PR should not be merged")
assert.Equal(t, issues_model.PullRequestStatusMergeable, pr.Status, "PR should be mergeable")
scheduled, err := automerge.ScheduleAutoMerge(ctx, user, pr, repo_model.MergeStyleMerge, "Dummy")
scheduled, err := automerge.ScheduleAutoMerge(ctx, user, pr, repo_model.MergeStyleMerge, "Dummy", false)
require.NoError(t, err, "PR should be scheduled for automerge")
assert.True(t, scheduled, "PR should be scheduled for automerge")

12
tests/integration/pull_merge_test.go
View file

@ -880,12 +880,12 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) {
session.MakeRequest(t, req, http.StatusSeeOther)
// first time insert automerge record, return true
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test")
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.NoError(t, err)
assert.True(t, scheduled)
// second time insert automerge record, return false because it does exist
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test")
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.Error(t, err)
assert.False(t, scheduled)
@ -964,12 +964,12 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) {
session.MakeRequest(t, req, http.StatusSeeOther)
// first time insert automerge record, return true
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test")
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.NoError(t, err)
assert.True(t, scheduled)
// second time insert automerge record, return false because it does exist
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test")
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.Error(t, err)
assert.False(t, scheduled)
@ -1093,12 +1093,12 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing.
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
// first time insert automerge record, return true
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test")
scheduled, err := automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.NoError(t, err)
assert.True(t, scheduled)
// second time insert automerge record, return false because it does exist
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test")
scheduled, err = automerge.ScheduleAutoMerge(db.DefaultContext, user1, pr, repo_model.MergeStyleMerge, "auto merge test", false)
require.Error(t, err)
assert.False(t, scheduled)

2
web_src/js/components/PullRequestMergeForm.vue
View file

@ -130,7 +130,7 @@ export default {
{{ mergeForm.textCancel }}
</button>
<div class="ui checkbox tw-ml-1" v-if="mergeForm.isPullBranchDeletable && !autoMergeWhenSucceed">
<div class="ui checkbox tw-ml-1" v-if="mergeForm.isPullBranchDeletable">
<input name="delete_branch_after_merge" type="checkbox" v-model="deleteBranchAfterMerge" id="delete-branch-after-merge">
<label for="delete-branch-after-merge">{{ mergeForm.textDeleteBranch }}</label>
</div>