Skip to content

Conversation

@jonpryor
Copy link
Contributor

Context: 7a32bb9
Context: https://stackoverflow.com/questions/55853220/handling-change-in-newlines-by-xml-transformation-for-cdata-from-java-8-to-java
Context: https://bugs.java.com/bugdatabase/view_bug?bug_id=8223291

Background: Since commit bc5bcf4, two JDKs were required in order to fully build and test Java.Interop: JDK 1.8 and JDK-11. This is because src/Java.Interop requires JDK-11+ to build, while some unit tests required JDK 1.8 to pass (because Java XML output changed between JDK 1.8 and JDK-11; see also 7a32bb9).

Recently, a question arose: how well does .NET Android work with JDK 17? (Android Studio recently bumped the bundled JDK from JDK-11 to JDK-17.) The "straightforward" approach of "provision JDK-17 and just build everything with JDK-17" quickly meant that Java.Interop needed to build under JDK-17.

This in turn segued into a "how do I make the MSBuild property meanings clearer", as $(JavaCPath) would be for JDK 1.8, while $(JavaC11Path) was for JDK-11, but with JDK-17 being provisioned $(JavaC11Path) actually was for JDK-17, which is just confusing.

After discussion, we decided that we don't need to continue using JDK 1.8 anymore. Android API-31 requires JDK-11 in order to use various Android SDK build tools, and the Google Play Store requires a target SDK version of API-33 starting 2023-Aug. There is not much point in maintaining JDK 1.8 support.

JDK 11 or later is now required.

Update to use Gradle 8.1.1. This is needed for later JDK-17 support.

java/util/Collection.java existed to help test API documentation import (when $ANDROID_SDK_PATH is set). JDK-11 does not support compiling java/util/Collection.java anymore; it errors out with:

java/java/util/Collection.java:1: error: package exists in another module: java.base
package java.util;
^

"Rename" this type to android/animation/TypeEvaluator.java, and update the API documentation import tests accordingly.

Update the ExpectedTypeDeclaration.MajorVersion values to 0x37.

The .class files for nested types has seen the addition of a new NestHost constant; see also:

Update ExpectedTypeDeclaration.ConstantPoolCount as appropriate.

Additional NestHost-related fallout is that JavaType.class now includes com/xamarin/JavaType$RNC$RPNC in the InnerClasses table.

Update tools/java-source-utils unit tests so that JDK-11 can now be used to run (and pass!) the unit tests. (This previously required JDK 1.8.)

Context: 7a32bb9
Context: https://stackoverflow.com/questions/55853220/handling-change-in-newlines-by-xml-transformation-for-cdata-from-java-8-to-java
Context: https://bugs.java.com/bugdatabase/view_bug?bug_id=8223291

**Background**: Since commit bc5bcf4, *two* JDKs were required in
order to *fully* build and test Java.Interop: JDK 1.8 and JDK-11.
This is because `src/Java.Interop` requires JDK-11+ to build, while
some unit tests required JDK 1.8 to pass (because Java XML output
changed between JDK 1.8 and JDK-11; see also 7a32bb9).

Recently, a question arose: how well does .NET Android work
with JDK 17?  ([Android Studio recently bumped][0] the bundled JDK
from JDK-11 to JDK-17.)  The "straightforward" approach of
"provision JDK-17 and just build everything with JDK-17" quickly
meant that Java.Interop needed to build under JDK-17.

This in turn segued into a "how do I make the MSBuild property
meanings clearer", as `$(JavaCPath)` would be for JDK 1.8, while
`$(JavaC11Path)` was for JDK-11, but with JDK-17 being provisioned
`$(JavaC11Path)` *actually* was for JDK-17, which is just confusing.

After discussion, we decided that we don't need to continue using
JDK 1.8 anymore.  Android API-31 requires JDK-11 in order to use
various Android SDK build tools, and the Google Play Store requires
a target SDK version of API-33 starting 2023-Aug.  There is not much
point in maintaining JDK 1.8 support.

JDK 11 or later is now required.

Update to use Gradle 8.1.1.  This is needed for later JDK-17 support.

`java/util/Collection.java` existed to help test API documentation
import (when `$ANDROID_SDK_PATH` is set).  JDK-11 does not support
compiling `java/util/Collection.java` anymore; it errors out with:

	java/java/util/Collection.java:1: error: package exists in another module: java.base
	package java.util;
	^

"Rename" this type to `android/animation/TypeEvaluator.java`, and
update the API documentation import tests accordingly.

Update the `ExpectedTypeDeclaration.MajorVersion` values to 0x37.

The `.class` files for nested types has seen the addition of a new
`NestHost` constant; see also:

  * https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-4.html#jvms-4.7.28
  * https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-5.html#jvms-5.4.4

Update `ExpectedTypeDeclaration.ConstantPoolCount` as appropriate.

Additional `NestHost`-related fallout is that `JavaType.class` now
includes `com/xamarin/JavaType$RNC$RPNC` in the `InnerClasses` table.

Update `tools/java-source-utils` unit tests so that JDK-11 can now be
used to run (and pass!) the unit tests.  (This previously required
JDK 1.8.)

[0]: https://web.archive.org/web/20230507035529/https://developer.android.com/studio/releases/#jdk-17
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jun 1, 2023
@jonpryor jonpryor marked this pull request as ready for review June 1, 2023 00:36
@jonpryor jonpryor requested review from jpobst and pjcollins June 1, 2023 00:36
Copy link
Contributor

@jpobst jpobst left a comment

Choose a reason for hiding this comment

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

Should we run a test XA bump against this?

@jonpryor
Copy link
Contributor Author

jonpryor commented Jun 2, 2023

@jpobst : we did: dotnet/android#8092

Everything was green except the "linux tests", which suffered bizarro network issues.

@jonpryor jonpryor merged commit 738de61 into dotnet:main Jun 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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.

3 participants