mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-06 21:07:06 +02:00
feat(api): support ref suffixes in compare
Resolve a `^` or `~N` revision suffix to a commit before comparing, so the compare API accepts `base...head^` and `~N` on either ref. Pull request creation still requires branch refs and the web compare page keeps rejecting suffixes. Closes #33943 Assisted-by: Claude Code:claude-opus-4-8
This commit is contained in:
10 files changed
+133
-23
No files matched your search
+9
-6
@@ -228,13 +228,16 @@ func (ref RefName) RefWebLinkPath() string {
|
|||||||
return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
|
return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
|
||||||
}
|
}
|
||||||
|
|
||||||
func ParseRefSuffix(ref string) (string, string) {
|
func ParseRefSuffix(ref string) (refName, refSuffix string) {
|
||||||
// Partially support https://git-scm.com/docs/gitrevisions
|
// Partially support https://git-scm.com/docs/gitrevisions
|
||||||
if idx := strings.Index(ref, "@{"); idx != -1 {
|
suffixIdx := -1 // earliest suffix mark, so a combined suffix like "main~2^" stays intact
|
||||||
return ref[:idx], ref[idx:]
|
for _, mark := range []string{"@{", "^", "~"} {
|
||||||
|
if idx := strings.Index(ref, mark); idx != -1 && (suffixIdx == -1 || idx < suffixIdx) {
|
||||||
|
suffixIdx = idx
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if idx := strings.Index(ref, "^"); idx != -1 {
|
if suffixIdx == -1 {
|
||||||
return ref[:idx], ref[idx:]
|
return ref, ""
|
||||||
}
|
}
|
||||||
return ref, ""
|
return ref[:suffixIdx], ref[suffixIdx:]
|
||||||
}
|
}
|
||||||
@@ -37,3 +37,22 @@ func TestRefWebLinkPath(t *testing.T) {
|
|||||||
assert.Equal(t, "tag/foo", RefName("refs/tags/foo").RefWebLinkPath())
|
assert.Equal(t, "tag/foo", RefName("refs/tags/foo").RefWebLinkPath())
|
||||||
assert.Equal(t, "commit/c0ffee", RefName("c0ffee").RefWebLinkPath())
|
assert.Equal(t, "commit/c0ffee", RefName("c0ffee").RefWebLinkPath())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestParseRefSuffix(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
ref, name, suffix string
|
||||||
|
}{
|
||||||
|
{"main", "main", ""},
|
||||||
|
{"main^", "main", "^"},
|
||||||
|
{"main^2", "main", "^2"},
|
||||||
|
{"main~3", "main", "~3"},
|
||||||
|
{"main@{yesterday}", "main", "@{yesterday}"},
|
||||||
|
{"main~2^", "main", "~2^"},
|
||||||
|
{"main^~2", "main", "^~2"},
|
||||||
|
}
|
||||||
|
for _, c := range cases {
|
||||||
|
name, suffix := ParseRefSuffix(c.ref)
|
||||||
|
assert.Equal(t, c.name, name, "ref: %s", c.ref)
|
||||||
|
assert.Equal(t, c.suffix, suffix, "ref: %s", c.ref)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -38,7 +38,7 @@ func CompareDiff(ctx *context.APIContext) {
|
|||||||
// required: true
|
// required: true
|
||||||
// - name: basehead
|
// - name: basehead
|
||||||
// in: path
|
// in: path
|
||||||
// description: compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs, including branch names that contain slashes.
|
// description: compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs (including branch names that contain slashes), optionally with a `^` or `~N` revision suffix.
|
||||||
// type: string
|
// type: string
|
||||||
// required: true
|
// required: true
|
||||||
// - name: output
|
// - name: output
|
||||||
|
|||||||
@@ -1087,12 +1087,6 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git
|
|||||||
baseRepo := ctx.Repo.Repository
|
baseRepo := ctx.Repo.Repository
|
||||||
compareReq := common.ParseCompareRouterParam(compareParam)
|
compareReq := common.ParseCompareRouterParam(compareParam)
|
||||||
|
|
||||||
// remove the check when we support compare with carets
|
|
||||||
if compareReq.BaseOriRefSuffix != "" {
|
|
||||||
ctx.APIError(http.StatusBadRequest, "Unsupported comparison syntax: ref with suffix")
|
|
||||||
return nil, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
_, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq)
|
_, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq)
|
||||||
switch {
|
switch {
|
||||||
case errors.Is(err, util.ErrInvalidArgument):
|
case errors.Is(err, util.ErrInvalidArgument):
|
||||||
@@ -1152,10 +1146,10 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git
|
|||||||
return nil, nil
|
return nil, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.BaseOriRef, baseRepo.GetPullRequestTargetBranch(ctx)))
|
baseRef := common.ResolveRefWithSuffix(ctx.Repo.GitRepo, util.IfZero(compareReq.BaseOriRef, baseRepo.GetPullRequestTargetBranch(ctx)), compareReq.BaseOriRefSuffix)
|
||||||
headRef := headGitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch))
|
headRef := common.ResolveRefWithSuffix(headGitRepo, util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch), compareReq.HeadOriRefSuffix)
|
||||||
|
|
||||||
log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef, baseRef, compareReq.HeadOriRef, headRef)
|
log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef+compareReq.BaseOriRefSuffix, baseRef, compareReq.HeadOriRef+compareReq.HeadOriRefSuffix, headRef)
|
||||||
|
|
||||||
baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName())
|
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())
|
headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName())
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ package common
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
repo_model "gitea.dev/models/repo"
|
repo_model "gitea.dev/models/repo"
|
||||||
@@ -19,9 +20,10 @@ type CompareRouterReq struct {
|
|||||||
|
|
||||||
CompareSeparator string
|
CompareSeparator string
|
||||||
|
|
||||||
HeadOwner string
|
HeadOwner string
|
||||||
HeadRepoName string
|
HeadRepoName string
|
||||||
HeadOriRef string
|
HeadOriRef string
|
||||||
|
HeadOriRefSuffix string
|
||||||
}
|
}
|
||||||
|
|
||||||
func (cr *CompareRouterReq) DirectComparison() bool {
|
func (cr *CompareRouterReq) DirectComparison() bool {
|
||||||
@@ -80,8 +82,10 @@ func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
|
|||||||
basePart, headPart, ok = strings.Cut(routerParam, sep)
|
basePart, headPart, ok = strings.Cut(routerParam, sep)
|
||||||
if !ok {
|
if !ok {
|
||||||
headOwnerName, headRepoName, headRef := parseHead(routerParam)
|
headOwnerName, headRepoName, headRef := parseHead(routerParam)
|
||||||
|
headRef, headRefSuffix := git.ParseRefSuffix(headRef)
|
||||||
return &CompareRouterReq{
|
return &CompareRouterReq{
|
||||||
HeadOriRef: headRef,
|
HeadOriRef: headRef,
|
||||||
|
HeadOriRefSuffix: headRefSuffix,
|
||||||
HeadOwner: headOwnerName,
|
HeadOwner: headOwnerName,
|
||||||
HeadRepoName: headRepoName,
|
HeadRepoName: headRepoName,
|
||||||
CompareSeparator: "...",
|
CompareSeparator: "...",
|
||||||
@@ -92,9 +96,30 @@ func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
|
|||||||
ci := &CompareRouterReq{CompareSeparator: sep}
|
ci := &CompareRouterReq{CompareSeparator: sep}
|
||||||
ci.BaseOriRef, ci.BaseOriRefSuffix = git.ParseRefSuffix(basePart)
|
ci.BaseOriRef, ci.BaseOriRefSuffix = git.ParseRefSuffix(basePart)
|
||||||
ci.HeadOwner, ci.HeadRepoName, ci.HeadOriRef = parseHead(headPart)
|
ci.HeadOwner, ci.HeadRepoName, ci.HeadOriRef = parseHead(headPart)
|
||||||
|
ci.HeadOriRef, ci.HeadOriRefSuffix = git.ParseRefSuffix(ci.HeadOriRef)
|
||||||
return ci
|
return ci
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// validRefSuffix matches git ancestry navigation (^, ^N, ~, ~N and combinations). It rejects the
|
||||||
|
// ^{...}, @{...} and :path forms, which would let a reader probe object types or commit messages.
|
||||||
|
var validRefSuffix = regexp.MustCompile(`^(?:[~^][0-9]*)+$`)
|
||||||
|
|
||||||
|
// ResolveRefWithSuffix resolves oriRef plus an optional revision suffix (^, ~N) to a RefName,
|
||||||
|
// returning the peeled commit ID for a suffix or an empty RefName when it can not be resolved.
|
||||||
|
func ResolveRefWithSuffix(gitRepo *git.Repository, oriRef, refSuffix string) git.RefName {
|
||||||
|
if refSuffix == "" {
|
||||||
|
return gitRepo.UnstableGuessRefByShortName(oriRef)
|
||||||
|
}
|
||||||
|
if !validRefSuffix.MatchString(refSuffix) {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
commit, err := gitRepo.GetCommit(oriRef + refSuffix)
|
||||||
|
if err != nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
return git.RefNameFromCommit(commit.ID.String())
|
||||||
|
}
|
||||||
|
|
||||||
// maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository.
|
// maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository.
|
||||||
const maxForkTraverseLevel = 10
|
const maxForkTraverseLevel = 10
|
||||||
|
|
||||||
|
|||||||
@@ -97,6 +97,43 @@ func TestCompareRouterReq(t *testing.T) {
|
|||||||
HeadOriRef: "develop",
|
HeadOriRef: "develop",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
input: "main...develop^",
|
||||||
|
CompareRouterReq: &CompareRouterReq{
|
||||||
|
BaseOriRef: "main",
|
||||||
|
CompareSeparator: "...",
|
||||||
|
HeadOriRef: "develop",
|
||||||
|
HeadOriRefSuffix: "^",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
input: "main~2...develop",
|
||||||
|
CompareRouterReq: &CompareRouterReq{
|
||||||
|
BaseOriRef: "main",
|
||||||
|
BaseOriRefSuffix: "~2",
|
||||||
|
CompareSeparator: "...",
|
||||||
|
HeadOriRef: "develop",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
input: "main...lunny/forked_repo:develop~3",
|
||||||
|
CompareRouterReq: &CompareRouterReq{
|
||||||
|
BaseOriRef: "main",
|
||||||
|
CompareSeparator: "...",
|
||||||
|
HeadOwner: "lunny",
|
||||||
|
HeadRepoName: "forked_repo",
|
||||||
|
HeadOriRef: "develop",
|
||||||
|
HeadOriRefSuffix: "~3",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
input: "develop^",
|
||||||
|
CompareRouterReq: &CompareRouterReq{
|
||||||
|
CompareSeparator: "...",
|
||||||
|
HeadOriRef: "develop",
|
||||||
|
HeadOriRefSuffix: "^",
|
||||||
|
},
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, c := range cases {
|
for _, c := range cases {
|
||||||
|
|||||||
@@ -208,8 +208,8 @@ func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context, comparePa
|
|||||||
// 1 Parse compare router param
|
// 1 Parse compare router param
|
||||||
compareReq := common.ParseCompareRouterParam(compareParam)
|
compareReq := common.ParseCompareRouterParam(compareParam)
|
||||||
|
|
||||||
// remove the check when we support compare with carets
|
// remove the check when the web compare page supports carets
|
||||||
if compareReq.BaseOriRefSuffix != "" {
|
if compareReq.BaseOriRefSuffix != "" || compareReq.HeadOriRefSuffix != "" {
|
||||||
return util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix")
|
return util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Generated
+1
-1
@@ -8135,7 +8135,7 @@
|
|||||||
},
|
},
|
||||||
{
|
{
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs, including branch names that contain slashes.",
|
"description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs (including branch names that contain slashes), optionally with a `^` or `~N` revision suffix.",
|
||||||
"name": "basehead",
|
"name": "basehead",
|
||||||
"in": "path",
|
"in": "path",
|
||||||
"required": true
|
"required": true
|
||||||
|
|||||||
Generated
+1
-1
@@ -19490,7 +19490,7 @@
|
|||||||
}
|
}
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs, including branch names that contain slashes.",
|
"description": "compare two refs as `base...head` (or `base..head`); refs may be branches, tags, full or short SHAs (including branch names that contain slashes), optionally with a `^` or `~N` revision suffix.",
|
||||||
"in": "path",
|
"in": "path",
|
||||||
"name": "basehead",
|
"name": "basehead",
|
||||||
"required": true,
|
"required": true,
|
||||||
|
|||||||
@@ -45,6 +45,38 @@ func TestAPICompareBranches(t *testing.T) {
|
|||||||
assert.Len(t, apiResp.Commits, 1)
|
assert.Len(t, apiResp.Commits, 1)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("CompareWithRefSuffix", func(t *testing.T) {
|
||||||
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
|
||||||
|
// remove-files-b^ is the parent of the tip, so the range drops the tip and ends at that parent
|
||||||
|
const parentSHA = "b67e43a07d48243a5f670ace063acd5e13f719df"
|
||||||
|
req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b^").AddTokenAuth(token2)
|
||||||
|
resp := MakeRequest(t, req, http.StatusOK)
|
||||||
|
apiResp := DecodeJSON(t, resp, &api.Compare{})
|
||||||
|
assert.Equal(t, 1, apiResp.TotalCommits)
|
||||||
|
assert.Len(t, apiResp.Commits, 1)
|
||||||
|
assert.Equal(t, parentSHA, apiResp.Commits[0].SHA)
|
||||||
|
|
||||||
|
// the same suffix on the direct ".." comparison resolves to the same commit
|
||||||
|
req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv..remove-files-b^").AddTokenAuth(token2)
|
||||||
|
resp = MakeRequest(t, req, http.StatusOK)
|
||||||
|
apiResp = DecodeJSON(t, resp, &api.Compare{})
|
||||||
|
assert.Equal(t, 1, apiResp.TotalCommits)
|
||||||
|
assert.Equal(t, parentSHA, apiResp.Commits[0].SHA)
|
||||||
|
|
||||||
|
req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv~1...add-csv").AddTokenAuth(token2)
|
||||||
|
resp = MakeRequest(t, req, http.StatusOK)
|
||||||
|
apiResp = DecodeJSON(t, resp, &api.Compare{})
|
||||||
|
assert.Equal(t, 1, apiResp.TotalCommits)
|
||||||
|
assert.Len(t, apiResp.Commits, 1)
|
||||||
|
|
||||||
|
// an unresolvable suffix is not found, and only ^/~ navigation is accepted (^{...} is rejected)
|
||||||
|
req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b~50").AddTokenAuth(token2)
|
||||||
|
MakeRequest(t, req, http.StatusNotFound)
|
||||||
|
req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv...remove-files-b^{/Add}").AddTokenAuth(token2)
|
||||||
|
MakeRequest(t, req, http.StatusNotFound)
|
||||||
|
})
|
||||||
|
|
||||||
t.Run("CompareForkOnlyCommit", func(t *testing.T) {
|
t.Run("CompareForkOnlyCommit", func(t *testing.T) {
|
||||||
defer tests.PrintCurrentTest(t)()
|
defer tests.PrintCurrentTest(t)()
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user