From 6f93547388b7144fd33e535683dc54875a6d0adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliv=C3=A9r=20Falvai?= Date: Thu, 25 Apr 2024 16:01:59 +0200 Subject: [PATCH] Fix NDK path detection (#103) * Fix NDK path detection * Set JDK version * Fix error logging * Fix sample app * * * * * * * Remove unnecessary test * Simplify * Print more * Better logs * Cleanup * Docs --- e2e/bitrise.yml | 43 +++------------ main.go | 82 ++++++++++++++++++----------- main_test.go | 137 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 194 insertions(+), 68 deletions(-) create mode 100644 main_test.go diff --git a/e2e/bitrise.yml b/e2e/bitrise.yml index 8a9b665..21aa58a 100644 --- a/e2e/bitrise.yml +++ b/e2e/bitrise.yml @@ -132,24 +132,6 @@ workflows: inputs: - gradle_task: assembleDebug - test_support_library: - envs: - - SAMPLE_APP_URL: https://github.com/bitrise-io/sample-apps-android-x.git - - BRANCH: master - - GRADLE_BUILD_FILE_PATH: build.gradle - - GRADLEW_PATH: ./gradlew - before_run: - - _setup - steps: - - script: - title: Uninstall Android SDK - inputs: - - content: |- - #!/usr/bin/env bash - sdkmanager --uninstall "extras;android;m2repository" - after_run: - - _run_and_build - test_build_tools_not_installed: envs: - SAMPLE_APP_URL: https://github.com/bitrise-io/Bitrise-Android-Sample.git @@ -172,24 +154,6 @@ workflows: _setup: steps: - - script: - title: Enable JDK 11 - description: This is still needed on the vs4mac stack - run_if: $.IsCI - inputs: - - content: |- - #!/usr/bin/env bash - set -ex - if [[ "$OSTYPE" == "linux-gnu"* ]]; then - sudo update-alternatives --set javac /usr/lib/jvm/java-11-openjdk-amd64/bin/javac - sudo update-alternatives --set java /usr/lib/jvm/java-11-openjdk-amd64/bin/java - export JAVA_HOME="/usr/lib/jvm/java-11-openjdk-amd64" - envman add --key JAVA_HOME --value "/usr/lib/jvm/java-11-openjdk-amd64" - elif [[ "$OSTYPE" == "darwin"* ]]; then - jenv global 11 || jenv global 11.0 - export JAVA_HOME="$(jenv prefix)" - envman add --key JAVA_HOME --value "$(jenv prefix)" - fi - script: title: Clean _tmp dir inputs: @@ -209,6 +173,9 @@ workflows: - repository_url: $SAMPLE_APP_URL - clone_into_dir: . - branch: $BRANCH + - set-java-version: + inputs: + - set_java_version: "17" _run_and_build: steps: @@ -238,6 +205,10 @@ workflows: #!/usr/bin/env bash set -ex + # If there is no ndk-bundle (only side-by-side NDKs), we don't need to back it up + if [ ! -d $ANDROID_HOME/ndk-bundle ]; then + exit 0 + fi cp -R $ANDROID_HOME/ndk-bundle $ANDROID_HOME/ndk-bundle-original _restore_ndk_bundle: diff --git a/main.go b/main.go index 297881d..bf82c5d 100644 --- a/main.go +++ b/main.go @@ -13,8 +13,10 @@ import ( "github.com/bitrise-io/go-steputils/tools" "github.com/bitrise-io/go-utils/fileutil" "github.com/bitrise-io/go-utils/log" + "github.com/bitrise-io/go-utils/v2/env" "github.com/bitrise-io/go-utils/v2/errorutil" . "github.com/bitrise-io/go-utils/v2/exitcode" + "github.com/bitrise-io/go-utils/v2/log/colorstring" "github.com/bitrise-steplib/steps-install-missing-android-tools/androidcomponents" "github.com/hashicorp/go-version" "github.com/kballard/go-shellquote" @@ -117,14 +119,14 @@ func (i AndroidToolsInstaller) Run(config Config) error { } if err := updateNDK(config.NDKVersion, androidSdk); err != nil { - return fmt.Errorf("failed to install new NDK package: %w", err) + return fmt.Errorf("install new NDK package: %w", err) } } else { log.Infof("Clearing NDK environment") log.Printf("Unset ANDROID_NDK_HOME") if err := os.Unsetenv("ANDROID_NDK_HOME"); err != nil { - return fmt.Errorf("failed to unset environment variable: %w", err) + return fmt.Errorf("unset environment variable: %w", err) } if err := tools.ExportEnvironmentWithEnvman("ANDROID_NDK_HOME", ""); err != nil { @@ -173,58 +175,74 @@ func ndkVersion(ndkPath string) string { return "" } -func currentNDKHome() string { - if v := os.Getenv(androidNDKHome); v != "" { - return v +// targetNDKPath picks the correct path where the requested NDK version should be installed, +// as well as returning if the path needs to be cleaned up before install. +// There are many (deprecated) ways to install NDK, the logic is based on the following: +// https://github.com/android/ndk-samples/wiki/Configure-NDK-Path +// https://developer.android.com/tools/variables +func targetNDKPath(envRepo env.Repository, requestedNDKVersion string) (string, bool) { + if v := envRepo.Get(androidNDKHome); v != "" { + // $ANDROID_NDK_HOME is old and AGP no longer takes it into account, + // but it's an explicit path, so use it if it's set on the system. + // And because we don't know if it's `ndk-bundle` or a specific side-by-side version, + // we return true for cleanup. + log.Warnf("$%s is set to %s", androidNDKHome, v) + log.Warnf("This variable is deprecated and modern Android Gradle Plugin versions no longer take it into account.") + return v, true } - if v := os.Getenv("ANDROID_HOME"); v != "" { - // $ANDROID_HOME is deprecated - return filepath.Join(v, "ndk-bundle") + if androidHome := envRepo.Get("ANDROID_HOME"); androidHome != "" { + // The most modern way is to install NDK versions side-by-side at $ANDROID_HOME/ndk/version + // This is what `sdkmanager` does when installing a specific version (`sdkmanager "ndk;26.3.11579264"`). + ndkPath := filepath.Join(androidHome, "ndk", requestedNDKVersion) + return ndkPath, false } - if v := os.Getenv("ANDROID_SDK_ROOT"); v != "" { - // $ANDROID_SDK_ROOT is preferred over $ANDROID_HOME - return filepath.Join(v, "ndk-bundle") + if sdkRoot := envRepo.Get("ANDROID_SDK_ROOT"); sdkRoot != "" { + // $ANDROID_SDK_ROOT is deprecated, so it's lower in priority than $ANDROID_HOME + ndkPath := filepath.Join(sdkRoot, "ndk", requestedNDKVersion) + return ndkPath, false } - if v := os.Getenv("HOME"); v != "" { - return filepath.Join(v, "ndk-bundle") + if v := envRepo.Get("HOME"); v != "" { + // Worst case: just install to home (and later export as $ANDROID_NDK_HOME) + return filepath.Join(v, "ndk-bundle"), true } - return "ndk-bundle" + return "ndk-bundle", true } // updateNDK installs the requested NDK version (if not already installed to the correct location). -// NDK is installed to the `ndk/version` subdirectory of the SDK location, while updating $ANDROID_NDK_HOME for -// compatibility with older Android Gradle Plugin versions. -// Details: https://github.com/android/ndk-samples/wiki/Configure-NDK-Path func updateNDK(version string, androidSdk *sdk.Model) error { - currentNdkHome := currentNDKHome() - - currentVersion := ndkVersion(currentNdkHome) - if currentVersion == version { - log.Donef("NDK %s already installed at %s", version, currentNdkHome) - return nil + envRepo := env.NewRepository() + targetNDKPath, doCleanup := targetNDKPath(envRepo, version) + currentVersionAtPath := ndkVersion(targetNDKPath) + if currentVersionAtPath != "" { + log.Printf("NDK %s found at %s", colorstring.Cyan(currentVersionAtPath), targetNDKPath) + } else { + log.Printf("NDK %s %s found at %s", colorstring.Cyan(version), colorstring.Yellow("not"), targetNDKPath) } - if currentVersion != "" { - log.Printf("NDK %s found at: %s", currentVersion, currentNdkHome) + if currentVersionAtPath == version { + log.Donef("NDK %s is already installed", version) + return nil } - log.Printf("Removing existing NDK...") - if err := os.RemoveAll(currentNdkHome); err != nil { - return err + if currentVersionAtPath != "" || doCleanup { + log.Printf("Removing existing NDK...") + if err := os.RemoveAll(targetNDKPath); err != nil { + return err + } + log.Printf("Done") } - log.Printf("Done") - log.Printf("Installing NDK %s with sdkmanager", version) + log.Printf("Installing NDK %s with sdkmanager", colorstring.Cyan(version)) sdkManager, err := sdkmanager.New(androidSdk) if err != nil { return err } ndkComponent := sdkcomponent.NDK{Version: version} cmd := sdkManager.InstallCommand(ndkComponent) - output, err := cmd.RunAndReturnTrimmedOutput() + output, err := cmd.RunAndReturnTrimmedCombinedOutput() if err != nil { log.Errorf(output) - return err + return fmt.Errorf("run %s: %w", cmd.PrintableCommandArgs(), err) } newNDKHome := filepath.Join(androidSdk.GetAndroidHome(), ndkComponent.InstallPathInAndroidHome()) diff --git a/main_test.go b/main_test.go new file mode 100644 index 0000000..5978c75 --- /dev/null +++ b/main_test.go @@ -0,0 +1,137 @@ +package main + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_currentNDKHome(t *testing.T) { + type test struct { + name string + envs map[string]string + wantPath string + wantCleanup bool + } + + requestedNDKVersion := "23.0.7599858" + tests := []test{ + { + name: "ANDROID_NDK_HOME is set", + envs: map[string]string{ + "ANDROID_NDK_HOME": "/opt/android-ndk", + }, + wantPath: "/opt/android-ndk", + wantCleanup: true, + }, + { + name: "both ANDROID_NDK_HOME and ANDROID_HOME are set", + envs: map[string]string{ + "ANDROID_NDK_HOME": "/opt/android-ndk", + "ANDROID_HOME": "/opt/android-sdk", + }, + wantPath: "/opt/android-ndk", + wantCleanup: true, + }, + { + name: "only ndk-bundle is installed", + envs: map[string]string{ + "ANDROID_HOME": "/home/user/android-sdk", + }, + wantPath: "/home/user/android-sdk/ndk/23.0.7599858", + wantCleanup: false, + }, + { + name: "ndk-bundle and side-by-side NDK is installed", + envs: map[string]string{ + "ANDROID_HOME": "/home/user/android-sdk", + }, + wantPath: "/home/user/android-sdk/ndk/23.0.7599858", + wantCleanup: false, + }, + { + name: "the exact requested side-by-side NDK is installed", + envs: map[string]string{ + "ANDROID_HOME": "/home/user/android-sdk", + }, + wantPath: "/home/user/android-sdk/ndk/23.0.7599858", + wantCleanup: false, + }, + { + name: "a different side-by-side NDK is installed than requested", + envs: map[string]string{ + "ANDROID_HOME": "/home/user/android-sdk", + }, + wantPath: "/home/user/android-sdk/ndk/23.0.7599858", + wantCleanup: false, + }, + { + name: "both ANDROID_HOME and SDK_ROOT are set, pointing to the same dir", + envs: map[string]string{ + "ANDROID_HOME": "/home/user/android-sdk", + "SDK_ROOT": "/home/user/android-sdk", + }, + wantPath: "/home/user/android-sdk/ndk/23.0.7599858", + wantCleanup: false, + }, + { + name: "both ANDROID_HOME and SDK_ROOT are set, pointing to different dirs", + envs: map[string]string{ + "ANDROID_HOME": "/home/user/android-sdk-home", + "ANDROID_SDK_ROOT": "/home/user/android-sdk-root", + }, + wantPath: "/home/user/android-sdk-home/ndk/23.0.7599858", + wantCleanup: false, + }, + { + name: "only SDK_ROOT is set", + envs: map[string]string{ + "ANDROID_SDK_ROOT": "/home/user/android-sdk-root", + }, + wantPath: "/home/user/android-sdk-root/ndk/23.0.7599858", + wantCleanup: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + envRepo := fakeEnvRepo{envVars: tt.envs} + gotPath, gotCleanup := targetNDKPath(envRepo, requestedNDKVersion) + require.Equal(t, tt.wantPath, gotPath) + require.Equal(t, tt.wantCleanup, gotCleanup) + }) + } + +} + +type fakeEnvRepo struct { + envVars map[string]string +} + +func (repo fakeEnvRepo) Get(key string) string { + value, ok := repo.envVars[key] + if ok { + return value + } else { + return "" + } +} + +func (repo fakeEnvRepo) Set(key, value string) error { + repo.envVars[key] = value + return nil +} + +func (repo fakeEnvRepo) Unset(key string) error { + repo.envVars[key] = "" + return nil +} + +func (repo fakeEnvRepo) List() []string { + envs := []string{} + for k, v := range repo.envVars { + envs = append(envs, fmt.Sprintf("%s=%s", k, v)) + } + return envs +}