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 "Update Branch" button to Pull Requests #9784

Merged
merged 39 commits into from
Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
38a27df
add Divergence
6543 Jan 15, 2020
2c7fc31
add Update Button
6543 Jan 15, 2020
26d686f
first working version
6543 Jan 16, 2020
d8522ac
re-use code
6543 Jan 16, 2020
efe6982
split raw merge commands and db-change functions (notify, cache, ...)
6543 Jan 16, 2020
328ecc3
use rawMerge (remove redundant code)
6543 Jan 16, 2020
9249d47
own function to get Diverging of PRs
6543 Jan 16, 2020
454ee44
use FlashError
6543 Jan 16, 2020
5d5d919
correct Error Msg
6543 Jan 16, 2020
3c6157b
hook is triggerd ... so remove comment
6543 Jan 16, 2020
88fa4c5
add "branch2" to "user2/repo1" because it unit-test "TestPullView_Rev…
6543 Jan 16, 2020
7776738
move GetPerm to IsUserAllowedToUpdate
6543 Jan 16, 2020
e3adbd2
add Flash Success MSG
6543 Jan 16, 2020
05c0c1b
imprufe code
6543 Jan 16, 2020
f7d924a
fix-lint
6543 Jan 16, 2020
e2f96d9
TEST: add PullRequest ID:5
6543 Jan 16, 2020
8bad5d2
correct comments
6543 Jan 16, 2020
7442991
Merge branch 'master' into update-pr
6543 Jan 16, 2020
143d8e8
make PR5 outdated
6543 Jan 16, 2020
fb33b4b
fix Tests
6543 Jan 16, 2020
069a393
WIP: add pull update test
6543 Jan 16, 2020
720fbf1
update revs
6543 Jan 16, 2020
892ddd9
update locales
6543 Jan 16, 2020
7aa9e84
working TEST
6543 Jan 16, 2020
7502ff5
Merge branch 'master' into update-pr
6543 Jan 16, 2020
36a34cd
update UI
6543 Jan 16, 2020
18fadef
misspell
6543 Jan 16, 2020
97e51ec
change style
6543 Jan 16, 2020
18eb495
add 1s delay so rev exist
6543 Jan 16, 2020
2b58961
move row up (before merge row)
6543 Jan 16, 2020
688a53a
fix lint nit
6543 Jan 16, 2020
97a6f83
UI remove divider
6543 Jan 16, 2020
32c5eee
Merge branch 'master' into update-pr
6543 Jan 16, 2020
6400bcc
Update style
lafriks Jan 17, 2020
fa4779b
nits
6543 Jan 17, 2020
fbbc7f4
do it right
6543 Jan 17, 2020
717406e
introduce IsSameRepo
6543 Jan 17, 2020
f26c8b3
remove useless check
6543 Jan 17, 2020
246748f
Merge branch 'master' into update-pr
6543 Jan 17, 2020
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
Prev Previous commit
Next Next commit
first working version
  • Loading branch information
6543 committed Jan 16, 2020
commit 26d686fe080ed2a9b7aa27534a916ebe731ac287
69 changes: 69 additions & 0 deletions routers/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,75 @@ func ViewPullFiles(ctx *context.Context) {
ctx.HTML(200, tplPullFiles)
}

// UpdatePullRequest merge master into PR
func UpdatePullRequest(ctx *context.Context) {

6543 marked this conversation as resolved.
Show resolved Hide resolved
issue := checkPullInfo(ctx)
if ctx.Written() {
return
}
if issue.IsClosed {
ctx.NotFound("MergePullRequest", nil)
return
}

6543 marked this conversation as resolved.
Show resolved Hide resolved
if issue.PullRequest.HasMerged {
ctx.NotFound("MergePullRequest", nil)
return
}

if err := issue.PullRequest.LoadBaseRepo(); err != nil {
ctx.InternalServerError(err)
return
}
if err := issue.PullRequest.LoadHeadRepo(); err != nil {
ctx.InternalServerError(err)
return
}

headRepoPerm, err := models.GetUserRepoPermission(issue.PullRequest.HeadRepo, ctx.User)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
return
}

allowedUpdate, err := pull_service.IsUserAllowedToUpdate(issue.PullRequest, headRepoPerm, ctx.User)
if err != nil {
ctx.ServerError("IsUserAllowedToMerge", err)
return
}

// ToDo: add check if maintainers are allowed to change branch ... (need migration & co)
if !allowedUpdate {
ctx.Flash.Error(ctx.Tr("repo.pulls.update_not_allowed"))
6543 marked this conversation as resolved.
Show resolved Hide resolved
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
return
}

// default merge commit message
message := fmt.Sprintf("Merge branch '%s' into %s", issue.PullRequest.BaseBranch, issue.PullRequest.HeadBranch)
Copy link
Member

Choose a reason for hiding this comment

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

This should be configurable via .ini or via Crowdin.

Copy link
Member

@lunny lunny Jan 17, 2020

Choose a reason for hiding this comment

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

But for this one, sometimes, we use English commit messages even if I'm using a Chinese User Interface.

Copy link
Member

Choose a reason for hiding this comment

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

We can use app.ini like we did with closing keywords and WIP...


if err = pull_service.Update(issue.PullRequest, ctx.User, message); err != nil {
sanitize := func(x string) string {
runes := []rune(x)

if len(runes) > 512 {
x = "..." + string(runes[len(runes)-512:])
}

return strings.Replace(html.EscapeString(x), "\n", "<br>", -1)
}
if models.IsErrMergeConflicts(err) {
conflictError := err.(models.ErrMergeConflicts)
ctx.Flash.Error(ctx.Tr("repo.pulls.merge_conflict", sanitize(conflictError.StdErr), sanitize(conflictError.StdOut)))
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(issue.Index))
return
}
ctx.ServerError("Update", err)
return
}
}

// MergePullRequest response for merging pull request
func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
issue := checkPullInfo(ctx)
Expand Down
1 change: 1 addition & 0 deletions routers/routes/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,7 @@ func RegisterRoutes(m *macaron.Macaron) {
m.Get(".patch", repo.DownloadPullPatch)
m.Get("/commits", context.RepoRef(), repo.ViewPullCommits)
m.Post("/merge", context.RepoMustNotBeArchived(), reqRepoPullsWriter, bindIgnErr(auth.MergePullRequestForm{}), repo.MergePullRequest)
m.Post("/update", repo.UpdatePullRequest)
m.Post("/cleanup", context.RepoMustNotBeArchived(), context.RepoRef(), repo.CleanUpPullRequest)
m.Group("/files", func() {
m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.ViewPullFiles)
Expand Down
259 changes: 259 additions & 0 deletions services/pull/update.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
// Copyright 2020 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 pull

import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"
"time"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"

"github.com/mcuadros/go-version"
)

// Update ToDo wip ...
func Update(pull *models.PullRequest, doer *models.User, message string) (err error) {
binVersion, err := git.BinVersion()
if err != nil {
log.Error("git.BinVersion: %v", err)
return fmt.Errorf("Unable to get git version: %v", err)
}

//use merge functions but switch repo's and branches
pr := &models.PullRequest{
HeadRepoID: pull.BaseRepoID,
BaseRepoID: pull.HeadRepoID,
HeadBranch: pull.BaseBranch,
BaseBranch: pull.HeadBranch,
}

if err = pr.LoadHeadRepo(); err != nil {
log.Error("LoadHeadRepo: %v", err)
return fmt.Errorf("LoadHeadRepo: %v", err)
} else if err = pr.LoadBaseRepo(); err != nil {
log.Error("LoadBaseRepo: %v", err)
return fmt.Errorf("LoadBaseRepo: %v", err)
}

// Clone base repo.
tmpBasePath, err := createTemporaryRepo(pr)
if err != nil {
log.Error("CreateTemporaryPath: %v", err)
return err
}
defer func() {
if err := models.RemoveTemporaryPath(tmpBasePath); err != nil {
log.Error("Merge: RemoveTemporaryPath: %s", err)
}
}()

baseBranch := "base"
trackingBranch := "tracking"

var outbuf, errbuf strings.Builder

// Enable sparse-checkout
sparseCheckoutList, err := getDiffTree(tmpBasePath, baseBranch, trackingBranch)
if err != nil {
log.Error("getDiffTree(%s, %s, %s): %v", tmpBasePath, baseBranch, trackingBranch, err)
return fmt.Errorf("getDiffTree: %v", err)
}

infoPath := filepath.Join(tmpBasePath, ".git", "info")
if err := os.MkdirAll(infoPath, 0700); err != nil {
log.Error("Unable to create .git/info in %s: %v", tmpBasePath, err)
return fmt.Errorf("Unable to create .git/info in tmpBasePath: %v", err)
}

sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout")
if err := ioutil.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0600); err != nil {
log.Error("Unable to write .git/info/sparse-checkout file in %s: %v", tmpBasePath, err)
return fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %v", err)
}

var gitConfigCommand func() *git.Command
if version.Compare(binVersion, "1.8.0", ">=") {
gitConfigCommand = func() *git.Command {
return git.NewCommand("config", "--local")
}
} else {
gitConfigCommand = func() *git.Command {
return git.NewCommand("config")
}
}

// Switch off LFS process (set required, clean and smudge here also)
if err := gitConfigCommand().AddArguments("filter.lfs.process", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("git config [filter.lfs.process -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
return fmt.Errorf("git config [filter.lfs.process -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
}
outbuf.Reset()
errbuf.Reset()

if err := gitConfigCommand().AddArguments("filter.lfs.required", "false").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("git config [filter.lfs.required -> <false> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
return fmt.Errorf("git config [filter.lfs.required -> <false> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
}
outbuf.Reset()
errbuf.Reset()

if err := gitConfigCommand().AddArguments("filter.lfs.clean", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("git config [filter.lfs.clean -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
return fmt.Errorf("git config [filter.lfs.clean -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
}
outbuf.Reset()
errbuf.Reset()

if err := gitConfigCommand().AddArguments("filter.lfs.smudge", "").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("git config [filter.lfs.smudge -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
return fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
}
outbuf.Reset()
errbuf.Reset()

if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("git config [core.sparseCheckout -> true ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
return fmt.Errorf("git config [core.sparsecheckout -> true]: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
}
outbuf.Reset()
errbuf.Reset()

// Read base branch index
if err := git.NewCommand("read-tree", "HEAD").RunInDirPipeline(tmpBasePath, &outbuf, &errbuf); err != nil {
log.Error("git read-tree HEAD: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
return fmt.Errorf("Unable to read base branch in to the index: %v\n%s\n%s", err, outbuf.String(), errbuf.String())
}
outbuf.Reset()
errbuf.Reset()

// Determine if we should sign
signArg := ""
if version.Compare(binVersion, "1.7.9", ">=") {
sign, keyID, _ := pr.SignMerge(doer, tmpBasePath, "HEAD", trackingBranch)
if sign {
signArg = "-S" + keyID
} else if version.Compare(binVersion, "2.0.0", ">=") {
signArg = "--no-gpg-sign"
}
}

sig := doer.NewGitSig()
commitTimeStr := time.Now().Format(time.RFC3339)

// Because this may call hooks we should pass in the environment
env := append(os.Environ(),
"GIT_AUTHOR_NAME="+sig.Name,
"GIT_AUTHOR_EMAIL="+sig.Email,
"GIT_AUTHOR_DATE="+commitTimeStr,
"GIT_COMMITTER_NAME="+sig.Name,
"GIT_COMMITTER_EMAIL="+sig.Email,
"GIT_COMMITTER_DATE="+commitTimeStr,
)

// Merge commits.
cmd := git.NewCommand("merge", "--no-ff", "--no-commit", trackingBranch)
if err := runMergeCommand(pr, models.MergeStyleMerge, cmd, tmpBasePath); err != nil {
log.Error("Unable to merge tracking into base: %v", err)
return err
}

if err := commitAndSignNoAuthor(pr, message, signArg, tmpBasePath, env); err != nil {
log.Error("Unable to make final commit: %v", err)
return err
}

// OK we should cache our current head and origin/headbranch
mergeHeadSHA, err := git.GetFullCommitID(tmpBasePath, "HEAD")
if err != nil {
return fmt.Errorf("Failed to get full commit id for HEAD: %v", err)
}
mergeBaseSHA, err := git.GetFullCommitID(tmpBasePath, "original_"+baseBranch)
if err != nil {
return fmt.Errorf("Failed to get full commit id for origin/%s: %v", pr.BaseBranch, err)
}

// Now it's questionable about where this should go - either after or before the push
// I think in the interests of data safety - failures to push to the lfs should prevent
// the merge as you can always remerge.
if setting.LFS.StartServer {
if err := LFSPush(tmpBasePath, mergeHeadSHA, mergeBaseSHA, pr); err != nil {
return err
}
}

var headUser *models.User
err = pr.HeadRepo.GetOwner()
if err != nil {
if !models.IsErrUserNotExist(err) {
log.Error("Can't find user: %d for head repository - %v", pr.HeadRepo.OwnerID, err)
return err
}
log.Error("Can't find user: %d for head repository - defaulting to doer: %s - %v", pr.HeadRepo.OwnerID, doer.Name, err)
headUser = doer
} else {
headUser = pr.HeadRepo.Owner
}

env = models.FullPushingEnvironment(
headUser,
doer,
pr.BaseRepo,
pr.BaseRepo.Name,
pr.ID,
)

// Push back to upstream.
if err := git.NewCommand("push", "origin", baseBranch+":"+pr.BaseBranch).RunInDirTimeoutEnvPipeline(env, -1, tmpBasePath, &outbuf, &errbuf); err != nil {
if strings.Contains(errbuf.String(), "non-fast-forward") {
return models.ErrMergePushOutOfDate{
Style: models.MergeStyleMerge,
StdOut: outbuf.String(),
StdErr: errbuf.String(),
Err: err,
}
}
return fmt.Errorf("git push: %s", errbuf.String())
}
outbuf.Reset()
errbuf.Reset()

//notification.NotifyPullRequestUpdated(pr, doer)
//trigger hooks and co ..

return nil
}

// IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections
func IsUserAllowedToUpdate(pull *models.PullRequest, p models.Permission, user *models.User) (bool, error) {
if p.IsAdmin() {
return true, nil
}
if !p.CanWrite(models.UnitTypeCode) {
return false, nil
}
pr := &models.PullRequest{
HeadRepoID: pull.BaseRepoID,
BaseRepoID: pull.HeadRepoID,
HeadBranch: pull.BaseBranch,
BaseBranch: pull.HeadBranch,
}
err := pr.LoadProtectedBranch()
if err != nil {
return false, err
}

if pr.ProtectedBranch == nil || pr.ProtectedBranch.IsUserMergeWhitelisted(user.ID) {
return true, nil
}

return false, nil
}
11 changes: 11 additions & 0 deletions web_src/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,17 @@ function initRepository() {
$('#comment-form').submit();
});

// Pull Request update button
const $updatePRButton = $('#update-button');
$updatePRButton.on('click', () => {
$.post(`${window.location}/update`, {
_csrf: csrf
}).success(() => {
// eslint-disable-next-line no-restricted-globals
location.reload();
});
});

// Pull Request merge button
const $mergeButton = $('.merge-button > button');
$mergeButton.on('click', function (e) {
Expand Down