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

[Xamarin.Android.Build.Tasks] Bump MinimumSupportedJavaVersion=17 #9257

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

jonpryor
Copy link
Member

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.

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.
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

Is this raising the min JDK to 17? for our whole product?

Should we consider:

  • JDK 11 minimum for .NET 9
  • JDK 17 minimum for .NET 10

Just wondering what might break if this goes out in RC 2.

@jonpryor
Copy link
Member Author

@jonathanpeppers asked:

Is this raising the min JDK to 17? for our whole product?

Yes.

Which may sound bananas, but:

TODO? What did/does VSMac provision? (And should we even care?)

Thus, who could/would still be using JDK-11? Who could this break?

Then there's the "real" reason I'm suggesting this: bumping $(MinimumSupportedJavaVersion) means it's consistent with $(JavaSdkVersion) and @(JavaDependency). Without this change, we'll have a possible scenario of:

  1. JDK-11 is installed and preferred (which is my current machine, via $HOME/.config/xbuild/monodroid-config.xml)

  2. I run dotnet build -t:InstallAndroidDependencies …, on .NET 9, and it errors out (now! with .NET 9 P7):

    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.
    

    It errors out because InstallAndroidDependencies wants to provision JDK 17, but my current system configured JDK is JDK 11.

Bumping $(MinimumSupportedJavaVersion) is the "easy and straightforward" way to resolve this. It would instead cause my machine to produce a XA5300 error that my JDK is too old.

@jonpryor
Copy link
Member Author

Also, I don't think any of our unit tests have used JDK-11 for quite some time (please correct me if I'm wrong), so it's entirely plausible that we don't fully work on JDK-11 now

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

If it breaks someone, they have the workaround for their project:

<MinimumSupportedJavaVersion>11.0</MinimumSupportedJavaVersion>

So, I think we should just put this in RC 2 release notes, and see if anything goes wrong.

@jonpryor jonpryor merged commit df68c20 into main Aug 29, 2024
58 checks passed
@jonpryor jonpryor deleted the dev/jonp/jonp-bump-min-jdk-to-17 branch August 29, 2024 19:05
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants