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

Unsupported major.minor version in JdkImageWorkaround (jdk20) #646

Closed
cdsap opened this issue Jan 26, 2024 · 0 comments
Closed

Unsupported major.minor version in JdkImageWorkaround (jdk20) #646

cdsap opened this issue Jan 26, 2024 · 0 comments

Comments

@cdsap
Copy link
Member

cdsap commented Jan 26, 2024

When applying the plugin in a project using JDK 17 and this jvmToolchain conf:

kotlin {
   jvmToolchain {
      languageVersion.set(JavaLanguageVersion.of(20))
   }
}

the build returns the following error:

Execution failed for task ':app:compileDebugJavaWithJavac'.
> Could not resolve all files for configuration ':app:androidJdkImage'.
   > Failed to transform core-for-system-modules.jar to match attributes {artifactType=_internal_android_jdk_image_extracted, org.gradle.libraryelements=jar, org.gradle.usage=java-runtime}.
      > Execution failed for ExtractJdkImageTransform: /Users/kio/.gradle/caches/transforms-3/066939602350b840bee1b3625dbd3a64/transformed/output.
         > Unsupported major.minor version 64.0

Expanding the failure it points to:

at org.gradle.android.workarounds.JdkImageWorkaround$ExtractJdkImageTransform.captureModuleDescriptorWithoutVersion(JdkImageWorkaround.groovy:203)

Build Scan https://ge.solutions-team.gradle.com/s/yymfunyhmtmqy

Additional tests

Repo Example

https://github.com/cdsap/ReproducerJdkImageWorkaroundWithJDK20

cdsap added a commit that referenced this issue Feb 9, 2024
…659)

#646 describes the issue when applying the plugin using not aligned JDKs
in the toolchain and the build, build fails on the JdkImageWorkaround
with the following message:
```
Execution failed for task ':app:compileDebugJavaWithJavac'.
> Could not resolve all files for configuration ':app:androidJdkImage'.
   > Failed to transform core-for-system-modules.jar to match attributes {artifactType=_internal_android_jdk_image_extracted, org.gradle.libraryelements=jar, org.gradle.usage=java-runtime}.
      > Execution failed for ExtractJdkImageTransform: /Users/kio/.gradle/caches/transforms-3/066939602350b840bee1b3625dbd3a64/transformed/output.
         > Unsupported major.minor version 64.0
```

After syncing with the BT team, we are proposing this PR where the main
change is to not use the module descriptor after extracting the JDK
generated by AGP.

## Background
The main motivation of the `JdkImageWorkaround` is to normalize the
inconsequential differences between JDKs. A more detailed explanation
can be found
[here](https://issuetracker.google.com/u/1/issues/234820480).
Our workaround fixed completely the issue in different JDK/AGP versions
despite the claim that this issue was fixed in AGP 7.4. Last year we
experimented with different JDK vendors and we noticed the issue in some
specific versions of JDK 11 and JDK 17. We repeated the experiment last
week targeting AGP >= 8 and JDKs [17-21] observing the issue is
[present](https://ge.solutions-team.gradle.com/c/jgotrldjuvgks/wvig5icz75ybi/task-inputs?expanded=WyJhcmJ1cTRtb2x6eDR5LW9wdGlvbnMuY29tcGlsZXJhcmd1bWVudHByb3ZpZGVycy4kMC5qcnRmc2phciJd)
for JDK 21.
It makes sense to keep the workaround for recent Java/AGP versions.

## Toolchain issue
In all the different experiments we used aligned JDK versions between
the build and the toolchain configuration. However, when we use a
greater JDK version in the `JavaCompile` task , through toolchain conf,
than the JDK build, we get the exception when trying to apply the logic
of the workaround which creates a new module-descriptor file dropping
the unnecessary versions:
```
File moduleInfoFile = new File(targetDir, 'java.base/module-info.class')
ModuleDescriptor descriptor = captureModuleDescriptorWithoutVersion(moduleInfoFile)
File descriptorData = new File(targetDir, "module-descriptor.txt")
```
The exception makes sense because the "extracted jdk" is created with
the toolchain JDK version and we try to read it with a lower JDK
version.
We are able to reproduce the issue in AGP 7.x versions. 

## AGP fix on the JDK differences
Although the original issue(aligned jdks) is present in AGP 8.2.2/JDK
21, we analyzed the AGP changes in 7.4 fixing partially this issue
([details](https://issuetracker.google.com/u/1/issues/234820480#comment5)).
Following the AGP classes `JdkImageTransform` and
`JdkImageTransformDelegate`, we observed they are
[already](https://cs.android.com/android-studio/platform/tools/base/+/mirror-goog-studio-main:build-system/gradle-core/src/main/java/com/android/build/gradle/internal/dependency/JdkImageTransformDelegate.kt;l=264?q=JdkImageTransformDelegate)
creating a "module-info.java" file describing the contents of the
"module.class" dropping the versions as we are doing in the workaround.
We don't need to apply the same logic in the plugin.

## Using ModuleDescriptor based on the AGP version
Knowing we don't need to generate a new "module-info.java" in the
transform, it fixes the issue because the original exception is caused
by capturing the module descriptor of the class file to generate the new
info file. Still, we need to create the info file in AGP versions before
the fix was introduced:
```
if (Versions.CURRENT_ANDROID_VERSION.major < 8) {
   File moduleInfoFile = new File(targetDir, 'java.base/module-info.class')
   ModuleDescriptor descriptor = captureModuleDescriptorWithoutVersion(moduleInfoFile)
   File descriptorData = new File(targetDir, "module-descriptor.txt")
   ...
}
```

## Toolchain state after fix 

| | No workaround | Current workaround | This PR |

|-----------------------------------------|------------------------|--------------------|---------|
| AGP 8.x with toolchains - JDK 21 | Success | Error | Success |
| AGP 8.x without toolchains - JDK 21 | Build cache difference | Success
| Success |


## Tests
### JDK Build 17 - JDK Toolchain 20 
Before:
* Build Failed https://ge.solutions-team.gradle.com/s/75jkfgb652u3g
After:
* Build
https://ge.solutions-team.gradle.com/s/wnkkcskifuwks/timeline?details=arbuq4molzx4y&expanded=WyI0Il0
* JDK 17.0.2 Zulu vs JDK 17 Temurin
https://ge.solutions-team.gradle.com/c/wnkkcskifuwks/zndww6rospvws/task-inputs


## Additional notes
The issue is still present when using the workaround with AGP 7.x and
toolchains not aligned.
This PR adds new test using toolchain 19 for AGP > 8
@cdsap cdsap closed this as completed Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant