Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Android SDK root #2343

Merged
merged 1 commit into from
Jan 13, 2021
Merged

Change Android SDK root #2343

merged 1 commit into from
Jan 13, 2021

Conversation

dsame
Copy link
Contributor

@dsame dsame commented Dec 24, 2020

Description

Improvement

Create a hardlink C:\Android\android-sdk pointing to C:\Program Files (x86)\Android\android-sdk and set ANDROID_SDK_ROOT to C:\Android\android-sdk

Problem to solve:

ANDROID_NDK_PATH / HOME should not contain spaces. Otherwise, the script C:\Program Files (x86)\Android\android-sdk\ndk-bundle\ndk-build.cmd gives an error

 'C:\Program' is not recognized as an internal or external command,
operable program or batch file.

changing just ndk-build.cmd is not (nearly) enough. We depend on all the compiler wrapper scripts in ./toolchains/llvm/prebuilt/windows-x86_64/bin/-clang*.cmd being capable of handling spaces, and have not gone beyond checking what else might break.

Related issue: #1122

Check list

  • Related issue / work item is attached
  • Changes are tested and related VM images are successfully generated

@AlenaSviridenko AlenaSviridenko merged commit ada08c2 into actions:main Jan 13, 2021
$sdkRoot = "C:\Android\android-sdk"
Expand-Archive -Path .\android-sdk-licenses.zip -DestinationPath $sdkInstallRoot -Force
New-Item -Path "C:\Android" -ItemType Directory
New-Item -Path "$sdkRoot" -ItemType SymbolicLink -Value "$sdkInstallRoot"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsame You mention hardlink in the PR description, but this is clearly a SymbolicLink. Shouldn't it be HardLink or Junction?

I don't know if Windows has realpath-alike functionality like UNIX but those resolve symbolic links, resulting in the path with spaces.

MarijnS95 added a commit to MarijnS95/ndk that referenced this pull request Jan 27, 2021
In [1] this hack was introduced because GH Actions virtual-environments
installed the SDK in `Program Files` which includes a space, but this is
not supported by the NDK. Now that the issue has been addressed [2] and
the images deployed this workaround can be removed again.

[1]: rust-mobile#92
[2]: actions/runner-images#2343
MarijnS95 added a commit to MarijnS95/ndk that referenced this pull request Jan 27, 2021
In [1] this hack was introduced because GH Actions virtual-environments
installed the SDK in `Program Files` which includes a space, but this is
not supported by the NDK. Now that the issue has been addressed [2] and
the images deployed this workaround can be removed again.

[1]: rust-mobile#92
[2]: actions/runner-images#2343
MarijnS95 added a commit to MarijnS95/ndk that referenced this pull request Jan 27, 2021
In [1] this hack was introduced because GH Actions virtual-environments
installed the SDK in `Program Files` which includes a space, but this is
not supported by the NDK. Now that the issue has been addressed [2] and
the images deployed this workaround can be removed again.

[1]: rust-mobile#92
[2]: actions/runner-images#2343
MarijnS95 added a commit to rust-mobile/ndk that referenced this pull request Jan 28, 2021
#118)

* ci: Separate apk check and build steps

An obscure one-line `apk build` failure got shadowed by a much larger
deprecation warning shown by `cargo check` right above, deemed
irrelevant. Separating these steps will make it clear when the issue is
code-related (ndk, ndk-glue) or build-related (ndk-build, cargo-apk,
cargo-subcommand).

* ci: Remove Windows NDK symlink; GH images now include space-less NDK

In [1] this hack was introduced because GH Actions virtual-environments
installed the SDK in `Program Files` which includes a space, but this is
not supported by the NDK. Now that the issue has been addressed [2] and
the images deployed this workaround can be removed again.

[1]: #92
[2]: actions/runner-images#2343
topjohnwu added a commit to topjohnwu/Magisk that referenced this pull request Jan 29, 2021
rib pushed a commit to rust-mobile/ndk-glue that referenced this pull request Dec 6, 2022
…s (#118)

* ci: Separate apk check and build steps

An obscure one-line `apk build` failure got shadowed by a much larger
deprecation warning shown by `cargo check` right above, deemed
irrelevant. Separating these steps will make it clear when the issue is
code-related (ndk, ndk-glue) or build-related (ndk-build, cargo-apk,
cargo-subcommand).

* ci: Remove Windows NDK symlink; GH images now include space-less NDK

In [1] this hack was introduced because GH Actions virtual-environments
installed the SDK in `Program Files` which includes a space, but this is
not supported by the NDK. Now that the issue has been addressed [2] and
the images deployed this workaround can be removed again.

[1]: rust-mobile/ndk#92
[2]: actions/runner-images#2343
rib pushed a commit to rust-mobile/ndk-context that referenced this pull request Dec 7, 2022
…s (#118)

* ci: Separate apk check and build steps

An obscure one-line `apk build` failure got shadowed by a much larger
deprecation warning shown by `cargo check` right above, deemed
irrelevant. Separating these steps will make it clear when the issue is
code-related (ndk, ndk-glue) or build-related (ndk-build, cargo-apk,
cargo-subcommand).

* ci: Remove Windows NDK symlink; GH images now include space-less NDK

In [1] this hack was introduced because GH Actions virtual-environments
installed the SDK in `Program Files` which includes a space, but this is
not supported by the NDK. Now that the issue has been addressed [2] and
the images deployed this workaround can be removed again.

[1]: rust-mobile/ndk#92
[2]: actions/runner-images#2343
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants