why was slf4j-api added as a dependency?

[zoff99]

i read: https://github.com/xerial/sqlite-jdbc/issues/997#issuecomment-1776972487 and: https://github.com/xerial/sqlite-jdbc/commit/32082c0504d02ee4b0b379b6cc4f26fc9fcf95ab and: https://github.com/xerial/sqlite-jdbc/commit/89dbda1bb21bc7df004419edb32e26f17bbb0e19

but i can not see any reason this was done.

here it says "Read the Changelog", but there is nothing about slf4j-api in the changelog. why was this added? can you link to documentation about the decision or some code in sqlite that requires it?

why was it not possible to make this optional?

[gotson]

it was added because it's used in almost any library, and it lets the application control logging. You can check https://github.com/xerial/sqlite-jdbc/issues/802

[jinsihou19]

yes，it is trouble for me to add external dependencies. look it...

[headius]

I must also raise a concern about this. JRuby does not ship with slf4j and we are very reluctant to add new dependencies. As a result, when we support SQLite for Ruby on Rails we will also have to ship an slf4j library.

Most users of slf4j I know of allow their code to fall back on j.u.logging so that upstream users do not have to introduce a new dependency. This also conflicts with users running on other logging backends, forcing the use of two different logging frameworks for no good reason.

Can we please reopen this discussion and figure out a way to remove the slf4j hard dependency from sqlite-jdbc?

See https://github.com/jruby/activerecord-jdbc-adapter/pull/1149#issuecomment-2250639765

[headius]

In https://github.com/xerial/sqlite-jdbc/issues/802#issuecomment-1306572582 @gotson mentions that two other drivers also use slf4j: MySQL and H2. However on both of these databases, the slf4j usage and depedency is optional:

• MySQL: https://dev.mysql.com/doc/connector-j/en/connector-j-logging-slf4j.html
• H2: https://www.h2database.com/html/features.html#other_logging

Both databases default to a simple logger without external dependencies.

[headius]

Neither MySQL nor H1 maven artifacts depend on slf4j directly.

[KweezyCode]

remove the slf4j1 dependency, as this causes conflicts between different versions (for example i can't use JDA which uses slf4j2)

[headius]

@gotson It seems like this hard slf4j dependency is still causing trouble for people. Can we please have a discussion about making it optional? As I pointed out in https://github.com/xerial/sqlite-jdbc/issues/1094#issuecomment-2258707132, none of the databases you used as examples actually have a hard dependency on slf4j.

The only option for users that want to avoid the dependency is to fork the project and push their own artifacts. Nobody wants to have to do that, especially me!

[KweezyCode]

@gotson It seems like this hard slf4j dependency is still causing trouble for people. Can we please have a discussion about making it optional? As I pointed out in #1094 (comment), none of the databases you used as examples actually have a hard dependency on slf4j.

The only option for users that want to avoid the dependency is to fork the project and push their own artifacts. Nobody wants to have to do that, especially me!

i have made a fork, check my profile

[gotson]

@gotson It seems like this hard slf4j dependency is still causing trouble for people. Can we please have a discussion about making it optional? As I pointed out in #1094 (comment), none of the databases you used as examples actually have a hard dependency on slf4j.

The only option for users that want to avoid the dependency is to fork the project and push their own artifacts. Nobody wants to have to do that, especially me!

if someone can send a PR where SLF4J is a soft-dependency, with a fallback on JUL, i will have a look at it.

[headius]

if someone can send a PR where SLF4J is a soft-dependency, with a fallback on JUL, i will have a look at it.

@gotson Thank you for your flexibility! 🙇‍♂️

@KweezyCode You have commits that remove SLF4J altogether, do you want to take a stab at making it optional? We might be able to get this resolved quickly!

[KweezyCode]

if someone can send a PR where SLF4J is a soft-dependency, with a fallback on JUL, i will have a look at it.

@gotson Thank you for your flexibility! 🙇‍♂️

@KweezyCode You have commits that remove SLF4J altogether, do you want to take a stab at making it optional? We might be able to get this resolved quickly!

I don't support the idea of bloating the software, so it's better to remove something from the library than to make something optional (which will make the code grow even more)

[zoff99]

i hope we can meet on middle ground. making it optional sounds like a reasonable way to solve this issue.

[headius]

@KweezyCode Optionally using slf4j has become pretty standard as a way to allow users to configure other kinds of loggers without the original author having to add support for them. It is in most cases the last logging framework you ever need to add, because it acts as a shim on top of all the other ones. This pattern is quite common and adds very little bloat.

I think we also need to be pragmatic here. The two other database drivers mentioned both include slf4j as an optional logging backend and @gotson is meeting us halfway here by accepting a patch to make it optional in this driver. I for one just want to get this patched and released so my users can upgrade.

[michael-o]

Here is one serious problem I have hit today while upgrading to latest version: If you are running in a multi-classloader container like Tomcat and you have the JDBC driver in the server or common classloader to make it available to all webapps and the webapp uses SLF4J and its own SLF4J binding you will end with such a fuckup:

2024-09-12T19:55:45.438 INFORMATION [main] org.apache.jasper.servlet.TldScanner.scanJars At least one JAR was scanned for TLDs yet contained no TLDs. Enable debug logging for this logger for a complete list of JARs that were scanned but no TLDs were found in them. Skipping unneeded JARs during scanning can improve startup time and JSP compilation time.
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/var/opt/tomcat-smartld/webapps/smartld%23%23029/WEB-INF/lib/logback-classic-1.2.12.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/opt/ports/apache-tomcat-9.0.94/lib/jdbc/slf4j-simple-1.7.36.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Actual binding is of type [ch.qos.logback.classic.util.ContextSelectorStaticBinder]

This is not funny and there is not way to solve. The only workaround is to move the JDBC driver into the webapp classloader.

So adding SLF4J the way it is now wasn't good after all.

I support @headius' position.

I forgot to mention: The JAR installed has all native libraries stripped because they aren't suited. SQLite3 is globally installed, the JNI shim is linked against it and installed globally as well. No size overhead, no extraction overhead. Since it is a stripped-down version I cannot use the one from Maven Central as a dep in my webapp POM. The other one comes with a custom-tailed Tomcat. Loss-loss situation here.

[headius]

I have pushed #1178 which makes slf4j optional. It is no longer a hard dependency, and when not present the implementation will fall back on java.util.logging, similar to other JDBC drivers.

I will work with the xerial/sqlite-jdbc maintainers to get that PR merged. Could this issue be reopened please?

[headius]

@michael-o:

The JAR installed has all native libraries stripped because they aren't suited.

I think you should file a separate bug for this.

[gotson]

I have pushed #1178 which makes slf4j optional. It is no longer a hard dependency, and when not present the implementation will fall back on java.util.logging, similar to other JDBC drivers.

I will work with the xerial/sqlite-jdbc maintainers to get that PR merged. Could this issue be reopened please?

this issue will remain closed, it was a question. The PR being opened is enough.

[headius]

1178 was merged! We just need a release now.

[zoff99]

thanks for everybody who helped solve this.

[github-actions[bot]]

🎉 This issue has been resolved in 3.46.1.1 (Release Notes)

[headius]

Thank you for the release!