-
Notifications
You must be signed in to change notification settings - Fork 533
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
-t:InstallAndroidDependencies tries to install JDK when no JavaSdkDirectory is specified #9159
Comments
I've been looking at some of this this week, and one quick improvement I think we should make here would be to soften the version check to only compare major.minor, and not recommend or require installation in cases like the one above. We should also update our feeds to include the latest Open JDK versions and bump the recommended version to the latest. |
(Aside: while the VSCode docs suggest separately installing the JDK, I (personally) do not like that, because @Redth: I can't repro this for .NET 8, and I don't see how it can happen in .NET 8, because .NET 8 uses a <!-- Xamarin.Android.Common.Debugging.targets -->
<Target Name="_InstallAndroidJdk"
AfterTargets="InstallAndroidDependencies"
Condition=" '$(JavaSdkDirectory)' != '' and !Exists('$(JavaSdkDirectory)/release') ">
<PropertyGroup>
<AndroidJdkBaseUrl Condition=" '$(AndroidJdkBaseUrl)' == '' ">https://aka.ms/download-jdk/</AndroidJdkBaseUrl>
<_AndroidJdkDownloadVersion Condition=" '$(_AndroidJdkDownloadVersion)' == '' ">17.0.8</_AndroidJdkDownloadVersion>
<_AndroidUnamePath Condition=" '$(_AndroidUnamePath)' == '' ">/usr/bin/uname</_AndroidUnamePath>
<_AndroidTarPath Condition=" '$(_AndroidTarPath)' == '' ">/usr/bin/tar</_AndroidTarPath>
<_AndroidJdkTemp Condition=" '$(_AndroidJdkTemp)' == '' ">$(IntermediateOutputPath)jdk-temp/</_AndroidJdkTemp>
<_AndroidArchiveRoot Condition=" $([MSBuild]::IsOsPlatform('osx')) ">Contents/Home/</_AndroidArchiveRoot>
</PropertyGroup>
… My diagnostic log contains:
For .NET 9, everything is different: a JDK is listed in the Xamarin manifest (e.g. https://aka.ms/AndroidManifestFeed/d17-12) and installed "like" other Android SDK packages. My diagnostic log from .NET 9 Preview 6 (what I have quickly available) contains:
This is certainly a bug. |
@pjcollins: relatedly, we should ensure that the |
For .NET 9 Preview 7, |
What does it use as the default directory? |
@Redth: I was wrong. My testing accidentally used .NET 8 Preview 7, not .NET 9. .NET 9 Preview 7 does error out if |
https://github.com/xamarin/android-sdk-installer/pull/990 changes things, and will be part of .NET 9 RC1:
Additionally, the Thus, when setting up a new machine:
|
@pjcollins: one corner case that comes to mind: right now, Line 5 in 7aa8b84
…and/or 1.6?!
(We should probably remove the latter…) This value is inconsistent with The result is that if I have both JDK 11 and 17 installed, neither is set as the "default" (via Should we bump |
Context: #9159 (comment) Bump `$(MinimumSupportedJavaVersion)` to 17.0, and base this value on the `Configurables.Defaults.MicrosoftOpenJDK17Version` value so that if (when) we bump the JDK we build against, the major version value of `$(MinimumSupportedJavaVersion)` follows suit. Additionally, remove the `$(MinimumSupportedJavaVersion)` definition in `Microsoft.Android.Sdk.DefaultProperties.targets` so that it's only defined in one location.
…9257) Context: #9159 (comment) Bump `$(MinimumSupportedJavaVersion)` to 17.0, and base this value on the `Configurables.Defaults.MicrosoftOpenJDK17Version` value so that if (when) we bump the JDK we build against, the major version value of `$(MinimumSupportedJavaVersion)` follows suit. Additionally, remove the `$(MinimumSupportedJavaVersion)` definition in `Microsoft.Android.Sdk.DefaultProperties.targets` so that it's only defined in one location. This change ensures that the `@(JavaDependency)` values produced by the `GetAndroidDependencies` target and the `$(JavaSdkDirectory)` value produced by the `<ResolveSdks/>` task are consistent; `<ResolveSdks/>` won't find and use JDK-11, then cause a future `InstallAndroidDependencies` target invocation to attempt to update the JDK-11 install to JDK-17. Bumping `$(MinimumSupportedJavaVersion)` will prevent `<ResolveSdks/>` from using a JDK that would cause `InstallAndroidDependencies` to error out. If this breaks your build, you should be able to fix it by adding the following fragment to your `.csproj`: <PropertyGroup> <MinimumSupportedJavaVersion>11.0</MinimumSupportedJavaVersion> </PropertyGroup>
#9257 has been merged, which bumps |
Android framework version
net8.0-android, net9.0-android
Affected platform version
macOS, didn't test windows
Description
The VSCode docs suggest installing the OpenJDK17 by using the installer from the microsoft download page. This installs fine.
However, to get the Android SDK dependencies, the docs suggest using the
InstallAndroidDependencies
build task.First of all, it's confusing as it suggests specifying the
-p:JavaSdkDirectory
variable, which having just installed OpenJDK earlier, it's not clear where that even installed to for me to specify it, so perhaps we shouldn't suggest in the docs to set this variable at all.I think it's supposed to infer the idk path if I do not specify it, and it seems to do that, however it still appears to try and install the idk to that path, which then fails:
Steps to Reproduce
Did you find any workaround?
I can install to a user location such as
-p:JavaSdkDirectory=/Users/username/Library/Java/openjdk17
but then I've got an extra jdkRelevant log output
No response
The text was updated successfully, but these errors were encountered: