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 a simple way to rename branch like gh #15870

Merged
merged 15 commits into from
Oct 8, 2021
Merged
43 changes: 43 additions & 0 deletions integrations/rename_branch_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2021 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 integrations

import (
"net/http"
"testing"

"code.gitea.io/gitea/models"
"github.com/stretchr/testify/assert"
)

func TestRenameBranch(t *testing.T) {
// get branch setting page
session := loginUser(t, "user2")
req := NewRequest(t, "GET", "/user2/repo1/settings/branches")
resp := session.MakeRequest(t, req, http.StatusOK)
6543 marked this conversation as resolved.
Show resolved Hide resolved
htmlDoc := NewHTMLParser(t, resp.Body)

postData := map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"from": "master",
"to": "main",
}
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/rename_branch", postData)
session.MakeRequest(t, req, http.StatusFound)
6543 marked this conversation as resolved.
Show resolved Hide resolved

// check new branch link
req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/main/README.md", postData)
session.MakeRequest(t, req, http.StatusOK)
6543 marked this conversation as resolved.
Show resolved Hide resolved

// check old branch link
req = NewRequestWithValues(t, "GET", "/user2/repo1/src/branch/master/README.md", postData)
resp = session.MakeRequest(t, req, http.StatusFound)
6543 marked this conversation as resolved.
Show resolved Hide resolved
location := resp.HeaderMap.Get("Location")
assert.Equal(t, "/user2/repo1/src/branch/main/README.md", location)

// check db
repo1 := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
assert.Equal(t, "main", repo1.DefaultBranch)
}
80 changes: 80 additions & 0 deletions models/branches.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,3 +571,83 @@ func RemoveOldDeletedBranches(ctx context.Context, olderThan time.Duration) {
log.Error("DeletedBranchesCleanup: %v", err)
}
}

// RenamedBranch proivde renamed branch log
// will check it when an branch can't be found
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved
type RenamedBranch struct {
ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"INDEX NOT NULL"`
From string
To string
CreatedUnix timeutil.TimeStamp `xorm:"created"`
}
6543 marked this conversation as resolved.
Show resolved Hide resolved

// FindRenamedBranch check if a branch was renamed
func FindRenamedBranch(repoID int64, from string) (branch *RenamedBranch, exist bool, err error) {
branch = &RenamedBranch{
RepoID: repoID,
From: from,
}
exist, err = x.Get(branch)

return
}

// RenameBranch rename a branch
func (repo *Repository) RenameBranch(from, to string, gitAction func(isDefault bool) error) (err error) {
sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}

// 1. update default branch if needed
isDefault := repo.DefaultBranch == from
if isDefault {
repo.DefaultBranch = to
_, err = sess.ID(repo.ID).Cols("default_branch").Update(repo)
if err != nil {
return err
}
}

// 2. Update protected branch if needed
protectedBranch, err := getProtectedBranchBy(sess, repo.ID, from)
if err != nil {
return err
}

if protectedBranch != nil {
protectedBranch.BranchName = to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I seeing this wrong, or is it a bad idea to automatically apply branch protection to a "new" branch without testing if that branch should be protected?
Because in the current implementation, if I see this correctly, the following would lead to an inconsistent state:

  1. Have branch protection only applied to branches m.*
  2. -> Branch main is a protected branch
  3. Rename main -> default.
  4. default now has branch protection even though it shouldn't.

Furthermore, if I see that correctly, the type of branch protection also needs to be switched then if this branch is covered by another rule.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have branch protection only applied to branches m.*

sadly, gitea don't have the feature to protect more than one branches by one rule.
each branch protect rule only applied to one brach. so when the branch was renamed.
the old config is not usefull, which should be binded to new branch or be removed, I think bind it
to new branch is better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks for the clarification. I did not need to set up branch protection on Gitea until now, I thought it was implemented just as in GitHub using a pattern for branches. Gitea's approach definitely has its advantages as well as its disadvantages.
In that case, I completely understand why you keep the branch protection. Makes total sense.

_, err = sess.ID(protectedBranch.ID).Cols("branch_name").Update(protectedBranch)
if err != nil {
return err
}
}

// 3. Update all not merged pull request base branch name
_, err = sess.Table(new(PullRequest)).Where("base_repo_id=? AND base_branch=? AND has_merged=?",
repo.ID, from, false).
Update(map[string]interface{}{"base_branch": to})
if err != nil {
return err
}

// 4. do git action
if err = gitAction(isDefault); err != nil {
return err
}

// 5. insert renamed branch record
renamedBranch := &RenamedBranch{
RepoID: repo.ID,
From: from,
To: to,
}
_, err = sess.Insert(renamedBranch)
if err != nil {
return err
}

return sess.Commit()
}
49 changes: 49 additions & 0 deletions models/branches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,52 @@ func getDeletedBranch(t *testing.T, branch *DeletedBranch) *DeletedBranch {

return deletedBranch
}

func TestFindRenamedBranch(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
branch, exist, err := FindRenamedBranch(1, "dev")
assert.NoError(t, err)
assert.Equal(t, true, exist)
assert.Equal(t, "master", branch.To)

_, exist, err = FindRenamedBranch(1, "unknow")
assert.NoError(t, err)
assert.Equal(t, false, exist)
}

func TestRenameBranch(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())
repo1 := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
_isDefault := false

err := UpdateProtectBranch(repo1, &ProtectedBranch{
RepoID: repo1.ID,
BranchName: "master",
}, WhitelistOptions{})
assert.NoError(t, err)

assert.NoError(t, repo1.RenameBranch("master", "main", func(isDefault bool) error {
_isDefault = isDefault
return nil
}))

assert.Equal(t, true, _isDefault)
repo1 = AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
assert.Equal(t, "main", repo1.DefaultBranch)

pull := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest) // merged
assert.Equal(t, "master", pull.BaseBranch)

pull = AssertExistsAndLoadBean(t, &PullRequest{ID: 2}).(*PullRequest) // open
assert.Equal(t, "main", pull.BaseBranch)

renamedBranch := AssertExistsAndLoadBean(t, &RenamedBranch{ID: 2}).(*RenamedBranch)
assert.Equal(t, "master", renamedBranch.From)
assert.Equal(t, "main", renamedBranch.To)
assert.Equal(t, int64(1), renamedBranch.RepoID)

AssertExistsAndLoadBean(t, &ProtectedBranch{
RepoID: repo1.ID,
BranchName: "main",
})
}
5 changes: 5 additions & 0 deletions models/fixtures/renamed_branch.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-
id: 1
repo_id: 1
from: dev
to: master
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ var migrations = []Migration{
NewMigration("Add key is verified to gpg key", addKeyIsVerified),
// v189 -> v190
NewMigration("Unwrap ldap.Sources", unwrapLDAPSourceCfg),
// v190 -> v191
NewMigration("Add renamed_branch table", addRenamedBranchTable),
}

// GetCurrentDBVersion returns the current db version
Expand Down
20 changes: 20 additions & 0 deletions models/migrations/v190.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2021 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 (
"xorm.io/xorm"
)

func addRenamedBranchTable(x *xorm.Engine) error {
type RenamedBranch struct {
ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"INDEX NOT NULL"`
From string
To string
CreatedUnix int64 `xorm:"created"`
}
return x.Sync2(new(RenamedBranch))
}
1 change: 1 addition & 0 deletions models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func init() {
new(PushMirror),
new(RepoArchiver),
new(ProtectedTag),
new(RenamedBranch),
)

gonicNames := []string{"SSL", "UID"}
Expand Down
32 changes: 31 additions & 1 deletion modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,28 @@ func getRefName(ctx *Context, pathType RepoRefType) string {
ctx.Repo.TreePath = path
return ctx.Repo.Repository.DefaultBranch
case RepoRefBranch:
return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsBranchExist)
ref := getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsBranchExist)
if len(ref) == 0 {
// maybe it's a renamed branch
return getRefNameFromPath(ctx, path, func(s string) bool {
b, exist, err := models.FindRenamedBranch(ctx.Repo.Repository.ID, s)
if err != nil {
log.Error("FindRenamedBranch", err)
return false
}

if !exist {
return false
}

ctx.Data["IsRenamedBranch"] = true
ctx.Data["RenamedBranchName"] = b.To

return true
})
}

return ref
case RepoRefTag:
return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsTagExist)
case RepoRefCommit:
Expand Down Expand Up @@ -784,6 +805,15 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context
} else {
refName = getRefName(ctx, refType)
ctx.Repo.BranchName = refName
isRenamedBranch, has := ctx.Data["IsRenamedBranch"].(bool)
if isRenamedBranch && has {
renamedBranchName := ctx.Data["RenamedBranchName"].(string)
ctx.Flash.Info(ctx.Tr("repo.branch.renamed", refName, renamedBranchName))
link := strings.Replace(ctx.Req.RequestURI, refName, renamedBranchName, 1)
ctx.Redirect(link)
return
}

if refType.RefTypeIncludesBranches() && ctx.Repo.GitRepo.IsBranchExist(refName) {
ctx.Repo.IsViewBranch = true

Expand Down
6 changes: 6 additions & 0 deletions modules/git/repo_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,9 @@ func (repo *Repository) RemoveRemote(name string) error {
func (branch *Branch) GetCommit() (*Commit, error) {
return branch.gitRepo.GetBranchCommit(branch.Name)
}

// RenameBranch rename a branch
func (repo *Repository) RenameBranch(from, to string) error {
_, err := NewCommand("branch", "-m", from, to).RunInDir(repo.Path)
return err
}
7 changes: 7 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1974,6 +1974,12 @@ settings.lfs_pointers.inRepo=In Repo
settings.lfs_pointers.exists=Exists in store
settings.lfs_pointers.accessible=Accessible to User
settings.lfs_pointers.associateAccessible=Associate accessible %d OIDs
settings.rename_branch_failed_exist=can not rename branch because target branch %s is exist
settings.rename_branch_failed_not_exist= can not rename branch because target branch %s is not exist
settings.rename_branch_success = branch %s has been succed renamed to %s
settings.rename_branch_from=old branch name
settings.rename_branch_to=new branch name
settings.rename_branch = Rename branch
a1012112796 marked this conversation as resolved.
Show resolved Hide resolved

diff.browse_source = Browse Source
diff.parent = parent
Expand Down Expand Up @@ -2093,6 +2099,7 @@ branch.create_new_branch = Create branch from branch:
branch.confirm_create_branch = Create branch
branch.new_branch = Create new branch
branch.new_branch_from = Create new branch from '%s'
branch.renamed = Branch %s was renamed to %s.

tag.create_tag = Create tag <strong>%s</strong>
tag.create_success = Tag '%s' has been created.
Expand Down
38 changes: 38 additions & 0 deletions routers/web/repo/setting_protected_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms"
pull_service "code.gitea.io/gitea/services/pull"
"code.gitea.io/gitea/services/repository"
)

// ProtectedBranch render the page to protect the repository
Expand Down Expand Up @@ -284,3 +285,40 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
ctx.Redirect(fmt.Sprintf("%s/settings/branches", ctx.Repo.RepoLink))
}
}

// RenameBranchPost responses for rename a branch
func RenameBranchPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.RenameBranchForm)
6543 marked this conversation as resolved.
Show resolved Hide resolved

if !ctx.Repo.CanCreateBranch() {
ctx.NotFound("RenameBranch", nil)
return
}

if ctx.HasError() {
ctx.Flash.Error(ctx.GetErrMsg())
ctx.Redirect(fmt.Sprintf("%s/settings/branches", ctx.Repo.RepoLink))
return
}

msg, err := repository.RenameBranch(ctx.Repo.Repository, ctx.User, ctx.Repo.GitRepo, form.From, form.To)
if err != nil {
ctx.ServerError("RenameBranch", err)
return
}

if msg == "target_exist" {
ctx.Flash.Error(ctx.Tr("repo.settings.rename_branch_failed_exist", form.To))
ctx.Redirect(fmt.Sprintf("%s/settings/branches", ctx.Repo.RepoLink))
return
}

if msg == "from_not_exist" {
ctx.Flash.Error(ctx.Tr("repo.settings.rename_branch_failed_not_exist", form.From))
ctx.Redirect(fmt.Sprintf("%s/settings/branches", ctx.Repo.RepoLink))
return
}

ctx.Flash.Success(ctx.Tr("repo.settings.rename_branch_success", form.From, form.To))
ctx.Redirect(fmt.Sprintf("%s/settings/branches", ctx.Repo.RepoLink))
}
1 change: 1 addition & 0 deletions routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ func RegisterRoutes(m *web.Route) {
m.Combo("/*").Get(repo.SettingsProtectedBranch).
Post(bindIgnErr(forms.ProtectBranchForm{}), context.RepoMustNotBeArchived(), repo.SettingsProtectedBranchPost)
}, repo.MustBeNotEmpty)
m.Post("/rename_branch", bindIgnErr(forms.RenameBranchForm{}), context.RepoMustNotBeArchived(), repo.RenameBranchPost)
Comment on lines 612 to +615
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I have seen, the underscore (_) syntax is uncommon for routes. What about /branches/rename instead? That would then be

Suggested change
m.Combo("/*").Get(repo.SettingsProtectedBranch).
Post(bindIgnErr(forms.ProtectBranchForm{}), context.RepoMustNotBeArchived(), repo.SettingsProtectedBranchPost)
}, repo.MustBeNotEmpty)
m.Post("/rename_branch", bindIgnErr(forms.RenameBranchForm{}), context.RepoMustNotBeArchived(), repo.RenameBranchPost)
m.Combo("/*").Get(repo.SettingsProtectedBranch).
Post(bindIgnErr(forms.ProtectBranchForm{}), context.RepoMustNotBeArchived(), repo.SettingsProtectedBranchPost)
m.Post("/rename", bindIgnErr(forms.RenameBranchForm{}), context.RepoMustNotBeArchived(), repo.RenameBranchPost)
}, repo.MustBeNotEmpty)

Combined with a change inside the UI dialog.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I dont mind since it's only a web route - if you like to group settings bit more refactor pulls are welcome :)


m.Group("/tags", func() {
m.Get("", repo.Tags)
Expand Down
12 changes: 12 additions & 0 deletions services/forms/repo_branch_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,15 @@ func (f *NewBranchForm) Validate(req *http.Request, errs binding.Errors) binding
ctx := context.GetContext(req)
return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
}

// RenameBranchForm form for rename a branch
type RenameBranchForm struct {
From string `binding:"Required;MaxSize(100);GitRefName"`
To string `binding:"Required;MaxSize(100);GitRefName"`
}

// Validate validates the fields
func (f *RenameBranchForm) Validate(req *http.Request, errs binding.Errors) binding.Errors {
ctx := context.GetContext(req)
return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
}
Loading