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

Wrong files/lib definitions in certain *-capture.mod files? #6464

Closed
jnehlmeier opened this issue Jun 23, 2021 · 4 comments · Fixed by #6465
Closed

Wrong files/lib definitions in certain *-capture.mod files? #6464

jnehlmeier opened this issue Jun 23, 2021 · 4 comments · Fixed by #6465
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@jnehlmeier
Copy link

Jetty version(s)

  • 10.x
  • 11.x

Description

I just tried to create a jetty-base using all of the logging *-capture modules and ended up with a jar file on the root of jetty-base. That seemed weird so I looked into the files and found the following sections which seem to be broken.


https://github.com/eclipse/jetty.project/blob/dc21b2d73cafd78a56e8e532cf50863739bd7027/jetty-home/src/main/resources/modules/logging-jcl-capture.mod#L16-L20

The file should be downloaded to lib/logging/jcl-over-slf4j-${slf4j.version}.jar, right?


https://github.com/eclipse/jetty.project/blob/dc21b2d73cafd78a56e8e532cf50863739bd7027/jetty-home/src/main/resources/modules/logging-log4j1-capture.mod#L16-L20

Here jcl-over-slf4j-${slf4j.version}.jar is also downloaded to the root of jetty-base but lib/logging/log4j-to-slf4j-${slf4j.version}.jar is put on classpath. So log4j-to-slf4j-${slf4j.version}.jar should have been downloaded in the first place and put into lib/logging/, right?

@jnehlmeier jnehlmeier added the Bug For general bugs on Jetty side label Jun 23, 2021
@janbartel
Copy link
Contributor

Yup, looks like a bug to me - shouldn't be any jars in $jetty.base top level, and the logging-log4j1-capture.mod should download log4j-to-slf4j-${slf4j.version}.jar.

@joakime
Copy link
Contributor

joakime commented Jun 23, 2021

I'll take this and get a PR put up today.

@joakime joakime self-assigned this Jun 23, 2021
@jnehlmeier
Copy link
Author

jnehlmeier commented Jun 23, 2021

Yup, looks like a bug to me - shouldn't be any jars in $jetty.base top level, and the logging-log4j1-capture.mod should download log4j-to-slf4j-${slf4j.version}.jar.

Just looked at slf4j and the correct one should be maven://org.slf4j/log4j-over-slf4j/${slf4j.version}.

There is also a log4j-to-slf4j library provided by log4j itself, but that uses a different version, see: https://search.maven.org/artifact/org.apache.logging.log4j/log4j-to-slf4j/2.14.1/jar

joakime added a commit that referenced this issue Jun 23, 2021
+ Adding distribution tests for logging module combinations.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime linked a pull request Jun 23, 2021 that will close this issue
@joakime
Copy link
Contributor

joakime commented Jun 23, 2021

Opened PR #6465 with fix (and new distribution tests)

@lachlan-roberts lachlan-roberts added this to To do in Jetty 10.0.6/11.0.6 FROZEN via automation Jun 23, 2021
Jetty 10.0.6/11.0.6 FROZEN automation moved this from To do to Done Jun 23, 2021
joakime added a commit that referenced this issue Jun 23, 2021
…ad-location

Issue #6464 - fix logging capture modules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants