[mobile] x/mobile: modernize handling of Android SDK and NDK paths
Ben Schwartz (Gerrit)
Ben Schwartz has uploaded this change for review.
x/mobile: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME, which are deprecated. It also increases the
minimum API version to 16, as all SDKs that supported API 15
are now deprecated.
Fixes https://github.com/golang/go/issues/52470
Change-Id: I546365774a089e5d7ae1be0a538efd72741d92ac
---
M bind/java/seq_test.go
M cmd/gomobile/bind.go
M cmd/gomobile/bind_androidapp.go
M cmd/gomobile/bind_test.go
M cmd/gomobile/build.go
M cmd/gomobile/build_androidapp.go
M cmd/gomobile/build_test.go
M cmd/gomobile/env.go
M cmd/gomobile/env_test.go
M cmd/gomobile/gendex.go
M cmd/gomobile/init.go
M cmd/gomobile/version.go
M example/ivy/android/README.md
M internal/binres/binres.go
M internal/binres/binres_test.go
M internal/binres/genarsc.go
M internal/binres/sdk.go
M internal/binres/table.go
M internal/binres/testdata/bootstrap.arsc
M internal/binres/testdata/bootstrap.bin
M internal/binres/testdata/gen.sh
A internal/sdkpath/sdkpath.go
22 files changed, 567 insertions(+), 154 deletions(-)
diff --git a/bind/java/seq_test.go b/bind/java/seq_test.go
index 8f741c6..e573707 100644
--- a/bind/java/seq_test.go
+++ b/bind/java/seq_test.go
@@ -17,6 +17,7 @@
"testing"
"golang.org/x/mobile/internal/importers/java"
+ "golang.org/x/mobile/internal/sdkpath"
)
var gomobileBin string
@@ -98,8 +99,8 @@
// runTest runs the Android java test class specified with javaCls. If javaPkg is
// set, it is passed with the -javapkg flag to gomobile. The pkgNames lists the Go
// packages to bind for the test.
-// This requires the gradle command in PATH and
-// the Android SDK whose path is available through ANDROID_HOME environment variable.
+// This requires the gradle command to be in PATH and the Android SDK to be
+// installed.
func runTest(t *testing.T, pkgNames []string, javaPkg, javaCls string) {
if gomobileBin == "" {
t.Skipf("no gomobile on %s", runtime.GOOS)
@@ -108,8 +109,8 @@
if err != nil {
t.Skip("command gradle not found, skipping")
}
- if sdk := os.Getenv("ANDROID_HOME"); sdk == "" {
- t.Skip("ANDROID_HOME environment var not set, skipping")
+ if _, err := sdkpath.GetAndroidHome(); err != nil {
+ t.Skip("Android SDK not found, skipping")
}
cwd, err := os.Getwd()
diff --git a/cmd/gomobile/bind.go b/cmd/gomobile/bind.go
index efbc896..c4552e3 100644
--- a/cmd/gomobile/bind.go
+++ b/cmd/gomobile/bind.go
@@ -16,6 +16,7 @@
"path/filepath"
"strings"
+ "golang.org/x/mobile/internal/sdkpath"
"golang.org/x/mod/modfile"
"golang.org/x/tools/go/packages"
)
@@ -42,9 +43,10 @@
the module import wizard (File > New > New Module > Import .JAR or
.AAR package), and setting it as a new dependency
(File > Project Structure > Dependencies). This requires 'javac'
-(version 1.7+) and Android SDK (API level 15 or newer) to build the
-library for Android. The environment variable ANDROID_HOME must be set
-to the path to Android SDK. Use the -javapkg flag to specify the Java
+(version 1.7+) and Android SDK (API level 16 or newer) to build the
+library for Android. The ANDROID_HOME and ANDROID_NDK_HOME environment
+variables can be used to specify the Android SDK and NDK if they are
+not in the default locations. Use the -javapkg flag to specify the Java
package prefix for the generated classes.
By default, -target=android builds shared libraries for all supported
@@ -85,7 +87,7 @@
if bindPrefix != "" {
return fmt.Errorf("-prefix is supported only for Apple targets")
}
- if _, err := ndkRoot(); err != nil {
+ if _, err := ndkRoot(targets[0]); err != nil {
return err
}
} else {
@@ -156,7 +158,7 @@
if bindBootClasspath != "" {
return bindBootClasspath, nil
}
- apiPath, err := androidAPIPath()
+ apiPath, err := sdkpath.AndroidAPIPath(buildAndroidAPI)
if err != nil {
return "", err
}
diff --git a/cmd/gomobile/bind_androidapp.go b/cmd/gomobile/bind_androidapp.go
index 7703868..992776c 100644
--- a/cmd/gomobile/bind_androidapp.go
+++ b/cmd/gomobile/bind_androidapp.go
@@ -12,15 +12,15 @@
"os"
"os/exec"
"path/filepath"
- "strconv"
"strings"
+ "golang.org/x/mobile/internal/sdkpath"
"golang.org/x/tools/go/packages"
)
func goAndroidBind(gobind string, pkgs []*packages.Package, targets []targetInfo) error {
- if sdkDir := os.Getenv("ANDROID_HOME"); sdkDir == "" {
- return fmt.Errorf("this command requires ANDROID_HOME environment variable (path to the Android SDK)")
+ if _, err := sdkpath.GetAndroidHome(); err != nil {
+ return fmt.Errorf("this command requires the Android SDK to be installed")
}
// Run gobind to generate the bindings
@@ -270,7 +270,7 @@
const (
javacTargetVer = "1.7"
- minAndroidAPI = 15
+ minAndroidAPI = 16
)
func buildJar(w io.Writer, srcDir string) error {
@@ -370,46 +370,3 @@
}
return jarw.Close()
}
-
-// androidAPIPath returns an android SDK platform directory under ANDROID_HOME.
-// If there are multiple platforms that satisfy the minimum version requirement
-// androidAPIPath returns the latest one among them.
-func androidAPIPath() (string, error) {
- sdk := os.Getenv("ANDROID_HOME")
- if sdk == "" {
- return "", fmt.Errorf("ANDROID_HOME environment var is not set")
- }
- sdkDir, err := os.Open(filepath.Join(sdk, "platforms"))
- if err != nil {
- return "", fmt.Errorf("failed to find android SDK platform: %v", err)
- }
- defer sdkDir.Close()
- fis, err := sdkDir.Readdir(-1)
- if err != nil {
- return "", fmt.Errorf("failed to find android SDK platform (API level: %d): %v", buildAndroidAPI, err)
- }
-
- var apiPath string
- var apiVer int
- for _, fi := range fis {
- name := fi.Name()
- if !strings.HasPrefix(name, "android-") {
- continue
- }
- n, err := strconv.Atoi(name[len("android-"):])
- if err != nil || n < buildAndroidAPI {
- continue
- }
- p := filepath.Join(sdkDir.Name(), name)
- _, err = os.Stat(filepath.Join(p, "android.jar"))
- if err == nil && apiVer < n {
- apiPath = p
- apiVer = n
- }
- }
- if apiVer == 0 {
- return "", fmt.Errorf("failed to find android SDK platform (API level: %d) in %s",
- buildAndroidAPI, sdkDir.Name())
- }
- return apiPath, nil
-}
diff --git a/cmd/gomobile/bind_test.go b/cmd/gomobile/bind_test.go
index 0e9f401..adaa80a 100644
--- a/cmd/gomobile/bind_test.go
+++ b/cmd/gomobile/bind_test.go
@@ -11,21 +11,17 @@
"os/exec"
"path/filepath"
"runtime"
- "strings"
"testing"
"text/template"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
func TestBindAndroid(t *testing.T) {
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- t.Skip("ANDROID_HOME not found, skipping bind")
- }
- platform, err := androidAPIPath()
+ platform, err := sdkpath.AndroidAPIPath(minAndroidAPI)
if err != nil {
- t.Skip("No android API platform found in $ANDROID_HOME, skipping bind")
+ t.Skip("No compatible Android API platform found, skipping bind")
}
- platform = strings.Replace(platform, androidHome, "$ANDROID_HOME", -1)
defer func() {
xout = os.Stderr
@@ -273,12 +269,8 @@
t.Run(target, func(t *testing.T) {
switch target {
case "android":
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- t.Skip("ANDROID_HOME not found, skipping bind")
- }
- if _, err := androidAPIPath(); err != nil {
- t.Skip("No android API platform found in $ANDROID_HOME, skipping bind")
+ if _, err := sdkpath.AndroidAPIPath(minAndroidAPI); err != nil {
+ t.Skip("No compatible Android API platform found, skipping bind")
}
case "ios":
if !xcodeAvailable() {
diff --git a/cmd/gomobile/build.go b/cmd/gomobile/build.go
index a11865a..f608367 100644
--- a/cmd/gomobile/build.go
+++ b/cmd/gomobile/build.go
@@ -60,7 +60,7 @@
The default version is 13.0.
Flag -androidapi sets the Android API version to compile against.
-The default and minimum is 15.
+The default and minimum is 16.
The -bundleid flag is required for -target ios and sets the bundle ID to use
with the app.
diff --git a/cmd/gomobile/build_androidapp.go b/cmd/gomobile/build_androidapp.go
index b06ea29..bcd2664 100644
--- a/cmd/gomobile/build_androidapp.go
+++ b/cmd/gomobile/build_androidapp.go
@@ -25,7 +25,7 @@
)
func goAndroidBuild(pkg *packages.Package, targets []targetInfo) (map[string]bool, error) {
- ndkRoot, err := ndkRoot()
+ ndkRoot, err := ndkRoot(targets...)
if err != nil {
return nil, err
}
diff --git a/cmd/gomobile/build_test.go b/cmd/gomobile/build_test.go
index ff21f87..448c6dd 100644
--- a/cmd/gomobile/build_test.go
+++ b/cmd/gomobile/build_test.go
@@ -14,6 +14,8 @@
"strings"
"testing"
"text/template"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
func TestRFC1034Label(t *testing.T) {
@@ -228,12 +230,8 @@
t.Run(target, func(t *testing.T) {
switch target {
case "android":
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- t.Skip("ANDROID_HOME not found, skipping bind")
- }
- if _, err := androidAPIPath(); err != nil {
- t.Skip("No android API platform found in $ANDROID_HOME, skipping bind")
+ if _, err := sdkpath.AndroidAPIPath(minAndroidAPI); err != nil {
+ t.Skip("No compatible android API platform found, skipping bind")
}
case "ios":
if !xcodeAvailable() {
diff --git a/cmd/gomobile/env.go b/cmd/gomobile/env.go
index a7f5485..c40111a 100644
--- a/cmd/gomobile/env.go
+++ b/cmd/gomobile/env.go
@@ -1,6 +1,8 @@
package main
import (
+ "bufio"
+ "encoding/json"
"errors"
"fmt"
"io/fs"
@@ -10,6 +12,8 @@
"path/filepath"
"runtime"
"strings"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
// General mobile build environment. Initialized by envInit.
@@ -280,29 +284,156 @@
return nil
}
-func ndkRoot() (string, error) {
+// Maps GOARCH values to Android ABI strings.
+// See https://developer.android.com/ndk/guides/abis
+func abi(goarch string) string {
+ switch goarch {
+ case "arm":
+ return "armeabi-v7a"
+ case "arm64":
+ return "arm64-v8a"
+ case "386":
+ return "x86"
+ case "amd64":
+ return "x86_64"
+ default:
+ return ""
+ }
+}
+
+// Returns nil if the NDK in `ndkDir` supports the current configured
+// API version and all the specified Android targets.
+func checkNdkRoot(ndkDir string, targets []targetInfo) error {
+ platformsJson, err := os.Open(filepath.Join(ndkDir, "meta", "platforms.json"))
+ if err != nil {
+ return err
+ }
+ decoder := json.NewDecoder(platformsJson)
+ supportedVersions := struct {
+ Min int
+ Max int
+ }{}
+ if err := decoder.Decode(&supportedVersions); err != nil {
+ return err
+ }
+ if supportedVersions.Min > buildAndroidAPI ||
+ supportedVersions.Max < buildAndroidAPI {
+ return fmt.Errorf("unsupported API version %d (not in %d..%d)", buildAndroidAPI, supportedVersions.Min, supportedVersions.Max)
+ }
+ abisJson, err := os.Open(filepath.Join(ndkDir, "meta", "abis.json"))
+ if err != nil {
+ return err
+ }
+ decoder = json.NewDecoder(abisJson)
+ abis := make(map[string]struct{})
+ if err := decoder.Decode(&abis); err != nil {
+ return err
+ }
+ for _, target := range targets {
+ if !isAndroidPlatform(target.platform) {
+ continue
+ }
+ if _, found := abis[abi(target.arch)]; !found {
+ return fmt.Errorf("ndk does not support %s", target.platform)
+ }
+ }
+ return nil
+}
+
+// Searches the side-by-side NDK dirs for compatible SDKs.
+func compatibleNdkRoots(ndkForest string, targets []targetInfo) ([]string, error) {
+ ndkDirs, err := ioutil.ReadDir(ndkForest)
+ if err != nil {
+ return nil, err
+ }
+
+ compatibleNdkRoots := []string{}
+ var lastErr error
+ for _, dirent := range ndkDirs {
+ ndkRoot := filepath.Join(ndkForest, dirent.Name())
+ lastErr = checkNdkRoot(ndkRoot, targets)
+ if lastErr == nil {
+ compatibleNdkRoots = append(compatibleNdkRoots, ndkRoot)
+ }
+ }
+ if len(compatibleNdkRoots) > 0 {
+ return compatibleNdkRoots, nil
+ }
+ return nil, lastErr
+}
+
+// Returns the full version number of an installed copy of the NDK.
+func ndkVersion(ndkRoot string) (string, error) {
+ properties, err := os.Open(filepath.Join(ndkRoot, "source.properties"))
+ if err != nil {
+ return "", err
+ }
+ // Parse the version number out of the .properties file.
+ // See https://en.wikipedia.org/wiki/.properties
+ scanner := bufio.NewScanner(properties)
+ for scanner.Scan() {
+ line := scanner.Text()
+ tokens := strings.SplitN(line, "=", 2)
+ if len(tokens) != 2 {
+ continue
+ }
+ if strings.TrimSpace(tokens[0]) == "Pkg.Revision" {
+ return strings.TrimSpace(tokens[1]), nil
+ }
+ }
+ return "", errors.New("no revision key found")
+}
+
+// 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
+func ndkRoot(targets ...targetInfo) (string, error) {
if buildN {
return "$NDK_PATH", nil
}
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome != "" {
- ndkRoot := filepath.Join(androidHome, "ndk-bundle")
- _, err := os.Stat(ndkRoot)
- if err == nil {
- return ndkRoot, nil
- }
- }
-
+ // Try the ANDROID_NDK_HOME variable. This approach is deprecated, but it
+ // has the highest priority because it represents an explicit user choice.
ndkRoot := os.Getenv("ANDROID_NDK_HOME")
if ndkRoot != "" {
- _, err := os.Stat(ndkRoot)
- if err == nil {
- return ndkRoot, nil
+ if err := checkNdkRoot(ndkRoot, targets); err != nil {
+ return "", fmt.Errorf("ANDROID_NDK_HOME specifies %s, which is unusable: %v", ndkRoot, err)
}
+ return ndkRoot, nil
}
- return "", fmt.Errorf("no Android NDK found in $ANDROID_HOME/ndk-bundle nor in $ANDROID_NDK_HOME")
+ androidHome, err := sdkpath.GetAndroidHome()
+ if err != nil {
+ return "", fmt.Errorf("could not locate Android SDK: %v", err)
+ }
+
+ // Use the newest compatible NDK under the side-by-side path arrangement.
+ ndkForest := filepath.Join(androidHome, "ndk")
+ ndkRoots, sideBySideErr := compatibleNdkRoots(ndkForest, targets)
+ if len(ndkRoots) == 1 {
+ return ndkRoots[0], nil
+ } else if len(ndkRoots) > 1 {
+ // Choose the latest version that supports the build configuration
+ selected := ndkRoots[0]
+ latestVersion := ""
+ for _, dirent := range ndkRoots {
+ version, err := ndkVersion(dirent)
+ if err != nil {
+ continue
+ } else if version > latestVersion {
+ selected = dirent
+ latestVersion = version
+ }
+ }
+ return selected, nil
+ }
+
+ // Try the deprecated NDK location.
+ ndkRoot = filepath.Join(androidHome, "ndk-bundle")
+ if legacyErr := checkNdkRoot(ndkRoot, targets); legacyErr != nil {
+ return "", fmt.Errorf("no usable NDK in %s: %v, %v", androidHome, sideBySideErr, legacyErr)
+ }
+ return ndkRoot, nil
}
func envClang(sdkName string) (clang, cflags string, err error) {
diff --git a/cmd/gomobile/env_test.go b/cmd/gomobile/env_test.go
index 2eef8ab..2c25706 100644
--- a/cmd/gomobile/env_test.go
+++ b/cmd/gomobile/env_test.go
@@ -25,10 +25,10 @@
os.RemoveAll(home)
}()
- os.Setenv("ANDROID_HOME", home)
sdkNDK := filepath.Join(home, "ndk-bundle")
envNDK := filepath.Join(home, "android-ndk")
os.Setenv("ANDROID_NDK_HOME", envNDK)
+ os.Unsetenv("ANDROID_HOME")
if ndk, err := ndkRoot(); err == nil {
t.Errorf("expected error but got %q", ndk)
@@ -38,15 +38,40 @@
if err := os.Mkdir(dir, 0755); err != nil {
t.Fatalf("couldn't mkdir %q", dir)
}
+ if err := os.Mkdir(filepath.Join(dir, "meta"), 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q/meta", dir)
+ }
+ f, err := os.Create(filepath.Join(dir, "meta", "platforms.json"))
+ if err != nil {
+ t.Fatalf("couldn't create platforms.json: %v", err)
+ }
+ if _, err := f.WriteString(`{"min":16,"max":32}`); err != nil {
+ t.Fatalf("couldn't populate platforms.json: %v", err)
+ }
+ if err := f.Close(); err != nil {
+ t.Fatalf("couldn't close platforms.json: %v", err)
+ }
+ f, err = os.Create(filepath.Join(dir, "meta", "abis.json"))
+ if err != nil {
+ t.Fatalf("couldn't create abis.json: %v", err)
+ }
+ if _, err := f.WriteString("{}"); err != nil {
+ t.Fatalf("couldn't populate abis.json: %v", err)
+ }
+ if err := f.Close(); err != nil {
+ t.Fatalf("couldn't close abis.json: %v", err)
+ }
}
- if ndk, _ := ndkRoot(); ndk != sdkNDK {
- t.Errorf("got %q want %q", ndk, sdkNDK)
+ if ndk, err := ndkRoot(); ndk != envNDK {
+ t.Log(err)
+ t.Errorf("got %q want %q", ndk, envNDK)
}
- os.Unsetenv("ANDROID_HOME")
+ os.Setenv("ANDROID_HOME", home)
- if ndk, _ := ndkRoot(); ndk != envNDK {
+ if ndk, err := ndkRoot(); ndk != envNDK {
+ t.Log(err)
t.Errorf("got %q want %q", ndk, envNDK)
}
@@ -56,9 +81,168 @@
t.Errorf("expected error but got %q", ndk)
}
- os.Setenv("ANDROID_HOME", home)
+ os.Unsetenv("ANDROID_NDK_HOME")
if ndk, _ := ndkRoot(); ndk != sdkNDK {
t.Errorf("got %q want %q", ndk, sdkNDK)
}
}
+
+func TestNdkRootSideBySide(t *testing.T) {
+ home, err := ioutil.TempDir("", "gomobile-test-")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ homeorig := os.Getenv("ANDROID_HOME")
+ ndkhomeorig := os.Getenv("ANDROID_NDK_HOME")
+ defer func() {
+ os.Setenv("ANDROID_HOME", homeorig)
+ os.Setenv("ANDROID_NDK_HOME", ndkhomeorig)
+ os.RemoveAll(home)
+ buildAndroidAPI = minAndroidAPI
+ }()
+
+ ndkForest := filepath.Join(home, "ndk")
+ // Newer NDK supports APIs 19-32 and arches arm, arm64, amd64
+ newerNDK := filepath.Join(ndkForest, "newer")
+ // Older NDK supports APIs 16-31 and arches arm, arm64, x86
+ olderNDK := filepath.Join(ndkForest, "older")
+ os.Unsetenv("ANDROID_NDK_HOME")
+ os.Setenv("ANDROID_HOME", home)
+
+ if err := os.Mkdir(ndkForest, 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q", ndkForest)
+ }
+
+ if err := os.Mkdir(newerNDK, 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q", newerNDK)
+ }
+ if err := os.Mkdir(filepath.Join(newerNDK, "meta"), 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q/meta", newerNDK)
+ }
+ f, err := os.Create(filepath.Join(newerNDK, "meta", "platforms.json"))
+ if err != nil {
+ t.Fatalf("couldn't create platforms.json: %v", err)
+ }
+ if _, err := f.WriteString(`{"min":19,"max":32}`); err != nil {
+ t.Fatalf("couldn't populate platforms.json: %v", err)
+ }
+ if err := f.Close(); err != nil {
+ t.Fatalf("couldn't close platforms.json: %v", err)
+ }
+ f, err = os.Create(filepath.Join(newerNDK, "meta", "abis.json"))
+ if err != nil {
+ t.Fatalf("couldn't create abis.json: %v", err)
+ }
+ if _, err := f.WriteString(`{"arm64-v8a": {}, "armeabi-v7a": {}, "x86_64": {}}`); err != nil {
+ t.Fatalf("couldn't populate abis.json: %v", err)
+ }
+ if err := f.Close(); err != nil {
+ t.Fatalf("couldn't close abis.json: %v", err)
+ }
+
+ if err := os.Mkdir(olderNDK, 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q", olderNDK)
+ }
+ if err := os.Mkdir(filepath.Join(olderNDK, "meta"), 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q/meta", olderNDK)
+ }
+ f, err = os.Create(filepath.Join(olderNDK, "meta", "platforms.json"))
+ if err != nil {
+ t.Fatalf("couldn't create platforms.json: %v", err)
+ }
+ if _, err := f.WriteString(`{"min":16,"max":31}`); err != nil {
+ t.Fatalf("couldn't populate platforms.json: %v", err)
+ }
+ if err := f.Close(); err != nil {
+ t.Fatalf("couldn't close platforms.json: %v", err)
+ }
+ f, err = os.Create(filepath.Join(olderNDK, "meta", "abis.json"))
+ if err != nil {
+ t.Fatalf("couldn't create abis.json: %v", err)
+ }
+ if _, err := f.WriteString(`{"arm64-v8a": {}, "armeabi-v7a": {}, "x86": {}}`); err != nil {
+ t.Fatalf("couldn't populate abis.json: %v", err)
+ }
+ if err := f.Close(); err != nil {
+ t.Fatalf("couldn't close abis.json: %v", err)
+ }
+
+ buildAndroidAPI = 15
+
+ if _, err = ndkRoot(); err == nil {
+ t.Error("Expected error due to unsupported version combination")
+ }
+
+ buildAndroidAPI = 16
+
+ ndk, err := ndkRoot()
+ if err != nil {
+ t.Error(err)
+ } else if ndk != olderNDK {
+ t.Errorf("got %s want %s", ndk, olderNDK)
+ }
+
+ ndk, err = ndkRoot(targetInfo{"android", "arm"})
+ if err != nil {
+ t.Error(err)
+ } else if ndk != olderNDK {
+ t.Errorf("got %s want %s", ndk, olderNDK)
+ }
+
+ ndk, err = ndkRoot(targetInfo{"android", "arm"}, targetInfo{"android", "arm64"}, targetInfo{"android", "386"})
+ if err != nil {
+ t.Error(err)
+ } else if ndk != olderNDK {
+ t.Errorf("got %s want %s", ndk, olderNDK)
+ }
+
+ if _, err = ndkRoot(targetInfo{"android", "x86_64"}); err == nil {
+ t.Error("Expected error due to unsupported version+arch combination")
+ }
+
+ buildAndroidAPI = 19
+
+ ndk, err = ndkRoot()
+ if err != nil {
+ t.Error(err)
+ } else if ndk != newerNDK {
+ t.Errorf("got %s want %s", ndk, newerNDK)
+ }
+
+ ndk, err = ndkRoot(targetInfo{"android", "arm"})
+ if err != nil {
+ t.Error(err)
+ } else if ndk != newerNDK {
+ t.Errorf("got %s want %s", ndk, newerNDK)
+ }
+
+ // Fall back to older NDK because newer NDK doesn't support x86.
+ ndk, err = ndkRoot(targetInfo{"android", "arm"}, targetInfo{"android", "arm64"}, targetInfo{"android", "386"})
+ if err != nil {
+ t.Error(err)
+ } else if ndk != olderNDK {
+ t.Errorf("got %s want %s", ndk, olderNDK)
+ }
+
+ buildAndroidAPI = 32
+
+ ndk, err = ndkRoot()
+ if err != nil {
+ t.Error(err)
+ } else if ndk != newerNDK {
+ t.Errorf("got %s want %s", ndk, newerNDK)
+ }
+
+ ndk, err = ndkRoot(targetInfo{"android", "arm"})
+ if err != nil {
+ t.Error(err)
+ } else if ndk != newerNDK {
+ t.Errorf("got %s want %s", ndk, newerNDK)
+ }
+
+ if _, err = ndkRoot(targetInfo{"android", "386"}); err == nil {
+ t.Error("Expected error due to unsupported version+arch combination")
+ }
+}
diff --git a/cmd/gomobile/gendex.go b/cmd/gomobile/gendex.go
index be7470c..1794e97 100644
--- a/cmd/gomobile/gendex.go
+++ b/cmd/gomobile/gendex.go
@@ -13,7 +13,7 @@
// however that would limit gomobile to working with newer versions of
// the Android OS, so we do this while we wait.
//
-// Requires ANDROID_HOME be set to the path of the Android SDK, and
+// Respects ANDROID_HOME to set the path of the Android SDK.
// javac must be on the PATH.
package main
@@ -29,6 +29,8 @@
"os"
"os/exec"
"path/filepath"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
var outfile = flag.String("o", "", "result will be written file")
@@ -52,9 +54,9 @@
}
func gendex() error {
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- return errors.New("ANDROID_HOME not set")
+ androidHome, err := sdkpath.GetAndroidHome()
+ if err != nil {
+ return fmt.Errorf("couldn't find Android SDK: %v", err)
}
if err := os.MkdirAll(tmpdir+"/work/org/golang/app", 0775); err != nil {
return err
diff --git a/cmd/gomobile/init.go b/cmd/gomobile/init.go
index 234b1c1..c1d3b2c 100644
--- a/cmd/gomobile/init.go
+++ b/cmd/gomobile/init.go
@@ -16,6 +16,8 @@
"runtime"
"strings"
"time"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
var (
@@ -122,11 +124,10 @@
if buildN {
cmake = "cmake"
} else {
- sdkRoot := os.Getenv("ANDROID_HOME")
- if sdkRoot == "" {
+ sdkRoot, err := sdkpath.GetAndroidHome()
+ if err != nil {
return nil
}
- var err error
cmake, err = exec.LookPath("cmake")
if err != nil {
cmakePath := filepath.Join(sdkRoot, "cmake")
diff --git a/cmd/gomobile/version.go b/cmd/gomobile/version.go
index b791556..27fd6bd 100644
--- a/cmd/gomobile/version.go
+++ b/cmd/gomobile/version.go
@@ -11,6 +11,8 @@
"os/exec"
"path/filepath"
"strings"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
var cmdVersion = &command{
@@ -56,8 +58,7 @@
platforms += "," + strings.Join(applePlatforms, ",")
}
- // ANDROID_HOME, sdk build tool version
- androidapi, _ := androidAPIPath()
+ androidapi, _ := sdkpath.AndroidAPIPath(buildAndroidAPI)
fmt.Printf("gomobile version %s (%s); androidSDK=%s\n", version, platforms, androidapi)
return nil
diff --git a/example/ivy/android/README.md b/example/ivy/android/README.md
index d5923ea..be8ad7a 100644
--- a/example/ivy/android/README.md
+++ b/example/ivy/android/README.md
@@ -11,16 +11,6 @@
- Android NDK
- `golang.org/x/mobile/cmd/gomobile`
-The `gomobile` command uses `ANDROID_HOME` and `ANDROID_NDK_HOME` environment variables.
-```
-export ANDROID_HOME=/path/to/sdk-directory
-export ANDROID_NDK_HOME=/path/to/ndk-directory
-```
-
-Note: Both ANDROID_SDK_ROOT and ANDROID_NDK_HOME are deprecated in Android tooling, but `gomobile` still uses it. In many cases, `ANDROID_HOME` corresponds to the new [`ANDROID_SDK_ROOT`](https://developer.android.com/studio/command-line/variables). If you installed NDK using [Android Studio's SDK manager](https://developer.android.com/studio/projects/install-ndk#default-version), use the `$ANDROID_SDK_ROOT/ndk/<ndk-version>/` directory as `ANDROID_NDK_HOME` (where `<ndk-version>` is the NDK version you want to use when compiling `robpike.io/ivy` for Android.
-
-(TODO: update `gomobile` to work with modern Android layout)
-
From this directory, run:
```sh
diff --git a/internal/binres/binres.go b/internal/binres/binres.go
index 78b874f..d77bde2 100644
--- a/internal/binres/binres.go
+++ b/internal/binres/binres.go
@@ -82,10 +82,13 @@
ResXMLResourceMap ResType = 0x0180
- ResTablePackage ResType = 0x0200
- ResTableType ResType = 0x0201
- ResTableTypeSpec ResType = 0x0202
- ResTableLibrary ResType = 0x0203
+ ResTablePackage ResType = 0x0200
+ ResTableType ResType = 0x0201
+ ResTableTypeSpec ResType = 0x0202
+ ResTableLibrary ResType = 0x0203
+ ResTableOverlayable ResType = 0x0204
+ ResTableOverlayablePolicy ResType = 0x0205
+ ResTableStagedAlias ResType = 0x0206
)
var (
@@ -247,14 +250,14 @@
Space: "",
Local: "platformBuildVersionCode",
},
- Value: "15",
+ Value: "16",
},
xml.Attr{
Name: xml.Name{
Space: "",
Local: "platformBuildVersionName",
},
- Value: "4.0.4-1406430",
+ Value: "4.1.2-1425332",
})
q = append(q, ltoken{tkn, line})
diff --git a/internal/binres/binres_test.go b/internal/binres/binres_test.go
index 93e7a18..7be872c 100644
--- a/internal/binres/binres_test.go
+++ b/internal/binres/binres_test.go
@@ -16,6 +16,8 @@
"sort"
"strings"
"testing"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
func init() {
@@ -408,10 +410,9 @@
}
}
if err == nil && v == "__" {
- if !strings.HasPrefix(x, "4.0.") {
+ if !strings.HasPrefix(x, "4.1.") {
// as of the time of this writing, the current version of build tools being targeted
- // reports 4.0.4-1406430. Previously, this was 4.0.3. This number is likely still due
- // to change so only report error if 4.x incremented.
+ // reports 4.1.2-1425332.
//
// TODO this check has the potential to hide real errors but can be fixed once more
// of the xml document is unmarshalled and XML can be queried to assure this is related
@@ -455,9 +456,8 @@
}
func TestOpenTable(t *testing.T) {
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- t.Skip("ANDROID_HOME env var not set")
+ if _, err := sdkpath.GetAndroidHome(); err != nil {
+ t.Skip("Could not locate Android SDK")
}
tbl, err := OpenTable()
if err != nil {
@@ -577,7 +577,7 @@
if typ.entryCount != xtyp.entryCount {
t.Fatal("typ.entryCount doesn't match")
}
- if typ.entriesStart != xtyp.entriesStart {
+ if typ.entriesStart-uint32(typ.headerByteSize) != xtyp.entriesStart-uint32(xtyp.headerByteSize) {
t.Fatal("typ.entriesStart doesn't match")
}
if len(typ.indices) != len(xtyp.indices) {
@@ -629,9 +629,8 @@
func checkResources(t *testing.T) {
t.Helper()
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- t.Skip("ANDROID_HOME env var not set")
+ if _, err := sdkpath.GetAndroidHome(); err != nil {
+ t.Skip("Could not locate Android SDK")
}
rscPath, err := apiResourcesPath()
if err != nil {
@@ -643,9 +642,8 @@
}
func BenchmarkTableRefByName(b *testing.B) {
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- b.Fatal("ANDROID_HOME env var not set")
+ if _, err := sdkpath.GetAndroidHome(); err != nil {
+ b.Fatal("Could not locate Android SDK")
}
b.ReportAllocs()
diff --git a/internal/binres/genarsc.go b/internal/binres/genarsc.go
index e93ae88..4ec35fb 100644
--- a/internal/binres/genarsc.go
+++ b/internal/binres/genarsc.go
@@ -8,8 +8,7 @@
// Genarsc generates stripped down version of android.jar resources used
// for validation of manifest entries.
//
-// Requires ANDROID_HOME be set to the path of the Android SDK and the
-// current sdk platform installed that matches MinSDK.
+// Requires the selected Android SDK to support the MinSDK platform version.
package main
import (
diff --git a/internal/binres/sdk.go b/internal/binres/sdk.go
index 6c1c343..7b4b851 100644
--- a/internal/binres/sdk.go
+++ b/internal/binres/sdk.go
@@ -8,12 +8,13 @@
"io"
"os"
"path"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
// MinSDK is the targeted sdk version for support by package binres.
-const MinSDK = 15
+const MinSDK = 16
-// Requires environment variable ANDROID_HOME to be set.
func apiResources() ([]byte, error) {
apiResPath, err := apiResourcesPath()
if err != nil {
@@ -50,14 +51,11 @@
}
func apiResourcesPath() (string, error) {
- // TODO(elias.naur): use the logic from gomobile's androidAPIPath and use the any installed version of the
- // Android SDK instead. Currently, the binres_test.go tests fail on anything newer than android-15.
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- return "", fmt.Errorf("ANDROID_HOME env var not set")
+ platformDir, err := sdkpath.AndroidAPIPath(MinSDK)
+ if err != nil {
+ return "", err
}
- platform := fmt.Sprintf("android-%v", MinSDK)
- return path.Join(sdkdir, "platforms", platform, "android.jar"), nil
+ return path.Join(platformDir, "android.jar"), nil
}
// PackResources produces a stripped down gzip version of the resources.arsc from api jar.
diff --git a/internal/binres/table.go b/internal/binres/table.go
index 304736d..acdfa98 100644
--- a/internal/binres/table.go
+++ b/internal/binres/table.go
@@ -298,7 +298,7 @@
return nil
}
- buf := bin[idOffset:]
+ buf := bin[idOffset:pkg.byteSize]
for len(buf) > 0 {
t := ResType(btou16(buf))
switch t {
@@ -317,8 +317,14 @@
last := pkg.specs[len(pkg.specs)-1]
last.types = append(last.types, typ)
buf = buf[typ.byteSize:]
+ case ResTableStagedAlias:
+ alias := new(StagedAlias)
+ if err := alias.UnmarshalBinary(buf); err != nil {
+ return err
+ }
+ buf = buf[alias.byteSize:]
default:
- return errWrongType(t, ResTableTypeSpec, ResTableType)
+ return errWrongType(t, ResTableTypeSpec, ResTableType, ResTableStagedAlias)
}
}
@@ -536,10 +542,10 @@
// fmt.Println("language/country:", u16tos(typ.config.locale.language), u16tos(typ.config.locale.country))
+ typ.indices = make([]uint32, typ.entryCount)
buf := bin[typ.headerByteSize:typ.entriesStart]
- for len(buf) > 0 {
- typ.indices = append(typ.indices, btou32(buf))
- buf = buf[4:]
+ for i := range typ.indices {
+ typ.indices[i] = btou32(buf[i*4:])
}
if len(typ.indices) != int(typ.entryCount) {
@@ -616,6 +622,65 @@
return bin, nil
}
+type StagedAliasEntry struct {
+ stagedID uint32
+ finalizedID uint32
+}
+
+func (ae *StagedAliasEntry) MarshalBinary() ([]byte, error) {
+ bin := make([]byte, 8)
+ putu32(bin, ae.stagedID)
+ putu32(bin[4:], ae.finalizedID)
+ return bin, nil
+}
+
+func (ae *StagedAliasEntry) UnmarshalBinary(bin []byte) error {
+ ae.stagedID = btou32(bin)
+ ae.finalizedID = btou32(bin[4:])
+ return nil
+}
+
+type StagedAlias struct {
+ chunkHeader
+ count uint32
+ entries []StagedAliasEntry
+}
+
+func (a *StagedAlias) UnmarshalBinary(bin []byte) error {
+ if err := (&a.chunkHeader).UnmarshalBinary(bin); err != nil {
+ return err
+ }
+ if a.typ != ResTableStagedAlias {
+ return errWrongType(a.typ, ResTableStagedAlias)
+ }
+ a.count = btou32(bin[8:])
+ a.entries = make([]StagedAliasEntry, a.count)
+ for i := range a.entries {
+ if err := a.entries[i].UnmarshalBinary(bin[12+i*8:]); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (a *StagedAlias) MarshalBinary() ([]byte, error) {
+ chunkHeaderBin, err := a.chunkHeader.MarshalBinary()
+ if err != nil {
+ return nil, err
+ }
+ countBin := make([]byte, 4)
+ putu32(countBin, a.count)
+ bin := append(chunkHeaderBin, countBin...)
+ for _, entry := range a.entries {
+ entryBin, err := entry.MarshalBinary()
+ if err != nil {
+ return nil, err
+ }
+ bin = append(bin, entryBin...)
+ }
+ return bin, nil
+}
+
// Entry is a resource key typically followed by a value or resource map.
type Entry struct {
size uint16
diff --git a/internal/binres/testdata/bootstrap.arsc b/internal/binres/testdata/bootstrap.arsc
index 60c1dda..4705191 100644
--- a/internal/binres/testdata/bootstrap.arsc
+++ b/internal/binres/testdata/bootstrap.arsc
Binary files differ
diff --git a/internal/binres/testdata/bootstrap.bin b/internal/binres/testdata/bootstrap.bin
index 35d3df6..ff5617c 100644
--- a/internal/binres/testdata/bootstrap.bin
+++ b/internal/binres/testdata/bootstrap.bin
Binary files differ
diff --git a/internal/binres/testdata/gen.sh b/internal/binres/testdata/gen.sh
index e37f21b..b19318f 100755
--- a/internal/binres/testdata/gen.sh
+++ b/internal/binres/testdata/gen.sh
@@ -1,10 +1,10 @@
#! /usr/bin/env bash
# version of build-tools tests run against
-AAPT=${ANDROID_HOME}/build-tools/23.0.1/aapt
+AAPT=${ANDROID_HOME:-${HOME}/Android/Sdk}/build-tools/32.0.0/aapt
# minimum version of android api for resource identifiers supported
-APIJAR=${ANDROID_HOME}/platforms/android-15/android.jar
+APIJAR=${ANDROID_HOME:-${HOME}/Android/Sdk}/platforms/android-16/android.jar
for f in *.xml; do
RES=""
diff --git a/internal/sdkpath/sdkpath.go b/internal/sdkpath/sdkpath.go
new file mode 100644
index 0000000..f6e6ee9
--- /dev/null
+++ b/internal/sdkpath/sdkpath.go
@@ -0,0 +1,75 @@
+package sdkpath
+
+import (
+ "fmt"
+ "os"
+ "path"
+ "path/filepath"
+ "strconv"
+ "strings"
+)
+
+// Package sdkpath provides functions for locating the Android SDK.
+// These functions respect the ANDROID_HOME environment variable, and
+// otherwise use the default SDK location.
+
+func GetAndroidHome() (string, error) {
+ androidHome := os.Getenv("ANDROID_HOME")
+ if androidHome == "" {
+ home, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ androidHome = path.Join(home, "Android", "Sdk")
+ }
+ info, err := os.Stat(androidHome)
+ if err != nil {
+ return "", fmt.Errorf("%v; Android Studio was not found at %s", err, androidHome)
+ } else if !info.IsDir() {
+ return "", fmt.Errorf("%s is not a directory", androidHome)
+ }
+ return androidHome, nil
+}
+
+// AndroidAPIPath returns an android SDK platform directory within the configured SDK.
+// If there are multiple platforms that satisfy the minimum version requirement,
+// androidAPIPath returns the latest one among them.
+func AndroidAPIPath(api int) (string, error) {
+ sdk, err := GetAndroidHome()
+ if err != nil {
+ return "", err
+ }
+ sdkDir, err := os.Open(filepath.Join(sdk, "platforms"))
+ if err != nil {
+ return "", fmt.Errorf("failed to find android SDK platform: %v", err)
+ }
+ defer sdkDir.Close()
+ fis, err := sdkDir.Readdir(-1)
+ if err != nil {
+ return "", fmt.Errorf("failed to find android SDK platform (API level: %d): %v", api, err)
+ }
+
+ var apiPath string
+ var apiVer int
+ for _, fi := range fis {
+ name := fi.Name()
+ if !strings.HasPrefix(name, "android-") {
+ continue
+ }
+ n, err := strconv.Atoi(name[len("android-"):])
+ if err != nil || n < api {
+ continue
+ }
+ p := filepath.Join(sdkDir.Name(), name)
+ _, err = os.Stat(filepath.Join(p, "android.jar"))
+ if err == nil && apiVer < n {
+ apiPath = p
+ apiVer = n
+ }
+ }
+ if apiVer == 0 {
+ return "", fmt.Errorf("failed to find android SDK platform (API level: %d) in %s",
+ api, sdkDir.Name())
+ }
+ return apiPath, nil
+}
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Ben Schwartz uploaded patch set #2 to this change.
22 files changed, 567 insertions(+), 155 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Changkun Ou (Gerrit)
Attention is currently required from: Ben Schwartz, Hyang-Ah Hana Kim.
Patch set 2:Run-TryBot +1
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #3 to this change.
22 files changed, 573 insertions(+), 156 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Hyang-Ah Hana Kim.
1 comment:
Patchset:
1 of 2 TryBots failed. […]
Hopefully fixed in patch set 3
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Changkun Ou (Gerrit)
Attention is currently required from: Ben Schwartz, Hyang-Ah Hana Kim.
Patch set 3:Run-TryBot +1
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Hyang-Ah Hana Kim.
16 comments:
Commit Message:
Patch Set #3, Line 14:
Fixes https://github.com/golang/go/issues/52470Fixes golang/go#52470
File cmd/gomobile/env.go:
Patch Set #3, Line 287:
// Maps GOARCH values to Android ABI strings.// abi maps...
Patch Set #3, Line 304:
// Returns nil if the NDK in `ndkDir` supports the current configured// checkNdkRoot returns...
Patch Set #3, Line 343:
// Searches the side-by-side NDK dirs for compatible SDKs.// compatibleNdkRoots searches...
Patch Set #3, Line 365:
// Returns the full version number of an installed copy of the NDK.// ndkVersion returns...
Patch Set #3, Line 387:
// Returns the root path of an installed NDK that supports all the specified// ndkRoot returns...
Patch Set #3, Line 400:
return "", fmt.Errorf("ANDROID_NDK_HOME specifies %s, which is unusable: %v", ndkRoot, err)Use %w
Patch Set #3, Line 407:
return "", fmt.Errorf("could not locate Android SDK: %v", err)Use %w
Patch Set #3, Line 415:
} else if len(ndkRoots) > 1 {Do we need separating the logic for len(ndkRoots) == 1 and len(ndkRoots) > 1?
Patch Set #3, Line 434:
return "", fmt.Errorf("no usable NDK in %s: %v, %v", androidHome, sideBySideErr, legacyErr)Use %w
File cmd/gomobile/env_test.go:
Patch Set #3, Line 46:
t.Fatalf("couldn't create platforms.json: %v", err)Use %w
File cmd/gomobile/gendex.go:
Patch Set #3, Line 59:
return fmt.Errorf("couldn't find Android SDK: %v", err)Use %w
File example/ivy/android/README.md:
Patch Set #3, Line 14:
The `gomobile` command uses `ANDROID_HOME` and `ANDROID_NDK_HOME` environment variables.As ANDROID_HOME and ANDROID_NDK_HOME are still used as hints, can we update this documentation instead of removing this?
File internal/sdkpath/sdkpath.go:
Patch Set #3, Line 26:
return "", fmt.Errorf("%v; Android Studio was not found at %s", err, androidHome)Use %w
Patch Set #3, Line 43:
return "", fmt.Errorf("failed to find android SDK platform: %v", err)Use %w
Patch Set #3, Line 48:
return "", fmt.Errorf("failed to find android SDK platform (API level: %d): %v", api, err)Use %w
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Hyang-Ah Hana Kim.
1 comment:
File internal/sdkpath/sdkpath.go:
Patch Set #3, Line 22:
androidHome = filepath.Join(home, "Android", "Sdk")Is this true? For example, the default Android home is `$HOME/Library/Android/sdk` on macOS.
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
File internal/sdkpath/sdkpath.go:
Patch Set #3, Line 15:
func GetAndroidHome() (string, error) {`AndroidHome()` is better for Go's naming convention
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
File cmd/gomobile/env_test.go:
Patch Set #3, Line 46:
t.Fatalf("couldn't create platforms.json: %v", err)Use %w
NVM
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
File internal/sdkpath/sdkpath.go:
Patch Set #3, Line 1:
package sdkpathAdd the copyright comment
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #4 to this change.
x/mobile: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME, which are deprecated. It also increases the
minimum API version to 16, as all SDKs that supported API 15
are now deprecated.
Fixes golang/go#52470
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #5 to this change.
x/mobile: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME, which are deprecated. It also increases the
minimum API version to 16, as all SDKs that supported API 15
are now deprecated.
Fixes https://github.com/golang/go/issues/52470
Change-Id: I546365774a089e5d7ae1be0a538efd72741d92ac
---
M bind/java/seq_test.go
M cmd/gomobile/bind.go
M cmd/gomobile/bind_androidapp.go
M cmd/gomobile/bind_test.go
M cmd/gomobile/build.go
M cmd/gomobile/build_androidapp.go
M cmd/gomobile/build_test.go
M cmd/gomobile/env.go
M cmd/gomobile/env_test.go
M cmd/gomobile/gendex.go
M cmd/gomobile/init.go
M cmd/gomobile/version.go
M example/ivy/android/README.md
M internal/binres/binres.go
M internal/binres/binres_test.go
M internal/binres/genarsc.go
M internal/binres/sdk.go
M internal/binres/table.go
M internal/binres/testdata/bootstrap.arsc
M internal/binres/testdata/bootstrap.bin
M internal/binres/testdata/gen.sh
A internal/sdkpath/sdkpath.go
22 files changed, 588 insertions(+), 151 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
18 comments:
Commit Message:
Patch Set #3, Line 14:
Fixes https://github.com/golang/go/issues/52470Fixes golang/go#52470
Done
File cmd/gomobile/env.go:
Patch Set #3, Line 287:
// Maps GOARCH values to Android ABI strings.// abi maps...
Done
Patch Set #3, Line 304:
// Returns nil if the NDK in `ndkDir` supports the current configured// checkNdkRoot returns...
Done
Patch Set #3, Line 343:
// Searches the side-by-side NDK dirs for compatible SDKs.// compatibleNdkRoots searches...
Done
Patch Set #3, Line 365:
// Returns the full version number of an installed copy of the NDK.// ndkVersion returns...
Done
Patch Set #3, Line 387:
// Returns the root path of an installed NDK that supports all the specified// ndkRoot returns...
Done
Patch Set #3, Line 400:
return "", fmt.Errorf("ANDROID_NDK_HOME specifies %s, which is unusable: %v", ndkRoot, err)Use %w
Done
Patch Set #3, Line 407:
return "", fmt.Errorf("could not locate Android SDK: %v", err)Use %w
Done
Patch Set #3, Line 415:
} else if len(ndkRoots) > 1 {Do we need separating the logic for len(ndkRoots) == 1 and len(ndkRoots) > 1?
No, it's not necessary. Combined.
Patch Set #3, Line 434:
return "", fmt.Errorf("no usable NDK in %s: %v, %v", androidHome, sideBySideErr, legacyErr)Use %w
Done
File cmd/gomobile/gendex.go:
Patch Set #3, Line 59:
return fmt.Errorf("couldn't find Android SDK: %v", err)Use %w
Done
File example/ivy/android/README.md:
Patch Set #3, Line 14:
The `gomobile` command uses `ANDROID_HOME` and `ANDROID_NDK_HOME` environment variables.
As ANDROID_HOME and ANDROID_NDK_HOME are still used as hints, can we update this documentation inste […]
OK, I've adjusted the text here instead of removing it.
File internal/sdkpath/sdkpath.go:
Patch Set #3, Line 1:
package sdkpathAdd the copyright comment
Done
Patch Set #3, Line 15:
func GetAndroidHome() (string, error) {`AndroidHome()` is better for Go's naming convention
Done
Patch Set #3, Line 22:
androidHome = filepath.Join(home, "Android", "Sdk")Is this true? For example, the default Android home is `$HOME/Library/Android/sdk` on macOS.
You're right, it's different by platform. Fixed.
Patch Set #3, Line 26:
return "", fmt.Errorf("%v; Android Studio was not found at %s", err, androidHome)Use %w
Done
Patch Set #3, Line 43:
return "", fmt.Errorf("failed to find android SDK platform: %v", err)Use %w
Done
Patch Set #3, Line 48:
return "", fmt.Errorf("failed to find android SDK platform (API level: %d): %v", api, err)Use %w
Done
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
10 comments:
Commit Message:
Done
I thinks this was not updated
Patchset:
Thank you!
File cmd/gomobile/env.go:
Patch Set #5, Line 351:
var lastErr errorWhy do we keep the last error? (i.e., why don't we return as soon as possible after `checkNdkRoot` returns an error?)
Patch Set #5, Line 412:
ndkRoots, sideBySideErr := compatibleNdkRoots(ndkForest, targets)If `sideBySideErr` is not nil, shouldn't this function return earlier?
Patch Set #5, Line 420:
} else if version > latestVersion {nit: `else` is not needed
File cmd/gomobile/env_test.go:
Patch Set #5, Line 42:
t.Fatalf("couldn't mkdir %q/meta", dir)`fmt.Fatalf("couldn't mkdir %q", filepath.Join(dir, "meta"))`
Patch Set #5, Line 122:
t.Fatalf("couldn't mkdir %q/meta", newerNDK)ditto
Patch Set #5, Line 149:
t.Fatalf("couldn't mkdir %q/meta", olderNDK)ditto
File cmd/gomobile/gendex.go:
Patch Set #5, Line 59:
return fmt.Errorf("couldn't find Android SDK: %v", err)Use %w
File internal/sdkpath/sdkpath.go:
Patch Set #5, Line 44:
} else if !info.IsDir() {nit: `else` is not needed
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #6 to this change.
x/mobile: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME, which are deprecated. It also increases the
minimum API version to 16, as all SDKs that supported API 15
are now deprecated.
Fixes golang/go#52470
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #7 to this change.
x/mobile: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME, which are deprecated. It also increases the
minimum API version to 16, as all SDKs that supported API 15
are now deprecated.
Fixes https://github.com/golang/go/issues/52470
Change-Id: I546365774a089e5d7ae1be0a538efd72741d92ac
---
M bind/java/seq_test.go
M cmd/gomobile/bind.go
M cmd/gomobile/bind_androidapp.go
M cmd/gomobile/bind_test.go
M cmd/gomobile/build.go
M cmd/gomobile/build_androidapp.go
M cmd/gomobile/build_test.go
M cmd/gomobile/env.go
M cmd/gomobile/env_test.go
M cmd/gomobile/gendex.go
M cmd/gomobile/init.go
M cmd/gomobile/version.go
M example/ivy/android/README.md
M internal/binres/binres.go
M internal/binres/binres_test.go
M internal/binres/genarsc.go
M internal/binres/sdk.go
M internal/binres/table.go
M internal/binres/testdata/bootstrap.arsc
M internal/binres/testdata/bootstrap.bin
M internal/binres/testdata/gen.sh
A internal/sdkpath/sdkpath.go
22 files changed, 591 insertions(+), 151 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #8 to this change.
x/mobile: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME, which are deprecated. It also increases the
minimum API version to 16, as all SDKs that supported API 15
are now deprecated.
Fixes golang/go#52470
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Daniel Skinner, Hajime Hoshi, Hyang-Ah Hana Kim.
11 comments:
Commit Message:
I thinks this was not updated
I tried again...
File cmd/gomobile/env.go:
Patch Set #5, Line 351:
var lastErr error
Why do we keep the last error? (i.e. […]
checkNdkRoot will return an error if this NDK root is not compatible. We are looking for any compatible NDK root, so we only return an error if none of the installed NDKs are compatible.
Patch Set #5, Line 412:
ndkRoots, sideBySideErr := compatibleNdkRoots(ndkForest, targets)If `sideBySideErr` is not nil, shouldn't this function return earlier?
That would mean that if both `$ANDROID_HOME/ndk/<version X>` and `$ANDROID_HOME/ndk-bundle` exist, and version X is not suitable, we would not try the `/ndk-bundle` option. This is a possible behavior, but I don't see a benefit from refusing to try `/ndk-bundle` in this case.
Patch Set #5, Line 420:
} else if version > latestVersion {nit: `else` is not needed
OK, removed. (This expands the loop by two lines.)
File cmd/gomobile/env_test.go:
Patch Set #5, Line 42:
t.Fatalf("couldn't mkdir %q/meta", dir)
`fmt.Fatalf("couldn't mkdir %q", filepath. […]
Done (via temp variable).
Patch Set #5, Line 122:
t.Fatalf("couldn't mkdir %q/meta", newerNDK)ditto
Done
Patch Set #5, Line 149:
t.Fatalf("couldn't mkdir %q/meta", olderNDK)ditto
Done
File cmd/gomobile/gendex.go:
Patch Set #5, Line 59:
return fmt.Errorf("couldn't find Android SDK: %v", err)Use %w
Done
File internal/binres/binres_test.go:
Patch Set #5, Line 580:
-uint32(typ.headerByteSize)why does the test need to subtract the chunkheader bytesize from entriesStart?
I believe this is because the original header contains `StagedAliasEntry` elements that are discarded during parsing and not re-serialized.
File internal/binres/table.go:
Patch Set #5, Line 301:
pkg.byteSizewhy is buf being clipped here?
This is necessary for the tests to pass on my system. I believe it's because `resources.arsc` in modern Android SDK versions contains multiple "packages" concatenated.
File internal/sdkpath/sdkpath.go:
Patch Set #5, Line 44:
} else if !info.IsDir() {nit: `else` is not needed
OK, separated.
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Daniel Skinner, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/env.go:
Patch Set #5, Line 420:
} else if version > latestVersion {OK, removed. (This expands the loop by two lines. […]
Ah right, `version` is not available without `else`... It's ok to leave the `else`
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Daniel Skinner, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #9 to this change.
22 files changed, 587 insertions(+), 151 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Daniel Skinner, Hajime Hoshi, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/env.go:
Patch Set #5, Line 420:
} else if version > latestVersion {
Ah right, `version` is not available without `else`... […]
OK, done
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Changkun Ou, Daniel Skinner, Hajime Hoshi, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/env.go:
Patch Set #5, Line 351:
var lastErr errorcheckNdkRoot will return an error if this NDK root is not compatible. […]
I see.
Hmm, returning only the last error only when no NDK is available sounds unusual. Even if a specific version of NDK is not available, this is a usual case and doesn't seem an error. What about
- Renaming `checkNdkRoot` to `isNdkRootAvailable` and letting it return a boolean
- Not letting `compatibleNdkRoot` return an error
?
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Skinner (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
2 comments:
File internal/binres/binres_test.go:
Patch Set #5, Line 580:
-uint32(typ.headerByteSize)why does the test need to subtract the chunkheader bytesize from entriesStart?
File internal/binres/table.go:
Patch Set #5, Line 301:
pkg.byteSizewhy is buf being clipped here?
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Daniel Skinner, Hajime Hoshi, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #10 to this change.
22 files changed, 578 insertions(+), 149 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Daniel Skinner, Hajime Hoshi, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/env.go:
Patch Set #5, Line 351:
var lastErr error
I see. […]
OK, done. This is simpler (but provides less debug information to users in broken configurations).
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Daniel Skinner, Hyang-Ah Hana Kim.
1 comment:
Commit Message:
Patch Set #10, Line 7:
x/mobile: modernize handling of Android SDK and NDK pathsall:
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Daniel Skinner, Hyang-Ah Hana Kim.
4 comments:
File cmd/gomobile/env.go:
Patch Set #10, Line 304:
// isUsableNdkRoot returns nil if the NDK in `ndkRoot` supports the currentUpdate the comment.
Patch Set #10, Line 306:
func isUsableNdkRoot(ndkRoot string, targets []targetInfo) bool {`isNdkRootAvailable`? (disclaimer: I am not an English native speaker)
Patch Set #10, Line 350:
compatibleNdkRoots := []string{}`var compatibleNdkRoots []string` to avoid unnecessary allocations.
File cmd/gomobile/gendex.go:
Patch Set #10, Line 57:
androidHome, err := sdkpath.GetAndroidHome()`AndroidHome()`?
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Skinner (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
1 comment:
File internal/binres/binres_test.go:
Patch Set #5, Line 580:
-uint32(typ.headerByteSize)
I believe this is because the original header contains `StagedAliasEntry` elements that are discarde […]
These tests are meant to assure byte for byte comparison. A subtraction here because chunkheader sizes differ compromises the intent. Can this either be commented with a detailed TODO or perhaps have a separate explicit check used when chunkheader differs and call this out via a TODO. I'll defer to whichever you think is appropriate.
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #11 to this change.
all: modernize handling of Android SDK and NDK paths
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
5 comments:
Commit Message:
Patch Set #10, Line 7:
x/mobile: modernize handling of Android SDK and NDK pathsall:
Done
File cmd/gomobile/env.go:
Patch Set #10, Line 304:
// isUsableNdkRoot returns nil if the NDK in `ndkRoot` supports the currentUpdate the comment.
Done
Patch Set #10, Line 306:
func isUsableNdkRoot(ndkRoot string, targets []targetInfo) bool {`isNdkRootAvailable`? (disclaimer: I am not an English native speaker)
I think "is [this thing a] usable NDK root[?]" is more natural
Patch Set #10, Line 350:
compatibleNdkRoots := []string{}`var compatibleNdkRoots []string` to avoid unnecessary allocations.
Done
File cmd/gomobile/gendex.go:
Patch Set #10, Line 57:
androidHome, err := sdkpath.GetAndroidHome()`AndroidHome()`?
Done
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Patch set 11:Run-TryBot +1
1 comment:
Patchset:
TRY=android
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/bind_androidapp.go:
Patch Set #11, Line 23:
return fmt.Errorf("this command requires the Android SDK to be installed")nit: `fmt.Errorf("this command requires the Android SDK to be installed: %w", err)`
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/env_test.go:
Patch Set #5, Line 42:
t.Fatalf("couldn't mkdir %q/meta", dir)
Done (via temp variable).
Not fixed yet?
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #12 to this change.
22 files changed, 600 insertions(+), 155 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #13 to this change.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Daniel Skinner, Hajime Hoshi, Hyang-Ah Hana Kim.
3 comments:
File cmd/gomobile/bind_androidapp.go:
Patch Set #11, Line 23:
return fmt.Errorf("this command requires the Android SDK to be installed")
nit: `fmt. […]
Done
File cmd/gomobile/env_test.go:
Patch Set #5, Line 42:
t.Fatalf("couldn't mkdir %q/meta", dir)
Not fixed yet?
Hm, not sure how that happened. Fixed now.
File internal/binres/binres_test.go:
Patch Set #5, Line 580:
-uint32(typ.headerByteSize)
These tests are meant to assure byte for byte comparison. […]
After looking into this more, I've come to understand that this test was flawed in two ways:
1. It assumed that `Type.config` has the same size in both `typ` and `xtyp`. This is false because the Config struct is variable-size, growing as it gains features in each API version. Unrecognized fields are stripped in the parse-serialize process, resulting in a smaller size.
2. The constant size hardcoded in table.go was incorrect.
I've fixed these issues and adjusted this test condition to test the real invariant here.
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Skinner (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
Patch set 13:Code-Review +1
1 comment:
Patchset:
thanks
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Patch set 13:Run-TryBot +1Code-Review +2
1 comment:
Patchset:
TRY=android
Hana, PTAL
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/bind_test.go:
Patch Set #13, Line 27:
components = components[len(components)-2:]What is `2` here? Doesn't this work only on Linux?
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Patch set 13:-Code-Review
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
Ben Schwartz uploaded patch set #14 to this change.
22 files changed, 601 insertions(+), 155 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Hajime Hoshi, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/bind_test.go:
Patch Set #13, Line 27:
components = components[len(components)-2:]What is `2` here? Doesn't this work only on Linux?
No, this is platform-independent. There are two path components after $ANDROID_HOME in the return value of `sdkpath.AndroidAPIPath()`. I've added a comment to help clarify what is going on here.
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
Patch set 14:Code-Review +2
2 comments:
Patchset:
Thank you!
File cmd/gomobile/bind_test.go:
Patch Set #13, Line 27:
components = components[len(components)-2:]
No, this is platform-independent. […]
I see. `$ANDROID_HOME` is `/path/to/Android/sdk` in this case, right?
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Hyang-Ah Hana Kim.
1 comment:
File cmd/gomobile/bind_test.go:
Patch Set #13, Line 27:
components = components[len(components)-2:]
I see. […]
Yes, that's correct.
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou, Hyang-Ah Hana Kim.
1 comment:
Patchset:
Friendly ping, Hana
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou.
16 comments:
Commit Message:
Patch Set #14, Line 10:
which are deprecated.It looks like Android project couldn't set its mind and ANDROID_HOME is "un-deprecated".
Patchset:
I skipped internal/binres/table.go review - relying on other reviewers who are experts in this domain. (I will take a quick look before merging).
File cmd/gomobile/bind_test.go:
Patch Set #14, Line 28:
len(components)check len(components)>=2.
File cmd/gomobile/env.go:
Patch Set #14, Line 304:
isUsableNdkRoot(optional) isUsableNDKRoot
per https://github.com/golang/go/wiki/CodeReviewComments#initialismsOTOH, I see many existing usages of `Ndk` in this code base.
Patch Set #14, Line 321:
false(ok as a followup) I think it will be useful if isUsableNDKRoot returns an error or writes debug logs to explain why it isn't usable. (for example, the error message in line 428 can be cryptic)
Patch Set #14, Line 323:
abisJson, err := os.Open(filepath.Join(ndkRoot, "meta", "abis.json"))close open files.
(check all the os.Open calls in this change are followed by Close or deferred Close)if version, err := ndkVersion(ndkRoot); err != nil {
continueI am afraid I don't understand this:
It looks like the result of ndkVersion(ndkRoots[0]) won't affect the result. For example, if len(ndkRoots) == 1, this ndkRoot function always returns ndkRoots[0] as selected regardless of the result of ndkVersion() call. On the other hand, if ndkVersion(ndkRoots[1]) fails, it won't be selected. Why is the first element special?
File cmd/gomobile/env_test.go:
metaDir := filepath.Join(dir, "meta")
if err := os.Mkdir(metaDir, 0755); err != nil {
t.Fatalf("couldn't mkdir %q", metaDir)
}
f, err := os.Create(filepath.Join(metaDir, "platforms.json"))
if err != nil {
t.Fatalf("couldn't create platforms.json: %v", err)
}
if _, err := f.WriteString(`{"min":16,"max":32}`); err != nil {
t.Fatalf("couldn't populate platforms.json: %v", err)
}
if err := f.Close(); err != nil {
t.Fatalf("couldn't close platforms.json: %v", err)
}Use os.WriteFile to simplify this.
Same for TestNdkRootSideBySide.
t.Log(err)
t.Errorf("got %q want %q", ndk, envNDK)Combine t.Log and t.Error here.
t.Errorf("ndkRoot() = (%q, %v), want (%q, nil)", ndk, err, envNDK)
Same for line 75
Patch Set #14, Line 92:
func TestNdkRootSideBySide(t *testing.T) {Can you add a comment to describe what this test does?
I think taking out boilerplate common between this and TestNdkRoot can help readers focus on the actual test logic, rather than the test fixture setup.
Patch Set #14, Line 175:
buildAndroidAPI = 15(optional) Can you make this test to be table-driven test or subtest (https://go.dev/blog/subtests)?
File internal/binres/binres_test.go:
Patch Set #14, Line 460:
t.Skip("Could not locate Android SDK")include `err` in the message.
File internal/sdkpath/sdkpath.go:
// Package sdkpath provides functions for locating the Android SDK.
// These functions respect the ANDROID_HOME environment variable, and
// otherwise use the default SDK location.move this to the package declaration line (line 5) so this can serve as a normal package doc.
Patch Set #5, Line 30:
mastercan you use a permanent link - so this link can survive even when the files in android project move around? Same for the link below.
Patch Set #5, Line 43:
Android StudioReference of "Android Studio" here is odd.
Shouldn't it be ANDROID_HOME or Android SDK?Patch Set #5, Line 52:
androidAPIPatnit: AndroidAPIPath
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou.
Ben Schwartz uploaded patch set #15 to this change.
all: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME. Setting ANDROID_HOME is generally optional,
and ANDROID_NDK_HOME is deprecated.
This change also increases the minimum API version to 16, as
22 files changed, 546 insertions(+), 176 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou.
Ben Schwartz uploaded patch set #16 to this change.
22 files changed, 545 insertions(+), 176 deletions(-)
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Ben Schwartz (Gerrit)
Attention is currently required from: Changkun Ou, Hyang-Ah Hana Kim.
14 comments:
File cmd/gomobile/bind_test.go:
Patch Set #14, Line 28:
len(components)check len(components)>=2.
Done
File cmd/gomobile/env.go:
Patch Set #14, Line 304:
isUsableNdkRoot
(optional) isUsableNDKRoot […]
Done
Patch Set #14, Line 321:
false
(ok as a followup) I think it will be useful if isUsableNDKRoot returns an error or writes debug log […]
I agree, and that's how this CL originally worked. I changed it in response to reviewer comments. I have reverted that change, so this exposes detailed errors again.
Patch Set #14, Line 323:
abisJson, err := os.Open(filepath.Join(ndkRoot, "meta", "abis.json"))
close open files. […]
Done
if version, err := ndkVersion(ndkRoot); err != nil {
continueI am afraid I don't understand this:
OK, I've restructured it to try to make it clearer.
It looks like the result of ndkVersion(ndkRoots[0]) won't affect the result.
It does affect the result. For example, if len(ndkRoots) == 2, the one with the newer version will be selected.
For example, if len(ndkRoots) == 1, this ndkRoot function always returns ndkRoots[0] as selected regardless of the result of ndkVersion() call.
Yes. This is as intended. I didn't see a reason to be strict about rejecting ndkRoots because ndkVersion() fails.
On the other hand, if ndkVersion(ndkRoots[1]) fails, it won't be selected. Why is the first element special?
If len(ndkRoots) == 2, and ndkVersion() fails both times, then there is a tie, which must be broken in some arbitrary fashion. The new code makes this clearer (and incidentally breaks the tie in the other direction).
File cmd/gomobile/env_test.go:
metaDir := filepath.Join(dir, "meta")
if err := os.Mkdir(metaDir, 0755); err != nil {
t.Fatalf("couldn't mkdir %q", metaDir)
}
f, err := os.Create(filepath.Join(metaDir, "platforms.json"))
if err != nil {
t.Fatalf("couldn't create platforms.json: %v", err)
}
if _, err := f.WriteString(`{"min":16,"max":32}`); err != nil {
t.Fatalf("couldn't populate platforms.json: %v", err)
}
if err := f.Close(); err != nil {
t.Fatalf("couldn't close platforms.json: %v", err)
}
Use os.WriteFile to simplify this. […]
Done
t.Log(err)
t.Errorf("got %q want %q", ndk, envNDK)
Combine t.Log and t.Error here. […]
Done
Patch Set #14, Line 92:
func TestNdkRootSideBySide(t *testing.T) {
Can you add a comment to describe what this test does? […]
Done
Patch Set #14, Line 175:
buildAndroidAPI = 15
(optional) Can you make this test to be table-driven test or subtest (https://go. […]
Done
File internal/binres/binres_test.go:
Patch Set #14, Line 460:
t.Skip("Could not locate Android SDK")include `err` in the message.
Done
File internal/sdkpath/sdkpath.go:
// Package sdkpath provides functions for locating the Android SDK.
// These functions respect the ANDROID_HOME environment variable, and
// otherwise use the default SDK location.move this to the package declaration line (line 5) so this can serve as a normal package doc.
Done
Patch Set #5, Line 30:
master
can you use a permanent link - so this link can survive even when the files in android project move […]
Done
Patch Set #5, Line 43:
Android Studio
Reference of "Android Studio" here is odd. […]
Yes, thanks. Fixed.
Patch Set #5, Line 52:
androidAPIPatnit: AndroidAPIPath
Done
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hyang-Ah Hana Kim (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou.
Patch set 16:Run-TryBot +1Code-Review +2
1 comment:
File cmd/gomobile/env.go:
if version, err := ndkVersion(ndkRoot); err != nil {
continue
> I am afraid I don't understand this: […]
Thanks for the explanation and updating the comment.
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Suzy Mueller (Gerrit)
Attention is currently required from: Ben Schwartz, Changkun Ou.
Patch set 16:Code-Review +1
Hyang-Ah Hana Kim (Gerrit)
Hyang-Ah Hana Kim submitted this change.
all: modernize handling of Android SDK and NDK paths
This change removes Gomobile's dependency on ANDROID_HOME and
ANDROID_NDK_HOME. Setting ANDROID_HOME is generally optional,
and ANDROID_NDK_HOME is deprecated.
This change also increases the minimum API version to 16, as
all SDKs that supported API 15 are now deprecated.
Fixes golang/go#52470
Change-Id: I546365774a089e5d7ae1be0a538efd72741d92ac
Reviewed-on: https://go-review.googlesource.com/c/mobile/+/401574
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Daniel Skinner <dan...@dasa.cc>
Reviewed-by: Suzy Mueller <suz...@golang.org>
Reviewed-by: Hajime Hoshi <hajim...@gmail.com>
Reviewed-by: Hyang-Ah Hana Kim <hya...@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hya...@gmail.com>
---
M bind/java/seq_test.go
M cmd/gomobile/bind.go
M cmd/gomobile/bind_androidapp.go
M cmd/gomobile/bind_test.go
M cmd/gomobile/build.go
M cmd/gomobile/build_androidapp.go
M cmd/gomobile/build_test.go
M cmd/gomobile/env.go
M cmd/gomobile/env_test.go
M cmd/gomobile/gendex.go
M cmd/gomobile/init.go
M cmd/gomobile/version.go
M example/ivy/android/README.md
M internal/binres/binres.go
M internal/binres/binres_test.go
M internal/binres/genarsc.go
M internal/binres/sdk.go
M internal/binres/table.go
M internal/binres/testdata/bootstrap.arsc
M internal/binres/testdata/bootstrap.bin
M internal/binres/testdata/gen.sh
A internal/sdkpath/sdkpath.go
22 files changed, 552 insertions(+), 176 deletions(-)
diff --git a/bind/java/seq_test.go b/bind/java/seq_test.go
index 8f741c6..a5151d4 100644
--- a/bind/java/seq_test.go
+++ b/bind/java/seq_test.go
@@ -17,6 +17,7 @@
"testing"
"golang.org/x/mobile/internal/importers/java"
+ "golang.org/x/mobile/internal/sdkpath"
)
var gomobileBin string
@@ -98,8 +99,8 @@
// runTest runs the Android java test class specified with javaCls. If javaPkg is
// set, it is passed with the -javapkg flag to gomobile. The pkgNames lists the Go
// packages to bind for the test.
-// This requires the gradle command in PATH and
-// the Android SDK whose path is available through ANDROID_HOME environment variable.
+// This requires the gradle command to be in PATH and the Android SDK to be
+// installed.
func runTest(t *testing.T, pkgNames []string, javaPkg, javaCls string) {
if gomobileBin == "" {
t.Skipf("no gomobile on %s", runtime.GOOS)
@@ -108,8 +109,8 @@
if err != nil {
t.Skip("command gradle not found, skipping")
}
- if sdk := os.Getenv("ANDROID_HOME"); sdk == "" {
- t.Skip("ANDROID_HOME environment var not set, skipping")
+ if _, err := sdkpath.AndroidHome(); err != nil {
+ t.Skip("Android SDK not found, skipping")
}
cwd, err := os.Getwd()
diff --git a/cmd/gomobile/bind.go b/cmd/gomobile/bind.go
index efbc896..c4552e3 100644
--- a/cmd/gomobile/bind.go
+++ b/cmd/gomobile/bind.go
@@ -16,6 +16,7 @@
"path/filepath"
"strings"
+ "golang.org/x/mobile/internal/sdkpath"
"golang.org/x/mod/modfile"
"golang.org/x/tools/go/packages"
)
@@ -42,9 +43,10 @@
the module import wizard (File > New > New Module > Import .JAR or
.AAR package), and setting it as a new dependency
(File > Project Structure > Dependencies). This requires 'javac'
-(version 1.7+) and Android SDK (API level 15 or newer) to build the
-library for Android. The environment variable ANDROID_HOME must be set
-to the path to Android SDK. Use the -javapkg flag to specify the Java
+(version 1.7+) and Android SDK (API level 16 or newer) to build the
+library for Android. The ANDROID_HOME and ANDROID_NDK_HOME environment
+variables can be used to specify the Android SDK and NDK if they are
+not in the default locations. Use the -javapkg flag to specify the Java
package prefix for the generated classes.
By default, -target=android builds shared libraries for all supported
@@ -85,7 +87,7 @@
if bindPrefix != "" {
return fmt.Errorf("-prefix is supported only for Apple targets")
}
- if _, err := ndkRoot(); err != nil {
+ if _, err := ndkRoot(targets[0]); err != nil {
return err
}
} else {
@@ -156,7 +158,7 @@
if bindBootClasspath != "" {
return bindBootClasspath, nil
}
- apiPath, err := androidAPIPath()
+ apiPath, err := sdkpath.AndroidAPIPath(buildAndroidAPI)
if err != nil {
return "", err
}
diff --git a/cmd/gomobile/bind_androidapp.go b/cmd/gomobile/bind_androidapp.go
index 7703868..a56fd82 100644
--- a/cmd/gomobile/bind_androidapp.go
+++ b/cmd/gomobile/bind_androidapp.go
@@ -12,15 +12,15 @@
"os"
"os/exec"
"path/filepath"
- "strconv"
"strings"
+ "golang.org/x/mobile/internal/sdkpath"
"golang.org/x/tools/go/packages"
)
func goAndroidBind(gobind string, pkgs []*packages.Package, targets []targetInfo) error {
- if sdkDir := os.Getenv("ANDROID_HOME"); sdkDir == "" {
- return fmt.Errorf("this command requires ANDROID_HOME environment variable (path to the Android SDK)")
+ if _, err := sdkpath.AndroidHome(); err != nil {
+ return fmt.Errorf("this command requires the Android SDK to be installed: %w", err)
}
// Run gobind to generate the bindings
@@ -270,7 +270,7 @@
const (
javacTargetVer = "1.7"
- minAndroidAPI = 15
+ minAndroidAPI = 16
)
func buildJar(w io.Writer, srcDir string) error {
@@ -370,46 +370,3 @@
}
return jarw.Close()
}
-
-// androidAPIPath returns an android SDK platform directory under ANDROID_HOME.
-// If there are multiple platforms that satisfy the minimum version requirement
-// androidAPIPath returns the latest one among them.
-func androidAPIPath() (string, error) {
- sdk := os.Getenv("ANDROID_HOME")
- if sdk == "" {
- return "", fmt.Errorf("ANDROID_HOME environment var is not set")
- }
- sdkDir, err := os.Open(filepath.Join(sdk, "platforms"))
- if err != nil {
- return "", fmt.Errorf("failed to find android SDK platform: %v", err)
- }
- defer sdkDir.Close()
- fis, err := sdkDir.Readdir(-1)
- if err != nil {
- return "", fmt.Errorf("failed to find android SDK platform (API level: %d): %v", buildAndroidAPI, err)
- }
-
- var apiPath string
- var apiVer int
- for _, fi := range fis {
- name := fi.Name()
- if !strings.HasPrefix(name, "android-") {
- continue
- }
- n, err := strconv.Atoi(name[len("android-"):])
- if err != nil || n < buildAndroidAPI {
- continue
- }
- p := filepath.Join(sdkDir.Name(), name)
- _, err = os.Stat(filepath.Join(p, "android.jar"))
- if err == nil && apiVer < n {
- apiPath = p
- apiVer = n
- }
- }
- if apiVer == 0 {
- return "", fmt.Errorf("failed to find android SDK platform (API level: %d) in %s",
- buildAndroidAPI, sdkDir.Name())
- }
- return apiPath, nil
-}
diff --git a/cmd/gomobile/bind_test.go b/cmd/gomobile/bind_test.go
index 0e9f401..cac3511 100644
--- a/cmd/gomobile/bind_test.go
+++ b/cmd/gomobile/bind_test.go
@@ -14,18 +14,22 @@
"strings"
"testing"
"text/template"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
func TestBindAndroid(t *testing.T) {
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- t.Skip("ANDROID_HOME not found, skipping bind")
- }
- platform, err := androidAPIPath()
+ platform, err := sdkpath.AndroidAPIPath(minAndroidAPI)
if err != nil {
- t.Skip("No android API platform found in $ANDROID_HOME, skipping bind")
+ t.Skip("No compatible Android API platform found, skipping bind")
}
- platform = strings.Replace(platform, androidHome, "$ANDROID_HOME", -1)
+ // platform is a path like "/path/to/Android/sdk/platforms/android-32"
+ components := strings.Split(platform, string(filepath.Separator))
+ if len(components) < 2 {
+ t.Fatalf("API path is too short: %s", platform)
+ }
+ components = components[len(components)-2:]
+ platformRel := filepath.Join("$ANDROID_HOME", components[0], components[1])
defer func() {
xout = os.Stderr
@@ -77,7 +81,7 @@
JavaPkg string
}{
outputData: output,
- AndroidPlatform: platform,
+ AndroidPlatform: platformRel,
JavaPkg: tc.javaPkg,
}
@@ -273,12 +277,8 @@
t.Run(target, func(t *testing.T) {
switch target {
case "android":
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- t.Skip("ANDROID_HOME not found, skipping bind")
- }
- if _, err := androidAPIPath(); err != nil {
- t.Skip("No android API platform found in $ANDROID_HOME, skipping bind")
+ if _, err := sdkpath.AndroidAPIPath(minAndroidAPI); err != nil {
+ t.Skip("No compatible Android API platform found, skipping bind")
}
case "ios":
if !xcodeAvailable() {
diff --git a/cmd/gomobile/build.go b/cmd/gomobile/build.go
index 4c83ca0..64a28fe 100644
--- a/cmd/gomobile/build.go
+++ b/cmd/gomobile/build.go
@@ -17,6 +17,7 @@
"strconv"
"strings"
+ "golang.org/x/mobile/internal/sdkpath"
"golang.org/x/tools/go/packages"
)
@@ -60,7 +61,7 @@
The default version is 13.0.
Flag -androidapi sets the Android API version to compile against.
-The default and minimum is 15.
+The default and minimum is 16.
The -bundleid flag is required for -target ios and sets the bundle ID to use
with the app.
@@ -215,7 +216,7 @@
if tmpdir != "" {
cmd = strings.Replace(cmd, tmpdir, "$WORK", -1)
}
- if androidHome := os.Getenv("ANDROID_HOME"); androidHome != "" {
+ if androidHome, err := sdkpath.AndroidHome(); err == nil {
cmd = strings.Replace(cmd, androidHome, "$ANDROID_HOME", -1)
}
if gomobilepath != "" {
diff --git a/cmd/gomobile/build_androidapp.go b/cmd/gomobile/build_androidapp.go
index b06ea29..bcd2664 100644
--- a/cmd/gomobile/build_androidapp.go
+++ b/cmd/gomobile/build_androidapp.go
@@ -25,7 +25,7 @@
)
func goAndroidBuild(pkg *packages.Package, targets []targetInfo) (map[string]bool, error) {
- ndkRoot, err := ndkRoot()
+ ndkRoot, err := ndkRoot(targets...)
if err != nil {
return nil, err
}
diff --git a/cmd/gomobile/build_test.go b/cmd/gomobile/build_test.go
index ff21f87..448c6dd 100644
--- a/cmd/gomobile/build_test.go
+++ b/cmd/gomobile/build_test.go
@@ -14,6 +14,8 @@
"strings"
"testing"
"text/template"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
func TestRFC1034Label(t *testing.T) {
@@ -228,12 +230,8 @@
t.Run(target, func(t *testing.T) {
switch target {
case "android":
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- t.Skip("ANDROID_HOME not found, skipping bind")
- }
- if _, err := androidAPIPath(); err != nil {
- t.Skip("No android API platform found in $ANDROID_HOME, skipping bind")
+ if _, err := sdkpath.AndroidAPIPath(minAndroidAPI); err != nil {
+ t.Skip("No compatible android API platform found, skipping bind")
}
case "ios":
if !xcodeAvailable() {
diff --git a/cmd/gomobile/env.go b/cmd/gomobile/env.go
index ff32517..43f24b9 100644
--- a/cmd/gomobile/env.go
+++ b/cmd/gomobile/env.go
@@ -1,6 +1,8 @@
package main
import (
+ "bufio"
+ "encoding/json"
"errors"
"fmt"
"io/fs"
@@ -10,6 +12,8 @@
"path/filepath"
"runtime"
"strings"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
// General mobile build environment. Initialized by envInit.
@@ -276,29 +280,155 @@
return nil
}
-func ndkRoot() (string, error) {
+// abi maps GOARCH values to Android ABI strings.
+// See https://developer.android.com/ndk/guides/abis
+func abi(goarch string) string {
+ switch goarch {
+ case "arm":
+ return "armeabi-v7a"
+ case "arm64":
+ return "arm64-v8a"
+ case "386":
+ return "x86"
+ case "amd64":
+ return "x86_64"
+ default:
+ return ""
+ }
+}
+
+// checkNDKRoot returns nil if the NDK in `ndkRoot` supports the current configured
+// API version and all the specified Android targets.
+func checkNDKRoot(ndkRoot string, targets []targetInfo) error {
+ platformsJson, err := os.Open(filepath.Join(ndkRoot, "meta", "platforms.json"))
+ if err != nil {
+ return err
+ }
+ defer platformsJson.Close()
+ decoder := json.NewDecoder(platformsJson)
+ supportedVersions := struct {
+ Min int
+ Max int
+ }{}
+ if err := decoder.Decode(&supportedVersions); err != nil {
+ return err
+ }
+ if supportedVersions.Min > buildAndroidAPI ||
+ supportedVersions.Max < buildAndroidAPI {
+ return fmt.Errorf("unsupported API version %d (not in %d..%d)", buildAndroidAPI, supportedVersions.Min, supportedVersions.Max)
+ }
+ abisJson, err := os.Open(filepath.Join(ndkRoot, "meta", "abis.json"))
+ if err != nil {
+ return err
+ }
+ defer abisJson.Close()
+ decoder = json.NewDecoder(abisJson)
+ abis := make(map[string]struct{})
+ if err := decoder.Decode(&abis); err != nil {
+ return err
+ }
+ for _, target := range targets {
+ if !isAndroidPlatform(target.platform) {
+ continue
+ }
+ if _, found := abis[abi(target.arch)]; !found {
+ return fmt.Errorf("ndk does not support %s", target.platform)
+ }
+ }
+ return nil
+}
+
+// compatibleNDKRoots searches the side-by-side NDK dirs for compatible SDKs.
+func compatibleNDKRoots(ndkForest string, targets []targetInfo) ([]string, error) {
+ ndkDirs, err := ioutil.ReadDir(ndkForest)
+ if err != nil {
+ return nil, err
+ }
+ compatibleNDKRoots := []string{}
+ var lastErr error
+ for _, dirent := range ndkDirs {
+ ndkRoot := filepath.Join(ndkForest, dirent.Name())
+ lastErr = checkNDKRoot(ndkRoot, targets)
+ if lastErr == nil {
+ compatibleNDKRoots = append(compatibleNDKRoots, ndkRoot)
+ }
+ }
+ if len(compatibleNDKRoots) > 0 {
+ return compatibleNDKRoots, nil
+ }
+ return nil, lastErr
+}
+
+// ndkVersion returns the full version number of an installed copy of the NDK,
+// or "" if it cannot be determined.
+func ndkVersion(ndkRoot string) string {
+ properties, err := os.Open(filepath.Join(ndkRoot, "source.properties"))
+ if err != nil {
+ return ""
+ }
+ defer properties.Close()
+ // Parse the version number out of the .properties file.
+ // See https://en.wikipedia.org/wiki/.properties
+ scanner := bufio.NewScanner(properties)
+ for scanner.Scan() {
+ line := scanner.Text()
+ tokens := strings.SplitN(line, "=", 2)
+ if len(tokens) != 2 {
+ continue
+ }
+ if strings.TrimSpace(tokens[0]) == "Pkg.Revision" {
+ return strings.TrimSpace(tokens[1])
+ }
+ }
+ return ""
+}
+
+// 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
+func ndkRoot(targets ...targetInfo) (string, error) {
if buildN {
return "$NDK_PATH", nil
}
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome != "" {
- ndkRoot := filepath.Join(androidHome, "ndk-bundle")
- _, err := os.Stat(ndkRoot)
- if err == nil {
- return ndkRoot, nil
+ // Try the ANDROID_NDK_HOME variable. This approach is deprecated, but it
+ // has the highest priority because it represents an explicit user choice.
+ if ndkRoot := os.Getenv("ANDROID_NDK_HOME"); ndkRoot != "" {
+ if err := checkNDKRoot(ndkRoot, targets); err != nil {
+ return "", fmt.Errorf("ANDROID_NDK_HOME specifies %s, which is unusable: %w", ndkRoot, err)
}
+ return ndkRoot, nil
}
- ndkRoot := os.Getenv("ANDROID_NDK_HOME")
- if ndkRoot != "" {
- _, err := os.Stat(ndkRoot)
- if err == nil {
- return ndkRoot, nil
- }
+ androidHome, err := sdkpath.AndroidHome()
+ if err != nil {
+ return "", fmt.Errorf("could not locate Android SDK: %w", err)
}
- return "", fmt.Errorf("no Android NDK found in $ANDROID_HOME/ndk-bundle nor in $ANDROID_NDK_HOME")
+ // Use the newest compatible NDK under the side-by-side path arrangement.
+ ndkForest := filepath.Join(androidHome, "ndk")
+ ndkRoots, sideBySideErr := compatibleNDKRoots(ndkForest, targets)
+ if len(ndkRoots) != 0 {
+ // Choose the latest version that supports the build configuration.
+ // NDKs whose version cannot be determined will be least preferred.
+ // In the event of a tie, the later ndkRoot will win.
+ maxVersion := ""
+ var selected string
+ for _, ndkRoot := range ndkRoots {
+ version := ndkVersion(ndkRoot)
+ if version >= maxVersion {
+ maxVersion = version
+ selected = ndkRoot
+ }
+ }
+ return selected, nil
+ }
+ // Try the deprecated NDK location.
+ ndkRoot := filepath.Join(androidHome, "ndk-bundle")
+ if legacyErr := checkNDKRoot(ndkRoot, targets); legacyErr != nil {
+ return "", fmt.Errorf("no usable NDK in %s: %w, %v", androidHome, sideBySideErr, legacyErr)
+ }
+ return ndkRoot, nil
}
func envClang(sdkName string) (clang, cflags string, err error) {
diff --git a/cmd/gomobile/env_test.go b/cmd/gomobile/env_test.go
index 2eef8ab..e5ad282 100644
--- a/cmd/gomobile/env_test.go
+++ b/cmd/gomobile/env_test.go
@@ -5,6 +5,7 @@
package main
import (
+ "fmt"
"io/ioutil"
"os"
"path/filepath"
@@ -18,47 +19,143 @@
}
homeorig := os.Getenv("ANDROID_HOME")
+ os.Unsetenv("ANDROID_HOME")
ndkhomeorig := os.Getenv("ANDROID_NDK_HOME")
+ os.Unsetenv("ANDROID_NDK_HOME")
+
defer func() {
os.Setenv("ANDROID_HOME", homeorig)
os.Setenv("ANDROID_NDK_HOME", ndkhomeorig)
os.RemoveAll(home)
}()
- os.Setenv("ANDROID_HOME", home)
- sdkNDK := filepath.Join(home, "ndk-bundle")
- envNDK := filepath.Join(home, "android-ndk")
- os.Setenv("ANDROID_NDK_HOME", envNDK)
-
- if ndk, err := ndkRoot(); err == nil {
- t.Errorf("expected error but got %q", ndk)
- }
-
- for _, dir := range []string{sdkNDK, envNDK} {
+ makeMockNDK := func(path, version, platforms, abis string) string {
+ dir := filepath.Join(home, path)
if err := os.Mkdir(dir, 0755); err != nil {
t.Fatalf("couldn't mkdir %q", dir)
}
+ propertiesPath := filepath.Join(dir, "source.properties")
+ propertiesData := []byte("Pkg.Revision = " + version)
+ if err := os.WriteFile(propertiesPath, propertiesData, 0644); err != nil {
+ t.Fatalf("couldn't write source.properties: %v", err)
+ }
+ metaDir := filepath.Join(dir, "meta")
+ if err := os.Mkdir(metaDir, 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q", metaDir)
+ }
+ platformsPath := filepath.Join(metaDir, "platforms.json")
+ platformsData := []byte(platforms)
+ if err := os.WriteFile(platformsPath, platformsData, 0644); err != nil {
+ t.Fatalf("couldn't write platforms.json: %v", err)
+ }
+ abisPath := filepath.Join(metaDir, "abis.json")
+ abisData := []byte(abis)
+ if err := os.WriteFile(abisPath, abisData, 0644); err != nil {
+ t.Fatalf("couldn't populate abis.json: %v", err)
+ }
+ return dir
}
- if ndk, _ := ndkRoot(); ndk != sdkNDK {
- t.Errorf("got %q want %q", ndk, sdkNDK)
- }
+ t.Run("no NDK in the default location", func(t *testing.T) {
+ os.Setenv("ANDROID_HOME", home)
+ defer os.Unsetenv("ANDROID_HOME")
+ if ndk, err := ndkRoot(); err == nil {
+ t.Errorf("expected error but got %q", ndk)
+ }
+ })
- os.Unsetenv("ANDROID_HOME")
+ t.Run("NDK location is set but is wrong", func(t *testing.T) {
+ os.Setenv("ANDROID_NDK_HOME", filepath.Join(home, "no-such-path"))
+ defer os.Unsetenv("ANDROID_NDK_HOME")
+ if ndk, err := ndkRoot(); err == nil {
+ t.Errorf("expected error but got %q", ndk)
+ }
+ })
- if ndk, _ := ndkRoot(); ndk != envNDK {
- t.Errorf("got %q want %q", ndk, envNDK)
- }
+ t.Run("Two NDKs installed", func(t *testing.T) {
+ // Default path for pre-side-by-side NDKs.
+ sdkNDK := makeMockNDK("ndk-bundle", "fake-version", `{"min":16,"max":32}`, "{}")
+ defer os.RemoveAll(sdkNDK)
+ // Arbitrary location for testing ANDROID_NDK_HOME.
+ envNDK := makeMockNDK("custom-location", "fake-version", `{"min":16,"max":32}`, "{}")
- os.RemoveAll(envNDK)
+ // ANDROID_NDK_HOME is sufficient.
+ os.Setenv("ANDROID_NDK_HOME", envNDK)
+ if ndk, err := ndkRoot(); ndk != envNDK {
+ t.Errorf("got (%q, %v) want (%q, nil)", ndk, err, envNDK)
+ }
- if ndk, err := ndkRoot(); err == nil {
- t.Errorf("expected error but got %q", ndk)
- }
+ // ANDROID_NDK_HOME takes precedence over ANDROID_HOME.
+ os.Setenv("ANDROID_HOME", home)
+ if ndk, err := ndkRoot(); ndk != envNDK {
+ t.Errorf("got (%q, %v) want (%q, nil)", ndk, err, envNDK)
+ }
- os.Setenv("ANDROID_HOME", home)
+ // ANDROID_NDK_HOME is respected even if there is no NDK there.
+ os.RemoveAll(envNDK)
+ if ndk, err := ndkRoot(); err == nil {
+ t.Errorf("expected error but got %q", ndk)
+ }
- if ndk, _ := ndkRoot(); ndk != sdkNDK {
- t.Errorf("got %q want %q", ndk, sdkNDK)
- }
+ // ANDROID_HOME is used if ANDROID_NDK_HOME is not set.
+ os.Unsetenv("ANDROID_NDK_HOME")
+ if ndk, err := ndkRoot(); ndk != sdkNDK {
+ t.Errorf("got (%q, %v) want (%q, nil)", ndk, err, envNDK)
+ }
+ })
+
+ t.Run("Modern 'side-by-side' NDK selection", func(t *testing.T) {
+ defer func() {
+ buildAndroidAPI = minAndroidAPI
+ }()
+
+ ndkForest := filepath.Join(home, "ndk")
+ if err := os.Mkdir(ndkForest, 0755); err != nil {
+ t.Fatalf("couldn't mkdir %q", ndkForest)
+ }
+
+ path := filepath.Join("ndk", "newer")
+ platforms := `{"min":19,"max":32}`
+ abis := `{"arm64-v8a": {}, "armeabi-v7a": {}, "x86_64": {}}`
+ version := "17.2.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"
+ olderNDK := makeMockNDK(path, version, platforms, abis)
+
+ testCases := []struct {
+ api int
+ targets []targetInfo
+ wantNDKRoot string
+ }{
+ {15, nil, ""},
+ {16, nil, olderNDK},
+ {16, []targetInfo{{"android", "arm"}}, olderNDK},
+ {16, []targetInfo{{"android", "arm"}, {"android", "arm64"}}, olderNDK},
+ {16, []targetInfo{{"android", "x86_64"}}, ""},
+ {19, nil, newerNDK},
+ {19, []targetInfo{{"android", "arm"}}, newerNDK},
+ {19, []targetInfo{{"android", "arm"}, {"android", "arm64"}, {"android", "386"}}, olderNDK},
+ {32, nil, newerNDK},
+ {32, []targetInfo{{"android", "arm"}}, newerNDK},
+ {32, []targetInfo{{"android", "386"}}, ""},
+ }
+
+ for i, tc := range testCases {
+ t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+ buildAndroidAPI = tc.api
+ ndk, err := ndkRoot(tc.targets...)
+ if len(tc.wantNDKRoot) != 0 {
+ if ndk != tc.wantNDKRoot || err != nil {
+ t.Errorf("got (%q, %v), want (%q, nil)", ndk, err, tc.wantNDKRoot)
+ }
+ } else if err == nil {
+ t.Error("expected error")
+ }
+ })
+ }
+ })
}
diff --git a/cmd/gomobile/gendex.go b/cmd/gomobile/gendex.go
index be7470c..88cf554 100644
--- a/cmd/gomobile/gendex.go
+++ b/cmd/gomobile/gendex.go
@@ -13,7 +13,7 @@
// however that would limit gomobile to working with newer versions of
// the Android OS, so we do this while we wait.
//
-// Requires ANDROID_HOME be set to the path of the Android SDK, and
+// Respects ANDROID_HOME to set the path of the Android SDK.
// javac must be on the PATH.
package main
@@ -29,6 +29,8 @@
"os"
"os/exec"
"path/filepath"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
var outfile = flag.String("o", "", "result will be written file")
@@ -52,9 +54,9 @@
}
func gendex() error {
- androidHome := os.Getenv("ANDROID_HOME")
- if androidHome == "" {
- return errors.New("ANDROID_HOME not set")
+ androidHome, err := sdkpath.AndroidHome()
+ if err != nil {
+ return fmt.Errorf("couldn't find Android SDK: %w", err)
}
if err := os.MkdirAll(tmpdir+"/work/org/golang/app", 0775); err != nil {
return err
diff --git a/cmd/gomobile/init.go b/cmd/gomobile/init.go
index 26da302..e7392df 100644
--- a/cmd/gomobile/init.go
+++ b/cmd/gomobile/init.go
@@ -16,6 +16,8 @@
"runtime"
"strings"
"time"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
var (
@@ -122,11 +124,10 @@
if buildN {
cmake = "cmake"
} else {
- sdkRoot := os.Getenv("ANDROID_HOME")
- if sdkRoot == "" {
+ sdkRoot, err := sdkpath.AndroidHome()
+ if err != nil {
return nil
}
- var err error
cmake, err = exec.LookPath("cmake")
if err != nil {
cmakePath := filepath.Join(sdkRoot, "cmake")
diff --git a/cmd/gomobile/version.go b/cmd/gomobile/version.go
index b791556..27fd6bd 100644
--- a/cmd/gomobile/version.go
+++ b/cmd/gomobile/version.go
@@ -11,6 +11,8 @@
"os/exec"
"path/filepath"
"strings"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
var cmdVersion = &command{
@@ -56,8 +58,7 @@
platforms += "," + strings.Join(applePlatforms, ",")
}
- // ANDROID_HOME, sdk build tool version
- androidapi, _ := androidAPIPath()
+ androidapi, _ := sdkpath.AndroidAPIPath(buildAndroidAPI)
fmt.Printf("gomobile version %s (%s); androidSDK=%s\n", version, platforms, androidapi)
return nil
diff --git a/example/ivy/android/README.md b/example/ivy/android/README.md
index d5923ea..dc56712 100644
--- a/example/ivy/android/README.md
+++ b/example/ivy/android/README.md
@@ -11,16 +11,12 @@
- Android NDK
- `golang.org/x/mobile/cmd/gomobile`
-The `gomobile` command uses `ANDROID_HOME` and `ANDROID_NDK_HOME` environment variables.
+The `gomobile` command respects the `ANDROID_HOME` and `ANDROID_NDK_HOME` environment variables. If `gomobile` can't find your SDK and NDK, you can set these environment variables to specify their locations:
```
export ANDROID_HOME=/path/to/sdk-directory
export ANDROID_NDK_HOME=/path/to/ndk-directory
```
-Note: Both ANDROID_SDK_ROOT and ANDROID_NDK_HOME are deprecated in Android tooling, but `gomobile` still uses it. In many cases, `ANDROID_HOME` corresponds to the new [`ANDROID_SDK_ROOT`](https://developer.android.com/studio/command-line/variables). If you installed NDK using [Android Studio's SDK manager](https://developer.android.com/studio/projects/install-ndk#default-version), use the `$ANDROID_SDK_ROOT/ndk/<ndk-version>/` directory as `ANDROID_NDK_HOME` (where `<ndk-version>` is the NDK version you want to use when compiling `robpike.io/ivy` for Android.
-
-(TODO: update `gomobile` to work with modern Android layout)
-
From this directory, run:
```sh
diff --git a/internal/binres/binres.go b/internal/binres/binres.go
index 78b874f..d77bde2 100644
--- a/internal/binres/binres.go
+++ b/internal/binres/binres.go
@@ -82,10 +82,13 @@
ResXMLResourceMap ResType = 0x0180
- ResTablePackage ResType = 0x0200
- ResTableType ResType = 0x0201
- ResTableTypeSpec ResType = 0x0202
- ResTableLibrary ResType = 0x0203
+ ResTablePackage ResType = 0x0200
+ ResTableType ResType = 0x0201
+ ResTableTypeSpec ResType = 0x0202
+ ResTableLibrary ResType = 0x0203
+ ResTableOverlayable ResType = 0x0204
+ ResTableOverlayablePolicy ResType = 0x0205
+ ResTableStagedAlias ResType = 0x0206
)
var (
@@ -247,14 +250,14 @@
Space: "",
Local: "platformBuildVersionCode",
},
- Value: "15",
+ Value: "16",
},
xml.Attr{
Name: xml.Name{
Space: "",
Local: "platformBuildVersionName",
},
- Value: "4.0.4-1406430",
+ Value: "4.1.2-1425332",
})
q = append(q, ltoken{tkn, line})
diff --git a/internal/binres/binres_test.go b/internal/binres/binres_test.go
index 93e7a18..dbac875 100644
--- a/internal/binres/binres_test.go
+++ b/internal/binres/binres_test.go
@@ -16,6 +16,8 @@
"sort"
"strings"
"testing"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
func init() {
@@ -408,10 +410,9 @@
}
}
if err == nil && v == "__" {
- if !strings.HasPrefix(x, "4.0.") {
+ if !strings.HasPrefix(x, "4.1.") {
// as of the time of this writing, the current version of build tools being targeted
- // reports 4.0.4-1406430. Previously, this was 4.0.3. This number is likely still due
- // to change so only report error if 4.x incremented.
+ // reports 4.1.2-1425332.
//
// TODO this check has the potential to hide real errors but can be fixed once more
// of the xml document is unmarshalled and XML can be queried to assure this is related
@@ -455,9 +456,8 @@
}
func TestOpenTable(t *testing.T) {
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- t.Skip("ANDROID_HOME env var not set")
+ if _, err := sdkpath.AndroidHome(); err != nil {
+ t.Skipf("Could not locate Android SDK: %v", err)
}
tbl, err := OpenTable()
if err != nil {
@@ -577,8 +577,10 @@
if typ.entryCount != xtyp.entryCount {
t.Fatal("typ.entryCount doesn't match")
}
- if typ.entriesStart != xtyp.entriesStart {
- t.Fatal("typ.entriesStart doesn't match")
+ // Config size can differ after serialization due to the loss of extended fields
+ // during reserialization, but the fixed portions of the Type header must not change.
+ if uint32(typ.headerByteSize)-typ.config.size != uint32(xtyp.headerByteSize)-uint32(xtyp.config.size) {
+ t.Fatal("fixed size header portions don't match")
}
if len(typ.indices) != len(xtyp.indices) {
t.Fatal("typ.indices length don't match")
@@ -629,9 +631,8 @@
func checkResources(t *testing.T) {
t.Helper()
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- t.Skip("ANDROID_HOME env var not set")
+ if _, err := sdkpath.AndroidHome(); err != nil {
+ t.Skip("Could not locate Android SDK")
}
rscPath, err := apiResourcesPath()
if err != nil {
@@ -643,9 +644,8 @@
}
func BenchmarkTableRefByName(b *testing.B) {
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- b.Fatal("ANDROID_HOME env var not set")
+ if _, err := sdkpath.AndroidHome(); err != nil {
+ b.Fatal("Could not locate Android SDK")
}
b.ReportAllocs()
diff --git a/internal/binres/genarsc.go b/internal/binres/genarsc.go
index e93ae88..4ec35fb 100644
--- a/internal/binres/genarsc.go
+++ b/internal/binres/genarsc.go
@@ -8,8 +8,7 @@
// Genarsc generates stripped down version of android.jar resources used
// for validation of manifest entries.
//
-// Requires ANDROID_HOME be set to the path of the Android SDK and the
-// current sdk platform installed that matches MinSDK.
+// Requires the selected Android SDK to support the MinSDK platform version.
package main
import (
diff --git a/internal/binres/sdk.go b/internal/binres/sdk.go
index 6c1c343..607e0b7 100644
--- a/internal/binres/sdk.go
+++ b/internal/binres/sdk.go
@@ -7,13 +7,14 @@
"fmt"
"io"
"os"
- "path"
+ "path/filepath"
+
+ "golang.org/x/mobile/internal/sdkpath"
)
// MinSDK is the targeted sdk version for support by package binres.
-const MinSDK = 15
+const MinSDK = 16
-// Requires environment variable ANDROID_HOME to be set.
func apiResources() ([]byte, error) {
apiResPath, err := apiResourcesPath()
if err != nil {
@@ -50,14 +51,11 @@
}
func apiResourcesPath() (string, error) {
- // TODO(elias.naur): use the logic from gomobile's androidAPIPath and use the any installed version of the
- // Android SDK instead. Currently, the binres_test.go tests fail on anything newer than android-15.
- sdkdir := os.Getenv("ANDROID_HOME")
- if sdkdir == "" {
- return "", fmt.Errorf("ANDROID_HOME env var not set")
+ platformDir, err := sdkpath.AndroidAPIPath(MinSDK)
+ if err != nil {
+ return "", err
}
- platform := fmt.Sprintf("android-%v", MinSDK)
- return path.Join(sdkdir, "platforms", platform, "android.jar"), nil
+ return filepath.Join(platformDir, "android.jar"), nil
}
// PackResources produces a stripped down gzip version of the resources.arsc from api jar.
diff --git a/internal/binres/table.go b/internal/binres/table.go
index 304736d..b552cba 100644
--- a/internal/binres/table.go
+++ b/internal/binres/table.go
@@ -248,7 +248,8 @@
typePool *Pool // type names; e.g. theme
keyPool *Pool // resource names; e.g. Theme.NoTitleBar
- specs []*TypeSpec
+ aliases []*StagedAlias
+ specs []*TypeSpec
}
func (pkg *Package) UnmarshalBinary(bin []byte) error {
@@ -298,7 +299,7 @@
return nil
}
- buf := bin[idOffset:]
+ buf := bin[idOffset:pkg.byteSize]
for len(buf) > 0 {
t := ResType(btou16(buf))
switch t {
@@ -317,8 +318,15 @@
last := pkg.specs[len(pkg.specs)-1]
last.types = append(last.types, typ)
buf = buf[typ.byteSize:]
+ case ResTableStagedAlias:
+ alias := new(StagedAlias)
+ if err := alias.UnmarshalBinary(buf); err != nil {
+ return err
+ }
+ pkg.aliases = append(pkg.aliases, alias)
+ buf = buf[alias.byteSize:]
default:
- return errWrongType(t, ResTableTypeSpec, ResTableType)
+ return errWrongType(t, ResTableTypeSpec, ResTableType, ResTableStagedAlias)
}
}
@@ -371,6 +379,14 @@
bin = append(bin, b...)
}
+ for _, alias := range pkg.aliases {
+ b, err := alias.MarshalBinary()
+ if err != nil {
+ return nil, err
+ }
+ bin = append(bin, b...)
+ }
+
for _, spec := range pkg.specs {
b, err := spec.MarshalBinary()
if err != nil {
@@ -537,14 +553,15 @@
// fmt.Println("language/country:", u16tos(typ.config.locale.language), u16tos(typ.config.locale.country))
buf := bin[typ.headerByteSize:typ.entriesStart]
- for len(buf) > 0 {
- typ.indices = append(typ.indices, btou32(buf))
- buf = buf[4:]
+ if len(buf) != 4*int(typ.entryCount) {
+ return fmt.Errorf("index buffer len[%v] doesn't match entryCount[%v]", len(buf), typ.entryCount)
}
- if len(typ.indices) != int(typ.entryCount) {
- return fmt.Errorf("indices len[%v] doesn't match entryCount[%v]", len(typ.indices), typ.entryCount)
+ typ.indices = make([]uint32, typ.entryCount)
+ for i := range typ.indices {
+ typ.indices[i] = btou32(buf[i*4:])
}
+
typ.entries = make([]*Entry, typ.entryCount)
for i, x := range typ.indices {
@@ -572,9 +589,9 @@
putu32(bin[12:], uint32(len(typ.entries)))
putu32(bin[16:], uint32(56+len(typ.entries)*4))
- // assure typ.config.size is always written as 52; extended configuration beyond supported
+ // assure typ.config.size is always written as 36; extended configuration beyond supported
// API level is not supported by this marshal implementation but will be forward-compatible.
- putu32(bin[20:], 52)
+ putu32(bin[20:], 36)
putu16(bin[24:], typ.config.imsi.mcc)
putu16(bin[26:], typ.config.imsi.mnc)
@@ -616,6 +633,65 @@
return bin, nil
}
+type StagedAliasEntry struct {
+ stagedID uint32
+ finalizedID uint32
+}
+
+func (ae *StagedAliasEntry) MarshalBinary() ([]byte, error) {
+ bin := make([]byte, 8)
+ putu32(bin, ae.stagedID)
+ putu32(bin[4:], ae.finalizedID)
+ return bin, nil
+}
+
+func (ae *StagedAliasEntry) UnmarshalBinary(bin []byte) error {
+ ae.stagedID = btou32(bin)
+ ae.finalizedID = btou32(bin[4:])
+ return nil
+}
+
+type StagedAlias struct {
+ chunkHeader
+ count uint32
+ entries []StagedAliasEntry
+}
+
+func (a *StagedAlias) UnmarshalBinary(bin []byte) error {
+ if err := (&a.chunkHeader).UnmarshalBinary(bin); err != nil {
+ return err
+ }
+ if a.typ != ResTableStagedAlias {
+ return errWrongType(a.typ, ResTableStagedAlias)
+ }
+ a.count = btou32(bin[8:])
+ a.entries = make([]StagedAliasEntry, a.count)
+ for i := range a.entries {
+ if err := a.entries[i].UnmarshalBinary(bin[12+i*8:]); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (a *StagedAlias) MarshalBinary() ([]byte, error) {
+ chunkHeaderBin, err := a.chunkHeader.MarshalBinary()
+ if err != nil {
+ return nil, err
+ }
+ countBin := make([]byte, 4)
+ putu32(countBin, a.count)
+ bin := append(chunkHeaderBin, countBin...)
+ for _, entry := range a.entries {
+ entryBin, err := entry.MarshalBinary()
+ if err != nil {
+ return nil, err
+ }
+ bin = append(bin, entryBin...)
+ }
+ return bin, nil
+}
+
// Entry is a resource key typically followed by a value or resource map.
type Entry struct {
size uint16
diff --git a/internal/binres/testdata/bootstrap.arsc b/internal/binres/testdata/bootstrap.arsc
index 60c1dda..4705191 100644
--- a/internal/binres/testdata/bootstrap.arsc
+++ b/internal/binres/testdata/bootstrap.arsc
Binary files differ
diff --git a/internal/binres/testdata/bootstrap.bin b/internal/binres/testdata/bootstrap.bin
index 35d3df6..ff5617c 100644
--- a/internal/binres/testdata/bootstrap.bin
+++ b/internal/binres/testdata/bootstrap.bin
Binary files differ
diff --git a/internal/binres/testdata/gen.sh b/internal/binres/testdata/gen.sh
index e37f21b..b19318f 100755
--- a/internal/binres/testdata/gen.sh
+++ b/internal/binres/testdata/gen.sh
@@ -1,10 +1,10 @@
#! /usr/bin/env bash
# version of build-tools tests run against
-AAPT=${ANDROID_HOME}/build-tools/23.0.1/aapt
+AAPT=${ANDROID_HOME:-${HOME}/Android/Sdk}/build-tools/32.0.0/aapt
# minimum version of android api for resource identifiers supported
-APIJAR=${ANDROID_HOME}/platforms/android-15/android.jar
+APIJAR=${ANDROID_HOME:-${HOME}/Android/Sdk}/platforms/android-16/android.jar
for f in *.xml; do
RES=""
diff --git a/internal/sdkpath/sdkpath.go b/internal/sdkpath/sdkpath.go
new file mode 100644
index 0000000..91e9a51
--- /dev/null
+++ b/internal/sdkpath/sdkpath.go
@@ -0,0 +1,89 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package sdkpath provides functions for locating the Android SDK.
+// These functions respect the ANDROID_HOME environment variable, and
+// otherwise use the default SDK location.
+package sdkpath
+
+import (
+ "fmt"
+ "os"
+ "path/filepath"
+ "runtime"
+ "strconv"
+ "strings"
+)
+
+// AndroidHome returns the absolute path of the selected Android SDK,
+// if one can be found.
+func AndroidHome() (string, error) {
+ androidHome := os.Getenv("ANDROID_HOME")
+ if androidHome == "" {
+ home, err := os.UserHomeDir()
+ if err != nil {
+ return "", err
+ }
+ switch runtime.GOOS {
+ case "windows":
+ // See https://android.googlesource.com/platform/tools/adt/idea/+/85b4bfb7a10ad858a30ffa4003085b54f9424087/native/installer/win/setup_android_studio.nsi#100
+ androidHome = filepath.Join(home, "AppData", "Local", "Android", "sdk")
+ case "darwin":
+ // See https://android.googlesource.com/platform/tools/asuite/+/67e0cd9604379e9663df57f16a318d76423c0aa8/aidegen/lib/ide_util.py#88
+ androidHome = filepath.Join(home, "Library", "Android", "sdk")
+ default: // Linux, BSDs, etc.
+ // See LINUX_ANDROID_SDK_PATH in ide_util.py above.
+ androidHome = filepath.Join(home, "Android", "Sdk")
+ }
+ }
+ if info, err := os.Stat(androidHome); err != nil {
+ return "", fmt.Errorf("%w; Android SDK was not found at %s", err, androidHome)
+ } else if !info.IsDir() {
+ return "", fmt.Errorf("%s is not a directory", androidHome)
+ }
+ return androidHome, nil
+}
+
+// AndroidAPIPath returns an android SDK platform directory within the configured SDK.
+// If there are multiple platforms that satisfy the minimum version requirement,
+// AndroidAPIPath returns the latest one among them.
+func AndroidAPIPath(api int) (string, error) {
+ sdk, err := AndroidHome()
+ if err != nil {
+ return "", err
+ }
+ sdkDir, err := os.Open(filepath.Join(sdk, "platforms"))
+ if err != nil {
+ return "", fmt.Errorf("failed to find android SDK platform: %w", err)
+ }
+ defer sdkDir.Close()
+ fis, err := sdkDir.Readdir(-1)
+ if err != nil {
+ return "", fmt.Errorf("failed to find android SDK platform (API level: %d): %w", api, err)
+ }
+
+ var apiPath string
+ var apiVer int
+ for _, fi := range fis {
+ name := fi.Name()
+ if !strings.HasPrefix(name, "android-") {
+ continue
+ }
+ n, err := strconv.Atoi(name[len("android-"):])
+ if err != nil || n < api {
+ continue
+ }
+ p := filepath.Join(sdkDir.Name(), name)
+ _, err = os.Stat(filepath.Join(p, "android.jar"))
+ if err == nil && apiVer < n {
+ apiPath = p
+ apiVer = n
+ }
+ }
+ if apiVer == 0 {
+ return "", fmt.Errorf("failed to find android SDK platform (API level: %d) in %s",
+ api, sdkDir.Name())
+ }
+ return apiPath, nil
+}
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.
Hajime Hoshi (Gerrit)
Attention is currently required from: Ben Schwartz.
1 comment:
File cmd/gomobile/env.go:
Patch Set #17, Line 347:
compatibleNDKRoots := []string{}`var compatibleNDKRoots []string` was better to avoid unnecessary allocation
To view, visit change 401574. To unsubscribe, or for help writing mail filters, visit settings.