Skip to content

Including JdkImageWorkaround in newer versions #419

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jan 25, 2023
Merged

Including JdkImageWorkaround in newer versions #419

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
22a1121
including again the JdkImageWorkaround
cdsap Jan 18, 2023
68d6ea4
applying the workaround by base plugin
cdsap Jan 18, 2023
cf86322
removing java/lang/invoke in the default normalization
cdsap Jan 19, 2023
fa66728
simplifying implementation
cdsap Jan 19, 2023
efb429f
revert the change to minimize the diff
cdsap Jan 23, 2023
339aaa9
Merge branch 'main' into recover_jdkImageWorkaround
cdsap Jan 24, 2023
a7e4569
Merge branch 'main' into recover_jdkImageWorkaround
cdsap Jan 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions src/main/groovy/org/gradle/android/workarounds/JdkImageWorkaround.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import java.util.stream.Stream
* than Java 9. This normalizes out minor inconsequential differences between JDKs used to generate the
* custom runtime and improve cache hits between environments.
*/
@AndroidIssue(introducedIn = "7.1.0", fixedIn = ['7.4.0-alpha07'], link = "https://issuetracker.google.com/u/1/issues/234820480")
@AndroidIssue(introducedIn = "7.1.0", link = "https://issuetracker.google.com/u/1/issues/234820480")
class JdkImageWorkaround implements Workaround {
static final String WORKAROUND_ENABLED_PROPERTY = "org.gradle.android.cache-fix.JdkImageWorkaround.enabled"

Expand All @@ -53,16 +53,28 @@ class JdkImageWorkaround implements Workaround {
applyRuntimeClasspathNormalization(project)

applyToAllAndroidVariants(project) { variant ->
variant.javaCompileProvider.configure { JavaCompile task ->
def jdkImageInput = getJdkImageInput(task)
if (jdkImageInput != null) {
setupExtractedJdkImageInputTransform(project, getJvmHome(task))
replaceCommandLineProvider(task, jdkImageInput)
if (Versions.CURRENT_ANDROID_VERSION <= VersionNumber.parse("7.4.0-alpha01")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a newer replacement for variant.javaCompileProvider in 7.4 + ?

Copy link
Member Author

Choose a reason for hiding this comment

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

sadly nope, https://androidstudygroup.slack.com/archives/C03KKLGC2/p1591411391491800

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do something like variant.javaCompilation like we do in the RoomWorkaround?

https://github.com/gradle/android-cache-fix-gradle-plugin/blob/main/src/main/groovy/org/gradle/android/workarounds/RoomSchemaLocationWorkaround.groovy#L116

Copy link
Member Author

Choose a reason for hiding this comment

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

tried yesterday and same issue, at the time the variant is being processed we don't have agp jdkImageInput required to apply the workaround.

Copy link
Member Author

Choose a reason for hiding this comment

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

to extend the previous comment, in the previous AGP variant you have access to the Task Provider of Java task, because is lazy we can guarantee that the required ap options have been configured.
The new variant removes this functionality, in the case of the RoomWorkaround we don't require to have existing ap options because we are just inserting new argument provider in the variant block.

Copy link
Member Author

Choose a reason for hiding this comment

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

@runningcode, following Xavier's comments, there is no replacement for variant.javaCompileProvider but we can still use the old API before is removed in AGP 9.
I created new commit using the old API fa66728 and worked

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good. Thanks for getting Xav's feedback there.

variant.javaCompileProvider.configure { JavaCompile task ->
jdkTransform(project, task)
}
} else {
project.afterEvaluate {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need afterEvaluate here?

Copy link
Member Author

Choose a reason for hiding this comment

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

here correct me if I'm wrong, but my approach was based in:
because the variant.javaCompilerProvider is unavailable, I need to check if a compiler argument is defined for jdkImage. When I'm configuring the project, the AGP didn't create the compiler argument yet. That's the main reason. Ideas/suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

See above thread ^

project.tasks.withType(JavaCompile).configureEach { task ->
jdkTransform(project, task)
}
}
}
}
}

private static void jdkTransform(Project project, JavaCompile task) {
def jdkImageInput = getJdkImageInput(task)
if (jdkImageInput != null) {
setupExtractedJdkImageInputTransform(project, getJvmHome(task))
replaceCommandLineProvider(task, jdkImageInput)
}
}

private static void applyToAllAndroidVariants(Project project, Closure<?> configureVariant) {
if (Versions.CURRENT_ANDROID_VERSION <= VersionNumber.parse("7.4.0-alpha01")) {
applyToAllAndroidVariantsWithOldVariantApi(project, configureVariant)
Expand All @@ -86,14 +98,17 @@ class JdkImageWorkaround implements Workaround {
}

private static void applyToAllAndroidVariantsWithNewVariantApi(Project project, Closure<?> configureVariant) {
def androidComponents = project.extensions.findByName("androidComponents")
def selector = androidComponents.selector()
androidComponents.onVariants(selector.all(), configureVariant)
project.plugins.withId("com.android.base") {
def androidComponents = project.extensions.findByName("androidComponents")
def selector = androidComponents.selector()
androidComponents.onVariants(selector.all(), configureVariant)
}
}

static def applyRuntimeClasspathNormalization(Project project) {
project.normalization { handler ->
handler.runtimeClasspath {
it.ignore '**/java/lang/invoke/**'
it.metaInf { metaInfNormalization ->
metaInfNormalization.ignoreAttribute('Implementation-Version')
metaInfNormalization.ignoreAttribute('Implementation-Vendor')
Expand Down
4 changes: 2 additions & 2 deletions src/test/groovy/org/gradle/android/WorkaroundTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ class WorkaroundTest extends Specification {
workarounds.collect { it.class.simpleName.replaceAll(/Workaround/, "") }.sort() == expectedWorkarounds.sort()
where:
androidVersion | expectedWorkarounds
"8.0.0-alpha11" | ['MergeSourceSetFolders', 'RoomSchemaLocation', 'ZipMergingTask']
"7.4.0" | ['MergeSourceSetFolders', 'RoomSchemaLocation', 'ZipMergingTask']
"8.0.0-alpha11" | ['MergeSourceSetFolders', 'RoomSchemaLocation', 'ZipMergingTask', 'JdkImage']
"7.4.0" | ['MergeSourceSetFolders', 'RoomSchemaLocation', 'ZipMergingTask', 'JdkImage']
"7.3.1" | ['MergeSourceSetFolders', 'RoomSchemaLocation', 'ZipMergingTask', 'JdkImage']
"7.2.2" | ['MergeSourceSetFolders', 'RoomSchemaLocation', 'ZipMergingTask', 'JdkImage']
"7.1.3" | ['BundleLibraryClasses', 'CompileLibraryResources', 'DataBindingMergeDependencyArtifacts', 'LibraryJniLibs', 'MergeNativeLibs', 'MergeSourceSetFolders', 'RoomSchemaLocation', 'StripDebugSymbols', 'ZipMergingTask', 'JdkImage']
Expand Down