From 33ba8f74f45ebc971bb298f10c932671655fc325 Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:00:47 +0100 Subject: [PATCH 1/9] Change data model. Rename repository.pulls_allow_rebase to repository.pulls_allow_alt, and add repository.pulls_prefer_rebase. --- internal/database/migrations/migrations.go | 4 ++ internal/database/migrations/v23.go | 20 ++++++++ internal/database/migrations/v23_test.go | 56 ++++++++++++++++++++++ internal/database/repo.go | 3 +- 4 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 internal/database/migrations/v23.go create mode 100644 internal/database/migrations/v23_test.go diff --git a/internal/database/migrations/migrations.go b/internal/database/migrations/migrations.go index 04c1e3636..c431e46ec 100644 --- a/internal/database/migrations/migrations.go +++ b/internal/database/migrations/migrations.go @@ -63,6 +63,10 @@ var migrations = []Migration{ // on v22. Let's make a noop v22 to make sure every instance will not miss a // real future migration. NewMigration("noop", func(*gorm.DB) error { return nil }), + NewMigration( + "rename repository.pulls_allow_rebase -> repository.pulls_allow_alt", + renameRepoPullsAllowRebaseToPullsAllowAlt, + ), } var errMigrationSkipped = errors.New("the migration has been skipped") diff --git a/internal/database/migrations/v23.go b/internal/database/migrations/v23.go new file mode 100644 index 000000000..20b98ed35 --- /dev/null +++ b/internal/database/migrations/v23.go @@ -0,0 +1,20 @@ +// Copyright 2025 The Gogs 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 ( + "gorm.io/gorm" +) + +func renameRepoPullsAllowRebaseToPullsAllowAlt(db *gorm.DB) error { + type Repository struct { + PullsAllowRebase bool `gorm:"not null;default:FALSE"` + PullsAllowAlt bool `gorm:"not null;default:FALSE"` + } + if db.Migrator().HasColumn(&Repository{}, "PullsAllowAlt") { + return errMigrationSkipped + } + return db.Migrator().RenameColumn(&Repository{}, "PullsAllowRebase", "PullsAllowAlt") +} diff --git a/internal/database/migrations/v23_test.go b/internal/database/migrations/v23_test.go new file mode 100644 index 000000000..f5d158c92 --- /dev/null +++ b/internal/database/migrations/v23_test.go @@ -0,0 +1,56 @@ +// Copyright 2025 The Gogs 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 ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "gogs.io/gogs/internal/dbtest" +) + +type repositoryPreV22 struct { + PullsAllowRebase bool `xorm:"NOT NULL DEFAULT false" gorm:"not null;default:FALSE"` +} + +func (*repositoryPreV22) TableName() string { + return "repository" +} + +type repositoryV22 struct { + PullsAllowAlt bool `xorm:"NOT NULL DEFAULT false" gorm:"not null;default:FALSE"` +} + +func (*repositoryV22) TableName() string { + return "repository" +} + +func TestRenameRepoPullsAllowRebaseToPullsAllowAlt(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Parallel() + + db := dbtest.NewDB(t, "renamePullsAllowRebaseToPullsAllowAlt", new(repositoryPreV22)) + err := db.Create( + &repositoryPreV22{ + PullsAllowRebase: false, + }, + ).Error + require.NoError(t, err) + assert.False(t, db.Migrator().HasColumn(&repositoryV22{}, "PullsAllowAlt")) + assert.True(t, db.Migrator().HasColumn(&repositoryPreV22{}, "PullsAllowRebase")) + + err = renameRepoPullsAllowRebaseToPullsAllowAlt(db) + require.NoError(t, err) + assert.True(t, db.Migrator().HasColumn(&repositoryV22{}, "PullsAllowAlt")) + assert.False(t, db.Migrator().HasColumn(&repositoryPreV22{}, "PullsAllowRebase")) + + // Re-run should be skipped + err = renameRepoPullsAllowRebaseToPullsAllowAlt(db) + require.Equal(t, errMigrationSkipped, err) +} diff --git a/internal/database/repo.go b/internal/database/repo.go index f55c35160..23d007008 100644 --- a/internal/database/repo.go +++ b/internal/database/repo.go @@ -201,7 +201,8 @@ type Repository struct { ExternalMetas map[string]string `xorm:"-" gorm:"-" json:"-"` EnablePulls bool `xorm:"NOT NULL DEFAULT true" gorm:"not null;default:TRUE"` PullsIgnoreWhitespace bool `xorm:"NOT NULL DEFAULT false" gorm:"not null;default:FALSE"` - PullsAllowRebase bool `xorm:"NOT NULL DEFAULT false" gorm:"not null;default:FALSE"` + PullsPreferRebase bool `xorm:"NOT NULL DEFAULT false" gorm:"not null;default:FALSE"` + PullsAllowAlt bool `xorm:"NOT NULL DEFAULT false" gorm:"not null;default:FALSE"` IsFork bool `xorm:"NOT NULL DEFAULT false" gorm:"not null;default:FALSE"` ForkID int64 From 4699ba0e53091fd4a97333e986333aa66f7127bd Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:03:33 +0100 Subject: [PATCH 2/9] use new repo data model for PullRequest.Merge --- internal/database/pull.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/database/pull.go b/internal/database/pull.go index 06f88b877..eec127958 100644 --- a/internal/database/pull.go +++ b/internal/database/pull.go @@ -254,8 +254,12 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle remoteHeadBranch := "head_repo/" + pr.HeadBranch // Check if merge style is allowed, reset to default style if not - if mergeStyle == MERGE_STYLE_REBASE && !pr.BaseRepo.PullsAllowRebase { - mergeStyle = MERGE_STYLE_REGULAR + if !pr.BaseRepo.PullsAllowAlt { + if pr.BaseRepo.PullsPreferRebase { + mergeStyle = MERGE_STYLE_REBASE + } else { + mergeStyle = MERGE_STYLE_REGULAR + } } switch mergeStyle { From ed9ad914c2fdc44c640903ed82f167e600bfce02 Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:06:17 +0100 Subject: [PATCH 3/9] reintroduce Repository.PullsAllowRebase as a method, with Repository.PullsAllowMerge as a companion so that it doesn't get lonely during these cold and dark winter nights --- internal/database/repo.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/database/repo.go b/internal/database/repo.go index 23d007008..35572d8a0 100644 --- a/internal/database/repo.go +++ b/internal/database/repo.go @@ -289,6 +289,14 @@ func (repo *Repository) MustOwner() *User { return repo.mustOwner(x) } +func (repo *Repository) PullsAllowMerge() bool { + return !repo.PullsPreferRebase || repo.PullsAllowAlt +} + +func (repo *Repository) PullsAllowRebase() bool { + return repo.PullsPreferRebase || repo.PullsAllowAlt +} + func (repo *Repository) FullName() string { return repo.MustOwner().Name + "/" + repo.Name } From c0df133804632f672fb0abd9b1f0b2653806ecae Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:41:43 +0100 Subject: [PATCH 4/9] change the repo settings form over to the new data model, presenting the two settings as one composite --- conf/locale/locale_en-US.ini | 5 ++++- internal/form/repo.go | 2 +- templates/repo/settings/options.tmpl | 24 +++++++++++++++++++++--- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/conf/locale/locale_en-US.ini b/conf/locale/locale_en-US.ini index d39f709e7..a377bfda8 100644 --- a/conf/locale/locale_en-US.ini +++ b/conf/locale/locale_en-US.ini @@ -758,7 +758,10 @@ settings.tracker_issue_style.alphanumeric = Alphanumeric settings.tracker_url_format_desc = You can use placeholder {user} {repo} {index} for user name, repository name and issue index. settings.pulls_desc = Enable pull requests to accept contributions between repositories and branches settings.pulls.ignore_whitespace = Ignore changes in whitespace -settings.pulls.allow_rebase_merge = Allow use rebase to merge commits +settings.pulls.merge_only = Always use merge to merge commits +settings.pulls.merge_default = Merge by default, but allow to use rebase to merge commits +settings.pulls.rebase_default = Rebase by default, but allow to use merge to merge commits +settings.pulls.rebase_only = Always use rebase to merge commits settings.danger_zone = Danger Zone settings.cannot_fork_to_same_owner = You cannot fork a repository to its original owner. settings.new_owner_has_same_repo = The new owner already has a repository with same name. Please choose another name. diff --git a/internal/form/repo.go b/internal/form/repo.go index 67066de29..c94be28ab 100644 --- a/internal/form/repo.go +++ b/internal/form/repo.go @@ -117,7 +117,7 @@ type RepoSetting struct { TrackerIssueStyle string EnablePulls bool PullsIgnoreWhitespace bool - PullsAllowRebase bool + PullsMergeType string } func (f *RepoSetting) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors { diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 5122fc37c..98f400cfa 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -231,9 +231,27 @@
-
- - +
+ + +
+
+
+
+ + +
+
+
+
+ + +
+
+
+
+ +
From d4da1edb60d98c738ee0989b1e2c6a42c8318fc0 Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:43:55 +0100 Subject: [PATCH 5/9] add methods that convert the composite frontend representation of the new settings to its backend counterparts --- internal/form/repo.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/internal/form/repo.go b/internal/form/repo.go index c94be28ab..79b22f8d2 100644 --- a/internal/form/repo.go +++ b/internal/form/repo.go @@ -120,6 +120,14 @@ type RepoSetting struct { PullsMergeType string } +func (f *RepoSetting) PullsPreferRebase() bool { + return strings.HasPrefix(f.PullsMergeType, "rebase_") +} + +func (f *RepoSetting) PullsAllowAlt() bool { + return strings.HasSuffix(f.PullsMergeType, "_default") +} + func (f *RepoSetting) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors { return validate(errs, ctx.Data, f, ctx.Locale) } From 3a3bbe26fd4b940e2a9662adfa6633b6f258283d Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:47:15 +0100 Subject: [PATCH 6/9] save the new repo settings correctly --- internal/route/repo/setting.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/route/repo/setting.go b/internal/route/repo/setting.go index 196c1c62b..95059d2f8 100644 --- a/internal/route/repo/setting.go +++ b/internal/route/repo/setting.go @@ -155,7 +155,8 @@ func SettingsPost(c *context.Context, f form.RepoSetting) { repo.ExternalTrackerStyle = f.TrackerIssueStyle repo.EnablePulls = f.EnablePulls repo.PullsIgnoreWhitespace = f.PullsIgnoreWhitespace - repo.PullsAllowRebase = f.PullsAllowRebase + repo.PullsPreferRebase = f.PullsPreferRebase() + repo.PullsAllowAlt = f.PullsAllowAlt() if !repo.EnableWiki || repo.EnableExternalWiki { repo.AllowPublicWiki = false From 2229c305cbdcb14c3fa96990998726f3fd4f38a2 Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:49:02 +0100 Subject: [PATCH 7/9] validate that the new repo settings are submitted correctly --- internal/form/repo.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/form/repo.go b/internal/form/repo.go index 79b22f8d2..5485598d5 100644 --- a/internal/form/repo.go +++ b/internal/form/repo.go @@ -7,6 +7,7 @@ package form import ( "net/url" "strings" + "slices" "github.com/go-macaron/binding" "github.com/unknwon/com" @@ -128,7 +129,25 @@ func (f *RepoSetting) PullsAllowAlt() bool { return strings.HasSuffix(f.PullsMergeType, "_default") } +const ( + MERGE_STYLE_SETTING_MERGE_ONLY = "merge_only" + MERGE_STYLE_SETTING_MERGE_DEFAULT = "merge_default" + MERGE_STYLE_SETTING_REBASE_DEFAULT = "rebase_default" + MERGE_STYLE_SETTING_REBASE_ONLY = "rebase_only" +) + +const ERR_INVALID_MERGE_TYPE = "InvalidMergeTypeError" + func (f *RepoSetting) Validate(ctx *macaron.Context, errs binding.Errors) binding.Errors { + valid := []string{ + MERGE_STYLE_SETTING_MERGE_ONLY, + MERGE_STYLE_SETTING_MERGE_DEFAULT, + MERGE_STYLE_SETTING_REBASE_DEFAULT, + MERGE_STYLE_SETTING_REBASE_ONLY, + } + if mt := f.PullsMergeType; !slices.Contains(valid, mt) { + errs.Add([]string{mt}, ERR_INVALID_MERGE_TYPE, "InvalidMergeType") + } return validate(errs, ctx.Data, f, ctx.Locale) } From 08375a1b9863efef14e0f3142293ec9cba1dbb69 Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:52:11 +0100 Subject: [PATCH 8/9] make the regular merge option being present conditional in the same way as the rebase merge option in the frontend --- templates/repo/issue/view_content.tmpl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index b83b11d0d..46dc979ff 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -195,12 +195,14 @@
{{.CSRFTokenHTML}} -
-
- - + {{if .Issue.Repo.PullsAllowMerge}} +
+
+ + +
-
+ {{end}} {{if .Issue.Repo.PullsAllowRebase}}
From eb26b2121694eab4bfc64c65c85a1cdc7745b5c2 Mon Sep 17 00:00:00 2001 From: Gogs Date: Sun, 23 Feb 2025 21:53:18 +0100 Subject: [PATCH 9/9] make the merge style preselection depend on which style is preferred in the frontend --- templates/repo/issue/view_content.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index 46dc979ff..33a3eae38 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -198,7 +198,7 @@ {{if .Issue.Repo.PullsAllowMerge}}
- +
@@ -206,7 +206,7 @@ {{if .Issue.Repo.PullsAllowRebase}}
- +