[SQLite] Support linking against a system SQLite library

[mehcode]

• Is this a cargo feature flag? How does it work? Perhaps have sqlite (bundled) and sqlite_sys (unbundled) ?

• What is the default inclusion strategy? My preference leans heavily towards bundled.

[mehcode]

This is blocked on libsqlite3-sys allowing us to specify a much higher minimum SQLite version.

[jcgruenhage]

Ftr: This makes packaging software using sqlx a bit of a pain, because it breaks cross compilation in some situations. If you could make static linking of sqlite optional, I'd really appreciate it. Your reason for bundling sqlite doesn't apply to all distros, in this case, Void ships sqlite-3.32.3 at the moment, so that's more up to date than what libsqlite3-sys bundles.

Additionally, when you start to support this: Making this configurable via features only doesn't help distro packagers much. An env var would be much nicer for that. This would require a Cargo build script, but that's very doable. If you want this to mainly be a cargo feature, you can also set (or unset) the cargo feature from the build script. An env var override would really be appreciated.

[mehcode]

Making this configurable via features only doesn't help distro packagers much [...]

Can you help me understand how you'd use this? What does the environment variable look like?

---

[...] Your reason for bundling sqlite doesn't apply to all distros [...]

The libsqlite3-sys crate has 3 primary modes of compilation: bindgen where it generates the rust bindings from header files for the target and links against a system library; bundled where it compiles and statically links the bundled amalgamation; and the default, where it uses pre-generated rust bindings and links against a system library. I would rather not use bindgen as it greatly increases compile time and requires the header files to be present. However, the pre-generated bindings in libsqlite3-sys does not have several functions that we depend on: sqlite3_bind_blob64, sqlite3_bind_text64, sqlite3_prepare_v3 and sqlite3_value_dup.

[jcgruenhage]

Can you help me understand how you'd use this? What does the environment variable look like?

Sure. The variable could be SQLITE_COMPILATION_MODE_OVERRIDE=bindgen. We'd set that in the template of the application that depends on sqlx when building that. The build script could then read that env var and use it to override the selected feature. Does that make it clear enough? Otherwise I can try to build an example.

[...] I would rather not use bindgen as it greatly increases compile time and requires the header files to be present. However, the pre-generated bindings in libsqlite3-sys does not have several functions that we depend on: sqlite3_bind_blob64, sqlite3_bind_text64, sqlite3_prepare_v3 and sqlite3_value_dup.

That sounds quite sensible. Having bundled as the default sounds good then, and bindgen for distros that want to package software.

Now that I think more about it, maybe it'd make more sense for that override to live in libsqlite3-sys though? Does bundled vs bindgen result in enough difference in the API surface for your crate to fail compiling? If not, then putting that override into libsqlite3-sys sounds better.

[mehcode]

bundled vs bindgen shouldn't cause any differences to SQLx as long as a recent enough version of SQLite is used (v3.8.11+)

you're right that an env var override to change compilation mode makes more sense in libsqlite3-sys

once support for 3.8.11+ pre-generated bindings is available in libsqlite3-sys, we plan to remove the bundled feature flag from sqlx which means that an application using sqlx that wants bundled would do:

sqlx = { version = "0.5", features = [ "sqlite" ] }

# the * means we don't care about the version, its being constrained by sqlx anyway
libsqlite3-sys = { version = "*", features = [ "bundled" ] }

[thomcc]

once support for 3.8.11+ pre-generated bindings is available in libsqlite3-sys

I've meant to reach out to you about this, and do a write up, but we have an open issue in the rusqlite repository around the minimum/default supported version: https://github.com/rusqlite/rusqlite/issues/706. (FWIW there's no rule that says that rusqlite and libsqlite3-sys need to have the same values here, although it would certainly be convenient for me if they did).

That said it's been a rough year in general and I haven't found the time to do so yet. That said, if you have opinions here they'd be valuable. Even the knowledge that you care about 3.8.11+ is very helpful.

In general I think the way libsqlite3-sys / rusqlite handles versions to be suboptimal. In an ideal world, there would be only a few versions that control underlying features, and then we'd have feature flags for them. clang-sys does this like https://github.com/KyleMayes/clang-sys/blob/master/Cargo.toml#L22-L42 and is fairly okay as a result, although you can see the fractures of that approach starting to not scale very well.

However, the world we live in is one in which there are very very many versions we would have to do this way, far more than clang-sys has to deal with...

Edit: P.S. in the future please feel welcome to file bugs in the rusqlite repo against libsqlite3-sys for anything sqlx finds lacking (such as the versioning situation). In general we'd be more than happy to make changes to accommodate your needs.

[mehcode]

@thomcc

I'll echo what you've said. It's been a rough year. Please don't take anything I've said as a slight against the libsqlite3 or rustqlite teams. It's very much on us for not attempting to reach out yet on the minimum version issue. It's just at the back of my priority list as bundled works fine for us at the moment.

---

in the future please feel welcome to file bugs in the rusqlite repo against libsqlite3-sys for anything sqlx finds lacking (such as the versioning situation). In general we'd be more than happy to make changes to accommodate your needs.

❤️ I appreciate it. libsqlite3-sys has been generally very "boring" which is a good thing for a -sys library. It does what it's told. It'd be nice if Clion knew what it was doing but that's hardly the fault of the library.

[thomcc]

Please don't take anything I've said as a slight against the libsqlite3 or rustqlite teams

I didn't, no worries!

It'd be nice if Clion knew what it was doing but that's hardly the fault of the library.

I've considered moving it so that if it's using the "bundled" bindgen file it gets pulled in as a #[path = "..."] mod blah controlled by a cfg instead of an include!. I suspect this would solve that problem, and probably solve some other long-tail problems (like not working well with bazel/buck/cargo-raze and other tools that don't actually do build.rs the way you might expect/at all).

That said it's a bit tricky since it requires restructuring some things in our code. rust-analyzer also is able to "see" it sort of (although it's a bit confused and usually finds the wrong bindgen file), so personally I haven't felt this pain too hard (and so it hasn't pushed me to fix it -- not that that's really a good thing).

The other alternative would be to give up bindgen (and use e.g. ctest) the way stuff like openssl-sys works, but the SQLite API has new features all the time and so that doesn't feel like a good path for allowing easy updates.

Anyway I've filed https://github.com/rusqlite/rusqlite/issues/775 about this, although it's probably going to be on my back burner for a bit.

[abonander]

@mehcode @thomcc it's been another year, anything new here?

[thomcc]

There's been more discussion in https://github.com/rusqlite/rusqlite/issues/706. There's a plan, I just have to find the time to implement it.

[thomcc]

To be clear, the plan is that libsqlite3-sys will expose everything, and it will be up to users of the library (sqlx, rusqlite, diesel, etc) to only use the right parts. (e.g. we won't expose different things under different versions of sqlite).

[jirutka]

It’s two years and this is still causing a lot of pain for distro packagers. I ran into this problem when trying to package atuin.

[jirutka]

@abonander, can you please resolve this?

[dezren39]

This could be useful in using the sqlite fork libsql with sqlx.

[kchibisov]

I'm not sure how stable it'll be, but you can link to system sqlite by doing export LIBSQLITE3_SYS_USE_PKG_CONFIG=1 before the build, thus the libsqlite3-sys will use pkg-config and link to system library.

It would be really nice to not force the bundled version, since it makes it really hard to patch vulnerabilities in the bundled libraries, in general such behavior should be opt-in, not opt-out in the first place.

Failing to build and suggesting that the user should update their libsqlite3 library or enable bundled feature is much more explicit and doesn't get in the way of people who desire dynamic linking with the system library package, because their distributions demand that.

[abonander]

I don't know what the current state of things on the libsqlite3-sys side is, but I'd accept a PR that adds a sqlite-unbundled feature if it can work as-is now. We can't drop the default bundled feature when the sqlite feature is enabled without it being a breaking change, but adding a different feature that doesn't enable it would be fine.

[lilydjwg]

I've sent pr #3507.