Move JSpecify from provided to compile scope

[ppkarwasz]

JSpecify is a nullability annotation library with a large list of supporters. It is the recommended library to solve LOG4J2-1477.

The usage of JSpecify as provided library has however a drawback: if users don't have JSpecify on their stack, they'll get a Javac Linter warning whenever they use a class with nullability annotations (see #3110 for example). Since JSpecify is an annotation with RUNTIME retention and is not covered by JDK-8342833, the easiest solution provided by this PR is:

• Move JSpecify from the provided to the compile scope. This will inflate users dependency stack by a whooping 3819 bytes.
• Mark JSpecify as optional for OSGi and JPMS, so users can exclude the dependency if they wish to do it.

In practice, this change should be neutral for users and will allow us to add nullability annotations to important class, such as Logger without breaking the build of users that use the -Werror compiler flag.

[rgoers]

-1

We have always had a rule that Log4j-API will have no required dependencies.

[ppkarwasz]

Yes, I know this is against current policy, but can that policy be reevaluated?

The rationale is that keeping nullability annotations in the artifacts is mostly useful to log4j-api users. Log4j Core calls usually don't appear in user code, but Log4j API classes are more used.

While the implementation can certainly do null-checks, we might mark the format parameters of log calls as @NonNull: it doesn't make sense to log something, when the format string is null. Also the Level parameter should be marked @NonNull as well as many of the parameters and return types of ThreadContext.

[rgoers]

Yes, I am aware of all these arguments as they were raised when the dependency was first added. I made the same objection then but was assured that by using provided scope users would have no problems if the dependency is missing. If that really isn’t the case then the dependency should be removed and a different solution should be found. Note that even using Jakarta.annotations would be a problem as we also have a policy to limit the dependencies to Java.base in JPMS, which is work I believe you worked on as part of 3.0.

[ppkarwasz]

I made the same objection then but was assured that by using provided scope users would have no problems if the dependency is missing. If that really isn’t the case then the dependency should be removed and a different solution should be found.

I don't consider a compiler warning (if the -Xlint:all linter option is enabled) to be a real problem. Besides, users can always disable the classfile category in the linter: it contains only two warnings (see this comment) and both of them are pretty harmless.

If I understand correctly, your -1 only applies to the additional dependency in log4j-api, while you are OK with adding it to the other artifacts?

[rgoers]

Yes, the -1 is for the API only. While we have always tried to minimize the dependencies on Log4j-core that has never been as strict. I believe we made some effort in 3.0 to remove many dependencies but I don't believe we got rid of them all. You would probably know that better than me since you have been actively working on it recently.

[ppkarwasz]

I believe we made some effort in 3.0 to remove many dependencies but I don't believe we got rid of them all.

Version 3.0.0-beta3 of Log4j Core has no external dependencies (see MvnRepository). Most importantly, it has no optional dependencies, which are always a problem. Even if we exclude JPMS and OSGi, normal Java users pin their optional Log4j Core dependency in the POM file and they forget to upgrade them, when they upgrade Log4j Core. This will no longer be an issue.