ctubbsii commented on issue #3771: URL: https://github.com/apache/logging-log4j2/issues/3771#issuecomment-3313787645
@ppkarwasz wrote3: > To avoid either case, the safest setup is to add the `-A` options **only** in the module(s) that actually declare Log4j plugins, and to scope them to the appropriate execution (`default-compile` for `main`, `default-testCompile` for `test`): Right, so that's pretty unfriendly, and the fact that they have to be specified at all, in any module, is unfriendly. It means that this issue is not actually fixed, because the options are not truely optional. The processor requires them. > That’s a fair request. The challenge is that adding a new Log4j-specific “skip” property would increase the number of moving parts people need to check when annotation processing doesn’t run. The compiler already supports the inverse (explicitly enumerating processors to include), so introducing an extra opt-out switch can introduce unnecessary complexity. The problem with the inverse is that it's not very friendly to users who need to use autodiscovery for other annotation processors in their classpath. The idea that you can just enumerate the ones you want is thinking too narrowly, only about the log4j processors, and not a realistic large project environment downstream that uses many processors. I don't think it's much of a challenge. To make the arguments here truly Optional, to properly address this issue, the processors should behave more like log4j's PluginProcessor automatically does when it is run on a project that doesn't need it... that is to say, it should do nothing if the arguments are missing. That would make them truly optional, and would not cause a build error when a user doesn't specify them. For the people that want to use the processor, it's not too hard to add the arguments to make the processor work. > If we tried to use `-A` options as a “disable” flag, we’d still face the same problem: unused `-A` options trigger warnings in modules where no annotations are processed. And since javac doesn’t accept `-D` system properties as processor options, that path isn’t really available either. `System.env` environment variables would work, but it's not very "java". > What we _can_ do, however, is reconsider the severity: we could lower the message from `WARNING` to `NOTE`. That way the processor still runs correctly, GraalVM native-image users still get the metadata, and the only trade-off of not setting the options is a randomly-named JSON resource in the JAR. [@vy](https://github.com/vy), any thoughts? Lowering the severity so that the build doesn't fail would be nice. Another, perhaps better, option, is to just put the GraalVMProcessor in a separate jar artifact (and presumably, the PluginProcessor in yet another, although that's not really necessary, since that one already works in very friendly ways). If I can get the PluginProcessor on the processor paths, without the GraalVMProcessor, I'd still be able to use autodiscovery for errorprone without any requirement to configure the GraalVMProcessor that I don't need or want, making the whole processor optional overall. > ### Side note > I really appreciate teams that run with “zero warnings” policies. Out of curiosity, do you also enforce this philosophy with tools like Error Prone or SpotBugs alongside javac’s linter? In my experience, the biggest value comes when that strictness is applied consistently across static analysis, not only linter warnings. Yes. We use many QA tools, and require the whole build to run without warnings (even javadoc warnings are disallowed) in our build. We do relax some of the configuration of these tools to ignore some warnings by type, but we are regularly making these stricter and stricter, as our code quality improves. Maven itself does give some warnings sometimes, and some Maven plugins produce unavoidable warnings that we can't suppress. For example, using log4j 2.25.1, we get the following from maven-assembly-plugin, which I don't fully understand: ``` [INFO] --- assembly:3.7.1:single (binary-assembly) @ accumulo --- [INFO] Reading assembly descriptor: src/main/assemblies/binary-release.xml [WARNING] Failed to build parent project for org.apache.logging.log4j:log4j-1.2-api:jar:2.25.1 [WARNING] Failed to build parent project for org.apache.logging.log4j:log4j-api:jar:2.25.1 [WARNING] Failed to build parent project for org.apache.logging.log4j:log4j-core:jar:2.25.1 [WARNING] Failed to build parent project for org.apache.logging.log4j:log4j-jul:jar:2.25.1 [WARNING] Failed to build parent project for org.apache.logging.log4j:log4j-slf4j2-impl:jar:2.25.1 [WARNING] Failed to build parent project for org.apache.logging.log4j:log4j-web:jar:2.25.1 ``` I don't know if that's a known issue with log4j or not. I haven't looked into it yet. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
