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

[build] Add support for JDK 21 #1287

Merged
merged 1 commit into from
Jan 8, 2025
Merged

[build] Add support for JDK 21 #1287

merged 1 commit into from
Jan 8, 2025

Conversation

jonpryor
Copy link
Member

@jonpryor jonpryor commented Jan 7, 2025

Context: 5bb0d24
Context: 4273e5c
Context: 0355acf
Context: dotnet/android#9651

Does dotnet/android build with JDK-21? We don't know! But in order to answer that question, dotnet/java-interop needs to be able to build under JDK-21; a'la 5bb0d24:

  1. Install JDK-21

  2. Run dotnet build -t:Prepare, overriding $(JdksRoot):

    dotnet build -t:Prepare Java.Interop.sln -p:JdksRoot=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home
    
  3. Build: dotnet build

Unfortunately, this fails for three reasons:

  1. class-parse crashes when processing java.base.jmod.
  2. src/Java.Base needs updates to bind JDK-21's java.base.jmod.
  3. On Linux and Windows, Gradle 8.1 and JDK-21 don't mix.

~~ class-parse ~~

This only impacts Debug builds of dotnet/java-interop, but:

% dotnet "bin/Debug-net8.0/class-parse.dll" \
  "$HOME/android-toolchain/jdk-21/jmods/java.base.jmod" \
  "-o=obj/Debug-net8.0//mcw/api.xml"
…
Process terminated. Assertion failed.
Unexpected number of method parameters in `Ljdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter$Instantiation;.get(Ljava/lang/Object;)Ljava/lang/Object;`: expected 1, got 0
   at Xamarin.Android.Tools.Bytecode.MethodInfo.UpdateParametersFromMethodParametersAttribute(ParameterInfo[] parameters) in …/src/Xamarin.Android.Tools.Bytecode/Methods.cs:line 308
   …

The assertion?

var pinfo = methodParams.ParameterInfo;
int startIndex = 0;
while (startIndex < pinfo.Count &&
    pinfo [startIndex].AccessFlags.HasFlag (.Synthetic))
  startIndex++;
Debug.Assert (parameters.Length == pinfo.Count - startIndex, …);

This is part of 4273e5c and 0355acf, attempting to "skip over" the constructor parameters which non-static inner classes have which contain the outer class instance:

// Java
class Outer {
  /* non-static */ class Inner {
    public Inner () { … }
  }
}

At the ABI boundary, the Outer.Inner constructor is actually:

/* partial */ class Outer {
  /* partial */ class Inner {
    public Inner (Outer outer) { … }
  }
}

and we need to skip over the first parameter.

Which brings us to
jdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter.Instantiation.get():

% mkdir _x
% unzip $HOME/android-toolchain/jdk-21/jmods/java.base.jmod -d _x
% dotnet bin/Debug-net8.0/class-parse.dll --dump \
  _x/classes/jdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter\$Instantiation.class
…
	8: get (Ljava/lang/Object;)Ljava/lang/Object; Public, Bridge, Synthetic
		Code(6, Unknown[LineNumberTable](6), LocalVariableTableAttribute(LocalVariableTableEntry(Name='this', Descriptor='Ljdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter$Instantiation;', StartPC=0, Index=0)))
		MethodParametersAttribute(MethodParameterInfo(Name='', AccessFlags=Final, Synthetic))

The parameter for JSRInlinerAdapter.Instantiation.get(Object) is Synthetic, causing us to skip over it, which is why we have a parameter mismatch. The thing is, this parameter shouldn't be skipped; the skipping is intended for constructor parameters!

Update the code so that the loop only occurs for constructors. This allows class-parse to not assert, allowing src/Java.Base to build.

~~ Gradle ~~

Via dotnet/android#9651, gradle fails when building tools/java-source-utils, but only on Linux and Windows:

"/mnt/vss/_work/1/s/xamarin-android/external/Java.Interop/build-tools/gradle/gradlew" -d --stacktrace --no-daemon -PjavaSourceVer=11 -PjavaTargetVer=11 jar
…
[org.gradle.internal.buildevents.BuildExceptionReporter]
[org.gradle.internal.buildevents.BuildExceptionReporter] FAILURE: Build failed with an exception.
[org.gradle.internal.buildevents.BuildExceptionReporter]
[org.gradle.internal.buildevents.BuildExceptionReporter] * What went wrong:
[org.gradle.internal.buildevents.BuildExceptionReporter] Could not open settings generic class cache for settings file '/mnt/vss/_work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/settings.gradle' (/home/cloudtest/.gradle/caches/8.1.1/scripts/aiw0k2bokig45bv5yvkog3o3j).
[org.gradle.internal.buildevents.BuildExceptionReporter] > BUG! exception in phase 'semantic analysis' in source unit '_BuildScript_' Unsupported class file major version 65
[org.gradle.internal.buildevents.BuildExceptionReporter]
[org.gradle.internal.buildevents.BuildExceptionReporter] * Try:
[org.gradle.internal.buildevents.BuildExceptionReporter] > Run with --scan to get full insights.
[org.gradle.internal.buildevents.BuildExceptionReporter]
[org.gradle.internal.buildevents.BuildExceptionReporter] * Exception is:
[org.gradle.internal.buildevents.BuildExceptionReporter] org.gradle.cache.CacheOpenException: Could not open settings generic class cache for settings file '/mnt/vss/_work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/settings.gradle' (/home/cloudtest/.gradle/caches/8.1.1/scripts/aiw0k2bokig45bv5yvkog3o3j).
[org.gradle.internal.buildevents.BuildExceptionReporter] 	at org.gradle.cache.internal.DefaultPersistentDirectoryStore.open(DefaultPersistentDirectoryStore.java:91)
[org.gradle.internal.buildevents.BuildExceptionReporter]  …
[org.gradle.internal.buildevents.BuildExceptionReporter] Caused by: BUG! exception in phase 'semantic analysis' in source unit '_BuildScript_' Unsupported class file major version 65
[org.gradle.internal.buildevents.BuildExceptionReporter] 	at org.gradle.groovy.scripts.internal.DefaultScriptCompilationHandler.compileScript(DefaultScriptCompilationHandler.java:147)
[org.gradle.internal.buildevents.BuildExceptionReporter]  …
[org.gradle.internal.buildevents.BuildExceptionReporter] Caused by: java.lang.IllegalArgumentException: Unsupported class file major version 65
[org.gradle.internal.buildevents.BuildExceptionReporter] 	at groovyjarjarasm.asm.ClassReader.<init>(ClassReader.java:199)

🤔

As per the Gradle Compatibility Matrix, Java 21 requires Gradle 8.5 or later. (No idea why this works on macOS…)

Bump to Gradle 8.12, whic is the current latest stable version.

~~ TODO ~~

While dotnet/java-interop now builds with JDK-21, unit tests don't fully pass. In particular,
tests/Xamarin.Android.Tools.Bytecode-Tests will need to be updated because javac output has changed. (Again.)

Context: 5bb0d24
Context: 4273e5c
Context: 0355acf
Context: dotnet/android#9651

Does dotnet/android build with JDK-21?  We don't know!  But in order
to answer that question, dotnet/java-interop needs to be able to
build under JDK-21; a'la 5bb0d24:

 1. [Install JDK-21][0]

 2. Run `dotnet build -t:Prepare`, overriding `$(JdksRoot)`:

        dotnet build -t:Prepare Java.Interop.sln -p:JdksRoot=/Library/Java/JavaVirtualMachines/microsoft-21.jdk/Contents/Home

 3. Build: `dotnet build`

Unfortunately, this *fails* for three reasons:

 1. `class-parse` crashes when processing `java.base.jmod`.
 2. `src/Java.Base` needs updates to bind JDK-21's `java.base.jmod`.
 3. On Linux and Windows, Gradle 8.1 and JDK-21 don't mix.

~~ class-parse ~~

This only impacts Debug builds of dotnet/java-interop, but:

	% dotnet "bin/Debug-net8.0/class-parse.dll" \
	  "$HOME/android-toolchain/jdk-21/jmods/java.base.jmod" \
	  "-o=obj/Debug-net8.0//mcw/api.xml"
	…
	Process terminated. Assertion failed.
	Unexpected number of method parameters in `Ljdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter$Instantiation;.get(Ljava/lang/Object;)Ljava/lang/Object;`: expected 1, got 0
	   at Xamarin.Android.Tools.Bytecode.MethodInfo.UpdateParametersFromMethodParametersAttribute(ParameterInfo[] parameters) in …/src/Xamarin.Android.Tools.Bytecode/Methods.cs:line 308
	   …

The assertion?

	var pinfo = methodParams.ParameterInfo;
	int startIndex = 0;
	while (startIndex < pinfo.Count &&
	    pinfo [startIndex].AccessFlags.HasFlag (.Synthetic))
	  startIndex++;
	Debug.Assert (parameters.Length == pinfo.Count - startIndex, …);

This is part of 4273e5c and 0355acf, attempting to "skip over" the
constructor parameters which non-static inner classes have which
contain the outer class instance:

	// Java
	class Outer {
	  /* non-static */ class Inner {
	    public Inner () { … }
	  }
	}

At the ABI boundary, the `Outer.Inner` constructor is actually:

	/* partial */ class Outer {
	  /* partial */ class Inner {
	    public Inner (Outer outer) { … }
	  }
	}

and we need to skip over the first parameter.

Which brings us to
`jdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter.Instantiation.get()`:

	% mkdir _x
	% unzip $HOME/android-toolchain/jdk-21/jmods/java.base.jmod -d _x
	% dotnet bin/Debug-net8.0/class-parse.dll --dump \
	  _x/classes/jdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter\$Instantiation.class
	…
		8: get (Ljava/lang/Object;)Ljava/lang/Object; Public, Bridge, Synthetic
			Code(6, Unknown[LineNumberTable](6), LocalVariableTableAttribute(LocalVariableTableEntry(Name='this', Descriptor='Ljdk/internal/org/objectweb/asm/commons/JSRInlinerAdapter$Instantiation;', StartPC=0, Index=0)))
			MethodParametersAttribute(MethodParameterInfo(Name='', AccessFlags=Final, Synthetic))

The parameter for `JSRInlinerAdapter.Instantiation.get(Object)` is
`Synthetic`, causing us to skip over it, which is why we have a
parameter mismatch.  The thing is, this parameter *shouldn't* be
skipped; the skipping is intended for *constructor* parameters!

Update the code so that the loop only occurs for constructors.
This allows `class-parse` to *not assert*, allowing `src/Java.Base`
to build.

~~ Gradle ~~

Via dotnet/android#9651, `gradle` fails when building
`tools/java-source-utils`, but only on Linux and Windows:

	"/mnt/vss/_work/1/s/xamarin-android/external/Java.Interop/build-tools/gradle/gradlew" -d --stacktrace --no-daemon -PjavaSourceVer=11 -PjavaTargetVer=11 jar
	…
	[org.gradle.internal.buildevents.BuildExceptionReporter]
	[org.gradle.internal.buildevents.BuildExceptionReporter] FAILURE: Build failed with an exception.
	[org.gradle.internal.buildevents.BuildExceptionReporter]
	[org.gradle.internal.buildevents.BuildExceptionReporter] * What went wrong:
	[org.gradle.internal.buildevents.BuildExceptionReporter] Could not open settings generic class cache for settings file '/mnt/vss/_work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/settings.gradle' (/home/cloudtest/.gradle/caches/8.1.1/scripts/aiw0k2bokig45bv5yvkog3o3j).
	[org.gradle.internal.buildevents.BuildExceptionReporter] > BUG! exception in phase 'semantic analysis' in source unit '_BuildScript_' Unsupported class file major version 65
	[org.gradle.internal.buildevents.BuildExceptionReporter]
	[org.gradle.internal.buildevents.BuildExceptionReporter] * Try:
	[org.gradle.internal.buildevents.BuildExceptionReporter] > Run with --scan to get full insights.
	[org.gradle.internal.buildevents.BuildExceptionReporter]
	[org.gradle.internal.buildevents.BuildExceptionReporter] * Exception is:
	[org.gradle.internal.buildevents.BuildExceptionReporter] org.gradle.cache.CacheOpenException: Could not open settings generic class cache for settings file '/mnt/vss/_work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/settings.gradle' (/home/cloudtest/.gradle/caches/8.1.1/scripts/aiw0k2bokig45bv5yvkog3o3j).
	[org.gradle.internal.buildevents.BuildExceptionReporter] 	at org.gradle.cache.internal.DefaultPersistentDirectoryStore.open(DefaultPersistentDirectoryStore.java:91)
	[org.gradle.internal.buildevents.BuildExceptionReporter]  …
	[org.gradle.internal.buildevents.BuildExceptionReporter] Caused by: BUG! exception in phase 'semantic analysis' in source unit '_BuildScript_' Unsupported class file major version 65
	[org.gradle.internal.buildevents.BuildExceptionReporter] 	at org.gradle.groovy.scripts.internal.DefaultScriptCompilationHandler.compileScript(DefaultScriptCompilationHandler.java:147)
	[org.gradle.internal.buildevents.BuildExceptionReporter]  …
	[org.gradle.internal.buildevents.BuildExceptionReporter] Caused by: java.lang.IllegalArgumentException: Unsupported class file major version 65
	[org.gradle.internal.buildevents.BuildExceptionReporter] 	at groovyjarjarasm.asm.ClassReader.<init>(ClassReader.java:199)

🤔

As per the [Gradle Compatibility Matrix][1], Java 21 requires
Gradle 8.5 or later.  (No idea why this works on macOS…)

Bump to Gradle 8.12, whic is the current latest stable version.

~~ TODO ~~

While dotnet/java-interop now *builds* with JDK-21, unit tests don't
fully pass.  In particular,
`tests/Xamarin.Android.Tools.Bytecode-Tests` will need to be updated
because `javac` output has changed.  (Again.)

[0]: https://learn.microsoft.com/en-us/java/openjdk/download#openjdk-21
[1]: https://docs.gradle.org/8.12/userguide/compatibility.html
@jonpryor jonpryor requested a review from Copilot January 7, 2025 20:13

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • build-tools/gradle/gradle/wrapper/gradle-wrapper.properties: Language not supported
jonpryor added a commit to dotnet/android that referenced this pull request Jan 7, 2025
Try dotnet/java-interop#1287, which contains a Gradle bump, which
is probably needed because previously Linux & Windows failed to build
because Gradle 8.1 and JDK-21 don't mix.

Update `$(LatestSupportedJavaVersion)` to 21.0.99 so that our unit
tests will actually *try* to do something under JDK-21, instead of
insta-failing with an XA0030 error:

> error XA0030: Building with JDK version `21.0.5` is not supported.
> Please install JDK version `17.0`. See https://aka.ms/xamarin/jdk9-errors

Doh.

Update to Gradle 8.12, for harmony with dotnet/java-interop#1287.
@jonpryor
Copy link
Member Author

jonpryor commented Jan 7, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor jonpryor merged commit ee47652 into main Jan 8, 2025
4 checks passed
@jonpryor jonpryor deleted the dev/jonp/jonp-jdk-21 branch January 8, 2025 18:35
jonpryor added a commit to dotnet/android that referenced this pull request Jan 8, 2025
jonpryor added a commit to dotnet/android that referenced this pull request Jan 9, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Feb 8, 2025
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.

2 participants