Compare commits

..
Author SHA1 Message Date
Eyüp Can AkmanandGitHub ef927f9fa3 feat(api): support ref suffixes in compare (#38148)
Compare API requests with a `^` or `~N` revision suffix (for example
`compare/main...feature^`) were rejected with `400 Unsupported
comparison syntax: ref with suffix`. The fix resolves the suffix to a
commit before comparing, so `base...head^` and `~N` work on either side,
the same as git.

Only `^`/`~N` navigation is resolved. Pull request creation still
requires plain branch refs, and the web compare page keeps rejecting
suffixes since its branch selectors need separate UI work.

Closes #33943
2026-06-24 05:38:02 +00:00
15 changed files with 320 additions and 85 deletions
+9 -6
View File
@@ -228,13 +228,16 @@ func (ref RefName) RefWebLinkPath() string {
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
if idx := strings.Index(ref, "@{"); idx != -1 {
return ref[:idx], ref[idx:]
suffixIdx := -1 // earliest suffix mark, so a combined suffix like "main~2^" stays intact
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 {
return ref[:idx], ref[idx:]
if suffixIdx == -1 {
return ref, ""
}
return ref, ""
return ref[:suffixIdx], ref[suffixIdx:]
}
+19
View File
@@ -37,3 +37,22 @@ func TestRefWebLinkPath(t *testing.T) {
assert.Equal(t, "tag/foo", RefName("refs/tags/foo").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)
}
}
+4 -4
View File
@@ -1,6 +1,6 @@
{
"type": "module",
"packageManager": "pnpm@11.8.0",
"packageManager": "pnpm@11.7.0",
"engines": {
"node": ">= 22.18.0",
"pnpm": ">= 11.0.0"
@@ -36,7 +36,7 @@
"@resvg/resvg-wasm": "2.6.2",
"@vitejs/plugin-vue": "6.0.7",
"ansi_up": "6.0.6",
"asciinema-player": "3.16.0",
"asciinema-player": "3.15.1",
"chart.js": "4.5.1",
"chartjs-adapter-dayjs-4": "1.0.4",
"chartjs-plugin-zoom": "2.2.0",
@@ -99,13 +99,13 @@
"eslint-plugin-import-x": "4.16.2",
"eslint-plugin-playwright": "2.10.4",
"eslint-plugin-regexp": "3.1.0",
"eslint-plugin-sonarjs": "4.1.0",
"eslint-plugin-sonarjs": "4.0.3",
"eslint-plugin-unicorn": "68.0.0",
"eslint-plugin-vue": "10.9.2",
"eslint-plugin-vue-scoped-css": "3.1.1",
"eslint-plugin-wc": "3.1.0",
"globals": "17.6.0",
"happy-dom": "20.10.6",
"happy-dom": "20.10.5",
"jiti": "2.7.0",
"markdownlint-cli": "0.49.0",
"material-icon-theme": "5.35.0",
+37 -47
View File
@@ -94,13 +94,13 @@ importers:
version: 2.6.2
'@vitejs/plugin-vue':
specifier: 6.0.7
version: 6.0.7(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0))(vue@3.5.38(typescript@6.0.3))
version: 6.0.7(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0))(vue@3.5.38(typescript@6.0.3))
ansi_up:
specifier: 6.0.6
version: 6.0.6
asciinema-player:
specifier: 3.16.0
version: 3.16.0
specifier: 3.15.1
version: 3.15.1
chart.js:
specifier: 4.5.1
version: 4.5.1
@@ -172,7 +172,7 @@ importers:
version: 5.32.6
tailwindcss:
specifier: 3.4.19
version: 3.4.19(yaml@2.9.0)
version: 3.4.19
throttle-debounce:
specifier: 5.0.2
version: 5.0.2
@@ -193,10 +193,10 @@ importers:
version: 0.7.2
vite:
specifier: 8.0.16
version: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)
version: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)
vite-string-plugin:
specifier: 2.0.4
version: 2.0.4(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0))
version: 2.0.4(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0))
vue:
specifier: 3.5.38
version: 3.5.38(typescript@6.0.3)
@@ -257,7 +257,7 @@ importers:
version: 8.61.1(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)
'@vitest/eslint-plugin':
specifier: 1.6.20
version: 1.6.20(@typescript-eslint/eslint-plugin@8.61.1(@typescript-eslint/parser@8.61.1(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)(vitest@4.1.9(@types/node@25.9.3)(happy-dom@20.10.6)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)))
version: 1.6.20(@typescript-eslint/eslint-plugin@8.61.1(@typescript-eslint/parser@8.61.1(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)(vitest@4.1.9(@types/node@25.9.3)(happy-dom@20.10.5)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)))
eslint:
specifier: 10.5.0
version: 10.5.0(jiti@2.7.0)
@@ -280,8 +280,8 @@ importers:
specifier: 3.1.0
version: 3.1.0(eslint@10.5.0(jiti@2.7.0))
eslint-plugin-sonarjs:
specifier: 4.1.0
version: 4.1.0(eslint@10.5.0(jiti@2.7.0))
specifier: 4.0.3
version: 4.0.3(eslint@10.5.0(jiti@2.7.0))
eslint-plugin-unicorn:
specifier: 68.0.0
version: 68.0.0(eslint@10.5.0(jiti@2.7.0))
@@ -298,8 +298,8 @@ importers:
specifier: 17.6.0
version: 17.6.0
happy-dom:
specifier: 20.10.6
version: 20.10.6
specifier: 20.10.5
version: 20.10.5
jiti:
specifier: 2.7.0
version: 2.7.0
@@ -344,7 +344,7 @@ importers:
version: 17.18.0
vitest:
specifier: 4.1.9
version: 4.1.9(@types/node@25.9.3)(happy-dom@20.10.6)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0))
version: 4.1.9(@types/node@25.9.3)(happy-dom@20.10.5)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0))
vue-tsc:
specifier: 3.3.5
version: 3.3.5(typescript@6.0.3)
@@ -1750,8 +1750,8 @@ packages:
resolution: {integrity: sha512-BNoCY6SXXPQ7gF2opIP4GBE+Xw7U+pHMYKuzjgCN3GwiaIR09UUeKfheyIry77QtrCBlC0KK0q5/TER/tYh3PQ==}
engines: {node: '>= 0.4'}
asciinema-player@3.16.0:
resolution: {integrity: sha512-bl6U6QD6/2as3F8AaNnAQ1dAZMq9Q/mvlSE5XxQrththGcDQS+jqAUCEr3jrUl2xatIQwblNIyJuFUpX8qjK+g==}
asciinema-player@3.15.1:
resolution: {integrity: sha512-agVYeNlPxthLyAb92l9AS7ypW0uhesqOuQzyR58Q4Sj+MvesQztZBgx86lHqNJkB8rQ6EP0LeA9czGytQUBpYw==}
asn1@0.2.6:
resolution: {integrity: sha512-ix/FxPn0MDjeyJ7i/yoHGFt/EX6LyNbxSEhPPXODPL+KB0VPk86UYfL0lMdy+KCnv+fmvIzySwaK5COwqVbWTQ==}
@@ -2565,8 +2565,8 @@ packages:
peerDependencies:
eslint: '>=9.38.0'
eslint-plugin-sonarjs@4.1.0:
resolution: {integrity: sha512-rh+FlVz0yfd2RNIb6WqSkuGh0addX/Qi5scwQ5FphXDFrM6fZKcxP1+attJ78yUKcyYfiu6MTaISPpAFPzqRJw==}
eslint-plugin-sonarjs@4.0.3:
resolution: {integrity: sha512-5drkJKLC9qQddIiaATV0e8+ygbUc7b0Ti6VB7M2d3jmKNh3X0RaiIJYTs3dr9xnlhlrxo+/s1FoO3Jgv6O/c7g==}
peerDependencies:
eslint: ^8.0.0 || ^9.0.0 || ^10.0.0
@@ -2886,8 +2886,8 @@ packages:
resolution: {integrity: sha512-tSQXBXS/MWQOn/RKckawJ61vvsDpCom87JgxiYdGwHdOa0ht0vzUWDlfioofFCRU0L+6NGDt6XzbgoJvZkMeRQ==}
engines: {node: '>=0.8.0'}
happy-dom@20.10.6:
resolution: {integrity: sha512-6QD0ilzDDt93tX44y8tbmZdAcdTRYDhUP+Asgi6pC8Pp5IA3cvaZGyoVN/EGtlq9ziT65iPuBBn3ASLr6hCgVw==}
happy-dom@20.10.5:
resolution: {integrity: sha512-0aA6BQoMnpcRE/c1E8ZyF2jXnET7MJskereWOXher4CJuYjrI5esN0Az/1NPMD4KeWUbampBGw2MGqabMPFIbg==}
engines: {node: '>=20.0.0'}
har-schema@2.0.0:
@@ -4761,11 +4761,6 @@ packages:
xmlchars@2.2.0:
resolution: {integrity: sha512-JZnDKK8B0RCDw84FNdDAIpZK+JuJw+s7Lz8nksI7SIuU3UXJJslUthsi+uWBUYOwPFwW7W7PRLRfUKpxjtjFCw==}
yaml@2.9.0:
resolution: {integrity: sha512-2AvhNX3mb8zd6Zy7INTtSpl1F15HW6Wnqj0srWlkKLcpYl/gMIMJiyuGq2KeI2YFxUPjdlB+3Lc10seMLtL4cA==}
engines: {node: '>= 14.6'}
hasBin: true
yocto-queue@0.1.0:
resolution: {integrity: sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==}
engines: {node: '>=10'}
@@ -6072,13 +6067,13 @@ snapshots:
d3-selection: 3.0.0
d3-transition: 3.0.1(d3-selection@3.0.0)
'@vitejs/plugin-vue@6.0.7(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0))(vue@3.5.38(typescript@6.0.3))':
'@vitejs/plugin-vue@6.0.7(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0))(vue@3.5.38(typescript@6.0.3))':
dependencies:
'@rolldown/pluginutils': 1.0.1
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)
vue: 3.5.38(typescript@6.0.3)
'@vitest/eslint-plugin@1.6.20(@typescript-eslint/eslint-plugin@8.61.1(@typescript-eslint/parser@8.61.1(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)(vitest@4.1.9(@types/node@25.9.3)(happy-dom@20.10.6)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)))':
'@vitest/eslint-plugin@1.6.20(@typescript-eslint/eslint-plugin@8.61.1(@typescript-eslint/parser@8.61.1(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)(vitest@4.1.9(@types/node@25.9.3)(happy-dom@20.10.5)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)))':
dependencies:
'@typescript-eslint/scope-manager': 8.61.1
'@typescript-eslint/utils': 8.61.1(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)
@@ -6086,7 +6081,7 @@ snapshots:
optionalDependencies:
'@typescript-eslint/eslint-plugin': 8.61.1(@typescript-eslint/parser@8.61.1(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3))(eslint@10.5.0(jiti@2.7.0))(typescript@6.0.3)
typescript: 6.0.3
vitest: 4.1.9(@types/node@25.9.3)(happy-dom@20.10.6)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0))
vitest: 4.1.9(@types/node@25.9.3)(happy-dom@20.10.5)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0))
transitivePeerDependencies:
- supports-color
@@ -6099,13 +6094,13 @@ snapshots:
chai: 6.2.2
tinyrainbow: 3.1.0
'@vitest/mocker@4.1.9(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0))':
'@vitest/mocker@4.1.9(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0))':
dependencies:
'@vitest/spy': 4.1.9
estree-walker: 3.0.3
magic-string: 0.30.21
optionalDependencies:
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)
'@vitest/pretty-format@4.1.9':
dependencies:
@@ -6331,7 +6326,7 @@ snapshots:
is-array-buffer: 3.0.5
optional: true
asciinema-player@3.16.0:
asciinema-player@3.15.1:
dependencies:
'@babel/runtime': 7.29.7
solid-js: 1.9.13
@@ -7253,7 +7248,7 @@ snapshots:
regexp-ast-analysis: 0.7.1
scslre: 0.3.0
eslint-plugin-sonarjs@4.1.0(eslint@10.5.0(jiti@2.7.0)):
eslint-plugin-sonarjs@4.0.3(eslint@10.5.0(jiti@2.7.0)):
dependencies:
'@eslint-community/regexpp': 4.12.2
builtin-modules: 3.3.0
@@ -7268,7 +7263,6 @@ snapshots:
semver: 7.8.4
ts-api-utils: 2.5.0(typescript@6.0.3)
typescript: 6.0.3
yaml: 2.9.0
eslint-plugin-unicorn@68.0.0(eslint@10.5.0(jiti@2.7.0)):
dependencies:
@@ -7638,7 +7632,7 @@ snapshots:
hammerjs@2.0.8: {}
happy-dom@20.10.6:
happy-dom@20.10.5:
dependencies:
'@types/node': 25.9.3
'@types/whatwg-mimetype': 3.0.2
@@ -8752,13 +8746,12 @@ snapshots:
camelcase-css: 2.0.1
postcss: 8.5.15
postcss-load-config@6.0.1(jiti@1.21.7)(postcss@8.5.15)(yaml@2.9.0):
postcss-load-config@6.0.1(jiti@1.21.7)(postcss@8.5.15):
dependencies:
lilconfig: 3.1.3
optionalDependencies:
jiti: 1.21.7
postcss: 8.5.15
yaml: 2.9.0
postcss-nested@6.2.0(postcss@8.5.15):
dependencies:
@@ -9359,7 +9352,7 @@ snapshots:
string-width: 4.2.3
strip-ansi: 6.0.1
tailwindcss@3.4.19(yaml@2.9.0):
tailwindcss@3.4.19:
dependencies:
'@alloc/quick-lru': 5.2.0
arg: 5.0.2
@@ -9378,7 +9371,7 @@ snapshots:
postcss: 8.5.15
postcss-import: 15.1.0(postcss@8.5.15)
postcss-js: 4.1.0(postcss@8.5.15)
postcss-load-config: 6.0.1(jiti@1.21.7)(postcss@8.5.15)(yaml@2.9.0)
postcss-load-config: 6.0.1(jiti@1.21.7)(postcss@8.5.15)
postcss-nested: 6.2.0(postcss@8.5.15)
postcss-selector-parser: 6.1.2
resolve: 1.22.12
@@ -9606,11 +9599,11 @@ snapshots:
core-util-is: 1.0.2
extsprintf: 1.3.0
vite-string-plugin@2.0.4(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)):
vite-string-plugin@2.0.4(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)):
dependencies:
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)
vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0):
vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0):
dependencies:
lightningcss: 1.32.0
picomatch: 4.0.4
@@ -9622,12 +9615,11 @@ snapshots:
esbuild: 0.28.1
fsevents: 2.3.3
jiti: 2.7.0
yaml: 2.9.0
vitest@4.1.9(@types/node@25.9.3)(happy-dom@20.10.6)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)):
vitest@4.1.9(@types/node@25.9.3)(happy-dom@20.10.5)(jsdom@20.0.3)(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)):
dependencies:
'@vitest/expect': 4.1.9
'@vitest/mocker': 4.1.9(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0))
'@vitest/mocker': 4.1.9(vite@8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0))
'@vitest/pretty-format': 4.1.9
'@vitest/runner': 4.1.9
'@vitest/snapshot': 4.1.9
@@ -9644,11 +9636,11 @@ snapshots:
tinyexec: 1.2.4
tinyglobby: 0.2.17
tinyrainbow: 3.1.0
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)(yaml@2.9.0)
vite: 8.0.16(@types/node@25.9.3)(esbuild@0.28.1)(jiti@2.7.0)
why-is-node-running: 2.3.0
optionalDependencies:
'@types/node': 25.9.3
happy-dom: 20.10.6
happy-dom: 20.10.5
jsdom: 20.0.3
transitivePeerDependencies:
- msw
@@ -9801,6 +9793,4 @@ snapshots:
xmlchars@2.2.0: {}
yaml@2.9.0: {}
yocto-queue@0.1.0: {}
+3 -1
View File
@@ -38,7 +38,7 @@ func CompareDiff(ctx *context.APIContext) {
// required: true
// - name: basehead
// 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
// required: true
// - name: output
@@ -51,6 +51,8 @@ func CompareDiff(ctx *context.APIContext) {
// responses:
// "200":
// "$ref": "#/responses/Compare"
// "400":
// "$ref": "#/responses/error"
// "404":
// "$ref": "#/responses/notFound"
+11 -9
View File
@@ -1087,12 +1087,6 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git
baseRepo := ctx.Repo.Repository
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)
switch {
case errors.Is(err, util.ErrInvalidArgument):
@@ -1152,10 +1146,18 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *git
return nil, nil
}
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.BaseOriRef, baseRepo.GetPullRequestTargetBranch(ctx)))
headRef := headGitRepo.UnstableGuessRefByShortName(util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch))
baseRef, err := common.ResolveRefWithSuffix(ctx.Repo.GitRepo, util.IfZero(compareReq.BaseOriRef, baseRepo.GetPullRequestTargetBranch(ctx)), compareReq.BaseOriRefSuffix)
if err != nil {
ctx.APIErrorAuto(err)
return nil, nil
}
headRef, err := common.ResolveRefWithSuffix(headGitRepo, util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch), compareReq.HeadOriRefSuffix)
if err != nil {
ctx.APIErrorAuto(err)
return nil, nil
}
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())
headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName())
+37 -5
View File
@@ -5,7 +5,9 @@ package common
import (
"context"
"regexp"
"strings"
"sync"
repo_model "gitea.dev/models/repo"
user_model "gitea.dev/models/user"
@@ -19,9 +21,10 @@ type CompareRouterReq struct {
CompareSeparator string
HeadOwner string
HeadRepoName string
HeadOriRef string
HeadOwner string
HeadRepoName string
HeadOriRef string
HeadOriRefSuffix string
}
func (cr *CompareRouterReq) DirectComparison() bool {
@@ -79,9 +82,11 @@ func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
sep = ".."
basePart, headPart, ok = strings.Cut(routerParam, sep)
if !ok {
headOwnerName, headRepoName, headRef := parseHead(routerParam)
headOwnerName, headRepoName, headOriRef := parseHead(routerParam)
headOriRef, headOriRefSuffix := git.ParseRefSuffix(headOriRef)
return &CompareRouterReq{
HeadOriRef: headRef,
HeadOriRef: headOriRef,
HeadOriRefSuffix: headOriRefSuffix,
HeadOwner: headOwnerName,
HeadRepoName: headRepoName,
CompareSeparator: "...",
@@ -92,9 +97,36 @@ func ParseCompareRouterParam(routerParam string) *CompareRouterReq {
ci := &CompareRouterReq{CompareSeparator: sep}
ci.BaseOriRef, ci.BaseOriRefSuffix = git.ParseRefSuffix(basePart)
ci.HeadOwner, ci.HeadRepoName, ci.HeadOriRef = parseHead(headPart)
ci.HeadOriRef, ci.HeadOriRefSuffix = git.ParseRefSuffix(ci.HeadOriRef)
return ci
}
// validRefSuffix matches only ^/~ ancestry navigation. The ^{...}, @{...} and :path forms address
// other objects (trees, blobs) or reflog/upstream state that compare does not resolve, so they are rejected.
var validRefSuffix = sync.OnceValue(func() *regexp.Regexp {
return regexp.MustCompile(`^(?:[~^][0-9]*)+$`)
})
// ResolveRefWithSuffix resolves oriRef plus an optional revision suffix (^, ~N) to a RefName.
// A nil error guarantees a usable RefName: an unsupported suffix yields an invalid-argument error
// and an unresolvable ref yields a not-found error.
func ResolveRefWithSuffix(gitRepo *git.Repository, oriRef, refSuffix string) (git.RefName, error) {
if refSuffix == "" {
if refName := gitRepo.UnstableGuessRefByShortName(oriRef); refName != "" {
return refName, nil
}
return "", util.NewNotExistErrorf("ref %q does not exist", oriRef)
}
if !validRefSuffix().MatchString(refSuffix) {
return "", util.NewInvalidArgumentErrorf("unsupported ref suffix %q", refSuffix)
}
commit, err := gitRepo.GetCommit(oriRef + refSuffix)
if err != nil {
return "", util.NewNotExistErrorf("ref %q does not exist", oriRef+refSuffix)
}
return git.RefNameFromCommit(commit.ID.String()), nil
}
// maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository.
const maxForkTraverseLevel = 10
+49
View File
@@ -6,6 +6,8 @@ package common
import (
"testing"
"gitea.dev/modules/util"
"github.com/stretchr/testify/assert"
)
@@ -97,9 +99,56 @@ func TestCompareRouterReq(t *testing.T) {
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 {
assert.Equal(t, c.CompareRouterReq, ParseCompareRouterParam(c.input), "input: %s", c.input)
}
}
func TestResolveRefWithSuffix(t *testing.T) {
// The ^{...}, @{...} and :path forms address non-commit objects or reflog state, so they are
// rejected before any repository access and a nil repo is fine here.
for _, refSuffix := range []string{"^{/Add}", "^{commit}", "@{upstream}", "~1:path"} {
ref, err := ResolveRefWithSuffix(nil, "branch", refSuffix)
assert.ErrorIs(t, err, util.ErrInvalidArgument, "suffix %q", refSuffix)
assert.Empty(t, ref, "suffix %q", refSuffix)
}
}
+6 -11
View File
@@ -208,11 +208,6 @@ func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context, comparePa
// 1 Parse compare router param
compareReq := common.ParseCompareRouterParam(compareParam)
// remove the check when we support compare with carets
if compareReq.BaseOriRefSuffix != "" {
return util.NewInvalidArgumentErrorf("unsupported comparison syntax: ref with suffix")
}
// 2 get repository and owner for head
headOwner, headRepo, err := common.GetHeadOwnerAndRepo(ctx, baseRepo, compareReq)
if err != nil {
@@ -241,18 +236,18 @@ func (cpi *comparePageInfoType) parseCompareInfo(ctx *context.Context, comparePa
baseRefName := util.IfZero(compareReq.BaseOriRef, baseRepo.GetPullRequestTargetBranch(ctx))
headRefName := util.IfZero(compareReq.HeadOriRef, headRepo.DefaultBranch)
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefName)
if baseRef == "" {
return util.NewNotExistErrorf("no base ref: %s", baseRefName)
baseRef, err := common.ResolveRefWithSuffix(ctx.Repo.GitRepo, baseRefName, compareReq.BaseOriRefSuffix)
if err != nil {
return err
}
headGitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, headRepo)
if err != nil {
return err
}
headRef := headGitRepo.UnstableGuessRefByShortName(headRefName)
if headRef == "" {
return util.NewNotExistErrorf("no head ref: %s", headRefName)
headRef, err := common.ResolveRefWithSuffix(headGitRepo, headRefName, compareReq.HeadOriRefSuffix)
if err != nil {
return err
}
ctx.Data["BaseName"] = baseRepo.OwnerName
+4 -1
View File
@@ -8135,7 +8135,7 @@
},
{
"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",
"in": "path",
"required": true
@@ -8155,6 +8155,9 @@
"200": {
"$ref": "#/responses/Compare"
},
"400": {
"$ref": "#/responses/error"
},
"404": {
"$ref": "#/responses/notFound"
}
+4 -1
View File
@@ -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",
"name": "basehead",
"required": true,
@@ -19515,6 +19515,9 @@
"200": {
"$ref": "#/components/responses/Compare"
},
"400": {
"$ref": "#/components/responses/error"
},
"404": {
"$ref": "#/components/responses/notFound"
}
@@ -45,6 +45,40 @@ func TestAPICompareBranches(t *testing.T) {
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)
// a valid but unresolvable suffix is not found, while an unsupported suffix (^{...}) is a bad request
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.StatusBadRequest)
req = NewRequestf(t, "GET", "/api/v1/repos/user2/repo20/compare/add-csv^{/Add}...remove-files-b").AddTokenAuth(token2)
MakeRequest(t, req, http.StatusBadRequest)
})
t.Run("CompareForkOnlyCommit", func(t *testing.T) {
defer tests.PrintCurrentTest(t)()
+70
View File
@@ -14,7 +14,10 @@ import (
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
"gitea.dev/modules/git/gitcmd"
"gitea.dev/modules/gitrepo"
"gitea.dev/modules/test"
"gitea.dev/modules/util"
"gitea.dev/routers/common"
repo_service "gitea.dev/services/repository"
"gitea.dev/tests"
@@ -135,6 +138,73 @@ func TestCompareBranches(t *testing.T) {
inspectCompare(t, htmlDoc, diffCount, diffChanges)
}
func TestCompareWithRefSuffix(t *testing.T) {
defer tests.PrepareTestEnv(t)()
session := loginUser(t, "user2")
// remove-files-b^ resolves to the tip's parent, so the test.txt added by the tip is excluded
req := NewRequest(t, "GET", "/user2/repo20/compare/add-csv...remove-files-b^")
resp := session.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
inspectCompare(t, htmlDoc, 2, []string{"link_hi", "test.csv"})
// a suffix resolves to a commit rather than a branch, so the page offers no pull request to create
assert.Equal(t, 0, htmlDoc.doc.Find(".pullrequest-form").Length())
// the same suffix on the direct ".." comparison resolves to the same commit
req = NewRequest(t, "GET", "/user2/repo20/compare/add-csv..remove-files-b^")
resp = session.MakeRequest(t, req, http.StatusOK)
htmlDoc = NewHTMLParser(t, resp.Body)
inspectCompare(t, htmlDoc, 2, []string{"link_hi", "test.csv"})
// a ~N suffix on the base side resolves and renders the compare page, but also
// resolves to a commit rather than a branch, so no pull request form is offered
req = NewRequest(t, "GET", "/user2/repo20/compare/add-csv~1...remove-files-b")
resp = session.MakeRequest(t, req, http.StatusOK)
assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
htmlDoc = NewHTMLParser(t, resp.Body)
assert.Equal(t, 0, htmlDoc.doc.Find(".pullrequest-form").Length())
// the web handler folds an unsupported (^{...}) and an unresolvable (~50) suffix alike into 404
for _, basehead := range []string{
"add-csv...remove-files-b~50",
"add-csv...remove-files-b^{/Add}",
"add-csv^{/Add}...remove-files-b",
} {
req = NewRequest(t, "GET", "/user2/repo20/compare/"+basehead).SetHeader("Accept", "text/html")
resp = session.MakeRequest(t, req, http.StatusNotFound)
assert.True(t, test.IsNormalPageCompleted(resp.Body.String()))
}
}
func TestResolveRefWithSuffixContract(t *testing.T) {
defer tests.PrepareTestEnv(t)()
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 31})
gitRepo, err := gitrepo.OpenRepository(t.Context(), repo)
require.NoError(t, err)
defer gitRepo.Close()
// a nil error guarantees a usable RefName
ref, err := common.ResolveRefWithSuffix(gitRepo, "add-csv", "^")
require.NoError(t, err)
assert.NotEmpty(t, ref)
// a ref resolved with a suffix must be a commit SHA, not a branch ref
// (branch refs would break "New Pull Request" logic)
assert.False(t, ref.IsBranch(), "ref with suffix must not resolve to a branch")
// a missing ref and an unresolvable suffix both report not-found instead of an empty RefName
for _, tc := range []struct{ oriRef, suffix string }{
{"does-not-exist", ""},
{"add-csv", "~50"},
} {
ref, err := common.ResolveRefWithSuffix(gitRepo, tc.oriRef, tc.suffix)
assert.ErrorIs(t, err, util.ErrNotExist, "ref %q suffix %q", tc.oriRef, tc.suffix)
assert.Empty(t, ref, "ref %q suffix %q", tc.oriRef, tc.suffix)
}
}
func TestCompareBranchesNoCommonMergeBase(t *testing.T) {
defer tests.PrepareTestEnv(t)()
Vendored
+24
View File
@@ -18,6 +18,10 @@ declare module '*.vue' {
import type {DefineComponent} from 'vue';
const component: DefineComponent<unknown, unknown, any>;
export default component;
// Here we declare all exports from vue files so `tsc` or `tsgo` can work for
// non-vue files. To lint .vue files, `vue-tsc` must be used.
export function initDashboardRepoList(): void;
export function initRepositoryActionView(): void;
}
declare module 'idiomorph' {
@@ -32,6 +36,14 @@ declare module 'swagger-ui-dist/swagger-ui-es-bundle.js' {
export default value.SwaggerUIBundle;
}
declare module 'asciinema-player' {
interface AsciinemaPlayer {
create(src: string | {data: string}, element: HTMLElement, options?: Record<string, unknown>): void;
}
const exports: AsciinemaPlayer;
export = exports;
}
declare module '@citation-js/core' {
export class Cite {
constructor(data: string);
@@ -65,3 +77,15 @@ declare module 'vue-bar-graph' {
labelHeight?: number;
}>;
}
declare module '@mcaptcha/vanilla-glue' {
export let INPUT_NAME: string;
export default class Widget {
constructor(options: {
siteKey: {
instanceUrl: URL;
key: string;
};
});
}
}
+9
View File
@@ -1,10 +1,13 @@
interface JQuery {
areYouSure: any, // jquery.are-you-sure
fomanticExt: any; // fomantic extension
api: any, // fomantic
dimmer: any, // fomantic
dropdown: any; // fomantic
modal: any; // fomantic
tab: any; // fomantic
transition: any, // fomantic
search: any, // fomantic
}
interface JQueryStatic {
@@ -38,6 +41,7 @@ interface Window {
FolderOpenIcon?: string,
repoLink?: string,
repoActivityTopAuthors?: any[],
pullRequestMergeForm?: Record<string, any>,
dashboardRepoList?: Record<string, any>,
},
notificationSettings: {
@@ -75,3 +79,8 @@ interface Window {
// do not add more properties here unless it is a must
}
declare module '*?worker' {
const workerConstructor: new () => Worker;
export default workerConstructor;
}