From 94e02140fb815cf9268d29db6bec2420129a1549 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 22 Mar 2025 17:37:00 -0700 Subject: [PATCH 01/12] Remove repo field in branch struct of git module --- models/git/branch.go | 12 ++++++++++++ modules/git/repo_branch.go | 22 ++++++---------------- routers/api/v1/repo/branch.go | 27 ++++++++++----------------- services/convert/pull.go | 13 +++---------- 4 files changed, 31 insertions(+), 43 deletions(-) diff --git a/models/git/branch.go b/models/git/branch.go index d1caa35947b40..9ac6c45578f7c 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -173,6 +173,18 @@ func GetBranch(ctx context.Context, repoID int64, branchName string) (*Branch, e return &branch, nil } +// IsBranchExist returns true if the branch exists in the repository. +func IsBranchExist(ctx context.Context, repoID int64, branchName string) (bool, error) { + var branch Branch + has, err := db.GetEngine(ctx).Where("repo_id=?", repoID).And("name=?", branchName).Get(&branch) + if err != nil { + return false, err + } else if !has { + return false, nil + } + return !branch.IsDeleted, nil +} + func GetBranches(ctx context.Context, repoID int64, branchNames []string, includeDeleted bool) ([]*Branch, error) { branches := make([]*Branch, 0, len(branchNames)) diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index 916391f1677b6..d958e1a34acd3 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -29,8 +29,6 @@ func IsBranchExist(ctx context.Context, repoPath, name string) bool { type Branch struct { Name string Path string - - gitRepo *Repository } // GetHEADBranch returns corresponding branch of HEAD. @@ -49,9 +47,8 @@ func (repo *Repository) GetHEADBranch() (*Branch, error) { } return &Branch{ - Name: stdout[len(BranchPrefix):], - Path: stdout, - gitRepo: repo, + Name: stdout[len(BranchPrefix):], + Path: stdout, }, nil } @@ -73,9 +70,8 @@ func (repo *Repository) GetBranch(branch string) (*Branch, error) { return nil, ErrBranchNotExist{branch} } return &Branch{ - Path: repo.Path, - Name: branch, - gitRepo: repo, + Path: repo.Path, + Name: branch, }, nil } @@ -89,9 +85,8 @@ func (repo *Repository) GetBranches(skip, limit int) ([]*Branch, int, error) { branches := make([]*Branch, len(brs)) for i := range brs { branches[i] = &Branch{ - Path: repo.Path, - Name: brs[i], - gitRepo: repo, + Path: repo.Path, + Name: brs[i], } } @@ -147,11 +142,6 @@ func (repo *Repository) RemoveRemote(name string) error { return err } -// GetCommit returns the head commit of a branch -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").AddDynamicArguments(from, to).RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) diff --git a/routers/api/v1/repo/branch.go b/routers/api/v1/repo/branch.go index 9c6e572fb4e93..2e033a3919c7b 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -60,17 +60,16 @@ func GetBranch(ctx *context.APIContext) { branchName := ctx.PathParam("*") - branch, err := ctx.Repo.GitRepo.GetBranch(branchName) + exist, err := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, branchName) if err != nil { - if git.IsErrBranchNotExist(err) { - ctx.APIErrorNotFound(err) - } else { - ctx.APIErrorInternal(err) - } + ctx.APIErrorInternal(err) + return + } else if !exist { + ctx.APIErrorNotFound(err) return } - c, err := branch.GetCommit() + c, err := ctx.Repo.GitRepo.GetBranchCommit(branchName) if err != nil { ctx.APIErrorInternal(err) return @@ -82,7 +81,7 @@ func GetBranch(ctx *context.APIContext) { return } - br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branch.Name, c, branchProtection, ctx.Doer, ctx.Repo.IsAdmin()) + br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branchName, c, branchProtection, ctx.Doer, ctx.Repo.IsAdmin()) if err != nil { ctx.APIErrorInternal(err) return @@ -261,25 +260,19 @@ func CreateBranch(ctx *context.APIContext) { return } - branch, err := ctx.Repo.GitRepo.GetBranch(opt.BranchName) - if err != nil { - ctx.APIErrorInternal(err) - return - } - - commit, err := branch.GetCommit() + commit, err := ctx.Repo.GitRepo.GetBranchCommit(opt.BranchName) if err != nil { ctx.APIErrorInternal(err) return } - branchProtection, err := git_model.GetFirstMatchProtectedBranchRule(ctx, ctx.Repo.Repository.ID, branch.Name) + branchProtection, err := git_model.GetFirstMatchProtectedBranchRule(ctx, ctx.Repo.Repository.ID, opt.BranchName) if err != nil { ctx.APIErrorInternal(err) return } - br, err := convert.ToBranch(ctx, ctx.Repo.Repository, branch.Name, commit, branchProtection, ctx.Doer, ctx.Repo.IsAdmin()) + br, err := convert.ToBranch(ctx, ctx.Repo.Repository, opt.BranchName, commit, branchProtection, ctx.Doer, ctx.Repo.IsAdmin()) if err != nil { ctx.APIErrorInternal(err) return diff --git a/services/convert/pull.go b/services/convert/pull.go index 928534ce5e6b8..c2714b644c006 100644 --- a/services/convert/pull.go +++ b/services/convert/pull.go @@ -157,7 +157,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } if err == nil { - baseCommit, err = baseBranch.GetCommit() + baseCommit, err = gitRepo.GetBranchCommit(pr.BaseBranch) if err != nil && !git.IsErrNotExist(err) { log.Error("GetCommit[%s]: %v", baseBranch.Name, err) return nil @@ -169,13 +169,6 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } if pr.Flow == issues_model.PullRequestFlowAGit { - gitRepo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo) - if err != nil { - log.Error("OpenRepository[%s]: %v", pr.GetGitRefName(), err) - return nil - } - defer gitRepo.Close() - apiPullRequest.Head.Sha, err = gitRepo.GetRefCommitID(pr.GetGitRefName()) if err != nil { log.Error("GetRefCommitID[%s]: %v", pr.GetGitRefName(), err) @@ -226,7 +219,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u endCommitID = headCommitID } } else { - commit, err := headBranch.GetCommit() + commit, err := headGitRepo.GetBranchCommit(pr.HeadBranch) if err != nil && !git.IsErrNotExist(err) { log.Error("GetCommit[%s]: %v", headBranch.Name, err) return nil @@ -478,7 +471,7 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs apiPullRequest.Head.Sha = headCommitID } } else { - commit, err := headBranch.GetCommit() + commit, err := headGitRepo.GetBranchCommit(pr.HeadBranch) if err != nil && !git.IsErrNotExist(err) { log.Error("GetCommit[%s]: %v", headBranch.Name, err) return nil, err From c46d935034c7faf28cfea478eeb87a38b7647edc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 22 Mar 2025 17:47:46 -0700 Subject: [PATCH 02/12] Use a string represent a branch --- modules/git/repo_branch.go | 46 +++++------------------------ modules/gitrepo/branch.go | 4 +-- routers/api/v1/repo/commits.go | 2 +- routers/web/repo/view_home.go | 2 +- services/context/repo.go | 4 +-- services/convert/pull.go | 10 +++---- services/mirror/mirror_pull.go | 2 +- services/pull/pull.go | 2 +- services/repository/files/patch.go | 2 +- services/repository/files/update.go | 2 +- services/repository/migrate.go | 4 +-- 11 files changed, 25 insertions(+), 55 deletions(-) diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index d958e1a34acd3..ae8a818d57bcb 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -25,31 +25,22 @@ func IsBranchExist(ctx context.Context, repoPath, name string) bool { return IsReferenceExist(ctx, repoPath, BranchPrefix+name) } -// Branch represents a Git branch. -type Branch struct { - Name string - Path string -} - // GetHEADBranch returns corresponding branch of HEAD. -func (repo *Repository) GetHEADBranch() (*Branch, error) { +func (repo *Repository) GetHEADBranch() (string, error) { if repo == nil { - return nil, fmt.Errorf("nil repo") + return "", fmt.Errorf("nil repo") } stdout, _, err := NewCommand("symbolic-ref", "HEAD").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) if err != nil { - return nil, err + return "", err } stdout = strings.TrimSpace(stdout) if !strings.HasPrefix(stdout, BranchPrefix) { - return nil, fmt.Errorf("invalid HEAD branch: %v", stdout) + return "", fmt.Errorf("invalid HEAD branch: %v", stdout) } - return &Branch{ - Name: stdout[len(BranchPrefix):], - Path: stdout, - }, nil + return stdout[len(BranchPrefix):], nil } func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) { @@ -65,32 +56,11 @@ func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) { } // GetBranch returns a branch by it's name -func (repo *Repository) GetBranch(branch string) (*Branch, error) { +func (repo *Repository) GetBranch(branch string) (string, error) { if !repo.IsBranchExist(branch) { - return nil, ErrBranchNotExist{branch} - } - return &Branch{ - Path: repo.Path, - Name: branch, - }, nil -} - -// GetBranches returns a slice of *git.Branch -func (repo *Repository) GetBranches(skip, limit int) ([]*Branch, int, error) { - brs, countAll, err := repo.GetBranchNames(skip, limit) - if err != nil { - return nil, 0, err + return "", ErrBranchNotExist{branch} } - - branches := make([]*Branch, len(brs)) - for i := range brs { - branches[i] = &Branch{ - Path: repo.Path, - Name: brs[i], - } - } - - return branches, countAll, nil + return branch, nil } // DeleteBranchOptions Option(s) for delete branch diff --git a/modules/gitrepo/branch.go b/modules/gitrepo/branch.go index 25ea5abfca1bd..d7857819e496b 100644 --- a/modules/gitrepo/branch.go +++ b/modules/gitrepo/branch.go @@ -11,14 +11,14 @@ import ( // GetBranchesByPath returns a branch by its path // if limit = 0 it will not limit -func GetBranchesByPath(ctx context.Context, repo Repository, skip, limit int) ([]*git.Branch, int, error) { +func GetBranchesByPath(ctx context.Context, repo Repository, skip, limit int) ([]string, int, error) { gitRepo, err := OpenRepository(ctx, repo) if err != nil { return nil, 0, err } defer gitRepo.Close() - return gitRepo.GetBranches(skip, limit) + return gitRepo.GetBranchNames(skip, limit) } func GetBranchCommitID(ctx context.Context, repo Repository, branch string) (string, error) { diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 03489d777b0d8..696902cd46d37 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -186,7 +186,7 @@ func GetAllCommits(ctx *context.APIContext) { return } - baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(head.Name) + baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(head) if err != nil { ctx.APIErrorInternal(err) return diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index d538406035c7d..47f5d03b9a169 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -269,7 +269,7 @@ func handleRepoEmptyOrBroken(ctx *context.Context) { } else if reallyEmpty { showEmpty = true // the repo is really empty updateContextRepoEmptyAndStatus(ctx, true, repo_model.RepositoryReady) - } else if branches, _, _ := ctx.Repo.GitRepo.GetBranches(0, 1); len(branches) == 0 { + } else if branches, _, _ := ctx.Repo.GitRepo.GetBranchNames(0, 1); len(branches) == 0 { showEmpty = true // it is not really empty, but there is no branch // at the moment, other repo units like "actions" are not able to handle such case, // so we just mark the repo as empty to prevent from displaying these units. diff --git a/services/context/repo.go b/services/context/repo.go index 6eccd1312a971..c52fc79b9de3e 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -815,9 +815,9 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) { if reqPath == "" { refShortName = ctx.Repo.Repository.DefaultBranch if !gitrepo.IsBranchExist(ctx, ctx.Repo.Repository, refShortName) { - brs, _, err := ctx.Repo.GitRepo.GetBranches(0, 1) + brs, _, err := ctx.Repo.GitRepo.GetBranchNames(0, 1) if err == nil && len(brs) != 0 { - refShortName = brs[0].Name + refShortName = brs[0] } else if len(brs) == 0 { log.Error("No branches in non-empty repository %s", ctx.Repo.GitRepo.Path) } else { diff --git a/services/convert/pull.go b/services/convert/pull.go index c2714b644c006..f0e2355c09956 100644 --- a/services/convert/pull.go +++ b/services/convert/pull.go @@ -28,8 +28,8 @@ import ( // Optional - Merger func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User) *api.PullRequest { var ( - baseBranch *git.Branch - headBranch *git.Branch + baseBranch string + headBranch string baseCommit *git.Commit err error ) @@ -159,7 +159,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u if err == nil { baseCommit, err = gitRepo.GetBranchCommit(pr.BaseBranch) if err != nil && !git.IsErrNotExist(err) { - log.Error("GetCommit[%s]: %v", baseBranch.Name, err) + log.Error("GetCommit[%s]: %v", baseBranch, err) return nil } @@ -221,7 +221,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } else { commit, err := headGitRepo.GetBranchCommit(pr.HeadBranch) if err != nil && !git.IsErrNotExist(err) { - log.Error("GetCommit[%s]: %v", headBranch.Name, err) + log.Error("GetCommit[%s]: %v", headBranch, err) return nil } if err == nil { @@ -473,7 +473,7 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs } else { commit, err := headGitRepo.GetBranchCommit(pr.HeadBranch) if err != nil && !git.IsErrNotExist(err) { - log.Error("GetCommit[%s]: %v", headBranch.Name, err) + log.Error("GetCommit[%s]: %v", headBranch, err) return nil, err } if err == nil { diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 658747e7c83f1..d673248e94e00 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -419,7 +419,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo } for _, branch := range branches { - cache.Remove(m.Repo.GetCommitsCountCacheKey(branch.Name, true)) + cache.Remove(m.Repo.GetCommitsCountCacheKey(branch, true)) } m.UpdatedUnix = timeutil.TimeStampNow() diff --git a/services/pull/pull.go b/services/pull/pull.go index 4641d4ac40491..13cbb401104af 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -763,7 +763,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re var errs []error for _, branch := range branches { - prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch.Name) + prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(ctx, repo.ID, branch) if err != nil { return err } diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index 1941adb86ac37..7ac1651d0d9a0 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -77,7 +77,7 @@ func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_mode // If we aren't branching to a new branch, make sure user can commit to the given branch if opts.NewBranch != opts.OldBranch { existingBranch, err := gitRepo.GetBranch(opts.NewBranch) - if existingBranch != nil { + if existingBranch != "" { return git_model.ErrBranchAlreadyExists{ BranchName: opts.NewBranch, } diff --git a/services/repository/files/update.go b/services/repository/files/update.go index cade7ba2bf7db..87ab19aaf2b5c 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -146,7 +146,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use // If we aren't branching to a new branch, make sure user can commit to the given branch if opts.NewBranch != opts.OldBranch { existingBranch, err := gitRepo.GetBranch(opts.NewBranch) - if existingBranch != nil { + if existingBranch != "" { return nil, git_model.ErrBranchAlreadyExists{ BranchName: opts.NewBranch, } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 5b6feccb8d879..db29e9fdd798c 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -145,8 +145,8 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, if err != nil { return repo, fmt.Errorf("GetHEADBranch: %w", err) } - if headBranch != nil { - repo.DefaultBranch = headBranch.Name + if headBranch != "" { + repo.DefaultBranch = headBranch } } From 3c3a91714c624d0448469a376b39a3df25820bee Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 22 Mar 2025 18:24:23 -0700 Subject: [PATCH 03/12] Fix test --- models/fixtures/branch.yml | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 17b1869ab6364..a7a2e44faf28b 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -93,3 +93,51 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 16 + repo_id: 16 + name: 'master' + commit_id: '69554a64c1e6030f051e5c3f94bfbd773cd6a324' + commit_message: 'not signed commit' + commit_time: 1502042309 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 + +- + id: 17 + repo_id: 16 + name: 'not-signed' + commit_id: '69554a64c1e6030f051e5c3f94bfbd773cd6a324' + commit_message: 'not signed commit' + commit_time: 1502042309 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 + +- + id: 18 + repo_id: 16 + name: 'good-sign-not-yet-validated' + commit_id: '27566bd5738fc8b4e3fef3c5e72cce608537bd95' + commit_message: 'good signed commit (with not yet validated email)' + commit_time: 1502042234 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 + +- + id: 19 + repo_id: 16 + name: 'good-sign' + commit_id: 'f27c2b2b03dcab38beaf89b0ab4ff61f6de63441' + commit_message: 'good signed commit' + commit_time: 1502042101 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 From ce6e0cf85222b395299326a1bbafdbd6999b8022 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Mar 2025 00:16:30 -0700 Subject: [PATCH 04/12] Add missing fixture --- models/fixtures/branch.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index a7a2e44faf28b..c60b088af3f4f 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -141,3 +141,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 20 + repo_id: 1 + name: 'feature/1' + commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d' + commit_message: 'Initial commit' + commit_time: 1489950479 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 From f3f185760798454f31250a62c2dcda7415e390fd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Mar 2025 09:58:43 -0700 Subject: [PATCH 05/12] Fix test --- tests/integration/api_branch_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index d64dd97f93f0f..4372d8394ec25 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -303,7 +303,7 @@ func TestAPICreateBranchWithSyncBranches(t *testing.T) { RepoID: 1, }) assert.NoError(t, err) - assert.Len(t, branches, 4) + assert.Len(t, branches, 5) // make a broke repository with no branch on database _, err = db.DeleteByBean(db.DefaultContext, git_model.Branch{RepoID: 1}) @@ -320,7 +320,7 @@ func TestAPICreateBranchWithSyncBranches(t *testing.T) { RepoID: 1, }) assert.NoError(t, err) - assert.Len(t, branches, 5) + assert.Len(t, branches, 6) branches, err = db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{ RepoID: 1, From f5da7ed74280a6f0664f901ae3731a11af52c299 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Mar 2025 14:19:36 -0700 Subject: [PATCH 06/12] Remvoe unnecessary GetBranch method --- modules/git/repo_branch.go | 12 ++---------- routers/api/v1/repo/commits.go | 4 ++-- routers/web/repo/editor.go | 9 ++++----- services/convert/pull.go | 20 ++++++++++---------- services/repository/files/patch.go | 22 +++++++++------------- services/repository/files/update.go | 16 ++++++++++------ services/repository/migrate.go | 6 +++--- 7 files changed, 40 insertions(+), 49 deletions(-) diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index ae8a818d57bcb..8e7a908e8ce2a 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -25,8 +25,8 @@ func IsBranchExist(ctx context.Context, repoPath, name string) bool { return IsReferenceExist(ctx, repoPath, BranchPrefix+name) } -// GetHEADBranch returns corresponding branch of HEAD. -func (repo *Repository) GetHEADBranch() (string, error) { +// GetHEADBranchName returns corresponding branch of HEAD. +func (repo *Repository) GetHEADBranchName() (string, error) { if repo == nil { return "", fmt.Errorf("nil repo") } @@ -55,14 +55,6 @@ func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) { return strings.TrimPrefix(stdout, BranchPrefix), nil } -// GetBranch returns a branch by it's name -func (repo *Repository) GetBranch(branch string) (string, error) { - if !repo.IsBranchExist(branch) { - return "", ErrBranchNotExist{branch} - } - return branch, nil -} - // DeleteBranchOptions Option(s) for delete branch type DeleteBranchOptions struct { Force bool diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 696902cd46d37..e6b31c61d4491 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -180,13 +180,13 @@ func GetAllCommits(ctx *context.APIContext) { var baseCommit *git.Commit if len(sha) == 0 { // no sha supplied - use default branch - head, err := ctx.Repo.GitRepo.GetHEADBranch() + headBranchName, err := ctx.Repo.GitRepo.GetHEADBranchName() if err != nil { ctx.APIErrorInternal(err) return } - baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(head) + baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(headBranchName) if err != nil { ctx.APIErrorInternal(err) return diff --git a/routers/web/repo/editor.go b/routers/web/repo/editor.go index 113622f87293c..c181aad050f4e 100644 --- a/routers/web/repo/editor.go +++ b/routers/web/repo/editor.go @@ -668,7 +668,7 @@ func UploadFilePost(ctx *context.Context) { } if oldBranchName != branchName { - if _, err := ctx.Repo.GitRepo.GetBranch(branchName); err == nil { + if exist, err := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, branchName); err == nil && exist { ctx.Data["Err_NewBranchName"] = true ctx.RenderWithErr(ctx.Tr("repo.editor.branch_already_exists", branchName), tplUploadFile, &form) return @@ -875,12 +875,11 @@ func GetUniquePatchBranchName(ctx *context.Context) string { prefix := ctx.Doer.LowerName + "-patch-" for i := 1; i <= 1000; i++ { branchName := fmt.Sprintf("%s%d", prefix, i) - if _, err := ctx.Repo.GitRepo.GetBranch(branchName); err != nil { - if git.IsErrBranchNotExist(err) { - return branchName - } + if exist, err := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, branchName); err != nil { log.Error("GetUniquePatchBranchName: %v", err) return "" + } else if !exist { + return branchName } } return "" diff --git a/services/convert/pull.go b/services/convert/pull.go index f0e2355c09956..f469e009995eb 100644 --- a/services/convert/pull.go +++ b/services/convert/pull.go @@ -150,13 +150,13 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } defer gitRepo.Close() - baseBranch, err = gitRepo.GetBranch(pr.BaseBranch) - if err != nil && !git.IsErrBranchNotExist(err) { + exist, err := git_model.IsBranchExist(ctx, pr.BaseRepoID, pr.BaseBranch) + if err != nil { log.Error("GetBranch[%s]: %v", pr.BaseBranch, err) return nil } - if err == nil { + if exist { baseCommit, err = gitRepo.GetBranchCommit(pr.BaseBranch) if err != nil && !git.IsErrNotExist(err) { log.Error("GetCommit[%s]: %v", baseBranch, err) @@ -196,8 +196,8 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u } defer headGitRepo.Close() - headBranch, err = headGitRepo.GetBranch(pr.HeadBranch) - if err != nil && !git.IsErrBranchNotExist(err) { + exist, err = git_model.IsBranchExist(ctx, pr.HeadRepoID, pr.HeadBranch) + if err != nil { log.Error("GetBranch[%s]: %v", pr.HeadBranch, err) return nil } @@ -208,7 +208,7 @@ func ToAPIPullRequest(ctx context.Context, pr *issues_model.PullRequest, doer *u endCommitID string ) - if git.IsErrBranchNotExist(err) { + if !exist { headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref) if err != nil && !git.IsErrNotExist(err) { log.Error("GetCommit[%s]: %v", pr.HeadBranch, err) @@ -455,13 +455,13 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs defer headGitRepo.Close() } - headBranch, err := headGitRepo.GetBranch(pr.HeadBranch) - if err != nil && !git.IsErrBranchNotExist(err) { + exist, err := git_model.IsBranchExist(ctx, pr.HeadRepoID, pr.HeadBranch) + if err != nil { log.Error("GetBranch[%s]: %v", pr.HeadBranch, err) return nil, err } - if git.IsErrBranchNotExist(err) { + if !exist { headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref) if err != nil && !git.IsErrNotExist(err) { log.Error("GetCommit[%s]: %v", pr.HeadBranch, err) @@ -473,7 +473,7 @@ func ToAPIPullRequests(ctx context.Context, baseRepo *repo_model.Repository, prs } else { commit, err := headGitRepo.GetBranchCommit(pr.HeadBranch) if err != nil && !git.IsErrNotExist(err) { - log.Error("GetCommit[%s]: %v", headBranch, err) + log.Error("GetCommit[%s]: %v", pr.HeadBranch, err) return nil, err } if err == nil { diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go index 7ac1651d0d9a0..5fbf748206788 100644 --- a/services/repository/files/patch.go +++ b/services/repository/files/patch.go @@ -12,7 +12,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -62,29 +61,26 @@ func (opts *ApplyDiffPatchOptions) Validate(ctx context.Context, repo *repo_mode opts.NewBranch = opts.OldBranch } - gitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, repo) - if err != nil { - return err - } - defer closer.Close() - // oldBranch must exist for this operation - if _, err := gitRepo.GetBranch(opts.OldBranch); err != nil { + if exist, err := git_model.IsBranchExist(ctx, repo.ID, opts.OldBranch); err != nil { return err + } else if !exist { + return git_model.ErrBranchNotExist{ + BranchName: opts.OldBranch, + } } // A NewBranch can be specified for the patch to be applied to. // Check to make sure the branch does not already exist, otherwise we can't proceed. // If we aren't branching to a new branch, make sure user can commit to the given branch if opts.NewBranch != opts.OldBranch { - existingBranch, err := gitRepo.GetBranch(opts.NewBranch) - if existingBranch != "" { + exist, err := git_model.IsBranchExist(ctx, repo.ID, opts.NewBranch) + if err != nil { + return err + } else if exist { return git_model.ErrBranchAlreadyExists{ BranchName: opts.NewBranch, } } - if err != nil && !git.IsErrBranchNotExist(err) { - return err - } } else { protectedBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, opts.OldBranch) if err != nil { diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 87ab19aaf2b5c..f1ee601a114f8 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -107,8 +107,12 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use defer closer.Close() // oldBranch must exist for this operation - if _, err := gitRepo.GetBranch(opts.OldBranch); err != nil && !repo.IsEmpty { + if exist, err := git_model.IsBranchExist(ctx, repo.ID, opts.OldBranch); err != nil { return nil, err + } else if !exist && !repo.IsEmpty { + return nil, git_model.ErrBranchNotExist{ + BranchName: opts.OldBranch, + } } var treePaths []string @@ -145,15 +149,15 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use // Check to make sure the branch does not already exist, otherwise we can't proceed. // If we aren't branching to a new branch, make sure user can commit to the given branch if opts.NewBranch != opts.OldBranch { - existingBranch, err := gitRepo.GetBranch(opts.NewBranch) - if existingBranch != "" { + exist, err := git_model.IsBranchExist(ctx, repo.ID, opts.NewBranch) + if err != nil { + return nil, err + } + if exist { return nil, git_model.ErrBranchAlreadyExists{ BranchName: opts.NewBranch, } } - if err != nil && !git.IsErrBranchNotExist(err) { - return nil, err - } } else if err := VerifyBranchProtection(ctx, repo, doer, opts.OldBranch, treePaths); err != nil { return nil, err } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index f01b4c8f0cfdb..21b50d2083970 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -142,12 +142,12 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, if !repo.IsEmpty { if len(repo.DefaultBranch) == 0 { // Try to get HEAD branch and set it as default branch. - headBranch, err := gitRepo.GetHEADBranch() + headBranchName, err := gitRepo.GetHEADBranchName() if err != nil { return repo, fmt.Errorf("GetHEADBranch: %w", err) } - if headBranch != "" { - repo.DefaultBranch = headBranch + if headBranchName != "" { + repo.DefaultBranch = headBranchName } } From 12b9dc6c6e0f1e2d71ba7501e4b4fa527d525653 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Mar 2025 14:26:25 -0700 Subject: [PATCH 07/12] Remove unnecessary method --- modules/git/repo_branch.go | 19 ------------------- routers/api/v1/repo/commits.go | 8 +------- services/repository/migrate.go | 2 +- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index 8e7a908e8ce2a..e7ecf53f51ff6 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -7,7 +7,6 @@ package git import ( "context" "errors" - "fmt" "strings" ) @@ -25,24 +24,6 @@ func IsBranchExist(ctx context.Context, repoPath, name string) bool { return IsReferenceExist(ctx, repoPath, BranchPrefix+name) } -// GetHEADBranchName returns corresponding branch of HEAD. -func (repo *Repository) GetHEADBranchName() (string, error) { - if repo == nil { - return "", fmt.Errorf("nil repo") - } - stdout, _, err := NewCommand("symbolic-ref", "HEAD").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) - if err != nil { - return "", err - } - stdout = strings.TrimSpace(stdout) - - if !strings.HasPrefix(stdout, BranchPrefix) { - return "", fmt.Errorf("invalid HEAD branch: %v", stdout) - } - - return stdout[len(BranchPrefix):], nil -} - func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) { stdout, _, err := NewCommand("symbolic-ref", "HEAD").RunStdString(ctx, &RunOpts{Dir: repoPath}) if err != nil { diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index e6b31c61d4491..73a1279b4a91d 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -180,13 +180,7 @@ func GetAllCommits(ctx *context.APIContext) { var baseCommit *git.Commit if len(sha) == 0 { // no sha supplied - use default branch - headBranchName, err := ctx.Repo.GitRepo.GetHEADBranchName() - if err != nil { - ctx.APIErrorInternal(err) - return - } - - baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(headBranchName) + baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) if err != nil { ctx.APIErrorInternal(err) return diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 21b50d2083970..2a17e9acd93c7 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -142,7 +142,7 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, if !repo.IsEmpty { if len(repo.DefaultBranch) == 0 { // Try to get HEAD branch and set it as default branch. - headBranchName, err := gitRepo.GetHEADBranchName() + headBranchName, err := git.GetDefaultBranch(ctx, repoPath) if err != nil { return repo, fmt.Errorf("GetHEADBranch: %w", err) } From b08875afd8d976ebb9d4d206abf57b1b076e21fa Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Mar 2025 16:30:53 -0700 Subject: [PATCH 08/12] Fix test --- models/fixtures/branch.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index c60b088af3f4f..944669a702003 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -153,3 +153,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 21 + repo_id: 49 + name: 'master' + commit_id: 'aacbdfe9e1c4b47f60abe81849045fa4e96f1d75' + commit_message: "Add 'test/test.txt'" + commit_time: 1572535577 + pusher_id: 2 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 From 95d5b977ef2771d5b1765de6b8df5a80b0d4995c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 25 Mar 2025 18:50:38 -0700 Subject: [PATCH 09/12] Fix test --- models/fixtures/branch.yml | 12 ++++++++++++ services/repository/files/update.go | 1 + tests/integration/repofiles_change_test.go | 3 ++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 944669a702003..1ab13a1de1509 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -165,3 +165,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 22 + repo_id: 1 + name: 'develop' + commit_id: '65f1bf27bc3bf70f64657658635e66094edbcb4d' + commit_message: "Initial commit" + commit_time: 1489927679 + pusher_id: 1 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 diff --git a/services/repository/files/update.go b/services/repository/files/update.go index f1ee601a114f8..3f6255e77a77c 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -111,6 +111,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use return nil, err } else if !exist && !repo.IsEmpty { return nil, git_model.ErrBranchNotExist{ + RepoID: repo.ID, BranchName: opts.OldBranch, } } diff --git a/tests/integration/repofiles_change_test.go b/tests/integration/repofiles_change_test.go index ef520a2e51ce1..2dc03fc69bc6c 100644 --- a/tests/integration/repofiles_change_test.go +++ b/tests/integration/repofiles_change_test.go @@ -4,6 +4,7 @@ package integration import ( + "fmt" "net/url" "path/filepath" "strings" @@ -490,7 +491,7 @@ func TestChangeRepoFilesErrors(t *testing.T) { filesResponse, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, opts) assert.Error(t, err) assert.Nil(t, filesResponse) - expectedError := "branch does not exist [name: " + opts.OldBranch + "]" + expectedError := fmt.Sprintf("branch does not exist [repo_id: %d name: %s]", repo.ID, opts.OldBranch) assert.EqualError(t, err, expectedError) }) From 011136a9cab6e051d217864f443581811343b8df Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 26 Mar 2025 19:43:45 -0700 Subject: [PATCH 10/12] Fix test --- models/fixtures/branch.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 1ab13a1de1509..cd542c65eb5c8 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -177,3 +177,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 23 + repo_id: 3 + name: 'master' + commit_id: '2a47ca4b614a9f5a43abbd5ad851a54a616ffee6' + commit_message: "init project" + commit_time: 1497448461 + pusher_id: 1 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 From fe71956549851969689ad83c780ef3329ec7d8d4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 26 Mar 2025 21:59:37 -0700 Subject: [PATCH 11/12] Fix test --- models/fixtures/branch.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index cd542c65eb5c8..6536e1dda7b84 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -189,3 +189,15 @@ is_deleted: false deleted_by_id: 0 deleted_unix: 0 + +- + id: 24 + repo_id: 3 + name: 'test_branch' + commit_id: 'd22b4d4daa5be07329fcef6ed458f00cf3392da0' + commit_message: "test commit" + commit_time: 1602935385 + pusher_id: 1 + is_deleted: false + deleted_by_id: 0 + deleted_unix: 0 From 7a30f43b160b1f74ce77baee22461867e72adf70 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 27 Mar 2025 10:23:17 -0700 Subject: [PATCH 12/12] Fix test --- tests/integration/api_branch_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index 4372d8394ec25..01e184b5e1e39 100644 --- a/tests/integration/api_branch_test.go +++ b/tests/integration/api_branch_test.go @@ -303,7 +303,7 @@ func TestAPICreateBranchWithSyncBranches(t *testing.T) { RepoID: 1, }) assert.NoError(t, err) - assert.Len(t, branches, 5) + assert.Len(t, branches, 6) // make a broke repository with no branch on database _, err = db.DeleteByBean(db.DefaultContext, git_model.Branch{RepoID: 1}) @@ -320,7 +320,7 @@ func TestAPICreateBranchWithSyncBranches(t *testing.T) { RepoID: 1, }) assert.NoError(t, err) - assert.Len(t, branches, 6) + assert.Len(t, branches, 7) branches, err = db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{ RepoID: 1,