Skip to content

Commit c039ecc

Browse files
jpraetzeripath
authored and
zhaoxin
committed
Add checkbox to delete pull branch after successful merge (go-gitea#16049)
* Add checkbox to delete pull branch after successful merge * Omit DeleteBranchAfterMerge field in json * Log a warning instead of error when PR head branch deleted * Add DefaultDeleteBranchAfterMerge to PullRequestConfig * Add support for delete_branch_after_merge via API * Fix for API: the branch should be deleted from the HEAD repo If head and base repo are the same, reuse the already opened ctx.Repo.GitRepo * Don't delegate to CleanupBranch, only reuse branch deletion code CleanupBranch contains too much logic that has already been performed by the Merge * Reuse gitrepo in MergePullRequest Co-authored-by: Andrew Thornton <[email protected]>
1 parent 815fa0e commit c039ecc

File tree

14 files changed

+17979
-17842
lines changed

14 files changed

+17979
-17842
lines changed

‎models/repo_unit.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,15 @@ func (cfg *IssuesConfig) ToDB() ([]byte, error) {
9191

9292
// PullRequestsConfig describes pull requests config
9393
type PullRequestsConfig struct {
94-
IgnoreWhitespaceConflicts bool
95-
AllowMerge bool
96-
AllowRebase bool
97-
AllowRebaseMerge bool
98-
AllowSquash bool
99-
AllowManualMerge bool
100-
AutodetectManualMerge bool
101-
DefaultMergeStyle MergeStyle
94+
IgnoreWhitespaceConflicts bool
95+
AllowMerge bool
96+
AllowRebase bool
97+
AllowRebaseMerge bool
98+
AllowSquash bool
99+
AllowManualMerge bool
100+
AutodetectManualMerge bool
101+
DefaultDeleteBranchAfterMerge bool
102+
DefaultMergeStyle MergeStyle
102103
}
103104

104105
// FromDB fills up a PullRequestsConfig from serialized format.

‎modules/structs/repo.go

+2
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ type EditRepoOption struct {
172172
AllowManualMerge *bool `json:"allow_manual_merge,omitempty"`
173173
// either `true` to enable AutodetectManualMerge, or `false` to prevent it. `has_pull_requests` must be `true`, Note: In some special cases, misjudgments can occur.
174174
AutodetectManualMerge *bool `json:"autodetect_manual_merge,omitempty"`
175+
// set to `true` to delete pr branch after merge by default
176+
DefaultDeleteBranchAfterMerge *bool `json:"default_delete_branch_after_merge,omitempty"`
175177
// set to a merge style to be used by this repository: "merge", "rebase", "rebase-merge", or "squash". `has_pull_requests` must be `true`.
176178
DefaultMergeStyle *string `json:"default_merge_style,omitempty"`
177179
// set to `true` to archive this repository.

‎options/locale/locale_en-US.ini

+1
Original file line numberDiff line numberDiff line change
@@ -1664,6 +1664,7 @@ settings.pulls.allow_rebase_merge_commit = Enable Rebasing with explicit merge c
16641664
settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits
16651665
settings.pulls.allow_manual_merge = Enable Mark PR as manually merged
16661666
settings.pulls.enable_autodetect_manual_merge = Enable autodetect manual merge (Note: In some special cases, misjudgments can occur)
1667+
settings.pulls.default_delete_branch_after_merge = Delete pull request branch after merge by default
16671668
settings.projects_desc = Enable Repository Projects
16681669
settings.admin_settings = Administrator Settings
16691670
settings.admin_enable_health_check = Enable Repository Health Checks (git fsck)

‎routers/api/v1/repo/pull.go

+34
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package repo
66

77
import (
8+
"errors"
89
"fmt"
910
"math"
1011
"net/http"
@@ -25,6 +26,7 @@ import (
2526
"code.gitea.io/gitea/services/forms"
2627
issue_service "code.gitea.io/gitea/services/issue"
2728
pull_service "code.gitea.io/gitea/services/pull"
29+
repo_service "code.gitea.io/gitea/services/repository"
2830
)
2931

3032
// ListPullRequests returns a list of all PRs
@@ -878,6 +880,38 @@ func MergePullRequest(ctx *context.APIContext) {
878880
}
879881

880882
log.Trace("Pull request merged: %d", pr.ID)
883+
884+
if form.DeleteBranchAfterMerge {
885+
var headRepo *git.Repository
886+
if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
887+
headRepo = ctx.Repo.GitRepo
888+
} else {
889+
headRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath())
890+
if err != nil {
891+
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
892+
return
893+
}
894+
defer headRepo.Close()
895+
}
896+
if err := repo_service.DeleteBranch(ctx.User, pr.HeadRepo, headRepo, pr.HeadBranch); err != nil {
897+
switch {
898+
case git.IsErrBranchNotExist(err):
899+
ctx.NotFound(err)
900+
case errors.Is(err, repo_service.ErrBranchIsDefault):
901+
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
902+
case errors.Is(err, repo_service.ErrBranchIsProtected):
903+
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
904+
default:
905+
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
906+
}
907+
return
908+
}
909+
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, pr.Issue.ID, pr.HeadBranch); err != nil {
910+
// Do not fail here as branch has already been deleted
911+
log.Error("DeleteBranch: %v", err)
912+
}
913+
}
914+
881915
ctx.Status(http.StatusOK)
882916
}
883917

‎routers/api/v1/repo/repo.go

+12-8
Original file line numberDiff line numberDiff line change
@@ -833,14 +833,15 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
833833
if err != nil {
834834
// Unit type doesn't exist so we make a new config file with default values
835835
config = &models.PullRequestsConfig{
836-
IgnoreWhitespaceConflicts: false,
837-
AllowMerge: true,
838-
AllowRebase: true,
839-
AllowRebaseMerge: true,
840-
AllowSquash: true,
841-
AllowManualMerge: true,
842-
AutodetectManualMerge: false,
843-
DefaultMergeStyle: models.MergeStyleMerge,
836+
IgnoreWhitespaceConflicts: false,
837+
AllowMerge: true,
838+
AllowRebase: true,
839+
AllowRebaseMerge: true,
840+
AllowSquash: true,
841+
AllowManualMerge: true,
842+
AutodetectManualMerge: false,
843+
DefaultDeleteBranchAfterMerge: false,
844+
DefaultMergeStyle: models.MergeStyleMerge,
844845
}
845846
} else {
846847
config = unit.PullRequestsConfig()
@@ -867,6 +868,9 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
867868
if opts.AutodetectManualMerge != nil {
868869
config.AutodetectManualMerge = *opts.AutodetectManualMerge
869870
}
871+
if opts.DefaultDeleteBranchAfterMerge != nil {
872+
config.DefaultDeleteBranchAfterMerge = *opts.DefaultDeleteBranchAfterMerge
873+
}
870874
if opts.DefaultMergeStyle != nil {
871875
config.DefaultMergeStyle = models.MergeStyle(*opts.DefaultMergeStyle)
872876
}

‎routers/web/repo/pull.go

+48-11
Original file line numberDiff line numberDiff line change
@@ -965,6 +965,22 @@ func MergePullRequest(ctx *context.Context) {
965965
}
966966

967967
log.Trace("Pull request merged: %d", pr.ID)
968+
969+
if form.DeleteBranchAfterMerge {
970+
var headRepo *git.Repository
971+
if ctx.Repo != nil && ctx.Repo.Repository != nil && pr.HeadRepoID == ctx.Repo.Repository.ID && ctx.Repo.GitRepo != nil {
972+
headRepo = ctx.Repo.GitRepo
973+
} else {
974+
headRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath())
975+
if err != nil {
976+
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
977+
return
978+
}
979+
defer headRepo.Close()
980+
}
981+
deleteBranch(ctx, pr, headRepo)
982+
}
983+
968984
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + fmt.Sprint(pr.Index))
969985
}
970986

@@ -1170,19 +1186,35 @@ func CleanUpPullRequest(ctx *context.Context) {
11701186

11711187
fullBranchName := pr.HeadRepo.Owner.Name + "/" + pr.HeadBranch
11721188

1173-
gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
1174-
if err != nil {
1175-
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
1176-
return
1189+
var gitBaseRepo *git.Repository
1190+
1191+
// Assume that the base repo is the current context (almost certainly)
1192+
if ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.BaseRepoID && ctx.Repo.GitRepo != nil {
1193+
gitBaseRepo = ctx.Repo.GitRepo
1194+
} else {
1195+
// If not just open it
1196+
gitBaseRepo, err = git.OpenRepository(pr.BaseRepo.RepoPath())
1197+
if err != nil {
1198+
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err)
1199+
return
1200+
}
1201+
defer gitBaseRepo.Close()
11771202
}
1178-
defer gitRepo.Close()
11791203

1180-
gitBaseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
1181-
if err != nil {
1182-
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err)
1183-
return
1204+
// Now assume that the head repo is the same as the base repo (reasonable chance)
1205+
gitRepo := gitBaseRepo
1206+
// But if not: is it the same as the context?
1207+
if pr.BaseRepoID != pr.HeadRepoID && ctx.Repo != nil && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID == pr.HeadRepoID && ctx.Repo.GitRepo != nil {
1208+
gitRepo = ctx.Repo.GitRepo
1209+
} else if pr.BaseRepoID != pr.HeadRepoID {
1210+
// Otherwise just load it up
1211+
gitRepo, err = git.OpenRepository(pr.HeadRepo.RepoPath())
1212+
if err != nil {
1213+
ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err)
1214+
return
1215+
}
1216+
defer gitRepo.Close()
11841217
}
1185-
defer gitBaseRepo.Close()
11861218

11871219
defer func() {
11881220
ctx.JSON(http.StatusOK, map[string]interface{}{
@@ -1208,6 +1240,11 @@ func CleanUpPullRequest(ctx *context.Context) {
12081240
return
12091241
}
12101242

1243+
deleteBranch(ctx, pr, gitRepo)
1244+
}
1245+
1246+
func deleteBranch(ctx *context.Context, pr *models.PullRequest, gitRepo *git.Repository) {
1247+
fullBranchName := pr.HeadRepo.Owner.Name + "/" + pr.HeadBranch
12111248
if err := repo_service.DeleteBranch(ctx.User, pr.HeadRepo, gitRepo, pr.HeadBranch); err != nil {
12121249
switch {
12131250
case git.IsErrBranchNotExist(err):
@@ -1223,7 +1260,7 @@ func CleanUpPullRequest(ctx *context.Context) {
12231260
return
12241261
}
12251262

1226-
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, issue.ID, pr.HeadBranch); err != nil {
1263+
if err := models.AddDeletePRBranchComment(ctx.User, pr.BaseRepo, pr.IssueID, pr.HeadBranch); err != nil {
12271264
// Do not fail here as branch has already been deleted
12281265
log.Error("DeleteBranch: %v", err)
12291266
}

‎routers/web/repo/setting.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -416,14 +416,15 @@ func SettingsPost(ctx *context.Context) {
416416
RepoID: repo.ID,
417417
Type: models.UnitTypePullRequests,
418418
Config: &models.PullRequestsConfig{
419-
IgnoreWhitespaceConflicts: form.PullsIgnoreWhitespace,
420-
AllowMerge: form.PullsAllowMerge,
421-
AllowRebase: form.PullsAllowRebase,
422-
AllowRebaseMerge: form.PullsAllowRebaseMerge,
423-
AllowSquash: form.PullsAllowSquash,
424-
AllowManualMerge: form.PullsAllowManualMerge,
425-
AutodetectManualMerge: form.EnableAutodetectManualMerge,
426-
DefaultMergeStyle: models.MergeStyle(form.PullsDefaultMergeStyle),
419+
IgnoreWhitespaceConflicts: form.PullsIgnoreWhitespace,
420+
AllowMerge: form.PullsAllowMerge,
421+
AllowRebase: form.PullsAllowRebase,
422+
AllowRebaseMerge: form.PullsAllowRebaseMerge,
423+
AllowSquash: form.PullsAllowSquash,
424+
AllowManualMerge: form.PullsAllowManualMerge,
425+
AutodetectManualMerge: form.EnableAutodetectManualMerge,
426+
DefaultDeleteBranchAfterMerge: form.DefaultDeleteBranchAfterMerge,
427+
DefaultMergeStyle: models.MergeStyle(form.PullsDefaultMergeStyle),
427428
},
428429
})
429430
} else if !models.UnitTypePullRequests.UnitGlobalDisabled() {

‎services/forms/repo_form.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ type RepoSettingForm struct {
151151
PullsAllowManualMerge bool
152152
PullsDefaultMergeStyle string
153153
EnableAutodetectManualMerge bool
154+
DefaultDeleteBranchAfterMerge bool
154155
EnableTimetracker bool
155156
AllowOnlyContributorsToTrackTime bool
156157
EnableIssueDependencies bool
@@ -563,11 +564,12 @@ func (f *InitializeLabelsForm) Validate(req *http.Request, errs binding.Errors)
563564
type MergePullRequestForm struct {
564565
// required: true
565566
// enum: merge,rebase,rebase-merge,squash,manually-merged
566-
Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,manually-merged)"`
567-
MergeTitleField string
568-
MergeMessageField string
569-
MergeCommitID string // only used for manually-merged
570-
ForceMerge *bool `json:"force_merge,omitempty"`
567+
Do string `binding:"Required;In(merge,rebase,rebase-merge,squash,manually-merged)"`
568+
MergeTitleField string
569+
MergeMessageField string
570+
MergeCommitID string // only used for manually-merged
571+
ForceMerge *bool `json:"force_merge,omitempty"`
572+
DeleteBranchAfterMerge bool `json:"delete_branch_after_merge,omitempty"`
571573
}
572574

573575
// Validate validates the fields

‎services/pull/pull.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,11 @@ func AddTestPullRequestTask(doer *models.User, repoID int64, branch string, isSy
303303
for _, pr := range prs {
304304
divergence, err := GetDiverging(pr)
305305
if err != nil {
306-
log.Error("GetDiverging: %v", err)
306+
if models.IsErrBranchDoesNotExist(err) && !git.IsBranchExist(pr.HeadRepo.RepoPath(), pr.HeadBranch) {
307+
log.Warn("Cannot test PR %s/%d: head_branch %s no longer exists", pr.BaseRepo.Name, pr.IssueID, pr.HeadBranch)
308+
} else {
309+
log.Error("GetDiverging: %v", err)
310+
}
307311
} else {
308312
err = pr.UpdateCommitDivergence(divergence.Ahead, divergence.Behind)
309313
if err != nil {

‎services/pull/temp_repo.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,15 @@ func createTemporaryRepo(pr *models.PullRequest) (string, error) {
141141
trackingBranch := "tracking"
142142
// Fetch head branch
143143
if err := git.NewCommand("fetch", "--no-tags", remoteRepoName, git.BranchPrefix+pr.HeadBranch+":"+trackingBranch).RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
144-
log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String())
145144
if err := models.RemoveTemporaryPath(tmpBasePath); err != nil {
146145
log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err)
147146
}
147+
if !git.IsBranchExist(pr.HeadRepo.RepoPath(), pr.HeadBranch) {
148+
return "", models.ErrBranchDoesNotExist{
149+
BranchName: pr.HeadBranch,
150+
}
151+
}
152+
log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String())
148153
return "", fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, err, outbuf.String(), errbuf.String())
149154
}
150155
outbuf.Reset()

‎services/pull/update.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ func GetDiverging(pr *models.PullRequest) (*git.DivergeObject, error) {
8888

8989
tmpRepo, err := createTemporaryRepo(pr)
9090
if err != nil {
91-
log.Error("CreateTemporaryPath: %v", err)
91+
if !models.IsErrBranchDoesNotExist(err) {
92+
log.Error("CreateTemporaryRepo: %v", err)
93+
}
9294
return nil, err
9395
}
9496
defer func() {

‎templates/repo/issue/view_content/pull.tmpl

+30
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,12 @@
315315
<button class="ui button merge-cancel">
316316
{{$.i18n.Tr "cancel"}}
317317
</button>
318+
{{if .IsPullBranchDeletable}}
319+
<div class="ui checkbox ml-2">
320+
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
321+
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
322+
</div>
323+
{{end}}
318324
</form>
319325
</div>
320326
{{end}}
@@ -328,6 +334,12 @@
328334
<button class="ui button merge-cancel">
329335
{{$.i18n.Tr "cancel"}}
330336
</button>
337+
{{if .IsPullBranchDeletable}}
338+
<div class="ui checkbox ml-2">
339+
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
340+
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
341+
</div>
342+
{{end}}
331343
</form>
332344
</div>
333345
{{end}}
@@ -347,6 +359,12 @@
347359
<button class="ui button merge-cancel">
348360
{{$.i18n.Tr "cancel"}}
349361
</button>
362+
{{if .IsPullBranchDeletable}}
363+
<div class="ui checkbox ml-2">
364+
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
365+
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
366+
</div>
367+
{{end}}
350368
</form>
351369
</div>
352370
{{end}}
@@ -366,6 +384,12 @@
366384
<button class="ui button merge-cancel">
367385
{{$.i18n.Tr "cancel"}}
368386
</button>
387+
{{if .IsPullBranchDeletable}}
388+
<div class="ui checkbox ml-2">
389+
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
390+
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
391+
</div>
392+
{{end}}
369393
</form>
370394
</div>
371395
{{end}}
@@ -382,6 +406,12 @@
382406
<button class="ui button merge-cancel">
383407
{{$.i18n.Tr "cancel"}}
384408
</button>
409+
{{if .IsPullBranchDeletable}}
410+
<div class="ui checkbox ml-2">
411+
<input name="delete_branch_after_merge" type="checkbox" {{if $prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}}checked{{end}}>
412+
<label>{{$.i18n.Tr "repo.branch.delete" .HeadTarget}}</label>
413+
</div>
414+
{{end}}
385415
</form>
386416
</div>
387417
{{end}}

‎templates/repo/settings/options.tmpl

+6
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,12 @@
445445
<label>{{.i18n.Tr "repo.settings.pulls.enable_autodetect_manual_merge"}}</label>
446446
</div>
447447
</div>
448+
<div class="field">
449+
<div class="ui checkbox">
450+
<input name="default_delete_branch_after_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge)}}checked{{end}}>
451+
<label>{{.i18n.Tr "repo.settings.pulls.default_delete_branch_after_merge"}}</label>
452+
</div>
453+
</div>
448454
<div class="field">
449455
<p>
450456
{{.i18n.Tr "repo.settings.default_merge_style_desc"}}

0 commit comments

Comments
 (0)