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

Update sqlite-jdbc to 3.46 #1149

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

segiddins
Copy link
Contributor

@segiddins segiddins commented Jul 20, 2024

@segiddins
Copy link
Contributor Author

@headius this is needed to be able to use newer SQLite features that are available in cruby from the sqlite3 gem

@headius
Copy link
Member

headius commented Jul 23, 2024

Great, thanks! We will merge and get a release out soon.

@headius
Copy link
Member

headius commented Jul 23, 2024

@enebo @kares maybe you can release this week while I'm traveling?

@enebo
Copy link
Member

enebo commented Jul 24, 2024

I will likely find time this week. In process of packing/cleaning so it is just dependent on that.

@enebo enebo merged commit 45ec346 into jruby:master Jul 25, 2024
0 of 13 checks passed
@enebo
Copy link
Member

enebo commented Jul 25, 2024

Whoops. I should have looked at the run results (which in turn is a problem with a pretty red CI) to notice this version of sqlite3 depends on slf4j-api + transitive deps.

xerial/sqlite-jdbc@21c77a4

It appears since 3.28 a lot of deps have been added (although I am not sure how many are actually needed for runtime from this page): https://central.sonatype.com/artifact/org.xerial/sqlite-jdbc/3.40.0.0/dependencies

We probably need to vendor these but at the same time I expect we will see errors due to loading this in mixed environments.

@headius
Copy link
Member

headius commented Jul 30, 2024

this version of sqlite3 depends on slf4j-api + transitive deps

Ugh.

We probably need to vendor these

If it comes to that I think we'd rather start using jar-dependencies to fetch them, so we're not shipping our own copies of these jar files.

I will also make the case to xerial/sqlite-jdbc that adding these dependencies is a major nuisance.

@headius
Copy link
Member

headius commented Jul 30, 2024

It looks like this concern was already raised by two others: xerial/sqlite-jdbc#1094

I have added some fuel to that fire.

@segiddins
Copy link
Contributor Author

Given this was reverted, is there another ticket now tracking the sqlite version update?

@headius
Copy link
Member

headius commented Sep 10, 2024

@segiddins I have poked the issue again, but have no idea if they'll be receptive.

@enebo We may be forced to fork the project just to remove the dependency. We'd continue to merge but have our own release that just removes slf4j. I don't want to do it any more than you do!

@headius
Copy link
Member

headius commented Sep 11, 2024

It looks like we'll be able to get this patched in sqlite-jdbc once we get a patch to them that makes the sl4fj dependency optional. 🎉

I've asked another interested party if they could modify their slf4j removal patch to make it optional instead. If they don't do it I will.

@headius
Copy link
Member

headius commented Sep 18, 2024

I have pushed xerial/sqlite-jdbc#1178 to make slf4j optional. Just need to get the maintainers to merge it and release now.

@anovadox
Copy link

anovadox commented Oct 1, 2024

It looks like https://github.com/xerial/sqlite-jdbc/releases/tag/3.46.1.1 has been released 🎉

make slf4j optional with fallback on JUL (b8ea5ca), closes xerial/sqlite-jdbc#1094

xerial/sqlite-jdbc#1094 (comment)

@headius
Copy link
Member

headius commented Oct 2, 2024

@anovadox Good eye! We'll spin an update release.

headius added a commit to headius/activerecord-jdbc-adapter that referenced this pull request Oct 2, 2024
@headius
Copy link
Member

headius commented Oct 2, 2024

When #1158 looks good I'll merge and release.

@headius
Copy link
Member

headius commented Oct 2, 2024

jdbc-sqlite-3.46.1.1 has been released!

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 this pull request may close these issues.

4 participants