[mobile] cmd/gomobile: compare NDK revisions numerically
1 view
Skip to first unread message
race quite (Gerrit)
Aug 9, 2026, 3:32:24 PM (14 hours ago) Aug 9
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
race quite has uploaded the change for review![]( "Open in Gerrit")
Commit message
cmd/gomobile: compare NDK revisions numerically
The NDK selector reads Pkg.Revision from source.properties. The
revision was previously compared using Go string ordering, which is
incorrect for numeric version components. For example, 17.9.0 compares
greater than 17.10.0 lexicographically.
This change introduces numeric comparison of NDK revisions and adds
stable fallback behavior for unparsable values. It also covers missing
trailing components and prerelease suffixes such as beta and rc.
The full package test command is affected by the local Android SDK
configuration: the installed NDK does not support the API 16 test case.
The focused regression tests pass.
Fixex golang/go#80805
Change-Id: I35a64b390278104f627b900656f28a0666a83f46
Change diff
diff --git a/cmd/gomobile/env.go b/cmd/gomobile/env.go
index 42a9406..5da2747 100644
--- a/cmd/gomobile/env.go
+++ b/cmd/gomobile/env.go
@@ -15,6 +15,7 @@
"path/filepath"
"runtime"
"strings"
+ "unicode"
"golang.org/x/mobile/internal/sdkpath"
)
@@ -385,6 +386,132 @@
return ""
}
+// compareNDKVersions compares NDK revision strings and reports whether x is
+// less than, equal to, or greater than y.
+//
+// NDK revisions consist of dot-separated numeric components followed by an
+// optional prerelease suffix. Missing trailing numeric components are treated
+// as zero. If only one revision can be parsed, it is preferred. If neither can
+// be parsed, compareNDKVersions falls back to comparing the original strings.
+func compareNDKVersions(x, y string) int {
+ xNumbers, xSuffix, xOK := parseNDKVersion(x)
+ yNumbers, ySuffix, yOK := parseNDKVersion(y)
+ if xOK != yOK {
+ if xOK {
+ return 1
+ }
+ return -1
+ }
+ if !xOK {
+ return strings.Compare(x, y)
+ }
+
+ for i := 0; i < max(len(xNumbers), len(yNumbers)); i++ {
+ xNumber, yNumber := "0", "0"
+ if i < len(xNumbers) {
+ xNumber = xNumbers[i]
+ }
+ if i < len(yNumbers) {
+ yNumber = yNumbers[i]
+ }
+ if c := compareDecimalStrings(xNumber, yNumber); c != 0 {
+ return c
+ }
+ }
+
+ // A release is newer than a prerelease with the same numeric revision.
+ if xSuffix == "" || ySuffix == "" {
+ switch {
+ case xSuffix == ySuffix:
+ return 0
+ case xSuffix == "":
+ return 1
+ default:
+ return -1
+ }
+ }
+ return compareVersionSuffixes(xSuffix, ySuffix)
+}
+
+func parseNDKVersion(version string) (numbers []string, suffix string, ok bool) {
+ version = strings.TrimSpace(version)
+ if version == "" {
+ return nil, "", false
+ }
+
+ for {
+ start := 0
+ for start < len(version) && version[start] >= '0' && version[start] <= '9' {
+ start++
+ }
+ if start == 0 {
+ return nil, "", false
+ }
+ numbers = append(numbers, version[:start])
+ version = version[start:]
+ if version == "" {
+ return numbers, "", true
+ }
+ if version[0] != '.' {
+ return numbers, strings.TrimLeftFunc(version, isVersionSeparator), true
+ }
+ version = version[1:]
+ }
+}
+
+func isVersionSeparator(r rune) bool {
+ return r == '-' || r == '_' || r == '+' || unicode.IsSpace(r)
+}
+
+func compareDecimalStrings(x, y string) int {
+ x = strings.TrimLeft(x, "0")
+ y = strings.TrimLeft(y, "0")
+ if len(x) != len(y) {
+ if len(x) < len(y) {
+ return -1
+ }
+ return 1
+ }
+ return strings.Compare(x, y)
+}
+
+// compareVersionSuffixes compares suffixes naturally, so beta9 sorts before
+// beta10. Text is compared case-insensitively, with the original text used as
+// a deterministic fallback.
+func compareVersionSuffixes(x, y string) int {
+ for len(x) > 0 && len(y) > 0 {
+ xDigits := x[0] >= '0' && x[0] <= '9'
+ yDigits := y[0] >= '0' && y[0] <= '9'
+ if xDigits != yDigits {
+ return strings.Compare(strings.ToLower(x), strings.ToLower(y))
+ }
+
+ xEnd, yEnd := 0, 0
+ for xEnd < len(x) && (x[xEnd] >= '0' && x[xEnd] <= '9') == xDigits {
+ xEnd++
+ }
+ for yEnd < len(y) && (y[yEnd] >= '0' && y[yEnd] <= '9') == yDigits {
+ yEnd++
+ }
+
+ xPart, yPart := x[:xEnd], y[:yEnd]
+ var c int
+ if xDigits {
+ c = compareDecimalStrings(xPart, yPart)
+ } else {
+ c = strings.Compare(strings.ToLower(xPart), strings.ToLower(yPart))
+ if c == 0 {
+ c = strings.Compare(xPart, yPart)
+ }
+ }
+ if c != 0 {
+ return c
+ }
+ x, y = x[xEnd:], y[yEnd:]
+ }
+ return strings.Compare(x, y)
+}
+
// ndkRoot returns the root path of an installed NDK that supports all the
// specified Android targets. For details of NDK locations, see
// https://github.com/android/ndk-samples/wiki/Configure-NDK-Path
@@ -418,7 +545,7 @@
var selected string
for _, ndkRoot := range ndkRoots {
version := ndkVersion(ndkRoot)
- if version >= maxVersion {
+ if compareNDKVersions(version, maxVersion) >= 0 {
maxVersion = version
selected = ndkRoot
}
diff --git a/cmd/gomobile/env_test.go b/cmd/gomobile/env_test.go
index 9090eaa..bec6027 100644
--- a/cmd/gomobile/env_test.go
+++ b/cmd/gomobile/env_test.go
@@ -116,13 +116,13 @@
path := filepath.Join("ndk", "newer")
platforms := `{"min":19,"max":32}`
abis := `{"arm64-v8a": {}, "armeabi-v7a": {}, "x86_64": {}}`
- version := "17.2.0"
+ version := "17.10.0"
newerNDK := makeMockNDK(path, version, platforms, abis)
path = filepath.Join("ndk", "older")
platforms = `{"min":16,"max":31}`
abis = `{"arm64-v8a": {}, "armeabi-v7a": {}, "x86": {}}`
- version = "17.1.0"
+ version = "17.9.0"
olderNDK := makeMockNDK(path, version, platforms, abis)
testCases := []struct {
@@ -158,3 +158,32 @@
}
})
}
+
+func TestCompareNDKVersions(t *testing.T) {
+ testCases := []struct {
+ x, y string
+ want int
+ }{
+ {"27.9.0", "27.10.0", -1},
+ {"27.10", "27.10.0", 0},
+ {"27.10.0.1", "27.10.0", 1},
+ {"27.10.00000000000000000001", "27.10.1", 0},
+ {"27.10.0-beta9", "27.10.0-beta10", -1},
+ {"27.10.0-beta10", "27.10.0-rc1", -1},
+ {"27.10.0-rc1", "27.10.0", -1},
+ {"27.10.0", "invalid", 1},
+ {"invalid-2", "invalid-10", 1},
+ {"", "invalid", -1},
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.x+"_"+tc.y, func(t *testing.T) {
+ if got := compareNDKVersions(tc.x, tc.y); got != tc.want {
+ t.Errorf("compareNDKVersions(%q, %q) = %d, want %d", tc.x, tc.y, got, tc.want)
+ }
+ if got := compareNDKVersions(tc.y, tc.x); got != -tc.want {
+ t.Errorf("compareNDKVersions(%q, %q) = %d, want %d", tc.y, tc.x, got, -tc.want)
+ }
+ })
+ }
+}
Change information
Files:
- M cmd/gomobile/env.go
- M cmd/gomobile/env_test.go
Change size: M
Delta: 2 files changed, 159 insertions(+), 3 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hajime Hoshi (Gerrit)
12:24 AM (6 hours ago) 12:24 AM
to race quite, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention needed from race quite
Hajime Hoshi added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- race quite
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
race quite (Gerrit)
2:38 AM (3 hours ago) 2:38 AM
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from race quite
race quite uploaded new patchset![]( "Open in Gerrit")
race quite uploaded patch set #2 to this change.
Related details
Attention is currently required from:
- race quite
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
race quite (Gerrit)
2:39 AM (3 hours ago) 2:39 AM
to goph...@pubsubhelper.golang.org, Hajime Hoshi, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Hajime Hoshi
race quite added 1 comment![]( "Open in Gerrit")
Patchset-level comments
race quite
Yes, thanks.This would require much smaller changes.
I replaced the custom version parser with golang.org/x/mod/semver and kept the regression test covering 17.9.0 versus 17.10.0.
Related details
Attention is currently required from:
- Hajime Hoshi
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages