Skip to content

Commit

Permalink
Avoid capturing module descriptor in JdkImageWorkaround for AGP >= 8 (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
cdsap authored Feb 9, 2024
2 parents 5f4fc19 + 869ca61 commit e44132f
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.gradle.android.workarounds
import com.google.common.annotations.VisibleForTesting
import com.google.common.collect.Lists
import org.gradle.android.AndroidIssue
import org.gradle.android.Versions
import org.gradle.api.Project
import org.gradle.api.artifacts.transform.CacheableTransform
import org.gradle.api.artifacts.transform.InputArtifact
Expand Down Expand Up @@ -188,14 +189,16 @@ class JdkImageWorkaround implements Workaround {
)
}

// Capture the module descriptor ignoring the version, which is not enforced anyways
File moduleInfoFile = new File(targetDir, 'java.base/module-info.class')
ModuleDescriptor descriptor = captureModuleDescriptorWithoutVersion(moduleInfoFile)
File descriptorData = new File(targetDir, "module-descriptor.txt")
descriptorData.text = serializeDescriptor(descriptor)

fileOperations.delete {
delete(moduleInfoFile)
// Starting with AGP 8 only the major Java version is stored in the output so we don't need any normalization
if (Versions.CURRENT_ANDROID_VERSION.major < 8) {
// Capture the module descriptor ignoring the version, which is not enforced anyways
File moduleInfoFile = new File(targetDir, 'java.base/module-info.class')
ModuleDescriptor descriptor = captureModuleDescriptorWithoutVersion(moduleInfoFile)
File descriptorData = new File(targetDir, "module-descriptor.txt")
descriptorData.text = serializeDescriptor(descriptor)
fileOperations.delete {
delete(moduleInfoFile)
}
}
}

Expand Down
53 changes: 53 additions & 0 deletions src/test/groovy/org/gradle/android/JdkImageWorkaroundTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -257,4 +257,57 @@ class JdkImageWorkaroundTest extends AbstractTest {
buildResult.task(':library:compileDebugJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
buildResult.task(':library:compileReleaseJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
}

def "jdkImage is normalized when using different toolchain configuration"() {

Assume.assumeTrue("Android Gradle Plugin < 8", androidVersion >= VersionNumber.parse("8.0"))


def gradleVersion = TestVersions.latestSupportedGradleVersionFor(androidVersion)
SimpleAndroidApp.builder(temporaryFolder.root, cacheDir)
.withAndroidVersion(androidVersion)
.withKotlinDisabled()
.withToolchainVersion("19")
.withSourceCompatibility(JavaVersion.VERSION_1_9)
.withDatabindingDisabled()
.build()
.writeProject()

when:
BuildResult buildResult = withGradleVersion(gradleVersion.version)
.withProjectDir(temporaryFolder.root)
.withArguments(
"clean", "testDebug", "testRelease", "assemble",
"--build-cache"
).build()

then:
buildResult.task(':app:compileDebugJavaWithJavac').outcome == TaskOutcome.SUCCESS
buildResult.task(':library:compileDebugJavaWithJavac').outcome == TaskOutcome.SUCCESS

buildResult.task(':app:compileDebugUnitTestJavaWithJavac').outcome == TaskOutcome.SUCCESS
buildResult.task(':library:compileDebugUnitTestJavaWithJavac').outcome == TaskOutcome.SUCCESS

when:
buildResult = withGradleVersion(gradleVersion.version)
.withProjectDir(temporaryFolder.root)
.withArguments(
"clean", "testDebug", "testRelease", "assemble",
"--build-cache"
).build()

then:
buildResult.task(':app:compileDebugJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
buildResult.task(':app:compileReleaseJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
buildResult.task(':library:compileDebugJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
buildResult.task(':library:compileReleaseJavaWithJavac').outcome == TaskOutcome.FROM_CACHE

buildResult.task(':app:compileDebugUnitTestJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
buildResult.task(':app:compileReleaseUnitTestJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
buildResult.task(':library:compileDebugUnitTestJavaWithJavac').outcome == TaskOutcome.FROM_CACHE
buildResult.task(':library:compileReleaseUnitTestJavaWithJavac').outcome == TaskOutcome.FROM_CACHE

where:
androidVersion << TestVersions.latestAndroidVersions
}
}

0 comments on commit e44132f

Please sign in to comment.