clojars / clojars-web

A community repository for open-source Clojure libraries
https://clojars.org
Eclipse Public License 1.0
468 stars 114 forks source link

Clean up slf4j dependencies #842

Closed ajoberstar closed 1 year ago

ajoberstar commented 1 year ago

I was trying to set up my dev environment locally and couldn't start a REPL. I was getting the exception below, which seemed likely to be some kind of SLF4J mismatch. It may not have cropped up on others due to classpath ordering differences, I'm not positive if Leiningen does anything special to work around that.

Looking through the deps tree, I found a few dependencies that were pulling in other SLF4J libs that were quite different versions from the 2.0.0-alpha1 pulled in by your explicit dep on logback.

This has removed the SLF4J fork pulled in by bote and the slf4j-nop which is pulled in by Duct's hikari component (which kinda seems like a bug on their side).

The remaining jcl dep may be needed to route logs into logback, so I bumped the version to align with the logback dep's slf4j version.

Caused by: java.lang.IllegalAccessError: class org.slf4j.LoggerFactory tried to access private field org.slf4j.impl.StaticLoggerBinder.SINGLETON (org.slf4j.LoggerFactory and org.slf4j.impl.StaticLoggerBinder are in
unnamed module of loader 'app')
tobias commented 1 year ago

I'm not opposed to making these changes, but I would like to figure out how you were starting the repl to understand why it works for me.

I can start a repl (and start the app via (go)) without errors using java 17 and lein 2.9.7.

What version of java and lein are you using?

Do you have anything in ~/.lein/profiles.clj?

lein does provide a stable classpath order given the same inputs, so I don't think varying order should be the problem.

tobias commented 1 year ago

I just upgraded to lein 2.9.10, and do get the exception you are getting. Comparing the output of lein deps :tree for both versions, I don't see any dependency differences other than test-only dependencies now having a test scope. I'm not sure what impact that has, but it seems like it shouldn't matter.

I'm going to go ahead and merge this. Thanks for tracking it down!

ajoberstar commented 1 year ago

FYI, while I don't understand Leiningen well, I did a bisect to track down the commit between 2.9.7 and 2.9.10 where it started to fail. It does seem to have something to do with dependency scopes, with a mention of test scope in the message.

tobias commented 1 year ago

Interesting, thanks. I suspect having the classifiers changes classpath ordering.