Mirrors/gitea
0
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2025-10-31 07:21:36 +01:00
Code Issues Activity

Improve sync fork behavior (#33319) (#33332)

Browse Source 
Backport #33319
Fix #33271

The only conflict is `reqctx` in
`services/repository/merge_upstream.go`, which could keep using
`context.Context` in 1.23
This commit is contained in:
wxiaoguang 2025-01-20 15:50:38 +08:00 committed by GitHub
parent e72d001708
commit 8f45a11919
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 108 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
models/git/branch.go
View File

@ -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
}

2
options/locale/locale_en-US.ini
View File

@ -1951,7 +1951,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

17
services/repository/branch.go
View File

@ -638,9 +638,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
}
@ -651,11 +654,20 @@ func GetBranchDivergingInfo(ctx context.Context, baseRepo *repo_model.Repository
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 {
@ -692,5 +704,6 @@ func GetBranchDivergingInfo(ctx context.Context, baseRepo *repo_model.Repository
}
info.HeadCommitsBehind, info.HeadCommitsAhead = diff.Behind, diff.Ahead
info.BaseHasNewCommits = info.HeadCommitsBehind > 0
return info, nil
}

45
services/repository/merge_upstream.go
View File

@ -5,6 +5,7 @@ package repository
import (
"context"
"errors"
"fmt"
issue_model "code.gitea.io/gitea/models/issues"
@ -24,9 +25,17 @@ func MergeUpstream(ctx context.Context, doer *user_model.User, repo *repo_model.
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 {
@ -58,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)
@ -68,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 context.Context, forkRepo *repo_model.Repository, forkBranch string) (*BranchDivergingInfo, error) {
func GetUpstreamDivergingInfo(ctx context.Context, forkRepo *repo_model.Repository, forkBranch string) (*UpstreamDivergingInfo, error) {
if !forkRepo.IsFork {
return nil, util.NewInvalidArgumentErrorf("repo is not a fork")
}
@ -82,5 +98,26 @@ func GetUpstreamDivergingInfo(ctx context.Context, forkRepo *repo_model.Reposito
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
}

14
templates/repo/code/upstream_diverging_info.tmpl
View File

@ -1,10 +1,12 @@
{{if and .UpstreamDivergingInfo (or .UpstreamDivergingInfo.BaseHasNewCommits .UpstreamDivergingInfo.HeadCommitsBehind)}}
{{if and .UpstreamDivergingInfo .UpstreamDivergingInfo.BaseBranchHasNewCommits}}
<div class="ui message flex-text-block">
<div class="tw-flex-1">
{{$upstreamLink := printf "%s/src/branch/%s" .Repository.BaseRepo.Link (.Repository.BaseRepo.DefaultBranch|PathEscapeSegments)}}
{{$upstreamHtml := HTMLFormat `<a href="%s">%s:%s</a>` $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 `<a href="%s">%s</a>` $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}}
<button class="ui compact primary button tw-m-0 link-action"
data-modal-confirm-header="{{ctx.Locale.Tr "repo.pulls.upstream_diverging_merge"}}"
data-modal-confirm-content="{{ctx.Locale.Tr "repo.pulls.upstream_diverging_merge_confirm" .BranchName}}"
data-modal-confirm-content="{{ctx.Locale.Tr "repo.pulls.upstream_diverging_merge_confirm" $upstreamRepoBranchDisplay $thisRepoBranchDisplay}}"
data-url="{{.Repository.Link}}/branches/merge-upstream?branch={{.BranchName}}">
{{ctx.Locale.Tr "repo.pulls.upstream_diverging_merge"}}
</button>

51
tests/integration/repo_merge_upstream_test.go
View File

@ -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 <a href="/user2/repo1/src/branch/master">user2/repo1:master</a>`)
}, 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 <a href="/user2/repo1/src/branch/master">user2/repo1:master</a>`)
}, 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 <a href="/user2/repo1/src/branch/fork-branch">user2/repo1:fork-branch</a>`)
}, 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) {