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

Bump AGP to 8.1.0 #236

Merged
merged 2 commits into from
Aug 18, 2023
Merged

Bump AGP to 8.1.0 #236

merged 2 commits into from
Aug 18, 2023

Conversation

hfhbd
Copy link
Contributor

@hfhbd hfhbd commented Aug 16, 2023

JVM 20 isn't supported with the precompiled included build used by the tests, because Gradle 8.2.1 uses Kotlin 1.8.22. I could switch from Kotlin to Groovy, or we just use JVM 17 until Gradle 8.3 is out.

build.gradle Outdated
@@ -60,6 +60,23 @@ tasks.named("test") {
}
exceptionFormat "full"
}
// AGP 8+ requires JVM 17+.
javaLauncher = javaToolchains.launcherFor {
languageVersion = JavaLanguageVersion.of(17)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use 17 on CI too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please elaborate why?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@hfhbd hfhbd Aug 17, 2023

Choose a reason for hiding this comment

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

The used JDK to execute Gradle is independent of the JDK used to run the compiler or tests.
To actually run Gradle (this project), you need a JVM, depending on your setup either JAVA_HOME, your project JDK (IntelliJ) or the the specified JDK in GitHub workflow (using setup-java setting JAVA_HOME).
The used Gradle version + Gradle plugins by licensee requires JVM 11.
Now licensee specified JVM 11 for compiling, without any constraints. The same version is used on CI, so Gradle reuse the JDK.
The tests requires JDK 17 (due to AGP 8+), which is specified in the test task using the javaLauncher. So Gradle needs to get a JDK 17.
The auto-detection you mention is about the default paths Gradle looks for JDK 17. The hostedtoolchain paths on GitHub CI isn't used today by Gradle (gradle/gradle#14903), so we use the foojay resolver plugin. Without this plugin, Gradle is unable to download a request JDK.

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL, I always believe Gradle honored the output path of setup-java, thanks for your sharing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, it doesn't, but this will finally be fixed by the grade-build-action, according to the planned 2.8.0 release: gradle/gradle-build-action#561

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, good to know.

@hfhbd
Copy link
Contributor Author

hfhbd commented Aug 17, 2023

BTW Gradle 8.3 is out, so we can use JVM 20 too, after updating.

@JakeWharton JakeWharton merged commit 3419ff4 into cashapp:trunk Aug 18, 2023
2 checks passed
@hfhbd hfhbd deleted the renovate/major-agp branch August 18, 2023 04:00
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

Successfully merging this pull request may close these issues.

None yet

3 participants