Closed Bug 1626323 Opened 5 years ago Closed 5 years ago

Vendor rusqlite into mozilla-central

Categories

(Firefox :: Sync, task)

task
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 77
Tracking Status
firefox77 --- fixed

People

(Reporter: tcsc, Assigned: tcsc)

References

Details

Attachments

(1 file)

In order to replace the extension-storage backend and sync implementation with the rust-based one we're developing, we need to vendor rusqlite into mozilla-central. It's also required for many of the SACI teams other future plans, and I believe the RKV team is interested in implementing a sqlite backend based on rusqlite as well, which would require this.

Ultimately, this requires getting rusqlite (well, libsqlite3-sys, which is part of the rusqlite project and in the rusqlite repo) linking against the in-tree sqlite. On it's face, this doesn't seem so bad: it has a number of configuration options for pointing it to the desired sqlite (mostly via environment flags), and the maintainer of rusqlite in the past has been willing to merge changes we need for using it in mozilla products (and I now comaintain it). I also co-maintain rusqlite (and by extension libsqlite3-sys) now, and can help get what we need through here.

However, while in some configurations(?) we seem to build it as a library, it seems that generally we just produce an object file that gets built into libxul(?) or some of libnss(?).

  • Trying to get libsqlite3-sys to link against libxul for this failed.
  • As did trying to change the build system to output a library for SQLite and linking against that.
  • As did just attempting to just disable most of the linking shenanigans hte way cubeb-sys seems to do.

However, these may still be viable routes, that I just didn't figure out how to make them work. Even in the case where I managed to get something working, it proved unreliable, presumably because mozbuild didn't know that there's a dependency on having built sqlite already. Trying to manually force this to happen (via... gross makefile hacks) didn't really work reliably either, and I suspect it couldn't land if it did.

It's also unclear if we have to worry about linking to an external sqlite (non-mozilla) still -- (I've heard we don't support this anymore, but it seems like it still has some support in the tree?).

Anyway, I've tried for a while now to sort all this out, and even do somewhat hacky things to get us unstuck, but in the end I'm pretty lost, and would like help. :linacambridge tells me that :glandium is someone who can at least point me in the right direction? Sorry if not, feel free to redirect to someone better (as well as ask me further questions)

This is the bugzilla version of this JIRA ticket, however the JIRA version doesn't have much information. https://jira.mozilla.com/browse/SYNC-794 and https://github.com/mozilla/application-services/issues/2597, although the... assumption there (that this is just 'cargo vendor + fixups') was hopelessly naive, and based mostly on a prototype I previously did which managed to link against system sqlite somehow (would have expected this to have caused errors).

Flags: needinfo?(mh+mozilla)
Blocks: 1623245

(Ah, I didn't realize there was an existing bug for this, thanks Lina!)

No worries, I just duped the old one forward since yours has a lot more detail!

No longer blocks: 1623245

Can you add more details as to how each of your attempts failed (and what they were in change terms)? Intuitively, I'd say you should link nothing (at all), and if your rust wrapper ends in libxul, then it will get sqlite indirectly from there.

Flags: needinfo?(mh+mozilla)

I thought that would work too, and definitely tried it. I believe it was a pretty typical linker error (undefined reference to symbol _sqlite3_open or something along those lines).

I can't do another build for this right now (It's after work hours and I can't stay late), but I'm needinfoing myself to get the specific errors to you tomorrow.

Flags: needinfo?(tchiovoloni)

After adding this https://github.com/thomcc/rusqlite/blob/in-gecko/libsqlite3-sys/build.rs#L7-L11 to libsqlite3-sys (to link nothing), and adding this (below) to toolkit/library/rust/shared/Cargo.toml (and adding a extern crate rusqlite to /Users/thom/gecko/toolkit/library/rust/shared/lib.rs)

rusqlite = { git = "https://github.com/thomcc/rusqlite", features = ["modern_sqlite", "in_gecko"], rev = "13fba9838cbc0b24d4370ccc88530afb00e31084" }

I get the following log (complete log here, but errors reproduced below https://gist.github.com/thomcc/2a409b225a754687d03d33dfaea0d98d)

61:45.70 Undefined symbols for architecture x86_64:
61:45.70   "_sqlite3_bind_zeroblob", referenced from:
61:45.80       __ZN8rusqlite9statement9Statement14bind_parameter17hef052d043e32036fE in libgkrust.a(rusqlite-bb2a2db7ebb1c68f.rusqlite.8kny5wtm-cgu.3.rcgu.o)
61:46.16   "_sqlite3_threadsafe", referenced from:
61:46.24       __ZN8rusqlite16inner_connection33ensure_safe_sqlite_threading_mode17h1b6f52c9b5d5051dE in libgkrust.a(rusqlite-bb2a2db7ebb1c68f.rusqlite.8kny5wtm-cgu.4.rcgu.o)
61:46.64   "_sqlite3_stmt_busy", referenced from:
61:46.74       __ZN8rusqlite16inner_connection15InnerConnection7is_busy17h8273df19e73897cdE in libgkrust.a(rusqlite-bb2a2db7ebb1c68f.rusqlite.8kny5wtm-cgu.4.rcgu.o)
61:47.16   "_sqlite3_column_decltype", referenced from:
61:47.24       __ZN8rusqlite13raw_statement12RawStatement15column_decltype17h12a1fa4545f01149E in libgkrust.a(rusqlite-bb2a2db7ebb1c68f.rusqlite.8kny5wtm-cgu.12.rcgu.o)
61:48.26 ld: symbol(s) not found for architecture x86_64
61:48.46 clang-8: error: linker command failed with exit code 1 (use -v to see invocation)
61:48.47 make[4]: *** [XUL] Error 1
61:48.47 make[3]: *** [toolkit/library/build/target] Error 2
61:48.47 make[3]: *** Waiting for unfinished jobs....
62:03.98    Compiling osclientcerts-static v0.1.4 (/Users/thom/gecko/security/manager/ssl/osclientcerts)
62:09.37     Finished dev [optimized + debuginfo] target(s) in 57.87s
62:21.54    Compiling neqo-crypto v0.2.3 (https://github.com/mozilla/neqo?tag=v0.2.3#b10d8b3c)
62:33.29    Compiling neqo-transport v0.2.3 (https://github.com/mozilla/neqo?tag=v0.2.3#b10d8b3c)
62:36.50    Compiling neqo-qpack v0.2.3 (https://github.com/mozilla/neqo?tag=v0.2.3#b10d8b3c)
62:39.94    Compiling neqo-http3 v0.2.3 (https://github.com/mozilla/neqo?tag=v0.2.3#b10d8b3c)
62:43.94    Compiling http3server v0.1.1 (/Users/thom/gecko/netwerk/test/http3server)
62:46.61     Finished dev [optimized + debuginfo] target(s) in 1m 31s
63:00.29 make[2]: *** [compile] Error 2
63:00.29 make[1]: *** [default] Error 2
63:00.29 make: *** [build] Error 2
63:00.35 379 compiler warnings present.

Leaving the needinfo as I keep going to repeat rest of the builds to get error messages.

Sadly I wasn't able to quite figure out what I did to mozbuild to make it spit out a static lib -- FWIW I do believe it was the same error.

Some extra notes, as I can't stay late tonight either (sorry).

(In reply to Thom Chiovoloni [:tcsc] (please ni?) from comment #7)

Sadly I wasn't able to quite figure out what I did to mozbuild to make it spit out a static lib -- FWIW I do believe it was the same error.

Some extra notes, as I can't stay late tonight either (sorry).

I'm sure if you trigger the "plain" builds you'll see it happen on try too.

  • Release builds always work for me.

Probably because of dead-code elimination, which rusqlite, if you just add that, is entirely. But even if rusqlite was used, maybe the parts that use these functions would be still be unused...

  • Those four undefined symbols are not behind any build flags in sqlite, but we never use them in firefox (that said, I suspect there are plenty of pieces of rusqlite we never use in firefox...).

Actually, one of them is behind SQLITE_OMIT_DECLTYPE, which we define when building sqlite (see third_party/sqlite3/src/moz.build). None of them appear in third_party/sqlite3/src/sqlite.symbols, so they're not available when linking.

Yes, that works! Thanks a ton.

Flags: needinfo?(tchiovoloni)
Assignee: nobody → tchiovoloni
Status: NEW → ASSIGNED
Pushed by tchiovoloni@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/097f2bea1f1e Vendor rusqlite into mozilla-central. r=lina
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 77
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: