search

gradlex-org / jvm-dependency-conflict-resolution

Gradle plugin to improve Dependency Conflict Detection and Resolution
http://gradlex.org/jvm-dependency-conflict-resolution/
Apache License 2.0
45 stars 15 forks source link

`LOG4J_SLF4J_IMPL` vs. `LOG4J_SLF4J2_IMPL` for `enforceLog4J2()` #121

Open Vampire opened 3 months ago

Vampire commented 3 months ago

Calling enforceLog4J2() chooses LOG4J_SLF4J_IMPL as slf4j impl. Wouldn't it be more appropriate to select LOG4J_SLF4J2_IMPL?

ljacomet commented 1 month ago

According to the documentation, this depends on the version of slf4j in use.

I need to check what can be done there and whether this decision can be made automatically or if the user will need to make a choice. In the previous version of this plugin, I had an issue for this.

I am going to move this out of the 2.1 release as it is a bigger piece of work.

Vampire commented 1 month ago

Well, if you select log4j-slf4j-impl and anything in the build uses slf4j 2, it will not work, as the discovery changed from including a class with specific FQCN to a proper way.

If you select log4j-slf4j2-impl, this depends on slf4j 2 and thus upgrades the slf4j version if only 1.7 was used, so should work in both cases.

I think the the only potentially unwanted effect should be that slf4j is updated to 2+ when it could stay on 1.7, but why should what be a problem? And if it is one could still manually change configuration.

Vampire commented 1 month ago

The slf4j docs even explicitly state

From the client's perspective the slf4j-api, more specifically classes in the org.slf4j package, are backward compatible for all versions. Client code compiled with slf4j-api-N.jar will run perfectly fine with slf4j-api-M.jar for any N and M. You only need to ensure that the version of your provider/binding matches that of the slf4j-api.jar. You do not have to worry about the version of slf4j-api.jar used by a given dependency in your project.

So as adding the log4j-slf4j2-impl will also updating the slf4j-api, all should be fine.

The only thing is, that you break Java 5 - 7 compatibility which was supported by 1.7 but no longer in 2+, but honestly, ...