You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "jjohannes (via GitHub)" <gi...@apache.org> on 2025/02/06 16:11:05 UTC
[I] Compiling on Module Path with -Werror and -Xlint:all (logging-log4j2)
jjohannes opened a new issue, #3437:
URL: https://github.com/apache/logging-log4j2/issues/3437
This is about getting an error like the following when compiling with a `module-info.java` and the compiler flags `-Xlint:all` and `-Werror`:
```
> Compilation failed; see the compiler output below.
log4j-api-2.24.3.jar(/org/apache/logging/log4j/message/ParameterizedMessage.class): warning: Cannot find annotation method 'replacement()' in type 'InlineMe': class file for com.google.errorprone.annotations.InlineMe not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/message/ParameterizedMessage.class): warning: Cannot find annotation method 'imports()' in type 'InlineMe'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/status/StatusData.class): warning: Cannot find annotation method 'value()' in type 'SuppressFBWarnings': class file for edu.umd.cs.findbugs.annotations.SuppressFBWarnings not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/status/StatusData.class): warning: Cannot find annotation method 'justification()' in type 'SuppressFBWarnings'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/SystemPropertiesPropertySource.class): warning: Cannot find annotation method 'value()' in type 'ServiceProvider': class file for aQute.bnd.annotation.spi.ServiceProvider not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/SystemPropertiesPropertySource.class): warning: Cannot find annotation method 'resolution()' in type 'ServiceProvider'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'value()' in type 'Headers': class file for org.osgi.annotation.bundle.Headers not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'name()' in type 'Header': class file for org.osgi.annotation.bundle.Header not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'value()' in type 'Header'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'name()' in type 'Header'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'value()' in type 'Header'
error: warnings found and -Werror specified
1 error
11 warnings
```
The error is triggered by using classes from Log4j that use annotations on public members from the annotation libraries indicated in the error message. There are two _layers_ of missing dependencies that cause this:
1. The annotation library's Jar file (e.g. `com.google.errorprone:error_prone_annotations:2.36.0`) is not on the Classpath/Module Path
2. The annotation library's Java Module (e.g. `com.google.errorprone.annotations`) is not defined as `require static` in the corresponding `module-info.class` of Log4j
For (1) it's debatable if it can be fixed in a good way, as POM does not have a scope that resembles [Gradle's compileOnlyApi scope](https://www.baeldung.com/gradle-dependency-management#2-types-of-configuration): visible transitively but ONLY on the compile classpath.
For (2) you can argue that it is a bug, because if the code of Log4j would be compiled on the Module Path (i.e. with the compiler checking the `module-info.java`) it would not compile.
### Workarounds for users
(1)
- Adding the missing dependencies directly as `compileOnly` (`<provided>`) to your own project.
- Patching the POM metadata with a rule (possible in Gradle)
(2)
- Calling the compiler with additional `--add-reads` arguments
- Patching the `module-info.class` itself (possible in Gradle with plugin)
### Possible improvements in Log4j
(1) This was already discussed [here](https://github.com/apache/logging-log4j2/issues/3110#issuecomment-2423586754). The dependencies marked as `<provided>` [here](https://github.com/apache/logging-log4j2/blob/94d3227165f22993c126c9ec19f3ddc03abe967c/log4j-parent/pom.xml#L829-L858) would need to be switched to `<compile>`. But that has the unwanted effect that ALL users will get them as unnecessary RUNTIME dependencies.
There is the option to publish additional [Gradle Metadata](https://github.com/gradle/gradle/blob/master/platforms/documentation/docs/src/docs/design/gradle-module-metadata-latest-specification.md) that supports the `compileOnlyApi` scope. If you are interested, I can make a contribution for that as I maintain the [Maven plugin for publishing such metadata with Maven](https://github.com/jjohannes/gradle-module-metadata-maven-plugin) (it's used by Jackson for example). But I can understand if you don't want to have additional complexity in your build setup.
(2) Maybe you can check if the additional requires could be added to all `module-info`:
```
requires static transitive com.google.errorprone.annotations;
requires static transitive com.github.spotbugs.annotations;
requires static transitive biz.aQute.bnd.annotation;
requires static transitive org.osgi.annotation.bundle;
```
*`requires static` works also, but conceptually `requires static transitive` feels more correct as the annotations are transitively visible.
-----
All of this can also be avoided by turning of the `classfiles` line check of `javac`...
I can understand if there is nothing you can/want to do at this point. Then, feel free to close this issue.
Having this written down can be helpful for users who run into this in any case.
If you want to do something I can help with, let me know.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Compiling on Module Path with -Werror and -Xlint:all (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #3437:
URL: https://github.com/apache/logging-log4j2/issues/3437#issuecomment-2640456549
@jjohannes,
> (1) This was already discussed [here](https://github.com/apache/logging-log4j2/issues/3110#issuecomment-2423586754). The dependencies marked as `<provided>` [here](https://github.com/apache/logging-log4j2/blob/94d3227165f22993c126c9ec19f3ddc03abe967c/log4j-parent/pom.xml#L829-L858) would need to be switched to `<compile>`. But that has the unwanted effect that ALL users will get them as unnecessary RUNTIME dependencies. There is the option to publish additional [Gradle Metadata](https://github.com/gradle/gradle/blob/master/platforms/documentation/docs/src/docs/design/gradle-module-metadata-latest-specification.md) that supports the `compileOnlyApi` scope. If you are interested, I can make a contribution for that as I maintain the [Maven plugin for publishing such metadata with Maven](https://github.com/jjohannes/gradle-module-metadata-maven-plugin) (it's used by Jackson for example). But I can understand if you don't want to have additional complexity in your build setup.
Please submit a PR, 🥺 . I found out about your `gradle-module-metadata-maven-plugin` a couple of weeks ago, but I didn't have the time to check how it works.
> (2) Maybe you can check if the additional requires could be added to all `module-info`:
>
> ```
> requires static transitive com.google.errorprone.annotations;
> requires static transitive com.github.spotbugs.annotations;
> requires static transitive biz.aQute.bnd.annotation;
> requires static transitive org.osgi.annotation.bundle;
> ```
>
> *`requires static` works also, but conceptually `requires static transitive` feels more correct as the annotations are transitively visible.
A `requires static transitive` statement has some unwanted effects. When you compile using the module path, JPMS recursively enumerates all the modules marked as `transitive` and fails if one is missing. After your changes Gradle users will not be affected by this problem, but Maven users will. See #2929 for example.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Compiling on Module Path with -Werror and -Xlint:all [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz closed issue #3437: Compiling on Module Path with -Werror and -Xlint:all
URL: https://github.com/apache/logging-log4j2/issues/3437
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Compiling on Module Path with -Werror and -Xlint:all (logging-log4j2)
Posted by "jjohannes (via GitHub)" <gi...@apache.org>.
jjohannes commented on issue #3437:
URL: https://github.com/apache/logging-log4j2/issues/3437#issuecomment-2648686124
My understanding of the argument why annotations with `CLASS` retention should be visible transitively (in certain cases) at compile time is: If, for instance, a public member of a Log4j class is annotated, it can be argued that that annotation is part of the public API at compile time – even though it's gone at runtime. If I now use tooling at compile time that visits the public API of all things used on the compile classpath, it may encounter this annotated method. Then it may be (at least theoreticaly) important that the class behind the annotation is available. That's what, to my understanding, the `-Xlint:classfile` check is validating: that all annotations on public members used during compilations are available. (Similar to how the compiler would fail if a superclass in the hierarchy is missing even though our own code does not interact with it directly.)
As OSGi is only about runtime, it's no surprise that BND does not care about this as well. It took me some time and source code reading / debugging of BND's JPMS support, but I found the switch to add additional `require static` entries. As said above, I think these are harmless to add, as the compiler won't complain in general if they are missing. And at runtime, they are not of importance anyway.
Only having both things – Gradle Metadata for `compileOnlyApi` dependencies and the additional `require static` entries – will solve the case I outlined in the description completely.
Here is a PR that proposes to add both: https://github.com/apache/logging-log4j2/pull/3450
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Compiling on Module Path with -Werror and -Xlint:all (logging-log4j2)
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #3437:
URL: https://github.com/apache/logging-log4j2/issues/3437#issuecomment-2640572981
> This would be in line with the `requires static org.jspecify` you already have and it should do no harm. If you agree and if you give me a pointer how this would be added, I can also provide a PR for that.
We use the [BND JPMS Libraries](https://bnd.bndtools.org/chapters/330-jpms.html) to generate our `module-info.class`. I am not sure what is the exact logic it applies to annotations to decide whether they are included or not. I suspect that only annotation with a retention of `RUNTIME` are included. Unless I am mistaken, only JSpecify is retained at runtime, so it is the only one in `module-info.class`.
IMHO annotations that are not visible at runtime do not need to be mentioned in the `module-info.class`, but the opinion is not universally shared (see [JDK-8342833](https://bugs.openjdk.org/browse/JDK-8342833) for some counter arguments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Compiling on Module Path with -Werror and -Xlint:all (logging-log4j2)
Posted by "jjohannes (via GitHub)" <gi...@apache.org>.
jjohannes commented on issue #3437:
URL: https://github.com/apache/logging-log4j2/issues/3437#issuecomment-2640495652
Thanks for the quick response @ppkarwasz.
I'll prepare a PR for the Gradle Metadata in the next days.
> A `requires static transitive` statement has some unwanted effects.
Ah yes. I did not think about that. Luckily, if you would add them as `requires static` it is stille enough for the lint check to work (see my reproducer):
```
requires static com.google.errorprone.annotations;
requires static com.github.spotbugs.annotations;
requires static biz.aQute.bnd.annotation;
requires static org.osgi.annotation.bundle;
```
This would be in line with the `requires static org.jspecify` you already have and it should do no harm. If you agree and if you give me a pointer how this would be added, I can also provide a PR for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org