Redth commented 1 month ago

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:

/usr/local/share/dotnet/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Common.targets(19,3): error : Unable to install the Java SDK into `/Library/Java/JavaVirtualMachines/microsoft-17.jdk/Contents/Home`. Please set `$(JavaSdkDirectory)` to a valid non-administrator path.

Steps to Reproduce

Install openjdk17
Install .NET SDK, and the maui, android workloads.
Try to install the dependencies with the build task:

mkdir ~/mauiandroidtemp && cd ~/mauiandroidtemp
dotnet new android
dotnet build -t:InstallAndroidDependencies \
  -p:AndroidSdkDirectory=/Users/username/Library/Android/sdk \
  -p:AcceptAndroidSDKLicenses=True

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 jdk

Relevant log output

No response

pjcollins commented 1 month ago

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.

jonpryor commented 4 weeks ago

(Aside: while the VSCode docs suggest separately installing the JDK, I (personally) do not like that, because InstallAndroidDependencies will provision a JDK! That said, my personal idea does not reasonably work, because "⌘+⇧+P > .NET: Build" does not set -p:JavaSdkDirectory, even when configured in Settings > Maui > Configuration: Java Sdk Preferred Path!)

@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 _InstallAndroidJdk target, which is conditional on $(JavaSdkDirectory) not being empty and $(JavaSdkDirectorh)/release not existing!

<!-- 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:

                   Target "_InstallAndroidJdk" skipped, due to false condition; ( '$(JavaSdkDirectory)' != '' and !Exists('$(JavaSdkDirectory)/release') ) was evaluated as ( '/Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home' != '' and !Exists('/Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/release') ).

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:

13:58:36.846   1:7>Target "GetAndroidDependencies: (TargetId:65)" in file "…/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Android.Common.targets" from project "/Volumes/Xamarin-Work/tmp/android-Typeface.create/android-Typeface.create.csproj" (target "InstallAndroidDependencies" depends on it):
                     …
                     Output Item(s): 
                         JavaDependency=
                             jdk
                                     Version=17.0.8.1 (TaskId:59)
…
13:58:36.851   1:7>Target "InstallAndroidDependencies: (TargetId:66)" in file "…/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Common.targets" from project "/Volumes/Xamarin-Work/tmp/android-Typeface.create/android-Typeface.create.csproj" (entry point):
                   Assembly loaded during TaskRun: Xamarin.Installer.Build.Tasks, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null (location: …/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Build.Tasks.dll, MVID: 3a12f961-a852-491a-a7f2-3cf6c37d5e82, AssemblyLoadContext: MSBuild plugin …/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Build.Tasks.dll)
                   Assembly loaded during TaskRun: Xamarin.Installer.Common, Version=17.11.0.13, Culture=neutral, PublicKeyToken=null (location: …/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Common.dll, MVID: ce5bdfa8-cff9-4ddf-b1b0-6b3b877ad272, AssemblyLoadContext: MSBuild plugin …/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Build.Tasks.dll)
                   Using "InstallAndroidDependencies" task from assembly "…/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Build.Tasks.dll".
                   Task "InstallAndroidDependencies" (TaskId:60)
                     Task Parameter:
                         JavaDependencies=
                             jdk
                                     Version=17.0.8.1 (TaskId:60)
                     Task Parameter:AcceptAndroidSDKLicenses=True (TaskId:60)
                     Task Parameter:TimeoutInMinutes=10 (TaskId:60)
                     Task Parameter:InstallJavaDependencies=True (TaskId:60)
                     Task Parameter:AndroidSdkPath=/Users/jon/Developer/tmp/android-Typeface.create/sdk (TaskId:60)
                     Task Parameter:
                         Dependencies=
                             platforms/android-34
                             build-tools/34.0.0
                                     Version=34.0.0
                             platform-tools
                                     Version=34.0.5
                             cmdline-tools/11.0
                                     Version=11.0 (TaskId:60)
                     Task Parameter:JavaSdkPath=/Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home (TaskId:60)

13:59:00.465   1:7>…/dotnet-sdk-9.0.100-preview.6.24328.19-osx-x64/packs/Microsoft.Android.Sdk.Darwin/34.99.0-preview.6.340/tools/Xamarin.Installer.Common.targets(19,3): error : Unable to install the Java SDK into `/Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home`. Please set `$(JavaSdkDirectory)` to a valid non-administrator path. [/Volumes/Xamarin-Work/tmp/android-Typeface.create/android-Typeface.create.csproj]

This is certainly a bug.

jonpryor commented 2 weeks ago

@pjcollins: relatedly, we should ensure that the $(JavaSdkDirectory) MSBuild property is optional when running the InstallAndroidDependencies target.

jonpryor commented 2 weeks ago

For .NET 9 Preview 7, -p:JavaSdkDirectory is not required for -t:InstallAndroidDependencies.

Redth commented 2 weeks ago

For .NET 9 Preview 7, -p:JavaSdkDirectory is not required for -t:InstallAndroidDependencies.

What does it use as the default directory?

jonpryor commented 2 weeks ago

@Redth: I was wrong. My testing accidentally used .NET 8 Preview 7, not .NET 9.

.NET 9 Preview 7 does error out if -p:JavaSdkDirectory is not set. We should fix this.

jonpryor commented 2 weeks ago

https://github.com/xamarin/android-sdk-installer/pull/990 changes things, and will be part of .NET 9 RC1:

Eases the version check done when deciding if a Java SDK should be installed. Installation will now only be attempted if an existing JDK is not detected, or if the detected JDK has a major version lower than the one we recommend (currently 17).

Info and error messages have also been updated to improve clarity around out of date and invalid path install scenarios.

Additionally, the InstallAndroidDependencies target runs after the _ResolveSdks target, which will attempt to use JdkInfo to find a JDK with a version between $(MinimumSupportedJavaVersion) and $(LatestSupportedJavaVersion).

Thus, when setting up a new machine:

If no JDK is present and we run dotnet build -t:InstallAndroidDependencies without using -p:JavaSdkDirectory, we will error out because JdkInfo won't find anything, and the "internal" $(JavaSdkDirectory) property will be empty.

I believe that this is appropriate behavior.
If a JDK is present and we run dotnet build -t:InstallAndroidDependencies without using -p:JavaSdkDirectory, _ResolveSdks will try to find a JDK that fits criteria and set $(JavaSdkDirectory). In a post xamarin/android-sdk-installer#990 world, this will not try to alter the JDK (if it fits requirements), and will accept a JDK that is newer than the required version.

Meaning if the user installed any JDK 17, and we want "JDK 17", the user will be fine. No warnings, no errors.

If the user installs JDK 11 and we want JDK 17, we'll try to in-place update the JDK we found. If it's user writable, it's updated. If it's not, we error out.

jonpryor commented 2 weeks ago

@pjcollins: one corner case that comes to mind: right now, $(MinimumSupportedJavaVersion) is 11.0:

https://github.com/dotnet/android/blob/7aa8b84db52c439e87d399cfd0b48528785de54d/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets#L5

…and/or 1.6?!

https://github.com/dotnet/android/blob/7aa8b84db52c439e87d399cfd0b48528785de54d/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in#L20

(We should probably remove the latter…)

This value is inconsistent with @(JavaDependency)/$(JavaSdkVersion), which is 17.0.8.1.

The result is that if I have both JDK 11 and 17 installed, neither is set as the "default" (via $JAVA_HOME or $HOME/.config/xbuild/monodroid-config.xml or registry or…), and JdkInfo picks JDK 11 as the one to use, then InstallAndroidDependencies may error out if the JDK 11 install isn't writable.

Should we bump $(MinimumSupportedJavaVersion) to 17.0?

jonpryor commented 2 weeks ago

9257 has been merged, which bumps `$(MinimumSupportedJavaVersion)` to 17.0.

dotnet / android

-t:InstallAndroidDependencies tries to install JDK when no JavaSdkDirectory is specified #9159

Android framework version

Affected platform version

Description

Steps to Reproduce

Did you find any workaround?

Relevant log output

9257 has been merged, which bumps `$(MinimumSupportedJavaVersion)` to 17.0.

dotnet / android

-t:InstallAndroidDependencies tries to install JDK when no JavaSdkDirectory is specified #9159

Android framework version

Affected platform version

Description

Steps to Reproduce

Did you find any workaround?

Relevant log output

9257 has been merged, which bumps $(MinimumSupportedJavaVersion) to 17.0.

9257 has been merged, which bumps `$(MinimumSupportedJavaVersion)` to 17.0.