diff --git a/models/user/user.go b/models/user/user.go index 925be83713cd2..1797d3eefc2b5 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1461,3 +1461,15 @@ func GetUserOrOrgIDByName(ctx context.Context, name string) (int64, error) { } return id, nil } + +// GetUserOrOrgByName returns the user or org by name +func GetUserOrOrgByName(ctx context.Context, name string) (*User, error) { + var u User + has, err := db.GetEngine(ctx).Where("lower_name = ?", strings.ToLower(name)).Get(&u) + if err != nil { + return nil, err + } else if !has { + return nil, ErrUserNotExist{Name: name} + } + return &u, nil +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 981d9de2f8623..6ac612a481d13 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1862,6 +1862,7 @@ pulls.desc = Enable pull requests and code reviews. pulls.new = New Pull Request pulls.new.blocked_user = Cannot create pull request because you are blocked by the repository owner. pulls.new.must_collaborator = You must be a collaborator to create pull request. +pulls.new.already_existed = A pull request between these branches already exists pulls.edit.already_changed = Unable to save changes to the pull request. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes. pulls.view = View Pull Request pulls.compare_changes = New Pull Request 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 073c7842424af..209647e7d7617 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -28,13 +28,16 @@ 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" asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/automerge" "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" @@ -413,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 { @@ -504,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, } @@ -1056,70 +1059,42 @@ 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.GetUserByName(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 - - // Check if current user has fork of repository or in the same repository. - headRepo := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) - if headRepo == nil && !isSameRepo { - err = baseRepo.GetBaseRepo(ctx) - if err != nil { - ctx.APIErrorInternal(err) - return nil, nil - } + // 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 + } - // Check if baseRepo's base repository is the same as headUser's repository. - if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID { - log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID) - ctx.APIErrorNotFound("GetBaseRepo") - return nil, nil - } - // Assign headRepo so it can be used below. - headRepo = baseRepo.BaseRepo + _, 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 { - headRepo = ctx.Repo.Repository headGitRepo = ctx.Repo.GitRepo closer = func() {} // no need to close the head repo because it shares the base repo } else { @@ -1143,9 +1118,9 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return nil, nil } - if !permBase.CanReadIssuesOrPulls(true) || !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") + if !permBase.CanRead(unit.TypeCode) { + 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 } @@ -1162,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()) @@ -1175,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 @@ -1426,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) @@ -1435,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) @@ -1462,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, @@ -1556,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) @@ -1573,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 fda31a07ba736..749a11a7fba47 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -4,19 +4,201 @@ 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. +const maxForkTraverseLevel = 10 + +// FindHeadRepo tries to find the head repository based on the base repository and head user ID. +func FindHeadRepo(ctx context.Context, baseRepo *repo_model.Repository, headUserID int64) (*repo_model.Repository, error) { + if baseRepo.IsFork { + curRepo := baseRepo + for curRepo.OwnerID != headUserID { // We assume the fork deepth is not too deep. + if err := curRepo.GetBaseRepo(ctx); err != nil { + return nil, err + } + if curRepo.BaseRepo == nil { + return findHeadRepoFromRootBase(ctx, curRepo, headUserID, maxForkTraverseLevel) + } + curRepo = curRepo.BaseRepo + } + return curRepo, nil + } + + return findHeadRepoFromRootBase(ctx, baseRepo, headUserID, maxForkTraverseLevel) +} + +func findHeadRepoFromRootBase(ctx context.Context, baseRepo *repo_model.Repository, headUserID int64, traverseLevel int) (*repo_model.Repository, error) { + if traverseLevel == 0 { + return nil, nil + } + repo, err := repo_model.GetUserFork(ctx, baseRepo.ID, headUserID) + if err != nil { + return nil, err + } + if repo != nil { + return repo, nil + } + + firstLevelForkedRepos, err := repo_model.GetRepositoriesByForkID(ctx, baseRepo.ID) + if err != nil { + return nil, err + } + for _, repo := range firstLevelForkedRepos { + forked, err := findHeadRepoFromRootBase(ctx, repo, headUserID, traverseLevel-1) + if err != nil { + return nil, err + } + if forked != nil { + return forked, nil + } + } + return nil, nil +} + +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 } 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 7750278a8d4bd..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.GetUserByName(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 4353e00840f92..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 @@ -1340,6 +1341,17 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } + // 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.HeadRef.ShortName(), ci.BaseRef.ShortName(), issues_model.PullRequestFlowGithub) + if err != nil && !issues_model.IsErrPullRequestNotExist(err) { + ctx.ServerError("GetUnmergedPullRequest", err) + return + } + if pr != nil { + ctx.JSONError(ctx.Tr("repo.pulls.new.already_existed")) + return + } + content := form.Content if filename := ctx.Req.Form.Get("template-file"); filename != "" { if template, err := issue_template.UnmarshalFromRepo(ctx.Repo.GitRepo, ctx.Repo.Repository.DefaultBranch, filename); err == nil { @@ -1360,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 67% rename from services/pull/compare.go rename to services/git/compare.go index c2d39752e8656..0d6cb107bcdc1 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,22 +18,36 @@ 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 ) // We don't need a temporary remote for same repository. - if headGitRepo.Path != baseRepo.RepoPath() { + if headRepo.ID != baseRepo.ID { // Add a temporary remote tmpRemote = strconv.FormatInt(time.Now().UnixNano(), 10) if err = gitrepo.GitRemoteAdd(ctx, headRepo, tmpRemote, baseRepo.RepoPath()); err != nil { @@ -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 { diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index ddafdf33b837d..ff60b70cf9a86 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -4,6 +4,7 @@ package integration import ( + "encoding/base64" "fmt" "net/http" "net/http/httptest" @@ -17,7 +18,9 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git/gitcmd" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -153,8 +156,16 @@ func TestPullCreate(t *testing.T) { url := test.RedirectURL(resp) assert.Regexp(t, "^/user2/repo1/pulls/[0-9]*$", url) + // test create the pull request again and it should fail now + link := "/user2/repo1/compare/master...user1/repo1:master" + req := NewRequestWithValues(t, "POST", link, map[string]string{ + "_csrf": GetUserCSRFToken(t, session), + "title": "This is a pull title", + }) + session.MakeRequest(t, req, http.StatusBadRequest) + // check .diff can be accessed and matches performed change - req := NewRequest(t, "GET", url+".diff") + req = NewRequest(t, "GET", url+".diff") resp = session.MakeRequest(t, req, http.StatusOK) assert.Regexp(t, `\+Hello, World \(Edited\)`, resp.Body) assert.Regexp(t, "^diff", resp.Body) @@ -295,6 +306,95 @@ func TestPullCreatePrFromBaseToFork(t *testing.T) { }) } +func TestCreatePullRequestFromNestedOrgForks(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteOrganization) + + const ( + baseOrg = "test-fork-org1" + midForkOrg = "test-fork-org2" + leafForkOrg = "test-fork-org3" + repoName = "test-fork-repo" + patchBranch = "teabot-patch-1" + ) + + createOrg := func(name string) { + req := NewRequestWithJSON(t, "POST", "/api/v1/orgs", &api.CreateOrgOption{ + UserName: name, + Visibility: "public", + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + } + + createOrg(baseOrg) + createOrg(midForkOrg) + createOrg(leafForkOrg) + + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/repos", baseOrg), &api.CreateRepoOption{ + Name: repoName, + AutoInit: true, + DefaultBranch: "main", + Private: false, + Readme: "Default", + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusCreated) + var baseRepo api.Repository + DecodeJSON(t, resp, &baseRepo) + assert.Equal(t, "main", baseRepo.DefaultBranch) + + forkIntoOrg := func(srcOrg, dstOrg string) api.Repository { + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/forks", srcOrg, repoName), &api.CreateForkOption{ + Organization: util.ToPointer(dstOrg), + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusAccepted) + var forkRepo api.Repository + DecodeJSON(t, resp, &forkRepo) + assert.NotNil(t, forkRepo.Owner) + if forkRepo.Owner != nil { + assert.Equal(t, dstOrg, forkRepo.Owner.UserName) + } + return forkRepo + } + + forkIntoOrg(baseOrg, midForkOrg) + forkIntoOrg(midForkOrg, leafForkOrg) + + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", leafForkOrg, repoName, "patch-from-org3.txt"), &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + BranchName: "main", + NewBranchName: patchBranch, + Message: "create patch from org3", + }, + ContentBase64: base64.StdEncoding.EncodeToString([]byte("patch content")), + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + + prPayload := map[string]string{ + "head": fmt.Sprintf("%s:%s", leafForkOrg, patchBranch), + "base": "main", + "title": "test creating pull from test-fork-org3 to test-fork-org1", + } + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/pulls", baseOrg, repoName), prPayload).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusCreated) + var pr api.PullRequest + DecodeJSON(t, resp, &pr) + assert.Equal(t, prPayload["title"], pr.Title) + if assert.NotNil(t, pr.Head) { + assert.Equal(t, patchBranch, pr.Head.Ref) + if assert.NotNil(t, pr.Head.Repository) { + assert.Equal(t, fmt.Sprintf("%s/%s", leafForkOrg, repoName), pr.Head.Repository.FullName) + } + } + if assert.NotNil(t, pr.Base) { + assert.Equal(t, "main", pr.Base.Ref) + if assert.NotNil(t, pr.Base.Repository) { + assert.Equal(t, fmt.Sprintf("%s/%s", baseOrg, repoName), pr.Base.Repository.FullName) + } + } + }) +} + func TestPullCreateParallel(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { sessionFork := loginUser(t, "user1")