Upgrading to 2.24.1 produces a compile time warning: BaselineIgnore not found

lbergelson commented 4 weeks ago

Description

We're trying to update from 2.17.1 -> 2.24.1 but we've encountered compile warnings due to missing annotations. It looks like essentially the same same issue as https://github.com/apache/logging-log4j2/issues/2144 and https://github.com/apache/logging-log4j2/issues/2232

Configuration

Version:. 2.24.1 Operating system: OSX Sonoma 14.4 JDK: openjdk 17.0.12 2024-07-16 OpenJDK Runtime Environment Homebrew (build 17.0.12+0) OpenJDK 64-Bit Server VM Homebrew (build 17.0.12+0, mixed mode, sharing)

Logs

/gradlew compileTestjava

Task :compileJava /Users/louisb/.gradle/caches/modules-2/files-2.1/org.apache.logging.log4j/log4j-api/2.24.1/7ebeb12c20606373005af4232cd0ecca72613dda/log4j-api-2.24.1.jar(/org/apache/logging/log4j/Level.class): warning: Cannot find annotation method 'value()' in type 'BaselineIgnore': class file for aQute.bnd.annotation.baseline.BaselineIgnore not found

Reproduction

Update log4j to 2.24.1 in our build.gradle

    platform('org.apache.logging.log4j:log4j-bom:2.24.1)
    implementation 'org.apache.logging.log4j:log4j-api'
    implementation 'org.apache.logging.log4j:log4j-core'

This was very surprising to me. Have you considered changing the annotation dependencies to be included. Or they processed out of the published jar?

We need to include log4j-core at compile time, because we want to change the default logging level at runtime based on command line input from the user. If there is a way to do this through the API we would be happy to switch.

I've solved the warning by adding 2 additional dependencies

    implementation 'biz.aQute.bnd:biz.aQute.bnd.annotation'
    implementation 'org.osgi:org.osgi.annotation.bundle'

Just adding biz.aQute.bnd:biz.aQute.bnd.annotation causes additional failures because it includes OSGI bundle annotations which are not included in it's dependency list.

I feel like I'm doing something wrong but I'm not really sure what I should be doing instead.

ppkarwasz commented 4 weeks ago

Hi @lbergelson,

It's ironic that adding annotations for third-party static code analysis tools, causes warnings in Oracle's -Xlint tool. My recommendation would be to add -classfile to your custom -Xlint option. As far as I could find in my IDE, there are only two warnings in the whole javac compiler code that use that category. They are both in c.s.t.javac.jvm.ClassReader and concern:

Missing annotations at compile time. This includes annotations with a retention of CLASS, which are not used at runtime, like the ones you are reporting.
The presence in a classfile of attributes from a future Java version, e.g. the sealed attribute in a class that uses Java 8 bytecode. I can not think of a natural situation, when this could happen.

If you disable classfile, you won't miss anything important.

This was very surprising to me. Have you considered changing the annotation dependencies to be included. Or they processed out of the published jar?

We have considered both solutions and each one comes at a cost. Currently we are using the following annotations, which all have their specificity:

OSGi annotations (like @Version) and BND annotations. These are license (or dual licensed) under Apache-2.0 and have a retention of CLASS. OSGi users (admittedly a small group) need them in the classfiles.
Annotations for the SpotBugs static analysis tool. They are licensed under LGPL-2.1, so inclusion in some products might be problematic (I am not a lawyer). On the other hand I don't know if Log4j users can benefit from them. They have a retention of CLASS.
Annotations for the Error Prone static analysis tool. These are definitively useful for a large group of users (see the @InlineMe annotation we also use in our code base). Some of them have a retention of RUNTIME, but I believe we only use those with a retention of CLASS.
Nullability annotations from JSpecify. This seems to be the first standard for nullability annotations supported by multiple vendors and it could really benefit the users. For some reason they have a retention of RUNTIME.

Adding annotations to the `compile/implementation` scope

We certainly could not add Spotbugs annotations to our compile scope, at least our lawyers say so.

Some of these annotations would be very useful to Log4j API users (e.g. JSpecify), but in general the PMC is quite reticent to add any dependency to the compile scope for log4j-api. We only have OSGi dependencies in the provided scope (which corresponds to Gradle's compileOnly configuration and is not transitive). For Log4j Core there are even less reasons to add the (compile-time only) annotations to the compile scope, since most users have Log4j Core in the runtime scope (runtimeOnly for Gradle). These dependencies would end up in the applications even if users do not use them.

Removing annotations from the class files

The problem with removing annotations from the class files is that AFAIK, there is no tool for that! Admittedly it must be a couple of hundred lines of ASM 9 code, but we didn't have time to develop it yet. I have opened apache/logging-log4j-tools#153. If you are willing to contribute such a tool, we love PRs! :heart:

As I stated earlier, not all annotations are eligible for removal (I think JSpecify, the OSGi @Version and Error Prone @InlineMe should stay), but @BaselineIgnore does not give any useful information to the compiler. We only use it to make breaking changes according to Java, which are not breaking changes according to the documentation: e.g. removing a public class that was always documented as private.

lbergelson commented 3 weeks ago

Thank you for the in depth response. It sounds like you've done a lot more thinking about than this than I have.

It was just a surprising gotcha to trip over for me. I had never seen that warning pop up before, but missing class files sounded worrisome so I didn't want to disable it in general. It sounds like it's extremely niche and not gaining us much though.

I've worked around it and you're obviously aware so I'll close this out.

ppkarwasz commented 3 weeks ago

Since @BaselineIgnore is an annotation with CLASS retention and absolutely nothing wrong will happen if it is not available at compile time, I have reported this as JDK-8342833 bug.

apache / logging-log4j2