这是indexloc提供的服务，不要输入任何密码
Skip to content

Migrate to jakarta.inject.* in gradle plugin (requires jakarta.inject-api dependency) #13090

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

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Migrate to jakarta.inject.* in gradle plugin (requires jakarta.inject-api dependency) #13090

wants to merge 1 commit into from

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Jan 27, 2025

For Play itself not really relevant, this is just about the gradle plugin.

Currently gradle only supports javax.inject, which is hardcoded.

So we need to wait for:

mkurz
mkurz commented Jan 27, 2025
View reviewed changes
dev-mode/gradle-plugin/build.gradle.kts
@@ -37,6 +37,7 @@ dependencies {
compileOnly("org.playframework:play-routes-compiler_2.13:$playVersion")
compileOnly("org.playframework.twirl:gradle-twirl:${libs.versions.twirl.get()}")
implementation("org.playframework:play-run-support:$playVersion")
implementation("jakarta.inject:jakarta.inject-api:2.0.1")
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this will really be necessary, because it seems currently gradle implicitly adds the javax dependency, so maybe they will do the same with the jakarta one:

@mkurz
Copy link
Member Author

mkurz commented Jan 27, 2025

btw, currently this is failing with:

Welcome to Gradle 8.12!

Here are the highlights of this release:
 - Enhanced error and warning reporting with the Problems API
 - File-system watching support on Alpine Linux
 - Build and test Swift 6 libraries and apps

For more details see https://docs.gradle.org/8.12/release-notes.html

To honour the JVM settings for this build a single-use Daemon process will be forked. For more on this, please refer to https://docs.gradle.org/8.12/userguide/gradle_daemon.html#sec:disabling_the_daemon in the Gradle documentation.
Daemon will be stopped at the end of the build 
> Task :clean
> Task :checkKotlinGradlePluginConfigurationErrors SKIPPED
> Task :compileKotlin NO-SOURCE
Note: /home/runner/work/playframework/playframework/dev-mode/gradle-plugin/src/main/java/play/gradle/internal/RoutesCompileAction.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

> Task :compileJava

> Task :pluginDescriptors
> Task :processResources
> Task :classes
> Task :jar
> Task :compileTestKotlin NO-SOURCE
> Task :compileTestJava
> Task :pluginUnderTestMetadata
> Task :processTestResources NO-SOURCE
> Task :testClasses

> Task :test

PlayPluginTest > Play extension should be registered FAILED
    org.gradle.api.internal.plugins.PluginApplicationException at PlayPluginTest.java:23
        Caused by: org.gradle.api.plugins.PluginInstantiationException at PlayPluginTest.java:23
            Caused by: java.lang.IllegalArgumentException at PlayPluginTest.java:23

PlayRoutesPluginTest > Play Routes configuration should be registered FAILED
    org.gradle.api.internal.plugins.PluginApplicationException at PlayRoutesPluginTest.java:29
        Caused by: org.gradle.api.plugins.PluginInstantiationException at PlayRoutesPluginTest.java:29
            Caused by: java.lang.IllegalArgumentException at InjectUtil.java:57

PlayRoutesPluginTest > Routes source directory set should be registered for main source set FAILED
    org.gradle.api.internal.plugins.PluginApplicationException at PlayRoutesPluginTest.java:29
        Caused by: org.gradle.api.plugins.PluginInstantiationException at PlayRoutesPluginTest.java:29
            Caused by: java.lang.IllegalArgumentException at InjectUtil.java:57

PlayRoutesPluginTest > Routes source directory set should be registered for test source set FAILED
    org.gradle.api.internal.plugins.PluginApplicationException at PlayRoutesPluginTest.java:29
        Caused by: org.gradle.api.plugins.PluginInstantiationException at PlayRoutesPluginTest.java:29
            Caused by: java.lang.IllegalArgumentException at InjectUtil.java:57

PlayRoutesPluginTest > Routes compile task should be registered for main/test source sets FAILED
    org.gradle.api.internal.plugins.PluginApplicationException at PlayRoutesPluginTest.java:29
        Caused by: org.gradle.api.plugins.PluginInstantiationException at PlayRoutesPluginTest.java:29
            Caused by: java.lang.IllegalArgumentException at InjectUtil.java:57

PlayRunPluginTest > Check classpath with submodules FAILED
    org.gradle.api.internal.plugins.PluginApplicationException at PlayRunPluginTest.java:34
        Caused by: org.gradle.api.plugins.PluginInstantiationException at PlayRunPluginTest.java:34
            Caused by: java.lang.IllegalArgumentException at InjectUtil.java:57

6 tests completed, 6 failed

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: file:///home/runner/work/playframework/playframework/dev-mode/gradle-plugin/build/reports/tests/test/index.html

* Try:
> Run with --scan to get full insights.

BUILD FAILED in 1m 4s

> Task :test FAILED

[Incubating] Problems report is available at: file:///home/runner/work/playframework/playframework/dev-mode/gradle-plugin/build/reports/problems/problems-report.html
8 actionable tasks: 8 executed

ihostage
ihostage reviewed Apr 1, 2025
View reviewed changes
dev-mode/gradle-plugin/src/main/java/play/gradle/plugin/PlayPlugin.java
@@ -80,7 +80,7 @@ private void configureTwirlDefaultImports(final Project project) {
});
}
var annotations = ((TwirlSourceDirectorySet) twirlSource).getConstructorAnnotations();
if (annotations != null) annotations.add("@javax.inject.Inject()");
if (annotations != null) annotations.add("@jakarta.inject.Inject()");
Copy link
Member

Choose a reason for hiding this comment

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

I extracted this important change into separated PR #13226 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ihostage ihostage ihostage left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
status:block-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mkurz @ihostage