Add wasm32-unknown-unknown support
#1010
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1010 +/- ##
==========================================
- Coverage 81.31% 80.62% -0.69%
==========================================
Files 51 52 +1
Lines 10402 10493 +91
==========================================
+ Hits 8458 8460 +2
- Misses 1944 2033 +89 ☔ View full report in Codecov by Sentry. |
|
I reviewed https://sqlite.org/capi3ref.html, and did not see any instances of structs being passed or returned by value. @thomcc Does this PR look roughly acceptable, if I sort out the time and randomness-related bits? |
|
Give me some time to review it (probably sometime over the weekend), but thanks for the work. |
There was a problem hiding this comment.
Great effort, hope it makes it into rusqlite soon!
Reading up on https://github.com/jlongster/absurd-sql, I wonder how feasible it would be to implement a pluggable persistence backend like jslongster did in sql-js/sql.js#481 for rusqlite.
Have you seen that? What are your thoughts on it?
|
@wngr Thanks! It does seem plausible to make a VFS that uses IndexedDB as a backend; that would be very cool. I don't expect to personally have time to work on it in the near future, but would love to see it happen. |
|
A concern from the previous PR:
I believe what's happening (in the few cases of this that still exist) is that the amalgamation is relying on certain declarations existing, but never actually uses them in the ultimately-generated code. If they were used, the WASM file would fail to load with its cryptic version of an "undefined symbol" error, see for example rustwasm/wasm-bindgen#2570 This is essentially how I decided which libc files to include; provide the minimal declarations that allowed compilation, and then track down the symbols that needed to be added that would allow the WASM to load. Edit: Also, it seems clang provides builtins for memcpy & friends. |
trevyn
force-pushed
the
wasm32-unknown-unknown
branch
from
24b1454
to
94d45d3
Compare
|
@thomcc Ready for review.
|
trevyn
force-pushed
the
wasm32-unknown-unknown
branch
from
94d45d3
to
004cf26
Compare
|
@thomcc Any chance you could review this PR? wasm32-unknown-unknown support would be awesome 😄 |
|
I tried using this and ran into the following (click to expand)
|
|
For what it's worth, this is what happens if I don't set
|
|
@paulyoung For an Anyway, this still works for me with: rusqlite = {git = "https://github.com/trevyn/rusqlite", branch = "wasm32-unknown-unknown", features = ["bundled"]}Does this work for you? Are you able to link to the project that you're actually building? |
|
@trevyn I manage my projects and their dependencies using nix (not homebrew) so it's probably a matter of figuring out what versions are needed and making sure they're being used. I could create a minimal example project that exhibits this issue if it would help. |
|
I created a standalone project here: https://github.com/paulyoung/rusqlite-wasm32-unknown-unknown-nix and asked about it on the Nix forums here: https://discourse.nixos.org/t/help-building-rusqlite-for-wasm32-unknown-unknown-with-crane/21724 |
|
Fancy! In that case, |
|
hey, is this waiting for something? can it be merged? |
|
This built fine on my m1 mac, then it failed on docker: Would love to see this merged into master 🎉 |
Co-authored-by: Andelf <andelf@gmail.com>
|
If merged this would be a pretty big win for the Rust ecosystem imo |
|
Absolutely massive. Especially since sled is dead. |
|
Thanks for testing it. Didn't realize they are just performing run-time checks. I did some research and have discovered that the wasi-sdk project releases libclang_rt as a standalone download here: https://github.com/WebAssembly/wasi-sdk/releases/tag/wasi-sdk-21 Upon inspecting that file, it's an ar archive containing a bunch of wasm objects that appear mostly ready to link. Out of the f128 functions I identified above as missing, this archive contains implementations of every function apart from It doesn't seem trivial to link that archive in during the build though in a way that correctly links all of those functions. But it's perhaps one possible way to move forward. |
|
Oh, just noticed that is the same file as https://github.com/jedisct1/libclang_rt.builtins-wasm32.a which you mentioned in #1010 (comment)... Apologies. |
|
Ok after more testing, I can confirm that the following diff resolves all of the missing imports: diff --git a/libsqlite3-sys/build.rs b/libsqlite3-sys/build.rs
index a6b99c3..55a7488 100644
--- a/libsqlite3-sys/build.rs
+++ b/libsqlite3-sys/build.rs
@@ -291,6 +291,12 @@ mod build_bundled {
cfg.file("sqlite3/wasm32-unknown-unknown/libc/string/strncmp.c");
cfg.file("sqlite3/wasm32-unknown-unknown/libc/string/strrchr.c");
cfg.file("sqlite3/wasm32-unknown-unknown/libc/string/strspn.c");
+
+ cfg.file("/home/carl/dev/llvm-project/compiler-rt/lib/builtins/extenddftf2.c");
+ cfg.file("/home/carl/dev/llvm-project/compiler-rt/lib/builtins/comparetf2.c");
+ cfg.file("/home/carl/dev/llvm-project/compiler-rt/lib/builtins/multf3.c");
+ cfg.file("/home/carl/dev/llvm-project/compiler-rt/lib/builtins/trunctfdf2.c");
}
if cfg!(feature = "unlock_notify") {
cfg.flag("-DSQLITE_ENABLE_UNLOCK_NOTIFY"); |
bring in minimum compiler-rt dependencies
|
Nice! |
This reverts commit 7c6cd3b.
|
@trevyn I am trying to get this branch working with I don't want to hijack your pull request discussion for a support request, I was just hoping there was somewhere you could point me that would be a good place to discuss this. Thanks! (And thanks for this work, this is very exciting -- I'm hoping to use |
|
I suggest moving that line of questioning to wasm-pack land. Here is a similar issue: rustwasm/wasm-pack#1328 You'll need to figure out how to convince wasm-pack + cargo + rustc to recompile everything with the atomics and bulk-memory features in order to support shared memory. I did a bunch of investigation into this recently and it's a huge PITA (for now). I also suggest taking a look at the new wasm32-wasi-preview1-threads target which may make this easier (or harder): https://doc.rust-lang.org/rustc/platform-support/wasm32-wasi-preview1-threads.html |
|
@carlsverre After investigating a bit, this seems to be distinct from the wasm-pack issue -- it's not about rust code, but specifically about sqlite3.o, which is produced by libsqlite3-sys. The issue seems to be that the target feature flags set for rust are not being forwarded to clang when building sqlite3.c. I proposed a fix to @trevyn in a pull request to his fork's pull request branch (😅), not sure if that's the best approach. |
This allows code that e.g. relies on `-Ctarget-feature=+atomics,+bulk-memory` to build libsqlite3-sys.
|
FWIW, the list of supported features was pulled from https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/target_features.rs#L312 -- I'm not sure if there's a clean way to depend on |
I'm seeing this same issue as well on the latest commit of this branch. Upon some investigation, it seems like I run into this if I am executing statements via |
|
Sorry I'm not deep enough into understanding sqlite3 to propose a fix, but I have found out that this happens when journaling is enabled. When a transaction is started a |
All credit to @passchaos in #935 for the insight that SQLite only uses a very small subset of libc, that can easily be copied in-tree.
Significant changes from that PR:
.cfiles from OpenBSDlibsqlite3-sys/sqlite3/sqlite3.ckept pristinememvfs, replaced by a thin VFS that liberally returnsSQLITE_IOERR. This target is intended to be used only withConnection::open_in_memory()or equivalent.wasm-pack.The allocator'sfree()method is still a little weird, but it is documented that SQLite's default allocator stores the allocation length in the first 8 bytes: https://sqlite.org/malloc.html#the_default_memory_allocator . If we want to wrap it again, I'm happy to do that, just let me know.To do:
Closes #488, closes #827, closes #935