Fix compare dropdown for branches without common history (#37470)

This commit is contained in:
Nicolas
2026-04-28 23:03:50 +02:00
committed by GitHub
parent fedc9dc993
commit deec2b0929
8 changed files with 160 additions and 154 deletions

View File

@@ -189,8 +189,8 @@ func setCsvCompareContext(ctx *context.Context) {
}
}
// ParseCompareInfo parse compare info between two commit for preparing comparing references
func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
// parseCompareInfo parse compare info between two commit for preparing comparing references
func parseCompareInfo(ctx *context.Context) (*git_service.CompareInfo, error) {
baseRepo := ctx.Repo.Repository
fileOnly := ctx.FormBool("file-only")
@@ -199,47 +199,29 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
// remove the check when we support compare with carets
if compareReq.BaseOriRefSuffix != "" {
ctx.HTTPError(http.StatusBadRequest, "Unsupported comparison syntax: ref with suffix")
return nil
return nil, util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix")
}
// 2 get repository and owner for head
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
if err != nil {
return nil, err
}
isSameRepo := baseRepo.ID == headRepo.ID
// 3 permission check
// base repository's code unit read permission check has been done on web.go
permBase := ctx.Repo.Permission
// If we're not merging from the same repo:
isSameRepo := baseRepo.ID == headRepo.ID
if !isSameRepo {
// Assert ctx.Doer has permission to read headRepo's codes
permHead, err := access_model.GetDoerRepoPermission(ctx, headRepo, ctx.Doer)
if err != nil {
ctx.ServerError("GetDoerRepoPermission", err)
return nil
return nil, err
}
if !permHead.CanRead(unit.TypeCode) {
if log.IsTrace() {
log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v",
ctx.Doer,
headRepo,
permHead)
}
ctx.NotFound(nil)
return nil
return nil, util.NewNotExistErrorf("") // permission: no error message for end users
}
ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
}
@@ -250,24 +232,17 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefName)
if baseRef == "" {
ctx.NotFound(nil)
return nil
return nil, util.NewNotExistErrorf("no base ref: %s", baseRefName)
}
var headGitRepo *git.Repository
if isSameRepo {
headGitRepo = ctx.Repo.GitRepo
} else {
headGitRepo, err = gitrepo.OpenRepository(ctx, headRepo)
if err != nil {
ctx.ServerError("OpenRepository", err)
return nil
}
defer headGitRepo.Close()
headGitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, headRepo)
if err != nil {
ctx.ServerError("OpenRepository", err)
return nil, err
}
headRef := headGitRepo.UnstableGuessRefByShortName(headRefName)
if headRef == "" {
ctx.NotFound(nil)
return nil
return nil, util.NewNotExistErrorf("no head ref: %s", headRefName)
}
ctx.Data["BaseName"] = baseRepo.OwnerName
@@ -291,12 +266,9 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
var rootRepo *repo_model.Repository
if baseRepo.IsFork {
err = baseRepo.GetBaseRepo(ctx)
if err != nil {
if !repo_model.IsErrRepoNotExist(err) {
ctx.ServerError("Unable to find root repo", err)
return nil
}
} else {
if err != nil && !repo_model.IsErrRepoNotExist(err) {
return nil, err
} else if err == nil {
rootRepo = baseRepo.BaseRepo
}
}
@@ -313,42 +285,10 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
}
}
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 == 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 == headOwner.ID {
headRepo = ownForkRepo
has = true
}
// 5. If the headOwner has a fork of the baseRepo - use that
if !has {
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 {
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
if !isSameRepo && !has {
ctx.Data["PageIsComparePull"] = false
}
ctx.Data["HeadRepo"] = headRepo
ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository
// If we have a rootRepo and it's different from:
// If we have a rootRepo, and it's different from:
// 1. the computed base
// 2. the computed head
// then get the branches of it
@@ -361,17 +301,15 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
if !fileOnly {
branches, tags, err := getBranchesAndTagsForRepo(ctx, rootRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil
return nil, err
}
ctx.Data["RootRepoBranches"] = branches
ctx.Data["RootRepoTags"] = tags
}
}
}
// If we have a ownForkRepo and it's different from:
// If we have a ownForkRepo, and it's different from:
// 1. The computed base
// 2. The computed head
// 3. The rootRepo (if we have one)
@@ -386,8 +324,7 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
if !fileOnly {
branches, tags, err := getBranchesAndTagsForRepo(ctx, ownForkRepo)
if err != nil {
ctx.ServerError("GetBranchesForRepo", err)
return nil
return nil, err
}
ctx.Data["OwnForkRepoBranches"] = branches
ctx.Data["OwnForkRepoTags"] = tags
@@ -395,33 +332,21 @@ func ParseCompareInfo(ctx *context.Context) *git_service.CompareInfo {
}
}
// Treat as pull request if both references are branches
if ctx.Data["PageIsComparePull"] == nil {
ctx.Data["PageIsComparePull"] = baseRef.IsBranch() && headRef.IsBranch() && permBase.CanReadIssuesOrPulls(true)
}
if ctx.Data["PageIsComparePull"] == true && !permBase.CanReadIssuesOrPulls(true) {
if log.IsTrace() {
log.Trace("Permission Denied: User: %-v cannot create/read pull requests in Repo: %-v\nUser in baseRepo has Permissions: %-+v",
ctx.Doer,
baseRepo,
permBase)
}
ctx.NotFound(nil)
return nil
}
compareInfo, err := git_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef, headRef, compareReq.DirectComparison(), fileOnly)
if err != nil {
ctx.ServerError("GetCompareInfo", err)
return nil
return nil, err
}
// Treat as pull request if both references are branches
allowCreatePullRequest := baseRef.IsBranch() && headRef.IsBranch() && permBase.CanReadIssuesOrPulls(true)
allowCreatePullRequest = allowCreatePullRequest && compareInfo.MergeBase != ""
ctx.Data["PageIsComparePull"] = allowCreatePullRequest
if compareReq.DirectComparison() {
ctx.Data["BeforeCommitID"] = compareInfo.BaseCommitID
} else {
ctx.Data["BeforeCommitID"] = compareInfo.MergeBase
}
return &compareInfo
return &compareInfo, nil
}
func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*git_model.SignCommitWithStatuses) (title, content string) {
@@ -454,12 +379,11 @@ func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*g
return title, content
}
// PrepareCompareDiff renders compare diff page
func PrepareCompareDiff(
ctx *context.Context,
ci *git_service.CompareInfo,
whitespaceBehavior gitcmd.TrustedCmdArgs,
) (nothingToCompare bool) {
// prepareCompareDiff renders compare diff page. TODO: need to refactor it and other "compare diff" related functions together
func prepareCompareDiff(ctx *context.Context, ci *git_service.CompareInfo, whitespaceBehavior gitcmd.TrustedCmdArgs) (nothingToCompare bool) {
if ci.MergeBase == "" {
return true
}
repo := ctx.Repo.Repository
headCommitID := ci.HeadCommitID
@@ -568,9 +492,6 @@ func PrepareCompareDiff(
ctx.Data["CommitCount"] = len(commits)
ctx.Data["title"], ctx.Data["content"] = prepareNewPullRequestTitleContent(ci, commits)
ctx.Data["Username"] = ci.HeadRepo.OwnerName
ctx.Data["Reponame"] = ci.HeadRepo.Name
setCompareContext(ctx, beforeCommit, headCommit, ci.HeadRepo.OwnerName, repo.Name)
return false
@@ -594,16 +515,24 @@ func getBranchesAndTagsForRepo(ctx gocontext.Context, repo *repo_model.Repositor
// CompareDiff show different from one commit to another commit
func CompareDiff(ctx *context.Context) {
ci := ParseCompareInfo(ctx)
ci, err := parseCompareInfo(ctx)
if ctx.Written() {
return
}
if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrInvalidArgument) {
ctx.NotFound(nil)
return
} else if err != nil {
ctx.ServerError("ParseCompareInfo", err)
return
}
ctx.Data["PageIsViewCode"] = true
ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
ctx.Data["CompareInfo"] = ci
nothingToCompare := PrepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
// TODO: need to refactor "prepare compare" related functions together
nothingToCompare := prepareCompareDiff(ctx, ci, gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)))
if ctx.Written() {
return
}
@@ -621,16 +550,13 @@ func CompareDiff(ctx *context.Context) {
return
}
headBranches, err := git_model.FindBranchNames(ctx, git_model.FindBranchOptions{
RepoID: ci.HeadRepo.ID,
ListOptions: db.ListOptionsAll,
IsDeletedBranch: optional.Some(false),
})
headBranches, headTags, err := getBranchesAndTagsForRepo(ctx, ci.HeadRepo)
if err != nil {
ctx.ServerError("GetBranches", err)
ctx.ServerError("GetBranchesAndTagsForRepo", err)
return
}
ctx.Data["HeadBranches"] = headBranches
ctx.Data["HeadTags"] = headTags
// For compare repo branches
PrepareBranchList(ctx)
@@ -638,13 +564,20 @@ func CompareDiff(ctx *context.Context) {
return
}
headTags, err := repo_model.GetTagNamesByRepoID(ctx, ci.HeadRepo.ID)
if err != nil {
ctx.ServerError("GetTagNamesByRepoID", err)
return
if ci.MergeBase != "" {
prepareCreatePullRequestPage(ctx, ci, nothingToCompare)
if ctx.Written() {
return
}
} else {
ctx.Flash.Error(ctx.Tr("repo.pulls.no_common_history"), true)
ctx.Data["PageIsComparePull"] = false
ctx.Data["CommitCount"] = 0
}
ctx.Data["HeadTags"] = headTags
ctx.HTML(http.StatusOK, tplCompare)
}
func prepareCreatePullRequestPage(ctx *context.Context, ci *git_service.CompareInfo, nothingToCompare bool) {
if ctx.Data["PageIsComparePull"] == true {
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 {
@@ -685,7 +618,7 @@ func CompareDiff(ctx *context.Context) {
if content, ok := ctx.Data["content"].(string); ok && content != "" {
// If a template content is set, prepend the "content". In this case that's only
// applicable if you have one commit to compare and that commit has a message.
// In that case the commit message will be prepend to the template body.
// In that case the commit message will be prepended to the template body.
if templateContent, ok := ctx.Data[pullRequestTemplateKey].(string); ok && templateContent != "" {
// Re-use the same key as that's prioritized over the "content" key.
// Add two new lines between the content to ensure there's always at least
@@ -713,14 +646,8 @@ func CompareDiff(ctx *context.Context) {
ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.Permission.CanWrite(unit.TypePullRequests)
if unit, err := ctx.Repo.Repository.GetUnit(ctx, unit.TypePullRequests); err == nil {
config := unit.PullRequestsConfig()
ctx.Data["AllowMaintainerEdit"] = config.DefaultAllowMaintainerEdit
} else {
ctx.Data["AllowMaintainerEdit"] = false
}
ctx.HTML(http.StatusOK, tplCompare)
prConfig := ctx.Repo.Repository.MustGetUnit(ctx, unit.TypePullRequests).PullRequestsConfig()
ctx.Data["AllowMaintainerEdit"] = prConfig.DefaultAllowMaintainerEdit
}
// attachCommentsToLines attaches comments to their corresponding diff lines

View File

@@ -1281,24 +1281,26 @@ func PullsNewRedirect(ctx *context.Context) {
// CompareAndPullRequestPost response for creating pull request
func CompareAndPullRequestPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.CreateIssueForm)
ctx.Data["Title"] = ctx.Tr("repo.pulls.compare_changes")
ctx.Data["PageIsComparePull"] = true
ctx.Data["IsDiffCompare"] = true
ctx.Data["PullRequestWorkInProgressPrefixes"] = setting.Repository.PullRequest.WorkInProgressPrefixes
ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled
upload.AddUploadContext(ctx, "comment")
ctx.Data["HasIssuesOrPullsWritePermission"] = ctx.Repo.Permission.CanWrite(unit.TypePullRequests)
repo := ctx.Repo.Repository
var (
repo = ctx.Repo.Repository
attachments []string
)
ci := ParseCompareInfo(ctx)
ci, err := parseCompareInfo(ctx)
if ctx.Written() {
return
}
if errors.Is(err, util.ErrNotExist) {
ctx.JSONErrorNotFound()
return
} else if errors.Is(err, util.ErrInvalidArgument) {
ctx.JSONError(err.Error())
return
} else if err != nil {
ctx.ServerError("ParseCompareInfo", err)
return
}
if ci.MergeBase == "" {
ctx.JSONError(ctx.Tr("repo.pulls.no_common_history"))
return
}
validateRet := ValidateRepoMetasForNewIssue(ctx, *form, true)
if ctx.Written() {
return
@@ -1306,6 +1308,7 @@ func CompareAndPullRequestPost(ctx *context.Context) {
labelIDs, assigneeIDs, milestoneID, projectID := validateRet.LabelIDs, validateRet.AssigneeIDs, validateRet.MilestoneID, validateRet.ProjectID
var attachments []string
if setting.Attachment.Enabled {
attachments = form.Files
}