gradlex-org / extra-java-module-info

A Gradle 6.8+ plugin to use legacy Java libraries as Java Modules in a modular Java project
Apache License 2.0
103 stars 6 forks source link

deriveAutomaticModuleNamesFromFileNames Breaks with Dagger #86

Closed mikewacker closed 10 months ago

mikewacker commented 10 months ago

Here's a minimalistic repro using deriveAutomaticModuleNamesFromFileNames:

Java file:

package org.example;

import javax.inject.Inject;
import javax.inject.Singleton;

@Singleton
public final class Empty {

    @Inject
    public Empty() {}
}

Dependencies block:

dependencies {
    annotationProcessor("com.google.dagger:dagger-compiler:2.48.1")

    implementation("javax.inject:javax.inject:1")
}

Error:

Execution failed for task ':lib:compileJava'.
> Could not resolve all files for configuration ':lib:annotationProcessor'.
   > Failed to transform javac-shaded-9-dev-r4023-3.jar (com.google.errorprone:javac-shaded:9-dev-r4023-3) to match attributes {artifactType=jar, javaModule=true, org.gradle.category=library, org.gradle.libraryelements=jar, org.gradle.status=release, org.gradle.usage=java-runtime}.
      > Execution failed for ExtraJavaModuleInfoTransform: /home/mike/.gradle/caches/modules-2/files-2.1/com.google.errorprone/javac-shaded/9-dev-r4023-3/72b688efd290280a0afde5f9892b0fde6f362d1d/javac-shaded-9-dev-r4023-3.jar.
         > javac.shaded.9.dev.r4023: Invalid module name: '9' is not a Java identifier
jjohannes commented 10 months ago

This is because this is one of the cases where a Module Name cannot be derived from the Jar name (see ModuleFinder Javadoc). If you put the (unmodified) Jar onto the --module-path of java directly you get the same error:

$ java --module-path javac-shaded-9-dev-r4023-3.jar
Error occurred during initialization of boot layer
java.lang.module.FindException: Unable to derive module descriptor for javac-shaded-9-dev-r4023-3.jar
Caused by: java.lang.IllegalArgumentException: javac.shaded.9.dev.r4023: Invalid module name: '9' is not a Java identifier

Which means in this case you need to define a different name explicitly for com.google.errorprone:javac-shaded using automaticModule(...). Which is an advantage of using this plugin over giving the unmodified Jar to java. The unmodified Jar will never work on the Module Path as it is.


A special thing in this setup is that the Jar is on the annotation processor path. You might not care about these Jars turned into modules. If that is the case, you can deactivate what this plugin is doing (the Jar Transformation) for the annotationProcessor configuration:

configurations {
    annotationProcessor {
        attributes { attribute(Attribute.of("javaModule", Boolean::class.javaObjectType), false) }
    }
}

See: https://github.com/gradlex-org/extra-java-module-info#how-do-i-deactivate-the-plugin-functionality-for-a-certain-classpath

mikewacker commented 10 months ago

This does go against the "Can't things just work™ without all that configuration?" branding of deriveAutomaticModuleNamesFromFileNames, though. And "things just work™" in Maven.

In Gradle, module-info.java would look like this if you take a dependency on Dagger:

module org.example {
    requires dagger;
    requires java.compiler;
    requires javax.inject;
}

Without this plugin, you would get a module not found error for requires javax.inject.


Now here's what would happen with various options:

1. Maven

Things just work™. You will get this warning, though:

[INFO] Required filename-based automodules detected: [javax.inject-1.jar]. Please don't publish this project to a public artifact repository!

2. deriveAutomaticModuleNamesFromFileNames, for Direct Dependencies Only

I am fairly confident that this option would work. IIUC, to fix the compilation error, the only dependency that needs to be transformed to an (explicit) automatic module is javax.inject:javax.inject. (Transforming transitive dependencies is nice as well, but IIUC, it's not needed to make the build succeed.)

(The one edge case here could be if you require'd a module, but you didn't list the corresponding dependency in the dependencies block of build.gradle.kts; it got pulled in as the transitive dependency of another dependency.)

3. deriveAutomaticModuleNamesFromFileNames, for Transitive Dependencies

This is how the plugin currently behaves. This is where we run into the issue with javac-shaded-9-dev-r4023-3.jar.


To get back to a "things just work™" state, one fix would be to just allow those "special" filename-based automodules to exist as-is, and not attempt to transform those. Dagger is a fairly popular dependency injection framework.

It does look like that "special" JAR is only pulled in by the com.google.dagger:dagger-compiler annotationProcessor dependency; it's not pulled in by the com.google.dagger:dagger implementation dependency. Another option is to add a note to the "Can't things just work™ without all that configuration?"; this note would include that configuration {} snippet. A build engineer like yourself can fairly easily diagnose that error, but a typical software developer may not easily figure out the fix.

The potential downside with the latter option is that we're overfitting to Dagger; we may learn later that some other popular implementation dependency pulls in a "special" JAR as a transitive dependency. In which case we're back to ground zero.

mikewacker commented 10 months ago

An additional note on the solution when trying it in a real-world context (as opposed to a minimalistic repro): you may also need to do this for testAnnotationProcessor as well:

configurations {
    annotationProcessor {
        attributes { attribute(Attribute.of("javaModule", Boolean::class.javaObjectType), false) }
    }
    testAnnotationProcessor {
        attributes { attribute(Attribute.of("javaModule", Boolean::class.javaObjectType), false) }
    }
}

If you have a multi-module build that use the java-test-fixtures plugin (and we assume that you want to put this configuration block in buildSrc), it's a bit more complex:

  1. Add `java-test-fixtures` to the plugin block of buildSrc/src/main/kotlin/[package].java-conventions.gradle.kts (as opposed to adding it to individual modules that have test fixtures).
  2. You can then add the corresponding testFixturesAnnotationProcessor configuration to your configuration block.

(If you add the java-test-fixtures plugin to individual modules, then the configuration block in the buildSrc conventions file won't have access to testFixturesAnnotationProcessor.)

jjohannes commented 10 months ago

The problem is not about javax.inject-1.jar but about javac-shaded-9-dev-r4023-3.jar which is on the annotation processor path.

If you put com.google.dagger:dagger-compiler:2.48.1 and its transitive dependencies (that include com.google.errorprone:javac-shaded:9-dev-r4023-3) on the --processor-module-path directly, you get the same error from javac. It's just that the build tools ofter do not yet use the new --processor-module-path but put everything on the classpath-based --processor-path. That's why I wrote you might not care about the processor Jars being modules.

You can disable this plugin for all annotation processor paths by doing it for all source sets. I have updated the Readme to contain this as well:

sourceSets.all {
    configurations.getByName(annotationProcessorConfigurationName) {
        attributes { attribute(Attribute.of("javaModule", Boolean::class.javaObjectType), false) }
    }
}