Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rebase with merge commit merge style (#3844) #4052

Merged
merged 16 commits into from
Dec 27, 2018
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions integrations/pull_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,20 @@ func TestPullRebase(t *testing.T) {
testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebase)
}

func TestPullRebaseMerge(t *testing.T) {
// TODO
apricote marked this conversation as resolved.
Show resolved Hide resolved
prepareTestEnv(t)
session := loginUser(t, "user1")
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
testEditFile(t, session, "user1", "repo1", "master", "README.md", "Hello, World (Edited)\n")
// TODO
resp := testPullCreate(t, session, "user1", "repo1", "master", "This is a pull title")

elem := strings.Split(test.RedirectURL(resp), "/")
assert.EqualValues(t, "pulls", elem[3])
testPullMerge(t, session, elem[1], elem[2], elem[4], models.MergeStyleRebaseMerge)
}

func TestPullSquash(t *testing.T) {
prepareTestEnv(t)
session := loginUser(t, "user1")
Expand Down
4 changes: 2 additions & 2 deletions models/fixtures/repo_unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
id: 5
repo_id: 1
type: 3
config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowSquash\":true}"
config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}"
created_unix: 946684810

-
Expand All @@ -51,7 +51,7 @@
id: 8
repo_id: 3
type: 3
config: "{\"IgnoreWhitespaceConflicts\":true,\"AllowMerge\":true,\"AllowRebase\":false,\"AllowSquash\":false}"
config: "{\"IgnoreWhitespaceConflicts\":true,\"AllowMerge\":true,\"AllowRebase\":false,\"AllowRebaseMerge\":true,\"AllowSquash\":false}"
created_unix: 946684810

-
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ var migrations = []Migration{
NewMigration("add review", addReview),
// v73 -> v74
NewMigration("add must_change_password column for users table", addMustChangePassword),
// v74 -> 75
NewMigration("add pull request rebase with merge commit", addPullRequestRebaseWithMerge),
}

// Migrate database to current version
Expand Down
63 changes: 63 additions & 0 deletions models/migrations/v74.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2018 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"fmt"

"code.gitea.io/gitea/modules/util"

"github.com/go-xorm/xorm"
)

func addPullRequestRebaseWithMerge(x *xorm.Engine) error {
// RepoUnit describes all units of a repository
type RepoUnit struct {
ID int64
RepoID int64 `xorm:"INDEX(s)"`
Type int `xorm:"INDEX(s)"`
Config map[string]interface{} `xorm:"JSON"`
CreatedUnix util.TimeStamp `xorm:"INDEX CREATED"`
}

sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}

//Updating existing issue units
units := make([]*RepoUnit, 0, 100)
if err := sess.Where("`type` = ?", V16UnitTypePRs).Find(&units); err != nil {
return fmt.Errorf("Query repo units: %v", err)
}
for _, unit := range units {
if unit.Config == nil {
unit.Config = make(map[string]interface{})
}
// Allow the new merge style if all other merge styles are allowed
allowMergeRebase := true

if allowMerge, ok := unit.Config["AllowMerge"]; ok {
allowMergeRebase = allowMergeRebase && allowMerge.(bool)
}

if allowRebase, ok := unit.Config["AllowRebase"]; ok {
allowMergeRebase = allowMergeRebase && allowRebase.(bool)
}

if allowSquash, ok := unit.Config["AllowSquash"]; ok {
allowMergeRebase = allowMergeRebase && allowSquash.(bool)
}

if _, ok := unit.Config["AllowRebaseMerge"]; !ok {
unit.Config["AllowRebaseMerge"] = allowMergeRebase
}
if _, err := sess.ID(unit.ID).Cols("config").Update(unit); err != nil {
return err
}
}
return sess.Commit()
}
28 changes: 28 additions & 0 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ const (
MergeStyleMerge MergeStyle = "merge"
// MergeStyleRebase rebase before merging
MergeStyleRebase MergeStyle = "rebase"
// MergeStyleRebaseMerge rebase before merging with merge commit (--no-ff)
MergeStyleRebaseMerge MergeStyle = "rebase-merge"
// MergeStyleSquash squash commits into single commit before merging
MergeStyleSquash MergeStyle = "squash"
)
Expand Down Expand Up @@ -407,6 +409,32 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle
"git", "merge", "--ff-only", "-q", "head_repo_"+pr.HeadBranch); err != nil {
return fmt.Errorf("git merge --ff-only [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
}
case MergeStyleRebaseMerge:
// Checkout head branch
if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath),
"git", "checkout", "-b", "head_repo_"+pr.HeadBranch, "head_repo/"+pr.HeadBranch); err != nil {
return fmt.Errorf("git checkout: %s", stderr)
}
// Rebase before merging
if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git rebase): %s", tmpBasePath),
"git", "rebase", "-q", pr.BaseBranch); err != nil {
return fmt.Errorf("git rebase [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
}
// Checkout base branch again
if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath),
"git", "checkout", pr.BaseBranch); err != nil {
return fmt.Errorf("git checkout: %s", stderr)
}
// Merge with commit
if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
fmt.Sprintf("PullRequest.Merge (git rebase): %s", tmpBasePath),
"git", "merge", "--no-ff", "-q", "head_repo_"+pr.HeadBranch); err != nil {
return fmt.Errorf("git merge --no-ff [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
}

case MergeStyleSquash:
// Merge with squash
if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
Expand Down
2 changes: 1 addition & 1 deletion models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -1298,7 +1298,7 @@ func createRepository(e *xorm.Session, doer, u *User, repo *Repository) (err err
units = append(units, RepoUnit{
RepoID: repo.ID,
Type: tp,
Config: &PullRequestsConfig{AllowMerge: true, AllowRebase: true, AllowSquash: true},
Config: &PullRequestsConfig{AllowMerge: true, AllowRebase: true, AllowRebaseMerge: true, AllowSquash: true},
})
} else {
units = append(units, RepoUnit{
Expand Down
2 changes: 2 additions & 0 deletions models/repo_unit.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ type PullRequestsConfig struct {
IgnoreWhitespaceConflicts bool
AllowMerge bool
AllowRebase bool
AllowRebaseMerge bool
AllowSquash bool
}

Expand All @@ -108,6 +109,7 @@ func (cfg *PullRequestsConfig) ToDB() ([]byte, error) {
func (cfg *PullRequestsConfig) IsMergeStyleAllowed(mergeStyle MergeStyle) bool {
return mergeStyle == MergeStyleMerge && cfg.AllowMerge ||
mergeStyle == MergeStyleRebase && cfg.AllowRebase ||
mergeStyle == MergeStyleRebaseMerge && cfg.AllowRebaseMerge ||
mergeStyle == MergeStyleSquash && cfg.AllowSquash
}

Expand Down
3 changes: 2 additions & 1 deletion modules/auth/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ type RepoSettingForm struct {
PullsIgnoreWhitespace bool
PullsAllowMerge bool
PullsAllowRebase bool
PullsAllowRebaseMerge bool
PullsAllowSquash bool
EnableTimetracker bool
AllowOnlyContributorsToTrackTime bool
Expand Down Expand Up @@ -360,7 +361,7 @@ func (f *InitializeLabelsForm) Validate(ctx *macaron.Context, errs binding.Error

// MergePullRequestForm form for merging Pull Request
type MergePullRequestForm struct {
Do string `binding:"Required;In(merge,rebase,squash)"`
Do string `binding:"Required;In(merge,rebase,rebase-merge,squash)"`
MergeTitleField string
MergeMessageField string
}
Expand Down
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,7 @@ pulls.no_merge_helper = Enable merge options in the repository settings or merge
pulls.no_merge_wip = This pull request can not be merged because it is marked as being a work in progress.
pulls.merge_pull_request = Merge Pull Request
pulls.rebase_merge_pull_request = Rebase and Merge
pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff)
pulls.squash_merge_pull_request = Squash and Merge
pulls.invalid_merge_option = You cannot use this merge option for this pull request.
pulls.open_unmerged_pull_exists = `You cannot perform a reopen operation because there is a pending pull request (#%d) with identical properties.`
Expand Down Expand Up @@ -1012,6 +1013,7 @@ settings.pulls_desc = Enable Repository Pull Requests
settings.pulls.ignore_whitespace = Ignore Whitespace for Conflicts
settings.pulls.allow_merge_commits = Enable Commit Merging
settings.pulls.allow_rebase_merge = Enable Rebasing to Merge Commits
settings.pulls.allow_rebase_merge_commit = Enable Rebasing with explicit merge commits (--no-ff)
settings.pulls.allow_squash_commits = Enable Squashing to Merge Commits
settings.admin_settings = Administrator Settings
settings.admin_enable_health_check = Enable Repository Health Checks (git fsck)
Expand Down
2 changes: 2 additions & 0 deletions routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,8 @@ func ViewIssue(ctx *context.Context) {
ctx.Data["MergeStyle"] = models.MergeStyleMerge
} else if prConfig.AllowRebase {
ctx.Data["MergeStyle"] = models.MergeStyleRebase
} else if prConfig.AllowRebaseMerge {
ctx.Data["MergeStyle"] = models.MergeStyleRebaseMerge
} else if prConfig.AllowSquash {
ctx.Data["MergeStyle"] = models.MergeStyleSquash
} else {
Expand Down
3 changes: 3 additions & 0 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,9 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
if models.MergeStyle(form.Do) == models.MergeStyleMerge {
message = pr.GetDefaultMergeMessage()
}
if models.MergeStyle(form.Do) == models.MergeStyleRebaseMerge {
message = pr.GetDefaultMergeMessage()
}
if models.MergeStyle(form.Do) == models.MergeStyleSquash {
message = pr.GetDefaultSquashMessage()
}
Expand Down
1 change: 1 addition & 0 deletions routers/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) {
IgnoreWhitespaceConflicts: form.PullsIgnoreWhitespace,
AllowMerge: form.PullsAllowMerge,
AllowRebase: form.PullsAllowRebase,
AllowRebaseMerge: form.PullsAllowRebaseMerge,
AllowSquash: form.PullsAllowSquash,
},
})
Expand Down
21 changes: 20 additions & 1 deletion templates/repo/issue/view_content/pull.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
</div>
{{if .AllowMerge}}
{{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}}
{{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowSquash}}
{{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowRebaseMerge $prUnit.PullRequestsConfig.AllowSquash}}
<div class="ui divider"></div>
{{if $prUnit.PullRequestsConfig.AllowMerge}}
<div class="ui form merge-fields" style="display: none">
Expand Down Expand Up @@ -114,6 +114,19 @@
</form>
</div>
{{end}}
{{if $prUnit.PullRequestsConfig.AllowRebaseMerge}}
<div class="ui form rebase-merge-fields" style="display: none">
<form action="{{.Link}}/merge" method="post">
{{.CsrfTokenHtml}}
<button class="ui green button" type="submit" name="do" value="rebase-merge">
{{$.i18n.Tr "repo.pulls.rebase_merge_commit_pull_request"}}
</button>
<button class="ui button merge-cancel">
{{$.i18n.Tr "cancel"}}
</button>
</form>
</div>
{{end}}
{{if $prUnit.PullRequestsConfig.AllowSquash}}
<div class="ui form squash-fields" style="display: none">
<form action="{{.Link}}/merge" method="post">
Expand Down Expand Up @@ -143,6 +156,9 @@
{{if eq .MergeStyle "rebase"}}
{{$.i18n.Tr "repo.pulls.rebase_merge_pull_request"}}
{{end}}
{{if eq .MergeStyle "rebase-merge"}}
{{$.i18n.Tr "repo.pulls.rebase_merge_commit_pull_request"}}
{{end}}
{{if eq .MergeStyle "squash"}}
{{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}}
{{end}}
Expand All @@ -157,6 +173,9 @@
{{if $prUnit.PullRequestsConfig.AllowRebase}}
<div class="item{{if eq .MergeStyle "rebase"}} active selected{{end}}" data-do="rebase">{{$.i18n.Tr "repo.pulls.rebase_merge_pull_request"}}</div>
{{end}}
{{if $prUnit.PullRequestsConfig.AllowRebaseMerge}}
<div class="item{{if eq .MergeStyle "rebase-merge"}} active selected{{end}}" data-do="rebase-merge">{{$.i18n.Tr "repo.pulls.rebase_merge_commit_pull_request"}}</div>
{{end}}
{{if $prUnit.PullRequestsConfig.AllowSquash}}
<div class="item{{if eq .MergeStyle "squash"}} active selected{{end}}" data-do="squash">{{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}}</div>
{{end}}
Expand Down
6 changes: 6 additions & 0 deletions templates/repo/settings/options.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,12 @@
<label>{{.i18n.Tr "repo.settings.pulls.allow_rebase_merge"}}</label>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="pulls_allow_rebase_merge" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowRebaseMerge)}}checked{{end}}>
<label>{{.i18n.Tr "repo.settings.pulls.allow_rebase_merge_commit"}}</label>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="pulls_allow_squash" type="checkbox" {{if or (not $pullRequestEnabled) ($prUnit.PullRequestsConfig.AllowSquash)}}checked{{end}}>
Expand Down