diff --git a/models/git/branch.go b/models/git/branch.go index a264f555d9..d1caa35947 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -167,6 +167,9 @@ func GetBranch(ctx context.Context, repoID int64, branchName string) (*Branch, e BranchName: branchName, } } + // FIXME: this design is not right: it doesn't check `branch.IsDeleted`, it doesn't make sense to make callers to check IsDeleted again and again. + // It causes inconsistency with `GetBranches` and `git.GetBranch`, and will lead to strange bugs + // In the future, there should be 2 functions: `GetBranchExisting` and `GetBranchWithDeleted` return &branch, nil } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 41f6c5055d..533eb136f9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1953,7 +1953,7 @@ pulls.upstream_diverging_prompt_behind_1 = This branch is %[1]d commit behind %[ pulls.upstream_diverging_prompt_behind_n = This branch is %[1]d commits behind %[2]s pulls.upstream_diverging_prompt_base_newer = The base branch %s has new changes pulls.upstream_diverging_merge = Sync fork -pulls.upstream_diverging_merge_confirm = Would you like to merge base repository's default branch onto this repository's branch %s? +pulls.upstream_diverging_merge_confirm = Would you like to merge "%[1]s" onto "%[2]s"? pull.deleted_branch = (deleted):%s pull.agit_documentation = Review documentation about AGit diff --git a/services/repository/branch.go b/services/repository/branch.go index 08c53bbb6a..c80d367bbd 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -668,9 +668,12 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitR } // BranchDivergingInfo contains the information about the divergence of a head branch to the base branch. -// This struct is also used in templates, so it needs to search for all references before changing it. type BranchDivergingInfo struct { + // whether the base branch contains new commits which are not in the head branch BaseHasNewCommits bool + + // behind/after are number of commits that the head branch is behind/after the base branch, it's 0 if it's unable to calculate. + // there could be a case that BaseHasNewCommits=true while the behind/after are both 0 (unable to calculate). HeadCommitsBehind int HeadCommitsAhead int } @@ -681,11 +684,20 @@ func GetBranchDivergingInfo(ctx reqctx.RequestContext, baseRepo *repo_model.Repo if err != nil { return nil, err } - + if headGitBranch.IsDeleted { + return nil, git_model.ErrBranchNotExist{ + BranchName: headBranch, + } + } baseGitBranch, err := git_model.GetBranch(ctx, baseRepo.ID, baseBranch) if err != nil { return nil, err } + if baseGitBranch.IsDeleted { + return nil, git_model.ErrBranchNotExist{ + BranchName: baseBranch, + } + } info := &BranchDivergingInfo{} if headGitBranch.CommitID == baseGitBranch.CommitID { @@ -720,5 +732,6 @@ func GetBranchDivergingInfo(ctx reqctx.RequestContext, baseRepo *repo_model.Repo } info.HeadCommitsBehind, info.HeadCommitsAhead = diff.Behind, diff.Ahead + info.BaseHasNewCommits = info.HeadCommitsBehind > 0 return info, nil } diff --git a/services/repository/merge_upstream.go b/services/repository/merge_upstream.go index 008e7de385..34e01df723 100644 --- a/services/repository/merge_upstream.go +++ b/services/repository/merge_upstream.go @@ -4,7 +4,7 @@ package repository import ( - "context" + "errors" "fmt" issue_model "code.gitea.io/gitea/models/issues" @@ -18,16 +18,24 @@ import ( ) // MergeUpstream merges the base repository's default branch into the fork repository's current branch. -func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, branch string) (mergeStyle string, err error) { +func MergeUpstream(ctx reqctx.RequestContext, doer *user_model.User, repo *repo_model.Repository, branch string) (mergeStyle string, err error) { if err = repo.MustNotBeArchived(); err != nil { return "", err } if err = repo.GetBaseRepo(ctx); err != nil { return "", err } + divergingInfo, err := GetUpstreamDivergingInfo(ctx, repo, branch) + if err != nil { + return "", err + } + if !divergingInfo.BaseBranchHasNewCommits { + return "up-to-date", nil + } + err = git.Push(ctx, repo.BaseRepo.RepoPath(), git.PushOptions{ Remote: repo.RepoPath(), - Branch: fmt.Sprintf("%s:%s", repo.BaseRepo.DefaultBranch, branch), + Branch: fmt.Sprintf("%s:%s", divergingInfo.BaseBranchName, branch), Env: repo_module.PushingEnvironment(doer, repo), }) if err == nil { @@ -59,7 +67,7 @@ func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model. BaseRepoID: repo.BaseRepo.ID, BaseRepo: repo.BaseRepo, HeadBranch: branch, // maybe HeadCommitID is not needed - BaseBranch: repo.BaseRepo.DefaultBranch, + BaseBranch: divergingInfo.BaseBranchName, } fakeIssue.PullRequest = fakePR err = pull.Update(ctx, fakePR, doer, "merge upstream", false) @@ -69,8 +77,15 @@ func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model. return "merge", nil } +// UpstreamDivergingInfo is also used in templates, so it needs to search for all references before changing it. +type UpstreamDivergingInfo struct { + BaseBranchName string + BaseBranchHasNewCommits bool + HeadBranchCommitsBehind int +} + // GetUpstreamDivergingInfo returns the information about the divergence between the fork repository's branch and the base repository's default branch. -func GetUpstreamDivergingInfo(ctx reqctx.RequestContext, forkRepo *repo_model.Repository, forkBranch string) (*BranchDivergingInfo, error) { +func GetUpstreamDivergingInfo(ctx reqctx.RequestContext, forkRepo *repo_model.Repository, forkBranch string) (*UpstreamDivergingInfo, error) { if !forkRepo.IsFork { return nil, util.NewInvalidArgumentErrorf("repo is not a fork") } @@ -83,5 +98,26 @@ func GetUpstreamDivergingInfo(ctx reqctx.RequestContext, forkRepo *repo_model.Re return nil, err } - return GetBranchDivergingInfo(ctx, forkRepo.BaseRepo, forkRepo.BaseRepo.DefaultBranch, forkRepo, forkBranch) + // Do the best to follow the GitHub's behavior, suppose there is a `branch-a` in fork repo: + // * if `branch-a` exists in base repo: try to sync `base:branch-a` to `fork:branch-a` + // * if `branch-a` doesn't exist in base repo: try to sync `base:main` to `fork:branch-a` + info, err := GetBranchDivergingInfo(ctx, forkRepo.BaseRepo, forkBranch, forkRepo, forkBranch) + if err == nil { + return &UpstreamDivergingInfo{ + BaseBranchName: forkBranch, + BaseBranchHasNewCommits: info.BaseHasNewCommits, + HeadBranchCommitsBehind: info.HeadCommitsBehind, + }, nil + } + if errors.Is(err, util.ErrNotExist) { + info, err = GetBranchDivergingInfo(ctx, forkRepo.BaseRepo, forkRepo.BaseRepo.DefaultBranch, forkRepo, forkBranch) + if err == nil { + return &UpstreamDivergingInfo{ + BaseBranchName: forkRepo.BaseRepo.DefaultBranch, + BaseBranchHasNewCommits: info.BaseHasNewCommits, + HeadBranchCommitsBehind: info.HeadCommitsBehind, + }, nil + } + } + return nil, err } diff --git a/templates/repo/code/upstream_diverging_info.tmpl b/templates/repo/code/upstream_diverging_info.tmpl index 8d6e55959f..b3d35c05e5 100644 --- a/templates/repo/code/upstream_diverging_info.tmpl +++ b/templates/repo/code/upstream_diverging_info.tmpl @@ -1,10 +1,12 @@ -{{if and .UpstreamDivergingInfo (or .UpstreamDivergingInfo.BaseHasNewCommits .UpstreamDivergingInfo.HeadCommitsBehind)}} +{{if and .UpstreamDivergingInfo .UpstreamDivergingInfo.BaseBranchHasNewCommits}}
- {{$upstreamLink := printf "%s/src/branch/%s" .Repository.BaseRepo.Link (.Repository.BaseRepo.DefaultBranch|PathEscapeSegments)}} - {{$upstreamHtml := HTMLFormat `%s:%s` $upstreamLink .Repository.BaseRepo.FullName .Repository.BaseRepo.DefaultBranch}} - {{if .UpstreamDivergingInfo.HeadCommitsBehind}} - {{ctx.Locale.TrN .UpstreamDivergingInfo.HeadCommitsBehind "repo.pulls.upstream_diverging_prompt_behind_1" "repo.pulls.upstream_diverging_prompt_behind_n" .UpstreamDivergingInfo.HeadCommitsBehind $upstreamHtml}} + {{$upstreamLink := printf "%s/src/branch/%s" .Repository.BaseRepo.Link (.UpstreamDivergingInfo.BaseBranchName|PathEscapeSegments)}} + {{$upstreamRepoBranchDisplay := HTMLFormat "%s:%s" .Repository.BaseRepo.FullName .UpstreamDivergingInfo.BaseBranchName}} + {{$thisRepoBranchDisplay := HTMLFormat "%s:%s" .Repository.FullName .BranchName}} + {{$upstreamHtml := HTMLFormat `%s` $upstreamLink $upstreamRepoBranchDisplay}} + {{if .UpstreamDivergingInfo.HeadBranchCommitsBehind}} + {{ctx.Locale.TrN .UpstreamDivergingInfo.HeadBranchCommitsBehind "repo.pulls.upstream_diverging_prompt_behind_1" "repo.pulls.upstream_diverging_prompt_behind_n" .UpstreamDivergingInfo.HeadBranchCommitsBehind $upstreamHtml}} {{else}} {{ctx.Locale.Tr "repo.pulls.upstream_diverging_prompt_base_newer" $upstreamHtml}} {{end}} @@ -12,7 +14,7 @@ {{if .CanWriteCode}} diff --git a/tests/integration/repo_merge_upstream_test.go b/tests/integration/repo_merge_upstream_test.go index e3e423c51d..e928b04e9b 100644 --- a/tests/integration/repo_merge_upstream_test.go +++ b/tests/integration/repo_merge_upstream_test.go @@ -60,25 +60,54 @@ func TestRepoMergeUpstream(t *testing.T) { t.Run("HeadBeforeBase", func(t *testing.T) { // add a file in base repo + sessionBaseUser := loginUser(t, baseUser.Name) require.NoError(t, createOrReplaceFileInBranch(baseUser, baseRepo, "new-file.txt", "master", "test-content-1")) - // the repo shows a prompt to "sync fork" var mergeUpstreamLink string - require.Eventually(t, func() bool { - resp := session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/fork-branch", forkUser.Name), http.StatusOK) - htmlDoc := NewHTMLParser(t, resp.Body) - mergeUpstreamLink = queryMergeUpstreamButtonLink(htmlDoc) - if mergeUpstreamLink == "" { - return false - } - respMsg, _ := htmlDoc.Find(".ui.message:not(.positive)").Html() - return strings.Contains(respMsg, `This branch is 1 commit behind user2/repo1:master`) - }, 5*time.Second, 100*time.Millisecond) + t.Run("DetectDefaultBranch", func(t *testing.T) { + // the repo shows a prompt to "sync fork" (defaults to the default branch) + require.Eventually(t, func() bool { + resp := session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/fork-branch", forkUser.Name), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + mergeUpstreamLink = queryMergeUpstreamButtonLink(htmlDoc) + if mergeUpstreamLink == "" { + return false + } + respMsg, _ := htmlDoc.Find(".ui.message:not(.positive)").Html() + return strings.Contains(respMsg, `This branch is 1 commit behind user2/repo1:master`) + }, 5*time.Second, 100*time.Millisecond) + }) + + t.Run("DetectSameBranch", func(t *testing.T) { + // if the fork-branch name also exists in the base repo, then use that branch instead + req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/_new/branch/master", map[string]string{ + "_csrf": GetUserCSRFToken(t, sessionBaseUser), + "new_branch_name": "fork-branch", + }) + sessionBaseUser.MakeRequest(t, req, http.StatusSeeOther) + + require.Eventually(t, func() bool { + resp := session.MakeRequest(t, NewRequestf(t, "GET", "/%s/test-repo-fork/src/branch/fork-branch", forkUser.Name), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + mergeUpstreamLink = queryMergeUpstreamButtonLink(htmlDoc) + if mergeUpstreamLink == "" { + return false + } + respMsg, _ := htmlDoc.Find(".ui.message:not(.positive)").Html() + return strings.Contains(respMsg, `This branch is 1 commit behind user2/repo1:fork-branch`) + }, 5*time.Second, 100*time.Millisecond) + }) // click the "sync fork" button req = NewRequestWithValues(t, "POST", mergeUpstreamLink, map[string]string{"_csrf": GetUserCSRFToken(t, session)}) session.MakeRequest(t, req, http.StatusOK) checkFileContent("fork-branch", "test-content-1") + + // delete the "fork-branch" from the base repo + req = NewRequestWithValues(t, "POST", "/user2/repo1/branches/delete?name=fork-branch", map[string]string{ + "_csrf": GetUserCSRFToken(t, sessionBaseUser), + }) + sessionBaseUser.MakeRequest(t, req, http.StatusOK) }) t.Run("BaseChangeAfterHeadChange", func(t *testing.T) {