Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

log4j-slf4j-impl 2.22.0 appears to depend on slf4j-api 2 #2065

Closed
bpholt opened this issue Dec 6, 2023 · 12 comments · Fixed by #2113
Closed

log4j-slf4j-impl 2.22.0 appears to depend on slf4j-api 2 #2065

bpholt opened this issue Dec 6, 2023 · 12 comments · Fixed by #2113
Milestone

Comments

@bpholt
Copy link

bpholt commented Dec 6, 2023

Description

log4j-slf4j-impl 2.22.0 appears to depend on slf4j 2, and not slf4j 1 as described by the documentation.

Configuration

Version: 2.22.0

Operating system: MacOS 13.5.1

JDK: I've tried Eclipse Adoptium Java 11.0.18 and Amazon.com Inc. Java 17.0.9 but I don't think it would matter

Logs

Not applicable

Reproduction

Using the following build.sbt file:

libraryDependencies += "org.apache.logging.log4j" % "log4j-slf4j-impl" % "2.22.0"

then run sbt "show fullClasspath":

$ sbt "show fullClasspath"
[info] welcome to sbt 1.9.7 (Eclipse Adoptium Java 11.0.18)
[info] loading settings for project global-plugins from global.sbt ...
[info] loading global plugins from ~/.sbt/1.0/plugins
[info] loading project definition from ~/log4j2-slf4j/project
[info] loading settings for project log4j2-slf4j from build.sbt ...
[info] set current project to log4j2-slf4j (in build file:~/log4j2-slf4j/)
[info] * Attributed(~/log4j2-slf4j/target/scala-2.12/classes)
[info] * Attributed(~/.sbt/boot/scala-2.12.18/lib/scala-library.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/apache/logging/log4j/log4j-slf4j-impl/2.22.0/log4j-slf4j-impl-2.22.0.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/apache/logging/log4j/log4j-api/2.22.0/log4j-api-2.22.0.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/slf4j/slf4j-api/2.0.9/slf4j-api-2.0.9.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/apache/logging/log4j/log4j-core/2.22.0/log4j-core-2.22.0.jar)
[success] Total time: 1 s, completed Dec 5, 2023, 5:58:16 PM

Note the slf4j-api version: 2.0.9.

@ppkarwasz
Copy link
Contributor

@bpholt,

Thanks for using Log4j API[1] and noticing the problem. I believe that the source of the problem might be in Coursier.

We use both SLF4J 1.7.x and 2.x (for log4j-slf4j-impl) in our build. In the org.apache.logging.log4j:log4j artifact (parent of log4j-slf4j-impl) we define the version of SLF4J:

<properties>
  <slf4j.api>2.0.9</slf4j.api>
</properties>
<dependencies>
    <dependency>
      <groupId>org.slf4j</groupId>
      <artifactId>slf4j-api</artifactId>
      <version>${slf4j.version}</version>
    </dependency>
</dependencies>

while in the log4j-slf4j-impl we define:

<properties>
  <slf4j.version>1.7.25</slf4j.version>
</properties>

This way to override dependency version is supported by Maven (Spring Boot uses it extensively):

[INFO] --- dependency:2.8:tree (default-cli) @ sandbox ---
[INFO] eu.copernik.sandbox:sandbox:jar:0.0.1-SNAPSHOT
[INFO] \- org.apache.logging.log4j:log4j-slf4j-impl:jar:2.22.0:runtime
[INFO]    +- org.apache.logging.log4j:log4j-api:jar:2.22.0:runtime
[INFO]    +- org.slf4j:slf4j-api:jar:1.7.25:runtime
[INFO]    \- org.apache.logging.log4j:log4j-core:jar:2.22.0:runtime

Coursier might have a problem with this. You should probably report it there.

[1] the problem would not have appeared if you had slf4j-api as direct dependency.

@bpholt
Copy link
Author

bpholt commented Dec 6, 2023

That override scheme makes sense in principle and has a certain elegance, but I don't think this issue is specific to Coursier, nor does including slf4j-api as a direct dependency resolve the problem. I can include

ThisBuild / useCoursier := false

in the build.sbt to use Ivy for dependency resolution instead of Coursier, and it still brings in slf4j 2:

sbt:log4j2-slf4j> show fullClasspath
[info] * Attributed(~/log4j2-slf4j/target/scala-2.12/classes)
[info] * Attributed(~/.sbt/boot/scala-2.12.18/lib/scala-library.jar)
[info] * Attributed(~/.ivy2/cache/org.apache.logging.log4j/log4j-slf4j-impl/jars/log4j-slf4j-impl-2.22.0.jar)
[info] * Attributed(~/.ivy2/cache/org.apache.logging.log4j/log4j-api/jars/log4j-api-2.22.0.jar)
[info] * Attributed(~/.ivy2/cache/org.slf4j/slf4j-api/jars/slf4j-api-2.0.9.jar)
[info] * Attributed(~/.ivy2/cache/org.apache.logging.log4j/log4j-core/jars/log4j-core-2.22.0.jar)
[success] Total time: 0 s, completed Dec 6, 2023, 10:06:25 AM

(Note that the paths now refer to the .ivy2 cache directory instead of the Coursier cache.)

Similarly, regardless if Coursier or Ivy is used, setting:

libraryDependencies ++= Seq(
  "org.apache.logging.log4j" % "log4j-slf4j-impl" % "2.22.0",
  "org.slf4j" % "slf4j-api" % "1.7.36",
)

(such that slf4j-api is specified as a direct dependency) results in an eviction warning, but slf4j 2.x is still the version selected for the classpath:

sbt:log4j2-slf4j> evicted
[warn] Found version conflict(s) in library dependencies; some are suspected to be binary incompatible:
[warn] 	* org.slf4j:slf4j-api:2.0.9 is selected over 1.7.36
[warn] 	    +- org.apache.logging.log4j:log4j-slf4j-impl:2.22.0   (depends on 2.0.9)
[warn] 	    +- default:log4j2-slf4j_2.12:0.1.0-SNAPSHOT           (depends on 1.7.36)
[success] Total time: 0 s, completed Dec 6, 2023, 10:06:36 AM
sbt:log4j2-slf4j> 

FWIW, another workaround (other than downgrading) is to specify a dependency override:

dependencyOverrides += "org.slf4j" % "slf4j-api" % "1.7.36"

but it's mistake-prone to have to specify that everywhere, when we didn't have to before v2.22.0 of this library. In my experience, dependencyOverrides has been a source of issues itself, because it allows one to introduce version conflicts that the eviction warning scheme is meant to help prevent, so it makes me nervous to suggest it as a workaround as it feels like an anti-pattern.

Is there a name for this override scheme in Maven, so I can open tickets with Ivy and Coursier so that they might add support? In the meantime, until those tools support the scheme, could log4j revert to its earlier way of specifying the slf4j-api dependency, so this downstream effect is mitigated?

@ppkarwasz
Copy link
Contributor

@bpholt,

I don't think this scheme has a name. It is based on the fact that the "Inheritance Assembly" phase is performed before the "Model Interpolation" phase (cf. Maven Model Builder). I guess that Ivy and Coursier do it differently.

To mitigate the current problem we are open to PRs. As far as I recall only log4j-slf4j-impl and log4j-mongodb3 override a version from the parent.

@ppkarwasz
Copy link
Contributor

@pjfanning, as the resident Scala expert, do you have an idea how to fix this issue?

@pjfanning
Copy link
Contributor

I haven't tested this yet. I think this should be reported to sbt team though. It sounds like the issue might be in sbt itself.

https://repo1.maven.org/maven2/org/apache/logging/log4j/log4j-slf4j-impl/2.22.0/ and its pom.xml look correct to me (slf4j 1.7 dependency)

@bpholt
Copy link
Author

bpholt commented Dec 6, 2023

To mitigate the current problem we are open to PRs.

The easiest thing to do would be to revert #1920, but I'm not sure what other impact that would have.

Otherwise, do you think it would help to add the dependency version within

<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
(and maybe in log4j-mongodb3 too; I haven't looked at that one)?

@vy
Copy link
Member

vy commented Dec 7, 2023

I agree with @pjfanning's assessment. Generating the effective POM in Maven (using IDEA and/or help:effective-pom) also points that all SLF4J dependencies in log4j-slf4j-impl are fixed to 1.7.25. AFAIC, Log4j employs a valid, widely-used Maven convention, the issue should ideally be reported to Ivy/sbt.

@ppkarwasz
Copy link
Contributor

@bpholt,

Reverting #1920 would only move the problem from log4j-slf4j-impl to log4j-slf4j2-impl, so it is not an option. As you can use on MvnRepository (that must be using Scala), before 2.22.0 it was log4j-slf4j2-impl that had the same problem as you have.

Yes, I believe that adding an explicit version to log4j-to-slf4j/pom.xml should help, but I don't use SBT, so I can not check. If you submit a PR, please also add a comment near the explicit version pointing to this issue. Otherwise we might unintentionally remove the explicit version again.

@pjfanning
Copy link
Contributor

Interestingly, with org.apache.logging.log4j:log4j-slf4j-impl:2.21.1 - sbt resolves the slf4j dependency as 1.7.36 - which is ok since it's still 1.7.x.

I think sbt must be getting the slf4j dependency from somewhere other than the log4j-slf4j-impl pom.xml. Maybe it gets it from something like the log4j-parent.

@bpholt
Copy link
Author

bpholt commented Dec 7, 2023

I think sbt must be getting the slf4j dependency from somewhere other than the log4j-slf4j-impl pom.xml. Maybe it gets it from something like the log4j-parent.

That has been my guess as well, since the versions match up.

v2.21.0:

<slf4j.version>1.7.36</slf4j.version>

v2.22.0:

<slf4j.version>2.0.9</slf4j.version>

@bpholt
Copy link
Author

bpholt commented Dec 7, 2023

Reverting #1920 would only move the problem from log4j-slf4j-impl to log4j-slf4j2-impl, so it is not an option. As you can use on MvnRepository (that must be using Scala), before 2.22.0 it was log4j-slf4j2-impl that had the same problem as you have.

It's definitely a problem, but IMO a little less severe than the log4j1 artifact, because it works to specify the slf4j 2.x version (directly or transitively):

libraryDependencies ++= Seq(
  "org.slf4j" % "slf4j-api" % "2.0.9",
  "org.apache.logging.log4j" % "log4j-slf4j2-impl" % "2.21.0",
)
sbt:log4j2-slf4j> evicted
[warn] Found version conflict(s) in library dependencies; some are suspected to be binary incompatible:
[warn] 	* org.slf4j:slf4j-api:2.0.9 is selected over 1.7.36
[warn] 	    +- default:log4j2-slf4j_2.12:0.1.0-SNAPSHOT           (depends on 2.0.9)
[warn] 	    +- org.apache.logging.log4j:log4j-slf4j2-impl:2.21.0  (depends on 1.7.36)
[success] Total time: 0 s, completed Dec 7, 2023, 10:29:56 AM
sbt:log4j2-slf4j> show fullClasspath
[info] * Attributed(~/log4j2-slf4j/target/scala-2.12/classes)
[info] * Attributed(~/.sbt/boot/scala-2.12.18/lib/scala-library.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/slf4j/slf4j-api/2.0.9/slf4j-api-2.0.9.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/apache/logging/log4j/log4j-slf4j2-impl/2.21.0/log4j-slf4j2-impl-2.21.0.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/apache/logging/log4j/log4j-api/2.21.0/log4j-api-2.21.0.jar)
[info] * Attributed(~/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/apache/logging/log4j/log4j-core/2.21.0/log4j-core-2.21.0.jar)
[success] Total time: 0 s, completed Dec 7, 2023, 10:30:07 AM

(i.e. it still appears as an eviction warning but the classpath ends up being right)

@ppkarwasz ppkarwasz added this to the 2.22.1 milestone Dec 18, 2023
ppkarwasz added a commit to ppkarwasz/logging-log4j2 that referenced this issue Dec 20, 2023
Workaround a Coursier/Ivy dependency resolution bug that affects
`log4j-slf4j-impl` and `log4j-mongodb3`.

This bug also affects popular sites like MvnRepository (cf.
[`log4j-mongodb3:2.22.0`](https://mvnrepository.com/artifact/org.apache.logging.log4j/log4j-mongodb3/2.22.0)).

Closes apache#2065
@ppkarwasz
Copy link
Contributor

This is blocked by:

@ppkarwasz ppkarwasz modified the milestones: 2.22.1, 2.23.0 Dec 21, 2023
@ppkarwasz ppkarwasz modified the milestones: 2.23.0, 2.22.1 Dec 22, 2023
ppkarwasz added a commit to ppkarwasz/logging-log4j2 that referenced this issue Jan 3, 2024
Workaround a Coursier/Ivy dependency resolution bug that affects
`log4j-slf4j-impl` and `log4j-mongodb3`.

This bug also affects popular sites like MvnRepository (cf.
[`log4j-mongodb3:2.22.0`](https://mvnrepository.com/artifact/org.apache.logging.log4j/log4j-mongodb3/2.22.0)).

Closes apache#2065
ppkarwasz added a commit to ppkarwasz/logging-log4j2 that referenced this issue Jan 3, 2024
Workaround a Coursier/Ivy dependency resolution bug that affects
`log4j-slf4j-impl` and `log4j-mongodb3`.

This bug also affects popular sites like MvnRepository (cf.
[`log4j-mongodb3:2.22.0`](https://mvnrepository.com/artifact/org.apache.logging.log4j/log4j-mongodb3/2.22.0)).

Closes apache#2065
ppkarwasz added a commit to ppkarwasz/logging-log4j2 that referenced this issue Jan 5, 2024
Workaround a Coursier/Ivy dependency resolution bug that affects
`log4j-slf4j-impl` and `log4j-mongodb3`.

This bug also affects popular sites like MvnRepository (cf.
[`log4j-mongodb3:2.22.0`](https://mvnrepository.com/artifact/org.apache.logging.log4j/log4j-mongodb3/2.22.0)).

Closes apache#2065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants