diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index 6d427c8073422..6285138c27df5 100644 --- a/routers/api/v1/repo/compare.go +++ b/routers/api/v1/repo/compare.go @@ -5,7 +5,6 @@ package repo import ( "net/http" - "strings" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/gitrepo" @@ -52,18 +51,7 @@ func CompareDiff(ctx *context.APIContext) { } } - infoPath := ctx.PathParam("*") - infos := []string{ctx.Repo.Repository.DefaultBranch, ctx.Repo.Repository.DefaultBranch} - if infoPath != "" { - infos = strings.SplitN(infoPath, "...", 2) - if len(infos) != 2 { - if infos = strings.SplitN(infoPath, "..", 2); len(infos) != 2 { - infos = []string{ctx.Repo.Repository.DefaultBranch, infoPath} - } - } - } - - compareResult, closer := parseCompareInfo(ctx, api.CreatePullRequestOption{Base: infos[0], Head: infos[1]}) + compareInfo, closer := parseCompareInfo(ctx, ctx.PathParam("*")) if ctx.Written() { return } @@ -72,10 +60,10 @@ func CompareDiff(ctx *context.APIContext) { verification := ctx.FormString("verification") == "" || ctx.FormBool("verification") files := ctx.FormString("files") == "" || ctx.FormBool("files") - apiCommits := make([]*api.Commit, 0, len(compareResult.compareInfo.Commits)) + apiCommits := make([]*api.Commit, 0, len(compareInfo.Commits)) userCache := make(map[string]*user_model.User) - for i := 0; i < len(compareResult.compareInfo.Commits); i++ { - apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, compareResult.compareInfo.Commits[i], userCache, + for i := 0; i < len(compareInfo.Commits); i++ { + apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, compareInfo.Commits[i], userCache, convert.ToCommitOptions{ Stat: true, Verification: verification, @@ -89,7 +77,7 @@ func CompareDiff(ctx *context.APIContext) { } ctx.JSON(http.StatusOK, &api.Compare{ - TotalCommits: len(compareResult.compareInfo.Commits), + TotalCommits: len(compareInfo.Commits), Commits: apiCommits, }) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index b422c36d29f2d..209647e7d7617 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -28,6 +28,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/routers/common" @@ -36,6 +37,7 @@ import ( "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" "code.gitea.io/gitea/services/forms" + git_service "code.gitea.io/gitea/services/git" "code.gitea.io/gitea/services/gitdiff" issue_service "code.gitea.io/gitea/services/issue" notify_service "code.gitea.io/gitea/services/notify" @@ -414,20 +416,20 @@ func CreatePullRequest(ctx *context.APIContext) { ) // Get repo/branch information - compareResult, closer := parseCompareInfo(ctx, form) + compareResult, closer := parseCompareInfo(ctx, form.Base+".."+form.Head) if ctx.Written() { return } defer closer() - if !compareResult.baseRef.IsBranch() || !compareResult.headRef.IsBranch() { + if !compareResult.BaseRef.IsBranch() || !compareResult.HeadRef.IsBranch() { ctx.APIError(http.StatusUnprocessableEntity, "Invalid PullRequest: base and head must be branches") return } // Check if another PR exists with the same targets - existingPr, err := issues_model.GetUnmergedPullRequest(ctx, compareResult.headRepo.ID, ctx.Repo.Repository.ID, - compareResult.headRef.ShortName(), compareResult.baseRef.ShortName(), + existingPr, err := issues_model.GetUnmergedPullRequest(ctx, compareResult.HeadRepo.ID, ctx.Repo.Repository.ID, + compareResult.HeadRef.ShortName(), compareResult.BaseRef.ShortName(), issues_model.PullRequestFlowGithub, ) if err != nil { @@ -505,13 +507,13 @@ func CreatePullRequest(ctx *context.APIContext) { DeadlineUnix: deadlineUnix, } pr := &issues_model.PullRequest{ - HeadRepoID: compareResult.headRepo.ID, + HeadRepoID: compareResult.HeadRepo.ID, BaseRepoID: repo.ID, - HeadBranch: compareResult.headRef.ShortName(), - BaseBranch: compareResult.baseRef.ShortName(), - HeadRepo: compareResult.headRepo, + HeadBranch: compareResult.HeadRef.ShortName(), + BaseBranch: compareResult.BaseRef.ShortName(), + HeadRepo: compareResult.HeadRepo, BaseRepo: repo, - MergeBase: compareResult.compareInfo.MergeBase, + MergeBase: compareResult.MergeBase, Type: issues_model.PullRequestGitea, } @@ -1057,63 +1059,40 @@ func MergePullRequest(ctx *context.APIContext) { ctx.Status(http.StatusOK) } -type parseCompareInfoResult struct { - headRepo *repo_model.Repository - headGitRepo *git.Repository - compareInfo *pull_service.CompareInfo - baseRef git.RefName - headRef git.RefName -} - // parseCompareInfo returns non-nil if it succeeds, it always writes to the context and returns nil if it fails -func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) (result *parseCompareInfoResult, closer func()) { - var err error - // Get compared branches information - // format: ...[:] - // base<-head: master...head:feature - // same repo: master...feature +func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git_service.CompareInfo, closer func()) { baseRepo := ctx.Repo.Repository - baseRefToGuess := form.Base - - headUser := ctx.Repo.Owner - headRefToGuess := form.Head - if headInfos := strings.Split(form.Head, ":"); len(headInfos) == 1 { - // If there is no head repository, it means pull request between same repository. - // Do nothing here because the head variables have been assigned above. - } else if len(headInfos) == 2 { - // There is a head repository (the head repository could also be the same base repo) - headRefToGuess = headInfos[1] - headUser, err = user_model.GetUserOrOrgByName(ctx, headInfos[0]) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIErrorNotFound("GetUserByName") - } else { - ctx.APIErrorInternal(err) - } - return nil, nil - } - } else { - ctx.APIErrorNotFound() + compareReq, err := common.ParseCompareRouterParam(compareParam) + switch { + case errors.Is(err, util.ErrInvalidArgument): + ctx.APIError(http.StatusBadRequest, err.Error()) + return nil, nil + case err != nil: + ctx.APIErrorInternal(err) return nil, nil } - isSameRepo := ctx.Repo.Owner.ID == headUser.ID + // remove the check when we support compare with carets + if compareReq.CaretTimes > 0 { + ctx.APIError(http.StatusBadRequest, "Unsupported compare syntax with carets") + return nil, nil + } - var headRepo *repo_model.Repository - if isSameRepo { - headRepo = baseRepo - } else { - headRepo, err = common.FindHeadRepo(ctx, baseRepo, headUser.ID) - if err != nil { - ctx.APIErrorInternal(err) - return nil, nil - } - if headRepo == nil { - ctx.APIErrorNotFound("head repository not found") - return nil, nil - } + _, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq) + switch { + case errors.Is(err, util.ErrInvalidArgument): + ctx.APIError(http.StatusBadRequest, err.Error()) + return nil, nil + case errors.Is(err, util.ErrNotExist): + ctx.APIErrorNotFound() + return nil, nil + case err != nil: + ctx.APIErrorInternal(err) + return nil, nil } + isSameRepo := baseRepo.ID == headRepo.ID + var headGitRepo *git.Repository if isSameRepo { headGitRepo = ctx.Repo.GitRepo @@ -1140,8 +1119,8 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) } if !permBase.CanRead(unit.TypeCode) { - log.Trace("Permission Denied: User %-v cannot create/read pull requests or cannot read code in Repo %-v\nUser in baseRepo has Permissions: %-+v", ctx.Doer, baseRepo, permBase) - ctx.APIErrorNotFound("Can't read pulls or can't read UnitTypeCode") + log.Trace("Permission Denied: User %-v cannot read code in Repo %-v\nUser in baseRepo has Permissions: %-+v", ctx.Doer, baseRepo, permBase) + ctx.APIErrorNotFound("can't read baseRepo UnitTypeCode") return nil, nil } @@ -1158,10 +1137,10 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return nil, nil } - baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefToGuess) - headRef := headGitRepo.UnstableGuessRefByShortName(headRefToGuess) + baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.BaseOriRef, baseRepo.DefaultBranch)) + headRef := headGitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch)) - log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), baseRefToGuess, baseRef, headRefToGuess, headRef) + log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef, baseRef, compareReq.HeadOriRef, headRef) baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName()) headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName()) @@ -1171,14 +1150,13 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return nil, nil } - compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), false, false) + compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef, headRef, compareReq.DirectComparison(), false) if err != nil { ctx.APIErrorInternal(err) return nil, nil } - result = &parseCompareInfoResult{headRepo: headRepo, headGitRepo: headGitRepo, compareInfo: compareInfo, baseRef: baseRef, headRef: headRef} - return result, closer + return compareInfo, closer } // UpdatePullRequest merge PR's baseBranch into headBranch @@ -1422,7 +1400,7 @@ func GetPullRequestCommits(ctx *context.APIContext) { return } - var prInfo *pull_service.CompareInfo + var compareInfo *git_service.CompareInfo baseGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pr.BaseRepo) if err != nil { ctx.APIErrorInternal(err) @@ -1431,19 +1409,18 @@ func GetPullRequestCommits(ctx *context.APIContext) { defer closer.Close() if pr.HasMerged { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), false, false) + compareInfo, err = git_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, git.RefName(pr.MergeBase), git.RefName(pr.GetGitHeadRefName()), false, false) } else { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), false, false) + compareInfo, err = git_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, git.RefNameFromBranch(pr.BaseBranch), git.RefName(pr.GetGitHeadRefName()), false, false) } if err != nil { ctx.APIErrorInternal(err) return } - commits := prInfo.Commits listOptions := utils.GetListOptions(ctx) - totalNumberOfCommits := len(commits) + totalNumberOfCommits := len(compareInfo.Commits) totalNumberOfPages := int(math.Ceil(float64(totalNumberOfCommits) / float64(listOptions.PageSize))) userCache := make(map[string]*user_model.User) @@ -1458,7 +1435,7 @@ func GetPullRequestCommits(ctx *context.APIContext) { apiCommits := make([]*api.Commit, 0, limit) for i := start; i < start+limit; i++ { - apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, baseGitRepo, commits[i], userCache, + apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, baseGitRepo, compareInfo.Commits[i], userCache, convert.ToCommitOptions{ Stat: true, Verification: verification, @@ -1552,11 +1529,11 @@ func GetPullRequestFiles(ctx *context.APIContext) { baseGitRepo := ctx.Repo.GitRepo - var prInfo *pull_service.CompareInfo + var compareInfo *git_service.CompareInfo if pr.HasMerged { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.MergeBase, pr.GetGitHeadRefName(), true, false) + compareInfo, err = git_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, git.RefName(pr.MergeBase), git.RefName(pr.GetGitHeadRefName()), true, false) } else { - prInfo, err = pull_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, pr.BaseBranch, pr.GetGitHeadRefName(), true, false) + compareInfo, err = git_service.GetCompareInfo(ctx, pr.BaseRepo, pr.BaseRepo, baseGitRepo, git.RefNameFromBranch(pr.BaseBranch), git.RefName(pr.GetGitHeadRefName()), true, false) } if err != nil { ctx.APIErrorInternal(err) @@ -1569,7 +1546,7 @@ func GetPullRequestFiles(ctx *context.APIContext) { return } - startCommitID := prInfo.MergeBase + startCommitID := compareInfo.MergeBase endCommitID := headCommitID maxLines := setting.Git.MaxGitDiffLines diff --git a/routers/common/compare.go b/routers/common/compare.go index be689bbdb54a9..fa9e4e668e719 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -5,22 +5,110 @@ package common import ( "context" + "strings" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/git" - pull_service "code.gitea.io/gitea/services/pull" + "code.gitea.io/gitea/modules/util" ) -// CompareInfo represents the collected results from ParseCompareInfo -type CompareInfo struct { - HeadUser *user_model.User - HeadRepo *repo_model.Repository - HeadGitRepo *git.Repository - CompareInfo *pull_service.CompareInfo - BaseBranch string - HeadBranch string - DirectComparison bool +type CompareRouterReq struct { + BaseOriRef string + HeadOwner string + HeadRepoName string + HeadOriRef string + CaretTimes int // ^ times after base ref + DotTimes int +} + +func (cr *CompareRouterReq) DirectComparison() bool { + return cr.DotTimes == 2 || cr.DotTimes == 0 +} + +func parseBase(base string) (string, int) { + parts := strings.SplitN(base, "^", 2) + if len(parts) == 1 { + return base, 0 + } + return parts[0], len(parts[1]) + 1 +} + +func parseHead(head string) (string, string, string) { + paths := strings.SplitN(head, ":", 2) + if len(paths) == 1 { + return "", "", paths[0] + } + ownerRepo := strings.SplitN(paths[0], "/", 2) + if len(ownerRepo) == 1 { + return paths[0], "", paths[1] + } + return ownerRepo[0], ownerRepo[1], paths[1] +} + +// ParseCompareRouterParam Get compare information from the router parameter. +// A full compare url is of the form: +// +// 1. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headBranch} +// 2. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}:{:headBranch} +// 3. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}/{:headRepoName}:{:headBranch} +// 4. /{:baseOwner}/{:baseRepoName}/compare/{:headBranch} +// 5. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}:{:headBranch} +// 6. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}/{:headRepoName}:{:headBranch} +// +// Here we obtain the infoPath "{:baseBranch}...[{:headOwner}/{:headRepoName}:]{:headBranch}" as ctx.PathParam("*") +// with the :baseRepo in ctx.Repo. +// +// Note: Generally :headRepoName is not provided here - we are only passed :headOwner. +// +// How do we determine the :headRepo? +// +// 1. If :headOwner is not set then the :headRepo = :baseRepo +// 2. If :headOwner is set - then look for the fork of :baseRepo owned by :headOwner +// 3. But... :baseRepo could be a fork of :headOwner's repo - so check that +// 4. Now, :baseRepo and :headRepos could be forks of the same repo - so check that +// +// format: ...[:] +// base<-head: master...head:feature +// same repo: master...feature +func ParseCompareRouterParam(routerParam string) (*CompareRouterReq, error) { + if routerParam == "" { + return &CompareRouterReq{}, nil + } + + var basePart, headPart string + dotTimes := 3 + parts := strings.Split(routerParam, "...") + if len(parts) > 2 { + return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam) + } + if len(parts) != 2 { + parts = strings.Split(routerParam, "..") + if len(parts) == 1 { + headOwnerName, headRepoName, headRef := parseHead(routerParam) + return &CompareRouterReq{ + HeadOriRef: headRef, + HeadOwner: headOwnerName, + HeadRepoName: headRepoName, + DotTimes: dotTimes, + }, nil + } else if len(parts) > 2 { + return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam) + } + dotTimes = 2 + } + basePart, headPart = parts[0], parts[1] + + baseRef, caretTimes := parseBase(basePart) + headOwnerName, headRepoName, headRef := parseHead(headPart) + + return &CompareRouterReq{ + BaseOriRef: baseRef, + HeadOriRef: headRef, + HeadOwner: headOwnerName, + HeadRepoName: headRepoName, + CaretTimes: caretTimes, + DotTimes: dotTimes, + }, nil } // maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository. @@ -45,6 +133,48 @@ func FindHeadRepo(ctx context.Context, baseRepo *repo_model.Repository, headUser return findHeadRepoFromRootBase(ctx, baseRepo, headUserID, maxForkTraverseLevel) } +func GetHeadOwnerAndRepo(ctx context.Context, baseRepo *repo_model.Repository, compareReq *CompareRouterReq) (headOwner *user_model.User, headRepo *repo_model.Repository, err error) { + if compareReq.HeadOwner == "" { + if compareReq.HeadRepoName != "" { // unsupported syntax + return nil, nil, util.ErrorWrap(util.ErrInvalidArgument, "head owner must be specified when head repo name is given") + } + + return baseRepo.Owner, baseRepo, nil + } + + if compareReq.HeadOwner == baseRepo.Owner.Name { + headOwner = baseRepo.Owner + } else { + headOwner, err = user_model.GetUserOrOrgByName(ctx, compareReq.HeadOwner) + if err != nil { + return nil, nil, err + } + } + if compareReq.HeadRepoName == "" { + if headOwner.ID == baseRepo.OwnerID { + headRepo = baseRepo + } else { + headRepo, err = FindHeadRepo(ctx, baseRepo, headOwner.ID) + if err != nil { + return nil, nil, err + } + if headRepo == nil { + return nil, nil, util.ErrorWrap(util.ErrInvalidArgument, "the user %s does not have a fork of the base repository", headOwner.Name) + } + } + } else { + if compareReq.HeadOwner == baseRepo.Owner.Name && compareReq.HeadRepoName == baseRepo.Name { + headRepo = baseRepo + } else { + headRepo, err = repo_model.GetRepositoryByName(ctx, headOwner.ID, compareReq.HeadRepoName) + if err != nil { + return nil, nil, err + } + } + } + return headOwner, headRepo, nil +} + func findHeadRepoFromRootBase(ctx context.Context, baseRepo *repo_model.Repository, headUserID int64, traverseLevel int) (*repo_model.Repository, error) { if traverseLevel == 0 { return nil, nil diff --git a/routers/common/compare_test.go b/routers/common/compare_test.go new file mode 100644 index 0000000000000..a55f6607aec74 --- /dev/null +++ b/routers/common/compare_test.go @@ -0,0 +1,151 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package common + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestCompareRouterReq(t *testing.T) { + unittest.PrepareTestEnv(t) + + kases := []struct { + router string + CompareRouterReq *CompareRouterReq + }{ + { + router: "", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "", + HeadOriRef: "", + DotTimes: 0, + }, + }, + { + router: "main...develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main..develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + DotTimes: 2, + }, + }, + { + router: "main^...develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + CaretTimes: 1, + DotTimes: 3, + }, + }, + { + router: "main^^^^^...develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + CaretTimes: 5, + DotTimes: 3, + }, + }, + { + router: "develop", + CompareRouterReq: &CompareRouterReq{ + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main...lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main...lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main^...lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + CaretTimes: 1, + }, + }, + { + router: "v1.0...v1.1", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "v1.0", + HeadOriRef: "v1.1", + DotTimes: 3, + }, + }, + { + router: "teabot-patch-1...v0.0.1", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "teabot-patch-1", + HeadOriRef: "v0.0.1", + DotTimes: 3, + }, + }, + { + router: "teabot:feature1", + CompareRouterReq: &CompareRouterReq{ + HeadOwner: "teabot", + HeadOriRef: "feature1", + DotTimes: 3, + }, + }, + { + router: "8eb19a5ae19abae15c0666d4ab98906139a7f439...283c030497b455ecfa759d4649f9f8b45158742e", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "8eb19a5ae19abae15c0666d4ab98906139a7f439", + HeadOriRef: "283c030497b455ecfa759d4649f9f8b45158742e", + DotTimes: 3, + }, + }, + } + + for _, kase := range kases { + t.Run(kase.router, func(t *testing.T) { + r, err := ParseCompareRouterParam(kase.router) + assert.NoError(t, err) + assert.Equal(t, kase.CompareRouterReq, r) + }) + } +} diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index f66dabbf8703a..29a82b5dfcb93 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -41,8 +41,8 @@ import ( "code.gitea.io/gitea/routers/common" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" + git_service "code.gitea.io/gitea/services/git" "code.gitea.io/gitea/services/gitdiff" - pull_service "code.gitea.io/gitea/services/pull" user_service "code.gitea.io/gitea/services/user" ) @@ -192,133 +192,64 @@ func setCsvCompareContext(ctx *context.Context) { } // ParseCompareInfo parse compare info between two commit for preparing comparing references -func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { +func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo { baseRepo := ctx.Repo.Repository - ci := &common.CompareInfo{} - fileOnly := ctx.FormBool("file-only") - // Get compared branches information - // A full compare url is of the form: - // - // 1. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headBranch} - // 2. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}:{:headBranch} - // 3. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}/{:headRepoName}:{:headBranch} - // 4. /{:baseOwner}/{:baseRepoName}/compare/{:headBranch} - // 5. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}:{:headBranch} - // 6. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}/{:headRepoName}:{:headBranch} - // - // Here we obtain the infoPath "{:baseBranch}...[{:headOwner}/{:headRepoName}:]{:headBranch}" as ctx.PathParam("*") - // with the :baseRepo in ctx.Repo. - // - // Note: Generally :headRepoName is not provided here - we are only passed :headOwner. - // - // How do we determine the :headRepo? - // - // 1. If :headOwner is not set then the :headRepo = :baseRepo - // 2. If :headOwner is set - then look for the fork of :baseRepo owned by :headOwner - // 3. But... :baseRepo could be a fork of :headOwner's repo - so check that - // 4. Now, :baseRepo and :headRepos could be forks of the same repo - so check that - // - // format: ...[:] - // base<-head: master...head:feature - // same repo: master...feature - - var ( - isSameRepo bool - infoPath string - err error - ) - - infoPath = ctx.PathParam("*") - var infos []string - if infoPath == "" { - infos = []string{baseRepo.DefaultBranch, baseRepo.DefaultBranch} - } else { - infos = strings.SplitN(infoPath, "...", 2) - if len(infos) != 2 { - if infos = strings.SplitN(infoPath, "..", 2); len(infos) == 2 { - ci.DirectComparison = true - ctx.Data["PageIsComparePull"] = false - } else { - infos = []string{baseRepo.DefaultBranch, infoPath} - } - } + compareReq, err := common.ParseCompareRouterParam(ctx.PathParam("*")) + switch { + case errors.Is(err, util.ErrInvalidArgument): + ctx.HTTPError(http.StatusBadRequest, err.Error()) + return nil + case err != nil: + ctx.ServerError("ParseCompareRouterParam", err) + return nil + } + // remove the check when we support compare with carets + if compareReq.CaretTimes > 0 { + ctx.HTTPError(http.StatusBadRequest, "Unsupported compare syntax with carets") + return nil } - ctx.Data["BaseName"] = baseRepo.OwnerName - ci.BaseBranch = infos[0] - ctx.Data["BaseBranch"] = ci.BaseBranch - - // If there is no head repository, it means compare between same repository. - headInfos := strings.Split(infos[1], ":") - if len(headInfos) == 1 { - isSameRepo = true - ci.HeadUser = ctx.Repo.Owner - ci.HeadBranch = headInfos[0] - } else if len(headInfos) == 2 { - headInfosSplit := strings.Split(headInfos[0], "/") - if len(headInfosSplit) == 1 { - ci.HeadUser, err = user_model.GetUserOrOrgByName(ctx, headInfos[0]) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.NotFound(nil) - } else { - ctx.ServerError("GetUserByName", err) - } - return nil - } - ci.HeadBranch = headInfos[1] - isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID - if isSameRepo { - ci.HeadRepo = baseRepo - } - } else { - ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, headInfosSplit[0], headInfosSplit[1]) - if err != nil { - if repo_model.IsErrRepoNotExist(err) { - ctx.NotFound(nil) - } else { - ctx.ServerError("GetRepositoryByOwnerAndName", err) - } - return nil - } - if err := ci.HeadRepo.LoadOwner(ctx); err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.NotFound(nil) - } else { - ctx.ServerError("GetUserByName", err) - } - return nil - } - ci.HeadBranch = headInfos[1] - ci.HeadUser = ci.HeadRepo.Owner - isSameRepo = ci.HeadRepo.ID == ctx.Repo.Repository.ID - } - } else { + headOwner, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq) + switch { + case errors.Is(err, util.ErrInvalidArgument): + ctx.HTTPError(http.StatusBadRequest, err.Error()) + return nil + case errors.Is(err, util.ErrNotExist): ctx.NotFound(nil) return nil + case err != nil: + ctx.ServerError("GetHeadOwnerAndRepo", err) + return nil } - ctx.Data["HeadUser"] = ci.HeadUser - ctx.Data["HeadBranch"] = ci.HeadBranch + + baseBranch := util.IfZero(compareReq.BaseOriRef, baseRepo.DefaultBranch) + headBranch := util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch) + isSameRepo := baseRepo.ID == headRepo.ID + + ctx.Data["BaseName"] = baseRepo.OwnerName + ctx.Data["BaseBranch"] = baseBranch + ctx.Data["HeadUser"] = headOwner + ctx.Data["HeadBranch"] = headBranch ctx.Repo.PullRequest.SameRepo = isSameRepo // Check if base branch is valid. - baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(ci.BaseBranch) - baseIsBranch, _ := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, ci.BaseBranch) - baseIsTag := gitrepo.IsTagExist(ctx, ctx.Repo.Repository, ci.BaseBranch) + baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(baseBranch) + baseIsBranch, _ := git_model.IsBranchExist(ctx, ctx.Repo.Repository.ID, baseBranch) + baseIsTag := gitrepo.IsTagExist(ctx, ctx.Repo.Repository, baseBranch) if !baseIsCommit && !baseIsBranch && !baseIsTag { // Check if baseBranch is short sha commit hash - if baseCommit, _ := ctx.Repo.GitRepo.GetCommit(ci.BaseBranch); baseCommit != nil { - ci.BaseBranch = baseCommit.ID.String() - ctx.Data["BaseBranch"] = ci.BaseBranch + if baseCommit, _ := ctx.Repo.GitRepo.GetCommit(baseBranch); baseCommit != nil { + baseBranch = baseCommit.ID.String() + ctx.Data["BaseBranch"] = baseBranch baseIsCommit = true - } else if ci.BaseBranch == ctx.Repo.GetObjectFormat().EmptyObjectID().String() { + } else if baseBranch == ctx.Repo.GetObjectFormat().EmptyObjectID().String() { if isSameRepo { - ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(ci.HeadBranch)) + ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(headBranch)) } else { - ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(ci.HeadRepo.FullName()) + ":" + util.PathEscapeSegments(ci.HeadBranch)) + ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(headRepo.FullName()) + ":" + util.PathEscapeSegments(headBranch)) } return nil } else { @@ -368,31 +299,31 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { } } - has := ci.HeadRepo != nil + has := headRepo != nil // 3. If the base is a forked from "RootRepo" and the owner of // the "RootRepo" is the :headUser - set headRepo to that - if !has && rootRepo != nil && rootRepo.OwnerID == ci.HeadUser.ID { - ci.HeadRepo = rootRepo + if !has && rootRepo != nil && rootRepo.OwnerID == headOwner.ID { + headRepo = rootRepo has = true } // 4. If the ctx.Doer has their own fork of the baseRepo and the headUser is the ctx.Doer // set the headRepo to the ownFork - if !has && ownForkRepo != nil && ownForkRepo.OwnerID == ci.HeadUser.ID { - ci.HeadRepo = ownForkRepo + if !has && ownForkRepo != nil && ownForkRepo.OwnerID == headOwner.ID { + headRepo = ownForkRepo has = true } // 5. If the headOwner has a fork of the baseRepo - use that if !has { - ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ID) - has = ci.HeadRepo != nil + headRepo = repo_model.GetForkedRepo(ctx, headOwner.ID, baseRepo.ID) + has = headRepo != nil } // 6. If the baseRepo is a fork and the headUser has a fork of that use that if !has && baseRepo.IsFork { - ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ForkID) - has = ci.HeadRepo != nil + headRepo = repo_model.GetForkedRepo(ctx, headOwner.ID, baseRepo.ForkID) + has = headRepo != nil } // 7. Otherwise if we're not the same repo and haven't found a repo give up @@ -401,11 +332,11 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { } // 8. Finally open the git repo + var headGitRepo *git.Repository if isSameRepo { - ci.HeadRepo = ctx.Repo.Repository - ci.HeadGitRepo = ctx.Repo.GitRepo + headGitRepo = ctx.Repo.GitRepo } else if has { - ci.HeadGitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ci.HeadRepo) + headGitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, headRepo) if err != nil { ctx.ServerError("RepositoryFromRequestContextOrOpen", err) return nil @@ -415,7 +346,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { return nil } - ctx.Data["HeadRepo"] = ci.HeadRepo + ctx.Data["HeadRepo"] = headRepo ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository // Now we need to assert that the ctx.Doer has permission to read @@ -440,7 +371,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { // If we're not merging from the same repo: if !isSameRepo { // Assert ctx.Doer has permission to read headRepo's codes - permHead, err := access_model.GetUserRepoPermission(ctx, ci.HeadRepo, ctx.Doer) + permHead, err := access_model.GetUserRepoPermission(ctx, headRepo, ctx.Doer) if err != nil { ctx.ServerError("GetUserRepoPermission", err) return nil @@ -449,7 +380,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { if log.IsTrace() { log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v", ctx.Doer, - ci.HeadRepo, + headRepo, permHead) } ctx.NotFound(nil) @@ -463,7 +394,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { // 2. the computed head // then get the branches of it if rootRepo != nil && - rootRepo.ID != ci.HeadRepo.ID && + rootRepo.ID != headRepo.ID && rootRepo.ID != baseRepo.ID { canRead := access_model.CheckRepoUnitUser(ctx, rootRepo, ctx.Doer, unit.TypeCode) if canRead { @@ -487,7 +418,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { // 3. The rootRepo (if we have one) // then get the branches from it. if ownForkRepo != nil && - ownForkRepo.ID != ci.HeadRepo.ID && + ownForkRepo.ID != headRepo.ID && ownForkRepo.ID != baseRepo.ID && (rootRepo == nil || ownForkRepo.ID != rootRepo.ID) { canRead := access_model.CheckRepoUnitUser(ctx, ownForkRepo, ctx.Doer, unit.TypeCode) @@ -506,14 +437,14 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { } // Check if head branch is valid. - headIsCommit := ci.HeadGitRepo.IsCommitExist(ci.HeadBranch) - headIsBranch, _ := git_model.IsBranchExist(ctx, ci.HeadRepo.ID, ci.HeadBranch) - headIsTag := gitrepo.IsTagExist(ctx, ci.HeadRepo, ci.HeadBranch) + headIsCommit := headGitRepo.IsCommitExist(headBranch) + headIsBranch, _ := git_model.IsBranchExist(ctx, headRepo.ID, headBranch) + headIsTag := gitrepo.IsTagExist(ctx, headRepo, headBranch) if !headIsCommit && !headIsBranch && !headIsTag { // Check if headBranch is short sha commit hash - if headCommit, _ := ci.HeadGitRepo.GetCommit(ci.HeadBranch); headCommit != nil { - ci.HeadBranch = headCommit.ID.String() - ctx.Data["HeadBranch"] = ci.HeadBranch + if headCommit, _ := headGitRepo.GetCommit(headBranch); headCommit != nil { + headBranch = headCommit.ID.String() + ctx.Data["HeadBranch"] = headBranch headIsCommit = true } else { ctx.NotFound(nil) @@ -540,41 +471,41 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { return nil } - baseBranchRef := ci.BaseBranch + baseBranchRef := git.RefName(baseBranch) if baseIsBranch { - baseBranchRef = git.BranchPrefix + ci.BaseBranch + baseBranchRef = git.RefNameFromBranch(baseBranch) } else if baseIsTag { - baseBranchRef = git.TagPrefix + ci.BaseBranch + baseBranchRef = git.RefNameFromTag(baseBranch) } - headBranchRef := ci.HeadBranch + headBranchRef := git.RefName(headBranch) if headIsBranch { - headBranchRef = git.BranchPrefix + ci.HeadBranch + headBranchRef = git.RefNameFromBranch(headBranch) } else if headIsTag { - headBranchRef = git.TagPrefix + ci.HeadBranch + headBranchRef = git.RefNameFromTag(headBranch) } - ci.CompareInfo, err = pull_service.GetCompareInfo(ctx, baseRepo, ci.HeadRepo, ci.HeadGitRepo, baseBranchRef, headBranchRef, ci.DirectComparison, fileOnly) + compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseBranchRef, headBranchRef, compareReq.DirectComparison(), fileOnly) if err != nil { ctx.ServerError("GetCompareInfo", err) return nil } - if ci.DirectComparison { - ctx.Data["BeforeCommitID"] = ci.CompareInfo.BaseCommitID + if compareReq.DirectComparison() { + ctx.Data["BeforeCommitID"] = compareInfo.BaseCommitID } else { - ctx.Data["BeforeCommitID"] = ci.CompareInfo.MergeBase + ctx.Data["BeforeCommitID"] = compareInfo.MergeBase } - return ci + return compareInfo } // PrepareCompareDiff renders compare diff page func PrepareCompareDiff( ctx *context.Context, - ci *common.CompareInfo, + ci *git_service.CompareInfo, whitespaceBehavior gitcmd.TrustedCmdArgs, ) (nothingToCompare bool) { repo := ctx.Repo.Repository - headCommitID := ci.CompareInfo.HeadCommitID + headCommitID := ci.HeadCommitID ctx.Data["CommitRepoLink"] = ci.HeadRepo.Link() ctx.Data["AfterCommitID"] = headCommitID @@ -586,17 +517,15 @@ func PrepareCompareDiff( ctx.Data["TitleQuery"] = newPrFormTitle ctx.Data["BodyQuery"] = newPrFormBody - if (headCommitID == ci.CompareInfo.MergeBase && !ci.DirectComparison) || - headCommitID == ci.CompareInfo.BaseCommitID { + if (headCommitID == ci.MergeBase && !ci.DirectComparison) || + headCommitID == ci.BaseCommitID { ctx.Data["IsNothingToCompare"] = true if unit, err := repo.GetUnit(ctx, unit.TypePullRequests); err == nil { config := unit.PullRequestsConfig() if !config.AutodetectManualMerge { - allowEmptyPr := !(ci.BaseBranch == ci.HeadBranch && ctx.Repo.Repository.Name == ci.HeadRepo.Name) - ctx.Data["AllowEmptyPr"] = allowEmptyPr - - return !allowEmptyPr + ctx.Data["AllowEmptyPr"] = !ci.IsSameRef() + return ci.IsSameRef() } ctx.Data["AllowEmptyPr"] = false @@ -604,9 +533,9 @@ func PrepareCompareDiff( return true } - beforeCommitID := ci.CompareInfo.MergeBase + beforeCommitID := ci.MergeBase if ci.DirectComparison { - beforeCommitID = ci.CompareInfo.BaseCommitID + beforeCommitID = ci.BaseCommitID } maxLines, maxFiles := setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffFiles @@ -674,7 +603,7 @@ func PrepareCompareDiff( return false } - commits, err := processGitCommits(ctx, ci.CompareInfo.Commits) + commits, err := processGitCommits(ctx, ci.Commits) if err != nil { ctx.ServerError("processGitCommits", err) return false @@ -682,7 +611,7 @@ func PrepareCompareDiff( ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) - title := ci.HeadBranch + title := ci.HeadRef.ShortName() if len(commits) == 1 { c := commits[0] title = strings.TrimSpace(c.UserCommit.Summary()) @@ -706,10 +635,10 @@ func PrepareCompareDiff( } ctx.Data["title"] = title - ctx.Data["Username"] = ci.HeadUser.Name + ctx.Data["Username"] = ci.HeadRepo.OwnerName ctx.Data["Reponame"] = ci.HeadRepo.Name - setCompareContext(ctx, beforeCommit, headCommit, ci.HeadUser.Name, repo.Name) + setCompareContext(ctx, beforeCommit, headCommit, ci.HeadRepo.OwnerName, repo.Name) return false } @@ -790,7 +719,7 @@ func CompareDiff(ctx *context.Context) { ctx.Data["HeadTags"] = headTags if ctx.Data["PageIsComparePull"] == true { - pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, ctx.Repo.Repository.ID, ci.HeadBranch, ci.BaseBranch, issues_model.PullRequestFlowGithub) + pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, ctx.Repo.Repository.ID, ci.HeadRef.ShortName(), ci.BaseRef.ShortName(), issues_model.PullRequestFlowGithub) if err != nil { if !issues_model.IsErrPullRequestNotExist(err) { ctx.ServerError("GetUnmergedPullRequest", err) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 488389e204963..ec4dbd4b109b8 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -44,6 +44,7 @@ import ( "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" "code.gitea.io/gitea/services/forms" + git_service "code.gitea.io/gitea/services/git" "code.gitea.io/gitea/services/gitdiff" notify_service "code.gitea.io/gitea/services/notify" pull_service "code.gitea.io/gitea/services/pull" @@ -256,7 +257,7 @@ func GetMergedBaseCommitID(ctx *context.Context, issue *issues_model.Issue) stri return baseCommit } -func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_service.CompareInfo { +func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git_service.CompareInfo { if !issue.IsPull { return nil } @@ -267,7 +268,7 @@ func preparePullViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *p } // prepareMergedViewPullInfo show meta information for a merged pull request view page -func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_service.CompareInfo { +func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git_service.CompareInfo { pull := issue.PullRequest setMergeTarget(ctx, pull) @@ -275,8 +276,8 @@ func prepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) baseCommit := GetMergedBaseCommitID(ctx, issue) - compareInfo, err := pull_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.Repository, ctx.Repo.GitRepo, - baseCommit, pull.GetGitHeadRefName(), false, false) + compareInfo, err := git_service.GetCompareInfo(ctx, ctx.Repo.Repository, ctx.Repo.Repository, ctx.Repo.GitRepo, + git.RefName(baseCommit), git.RefName(pull.GetGitHeadRefName()), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") || strings.Contains(err.Error(), "unknown revision or path not in the working tree") { ctx.Data["IsPullRequestBroken"] = true @@ -321,7 +322,7 @@ type pullCommitStatusCheckData struct { } // prepareViewPullInfo show meta information for a pull request preview page -func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_service.CompareInfo { +func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git_service.CompareInfo { ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes repo := ctx.Repo.Repository @@ -383,8 +384,8 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_ ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(commitStatuses) } - compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, - pull.MergeBase, pull.GetGitHeadRefName(), false, false) + compareInfo, err := git_service.GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, + git.RefName(pull.MergeBase), git.RefName(pull.GetGitHeadRefName()), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { ctx.Data["IsPullRequestBroken"] = true @@ -550,8 +551,8 @@ func prepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *pull_ } } - compareInfo, err := pull_service.GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, - git.BranchPrefix+pull.BaseBranch, pull.GetGitHeadRefName(), false, false) + compareInfo, err := git_service.GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, + git.RefNameFromBranch(pull.BaseBranch), git.RefNameFromBranch(pull.GetGitHeadRefName()), false, false) if err != nil { if strings.Contains(err.Error(), "fatal: Not a valid object name") { ctx.Data["IsPullRequestBroken"] = true @@ -1341,7 +1342,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { } // Check if a pull request already exists with the same head and base branch. - pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, repo.ID, ci.HeadBranch, ci.BaseBranch, issues_model.PullRequestFlowGithub) + pr, err := issues_model.GetUnmergedPullRequest(ctx, ci.HeadRepo.ID, repo.ID, ci.HeadRef.ShortName(), ci.BaseRef.ShortName(), issues_model.PullRequestFlowGithub) if err != nil && !issues_model.IsErrPullRequestNotExist(err) { ctx.ServerError("GetUnmergedPullRequest", err) return @@ -1371,11 +1372,11 @@ func CompareAndPullRequestPost(ctx *context.Context) { pullRequest := &issues_model.PullRequest{ HeadRepoID: ci.HeadRepo.ID, BaseRepoID: repo.ID, - HeadBranch: ci.HeadBranch, - BaseBranch: ci.BaseBranch, + HeadBranch: ci.HeadRef.ShortName(), + BaseBranch: ci.BaseRef.ShortName(), HeadRepo: ci.HeadRepo, BaseRepo: repo, - MergeBase: ci.CompareInfo.MergeBase, + MergeBase: ci.MergeBase, Type: issues_model.PullRequestGitea, AllowMaintainerEdit: form.AllowMaintainerEdit, } diff --git a/services/pull/compare.go b/services/git/compare.go similarity index 68% rename from services/pull/compare.go rename to services/git/compare.go index fbdb17cfdd937..09d5c44eadfbb 100644 --- a/services/pull/compare.go +++ b/services/git/compare.go @@ -1,7 +1,7 @@ // Copyright 2025 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package pull +package git import ( "context" @@ -18,15 +18,29 @@ import ( // CompareInfo represents needed information for comparing references. type CompareInfo struct { - MergeBase string - BaseCommitID string - HeadCommitID string - Commits []*git.Commit - NumFiles int + BaseRepo *repo_model.Repository + BaseRef git.RefName + BaseCommitID string + HeadRepo *repo_model.Repository + HeadGitRepo *git.Repository + HeadRef git.RefName + HeadCommitID string + DirectComparison bool + MergeBase string + Commits []*git.Commit + NumFiles int +} + +func (ci *CompareInfo) IsSameRepository() bool { + return ci.BaseRepo.ID == ci.HeadRepo.ID +} + +func (ci *CompareInfo) IsSameRef() bool { + return ci.IsSameRepository() && ci.BaseRef == ci.HeadRef } // GetCompareInfo generates and returns compare information between base and head branches of repositories. -func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Repository, headGitRepo *git.Repository, baseBranch, headBranch string, directComparison, fileOnly bool) (_ *CompareInfo, err error) { +func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Repository, headGitRepo *git.Repository, baseRef, headRef git.RefName, directComparison, fileOnly bool) (_ *CompareInfo, err error) { var ( remoteBranch string tmpRemote string @@ -46,14 +60,21 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito }() } - compareInfo := new(CompareInfo) + compareInfo := &CompareInfo{ + BaseRepo: baseRepo, + BaseRef: baseRef, + HeadRepo: headRepo, + HeadRef: headRef, + DirectComparison: directComparison, + } - compareInfo.HeadCommitID, err = gitrepo.GetFullCommitID(ctx, headRepo, headBranch) + compareInfo.HeadCommitID, err = gitrepo.GetFullCommitID(ctx, headRepo, headRef.ShortName()) if err != nil { - compareInfo.HeadCommitID = headBranch + compareInfo.HeadCommitID = headRef.ShortName() } - compareInfo.MergeBase, remoteBranch, err = headGitRepo.GetMergeBase(tmpRemote, baseBranch, headBranch) + // FIXME: It seems we don't need mergebase if it's a direct comparison? + compareInfo.MergeBase, remoteBranch, err = headGitRepo.GetMergeBase(tmpRemote, baseRef.ShortName(), headRef.ShortName()) if err == nil { compareInfo.BaseCommitID, err = gitrepo.GetFullCommitID(ctx, headRepo, remoteBranch) if err != nil { @@ -68,7 +89,7 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito // We have a common base - therefore we know that ... should work if !fileOnly { - compareInfo.Commits, err = headGitRepo.ShowPrettyFormatLogToList(ctx, baseCommitID+separator+headBranch) + compareInfo.Commits, err = headGitRepo.ShowPrettyFormatLogToList(ctx, baseCommitID+separator+headRef.ShortName()) if err != nil { return nil, fmt.Errorf("ShowPrettyFormatLogToList: %w", err) } @@ -87,7 +108,7 @@ func GetCompareInfo(ctx context.Context, baseRepo, headRepo *repo_model.Reposito // Count number of changed files. // This probably should be removed as we need to use shortstat elsewhere // Now there is git diff --shortstat but this appears to be slower than simply iterating with --nameonly - compareInfo.NumFiles, err = headGitRepo.GetDiffNumChangedFiles(remoteBranch, headBranch, directComparison) + compareInfo.NumFiles, err = headGitRepo.GetDiffNumChangedFiles(remoteBranch, headRef.ShortName(), directComparison) if err != nil { return nil, err } diff --git a/services/pull/pull.go b/services/pull/pull.go index ecc0b2c7cebce..f8f64dd6501b9 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -34,6 +34,7 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" + git_service "code.gitea.io/gitea/services/git" issue_service "code.gitea.io/gitea/services/issue" notify_service "code.gitea.io/gitea/services/notify" ) @@ -1066,14 +1067,14 @@ func GetPullCommits(ctx context.Context, baseGitRepo *git.Repository, doer *user if pull.HasMerged { baseBranch = pull.MergeBase } - prInfo, err := GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, baseBranch, pull.GetGitHeadRefName(), true, false) + compareInfo, err := git_service.GetCompareInfo(ctx, pull.BaseRepo, pull.BaseRepo, baseGitRepo, git.RefNameFromBranch(baseBranch), git.RefName(pull.GetGitHeadRefName()), true, false) if err != nil { return nil, "", err } - commits := make([]CommitInfo, 0, len(prInfo.Commits)) + commits := make([]CommitInfo, 0, len(compareInfo.Commits)) - for _, commit := range prInfo.Commits { + for _, commit := range compareInfo.Commits { var committerOrAuthorName string var commitTime time.Time if commit.Author != nil {