diff --git a/models/fixtures/branch.yml b/models/fixtures/branch.yml index 17b1869ab6364..6536e1dda7b84 100644 --- a/models/fixtures/branch.yml +++ b/models/fixtures/branch.yml @@ -93,3 +93,111 @@ 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 + +- + 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 + +- + 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 + +- + 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 + +- + 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 + +- + 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 diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index 20a347a04075a..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,36 +24,6 @@ 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 - - gitRepo *Repository -} - -// GetHEADBranch returns corresponding branch of HEAD. -func (repo *Repository) GetHEADBranch() (*Branch, error) { - if repo == nil { - return nil, errors.New("nil repo") - } - stdout, _, err := NewCommand("symbolic-ref", "HEAD").RunStdString(repo.Ctx, &RunOpts{Dir: repo.Path}) - if err != nil { - return nil, err - } - stdout = strings.TrimSpace(stdout) - - if !strings.HasPrefix(stdout, BranchPrefix) { - return nil, fmt.Errorf("invalid HEAD branch: %v", stdout) - } - - return &Branch{ - Name: stdout[len(BranchPrefix):], - Path: stdout, - gitRepo: repo, - }, nil -} - func GetDefaultBranch(ctx context.Context, repoPath string) (string, error) { stdout, _, err := NewCommand("symbolic-ref", "HEAD").RunStdString(ctx, &RunOpts{Dir: repoPath}) if err != nil { @@ -67,37 +36,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) (*Branch, error) { - if !repo.IsBranchExist(branch) { - return nil, ErrBranchNotExist{branch} - } - return &Branch{ - Path: repo.Path, - Name: branch, - gitRepo: repo, - }, 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 - } - - branches := make([]*Branch, len(brs)) - for i := range brs { - branches[i] = &Branch{ - Path: repo.Path, - Name: brs[i], - gitRepo: repo, - } - } - - return branches, countAll, nil -} - // DeleteBranchOptions Option(s) for delete branch type DeleteBranchOptions struct { Force bool @@ -147,11 +85,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/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/branch.go b/routers/api/v1/repo/branch.go index c40ac0dee0014..fe82550fddae1 100644 --- a/routers/api/v1/repo/branch.go +++ b/routers/api/v1/repo/branch.go @@ -59,17 +59,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 @@ -81,7 +80,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 @@ -260,25 +259,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/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index c92fbeab09df2..34a81bd7f3c73 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -179,13 +179,7 @@ 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() - if err != nil { - ctx.APIErrorInternal(err) - return - } - - baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(head.Name) + baseCommit, err = ctx.Repo.GitRepo.GetBranchCommit(ctx.Repo.Repository.DefaultBranch) 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/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index f69336a014524..c6f462bccff37 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 7d0b44c42f68c..4e91e53e7d083 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -817,9 +817,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 c22b5282c8d5c..34c3b1bf9a568 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 ) @@ -150,16 +150,16 @@ 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 { - baseCommit, err = baseBranch.GetCommit() + if exist { + 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 } @@ -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) @@ -203,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 } @@ -215,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) @@ -226,9 +219,9 @@ 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) + log.Error("GetCommit[%s]: %v", headBranch, err) return nil } if err == nil { diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index fa5b9934ec90f..c43a4ef04a181 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -437,7 +437,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..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 != nil { + 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 cade7ba2bf7db..3f6255e77a77c 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -107,8 +107,13 @@ 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{ + RepoID: repo.ID, + BranchName: opts.OldBranch, + } } var treePaths []string @@ -145,15 +150,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 != nil { + 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 9a5c6ffb0fd7a..2a17e9acd93c7 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 := git.GetDefaultBranch(ctx, repoPath) if err != nil { return repo, fmt.Errorf("GetHEADBranch: %w", err) } - if headBranch != nil { - repo.DefaultBranch = headBranch.Name + if headBranchName != "" { + repo.DefaultBranch = headBranchName } } diff --git a/tests/integration/api_branch_test.go b/tests/integration/api_branch_test.go index 1f2d1055e8463..16e1f2812e596 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, 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, 5) + assert.Len(t, branches, 7) branches, err = db.Find[git_model.Branch](db.DefaultContext, git_model.FindBranchOptions{ RepoID: 1, diff --git a/tests/integration/repofiles_change_test.go b/tests/integration/repofiles_change_test.go index 9ae069c810b8e..11c4a349d1905 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) })