-
Notifications
You must be signed in to change notification settings - Fork 564
[build] Use Azure Pipelines vars for Android SDK/NDK paths #6719
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
Conversation
Configuration.props
Outdated
| <AndroidPreviousFrameworkVersion Condition=" '$(AndroidPreviousFrameworkVersion)' == '' ">v1.0</AndroidPreviousFrameworkVersion> | ||
| <AndroidToolchainCacheDirectory Condition=" '$(AndroidToolchainCacheDirectory)' == '' ">$(HOME)\android-archives</AndroidToolchainCacheDirectory> | ||
| <AndroidToolchainDirectory Condition=" '$(AndroidToolchainDirectory)' == '' And '$(RunningOnCI)' == 'true' And '$(HostOS)' == 'Darwin' ">$(HOME)\Library\Android</AndroidToolchainDirectory> | ||
| <AndroidToolchainDirectory Condition=" '$(AndroidToolchainDirectory)' == '' And '$(RunningOnCI)' == 'true' And '$(HostOS)' == 'Linux' ">/usr/local/lib/android</AndroidToolchainDirectory> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be hardcoding /usr/local/lib/android, or should this instead use the documented environment variable?
diff --git a/Configuration.props b/Configuration.props
index ae219baaf..b9543946a 100644
--- a/Configuration.props
+++ b/Configuration.props
@@ -78,7 +78,9 @@
<AndroidToolchainDirectory Condition=" '$(AndroidToolchainDirectory)' == '' ">$(HOME)\android-toolchain</AndroidToolchainDirectory>
<AndroidMxeInstallPrefix Condition=" '$(HostOS)' == 'Linux' ">\usr</AndroidMxeInstallPrefix>
<AndroidMxeInstallPrefix Condition=" '$(HostOS)' == 'Darwin' ">$(HostHomebrewPrefix)</AndroidMxeInstallPrefix>
+ <AndroidSdkDirectory Condition=" '$(AndroidSdkDirectory)' == '' And Exists($(ANDROID_SDK_ROOT)) ">$(ANDROID_SDK_ROOT)</AndroidSdkDirectory>
<AndroidSdkDirectory Condition=" '$(AndroidSdkDirectory)' == '' ">$(AndroidToolchainDirectory)\sdk</AndroidSdkDirectory>
+ <AndroidNdkDirectory Condition=" '$(AndroidNdkDirectory)' == '' And Exists($(ANDROID_NDK_ROOT)) ">$(ANDROID_NDK_ROOT)</AndroidNdkDirectory>
<AndroidNdkDirectory Condition=" '$(AndroidNdkDirectory)' == '' ">$(AndroidToolchainDirectory)\ndk</AndroidNdkDirectory>
<DotNetPreviewPath Condition=" '$(DotNetPreviewPath)' == '' ">$(AndroidToolchainDirectory)\dotnet\</DotNetPreviewPath>
<DotNetPreviewTool Condition=" '$(DotNetPreviewTool)' == '' ">$(DotNetPreviewPath)dotnet</DotNetPreviewTool>This might also warrant checking $(RunningOnCI), but I think it might be reasonable to default to $ANDROID_SDK_ROOT and $ANDROID_NDK_ROOT if those are set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with $ANDROID_NDK_LATEST_HOME, as it will hopefully continue to be a closer match to the version that we need going forward. Right now this version aligns with ours and allows us to skip installation of the NDK on hosted machines.
cd05b1e to
9e0f3ca
Compare
|
This seems to be working on the Linux side, we're no longer installing everything: Compared to previous runs: Windows test environments are also using the preinstalled paths: |
9e0f3ca to
f6604fb
Compare
Context: https://github.com/actions/virtual-environments/blob/5cdc2e5f50ec21dbac1d3b475ae0fe364f4a2d12/images/linux/Ubuntu2004-Readme.md#environment-variables-3
Context: https://github.com/actions/virtual-environments/blob/5cdc2e5f50ec21dbac1d3b475ae0fe364f4a2d12/images/win/Windows2022-Readme.md#environment-variables-2
Context: https://github.com/actions/virtual-environments/blob/5cdc2e5f50ec21dbac1d3b475ae0fe364f4a2d12/images/macos/macos-11-Readme.md#environment-variables-2
Build and test against Android SDK and NDK paths set by Azure Pipelines
hosted images if those paths exist.