Add wasm32-unknown-unknown target support for sqlite #4411

Spxg · 2024-12-29T13:40:31Z

I recently created a rust wrapper crate for sqlite-wasm and now diesel can run successfully on the wasm32-unknown-unknown platform.

Diesel’s integration tests and unit tests all run successfully，except for a few tests that required std::fs::* ~~and doc test (wasm-bindgen-test currently does not support this~~).
Branch: https://github.com/Spxg/diesel/tree/wasm32-test (There are many changes, not planning to merge them at the moment).

cargo install wasm-pack
cargo install wasm-bindgen-cli
git clone https://github.com/Spxg/diesel -b wasm32-test
cd diesel/diesel
cargo xtask run-tests --wasm
CHROMEDRIVER="your_chrome_driver_path" cargo +nightly test --doc --target wasm32-unknown-unknown --features sqlite -Zdoctest-xcompile

You will see output similar to this: https://raw.githubusercontent.com/Spxg/Spxg/refs/heads/master/resources/sqlitest.gif

insipx · 2025-01-02T18:00:48Z

nice, this seems to be cleaner than re-implementing the backend in a separate crate, and maybe better than storing them on function pointers like in the discussion?

How would SQLite updates work with this. Further, it would be most efficient to implement a VFS in rust rather than go to JS and back, would this wrapper crate allow for that from diesel?

Spxg · 2025-01-03T06:08:01Z

nice, this seems to be cleaner than re-implementing the backend in a separate crate, and maybe better than storing them on function pointers like in the discussion?

How would SQLite updates work with this. Further, it would be most efficient to implement a VFS in rust rather than go to JS and back, would this wrapper crate allow for that from diesel?

The current implementation of sqlite-wasm-rs is actually the same as what the "JS side" in the sqlite-wasm project does (using various wrappers to implement calls, including type conversion and wasm memory allocation).

It would be most efficient if a VFS could be implemented in rust and registered using sqlite3_vfs_register. It can be implemented directly in libsqlite3-sys, and ORM frameworks such as diesel and sqlx do not even need to modify the implementation.

However, I know very little about VFS used by the web, such as OPFS. The opfs used by sqlite-wasm is not intended to be exported as an API (https://github.com/sqlite/sqlite/blob/04364cb3cc108a044a0d9dc7162f4d550adb2f99/ext/wasm/api/sqlite3-vfs-opfs.c-pp.js#L54), but the wa-sqlite project has other VFS for reference (https://github.com/rhashimoto/wa-sqlite/tree/master/src/examples).

In short, if there is a VFS solution, I am happy to try it.

weiznich

Thanks for opening this PR. Overall I'm very much in favor of this approach as it looks really clean in terms of code changes. I've added two questions about some details.

That written: I personally would prefer to run at least the tests in CI to ensure that this does not break in the future.

As for the VFS discussion: I can imaging that diesel exposes the necessary traits to implement a custom VFS. A quick search on crates.io brought up https://docs.rs/sqlite-vfs/latest/sqlite_vfs/, which might serve as basis for such an implementation.

weiznich · 2025-01-03T09:41:34Z

diesel/src/lib.rs

+    // Before using C-API, you must initialize sqlite.
+    //
+    // Initializing the database is a one-time operation during
+    // the life of the program.
+    #[cfg(feature = "sqlite")]
+    #[cfg(all(target_family = "wasm", not(target_os = "wasi")))]
+    pub use sqlite_wasm_rs::init_sqlite;


I would rather not expose this function from diesel as this would couple diesel's API stability to that one of sqlite_wasm_rs. Ideally we would just call that function internally one, or even better sqlite_wasm_rs would provide a singelton helper function to do this.

We do something similar to this for the mysql backend already:

diesel/diesel/src/mysql/connection/raw.rs

Lines 251 to 277 in 3f69aa4

/// > In a non-multi-threaded environment, `mysql_init()` invokes

/// > `mysql_library_init()` automatically as necessary. However,

/// > `mysql_library_init()` is not thread-safe in a multi-threaded environment,

/// > and thus neither is `mysql_init()`. Before calling `mysql_init()`, either

/// > call `mysql_library_init()` prior to spawning any threads, or use a mutex

/// > to protect the `mysql_library_init()` call. This should be done prior to

/// > any other client library call.

///

/// <https://dev.mysql.com/doc/c-api/8.4/en/mysql-init.html>

static MYSQL_THREAD_UNSAFE_INIT: Once = Once::new();

fn perform_thread_unsafe_library_initialization() {

MYSQL_THREAD_UNSAFE_INIT.call_once(|| {

// mysql_library_init is defined by `#define mysql_library_init mysql_server_init`

// which isn't picked up by bindgen

let error_code = unsafe { ffi::mysql_server_init(0, ptr::null_mut(), ptr::null_mut()) };

if error_code != 0 {

// FIXME: This is documented as Nonzero if an error occurred.

// Presumably the value has some sort of meaning that we should

// reflect in this message. We are going to panic instead of return

// an error here, since the documentation does not indicate whether

// it is safe to call this function twice if the first call failed,

// so I will assume it is not.

panic!("Unable to perform MySQL global initialization");

}

})

}

Ideally we would just call that function internally one, or even better sqlite_wasm_rs would provide a singelton helper function to do this.

Yes, the best way is to implement a singleton function call internally. But unlike mysql_server_init, init_sqlite is asynchronous, and since the diesel function is called synchronously, there is no opportunity to execute init_sqlite.
Any good suggestions for this?

if this is exclusively for the web, you can make some assumptions about the environment and spawn the init future onto the web executor rather than immediately wait for it. I think this makes sense, and maybe you can even rename init_sqlite to init_sqlite_web_lazy. The function should return a handle to the spawned future to allow awaiting it to completion.

Diesel can store this future wherever it likes, like in another OnceCell. Then maybe add a new async fn to diesel to finish waiting for the spawned future handle. This should decouple the api and let the user wait for sqlite init.

Ideally we could avoid offloading the init to the user, but since diesel is pretty much 100% synchronous not sure how that could work

https://rustwasm.github.io/wasm-bindgen/api/wasm_bindgen_futures/fn.spawn_local.html

happy to make a PR if this is something that would be acceptable!

spawn the init future onto the web executor rather than immediately wait for it.

Spawn init_sqlite means that init_sqlite may not be called before sqlite ffi is called and wasm_bindgen_future::spawn relies on event loop, execution needs to wait until the main thread is idle.
Anyway this may cause the call to sqlite ffi to fail.

As you said, since diesel is now fully synchronous, there is no way to initialize sqlite in diesel.
I think there are only three ways to do it, called by the user:

export sqlite_wasm_rs::init_sqlite

Wrapper an async init function in diesel and call sqlite_wasm_rs::init_sqlite

diesel does not provide any init_sqlite method, and the user add sqlite-wasm-rs dependency somewhere and executes sqlite_wasm_rs::init_sqlite. As long as there is only one version of sqlite-wasm-rs in a .wasm it is acceptable.

As written above: My main concern for just reexporting this function is the API stability of the sqlite_wasm_rs crate. I'm not sure who is in charge of maintaining it, so I'm also not sure how future updates are planed.

For diesel it would be problematic if some of the functions used by diesel change between versions. If that's not planed it's even fine to issue new major version releases of this crate, as diesel could just support a version range then. If one of the functions used by diesel changes between versions it is essentially impossible for diesel to support the new sqlite_wasm_rs version without breaking its (diesels) semver promise as this exported function is then part of the public API. The other problematic point is: Even if we don't reexport that function the same public dependency relation ship would exist due to being a essentially a -sys crate. After all you need to call this function for the right version of sqlite_wasm_rs, right?

The other problematic point is: Even if we don't reexport that function the same public dependency relation ship would exist due to being a essentially a -sys crate. After all you need to call this function for the right version of sqlite_wasm_rs, right?

Is this problematic only in the case of using sqlite_wasm_rs and diesel in the same project, or even if there is a mysql-like init?

FWIW its not impossible to make this sync, although it is more involved:
Looking through sqlite/sqlite-wasm code for sqlite3InitModule

1.) sqlite/sqlite-wasm offers a way to pass an already-instantiated wasm module to sqlite3InitModule here
2.) Initializing a WebAssembly module by-default is async, but it can be done syncronously, and we can do it from rust and pass it to sqlite/sqlite-wasm from rust, using the InitOpts object. This InitOpts object is the same exact way sqlite-web-rs controls the same params, so I could also try it out there and report back on how it effects things
3.) Create an init that mirrors the mysql_server_init

may require minor modifications/PR to sqlite/sqlite-wasm repo and sqlite3InitModule

Instantiating the WASM module sync should be fine in this case b/c using OPFS needs to be done in a dedicated worker anyway, so it won't block the main thread.

That should handle the async part of sqlite3InitModule, which I think is the bulk of it but maybe I'm missing a promise somewhere in that file.

As written above: My main concern for just reexporting this function is the API stability of the sqlite_wasm_rs crate. I'm not sure who is in charge of maintaining it, so I'm also not sure how future updates are planed.

sqlite-wasm-rs is maintained by me, and its purpose is to wrap sqlite-wasm and provide the same C-Like API as libsqlite3-sys.

About the C-Like API, since the SQLite C API is stable, there will be no breaking change in this C-Like API.

About init_sqlite, since sqlite.wasm must be initialized, init_sqlite is also stable. Initialization is now an asynchronous OnceCell. If there is a synchronous initialization method later, init_sqlite still works, and the initialization can be called internally (when calling the C-Like API).

So there will be no breaking change in the future. The only problem is that if diesel use a new SQLite C API when developing new features, but sqlite-wasm-rs has not yet been wrapped, it will cause compilation failure.

Fortunately, the cost of wrapping a new API is not high, and I can support it quickly.

diesel/Cargo.toml

Spxg · 2025-01-04T02:26:50Z

That written: I personally would prefer to run at least the tests in CI to ensure that this does not break in the future.

I agree with you, but in order to solve the differences between native and wasm tests, I created the #[test_derive::test] macro to do this. Is it acceptable to replace #[test] with #[test_derive::test] in future maintenance?

In addition, some changes are required to run rustdoc tests

insipx · 2025-01-04T21:38:59Z

It would be most efficient if a VFS could be implemented in rust and registered using sqlite3_vfs_register. It can be implemented directly in libsqlite3-sys, and ORM frameworks such as diesel and sqlx do not even need to modify the implementation.

However, I know very little about VFS used by the web, such as OPFS. The opfs used by sqlite-wasm is not intended to be exported as an API (https://github.com/sqlite/sqlite/blob/04364cb3cc108a044a0d9dc7162f4d550adb2f99/ext/wasm/api/sqlite3-vfs-opfs.c-pp.js#L54), but the wa-sqlite project has other VFS for reference (https://github.com/rhashimoto/wa-sqlite/tree/master/src/examples).

The VFS Implementation is only interesting for web because of the variety of ways to accomplish a VFS, the immaturity of OPFS vs IndexedDb, as well as the newness of SQLite Wasm build itself. Therefore (and maybe my opinion only) it is useful to expose methods for a flexible VFS implementation rather than forcing a specific implementation lower in the stack. At least, until standards and best practices are formed.

Off the top, certain VFS implementations may play better with multiple open tabs to the same db, for instance, while other may completely disallow that kind of behavior.

I'm satisfied with Weiznich's suggestion, though, didn't even know about that crate! An API to simply register the vfs is all that would be needed

weiznich · 2025-01-07T18:36:33Z

I agree with you, but in order to solve the differences between native and wasm tests, I created the #[test_derive::test] macro to do this. Is it acceptable to replace #[test] with #[test_derive::test] in future maintenance?

I would prefer to handle this via #[cfg_attr(target = "wasm", wasm_bindgen::test)] and #[cfg_attr(not(target = "wasm"), test)] attributes instead if possible.

In addition, some changes are required to run rustdoc tests

I think it would be fine to just run unit and integration tests and skip the doc tests on this target.

Spxg · 2025-01-10T06:42:51Z

I'm ready for review.

Regarding the API stability issue before, I wrapper an export module inside sqlite-wasm-rs and exposed it to diesel, which provides stable APIs.
td::test macro is created. If it is a wasm platform test, it is replaced with wasm_bindgen_test::test and if the feature is sqlite, the init_sqlite step will be added before the test.
OPFS is no longer provided by default, because the conditions for using opfs are relatively strict. Currently, MemVFS is the default. The type of vfs to use can be specified through the url, like file:xxxx.db?vfs=[opfs|opfs-sahpool|xxxx].
Currently, the test can be run through cargo xtask run-tests --wasm (welcome to experience, but you need to install wasm-pack first)

The test has been successfully run in CI, but some CIs failed unexpectedly. I am not sure if it was caused by my changes.

@weiznich @insipx

weiznich · 2025-01-10T06:46:05Z

The test has been successfully run in CI, but some CIs failed unexpectedly. I am not sure if it was caused by my changes.

The CI failures are unrelated. They are caused by the new rust release and some changes to the CI runners. I've filled #4428 to address that. You likely need to rebase this PR after that one is merged.

I will do another round of reviews later today.

weiznich

Thanks for the update. Overall I think we are nearly there. I've noted a few minor organizational things that shouldn't be too hard to fix.

I'm also not happy with the test attribute crate name, sorry about that but I would really like to see a better/longer name there.

Beside these specific comments I think it would be great to have:

A short documentation entry in the sqlite connection documentation that states wasm32 is supported, outlines that you need to call the init function + contains some details about the vfs choices
A changelog entry as this is a rather large new feature

.github/workflows/ci.yml

diesel_tests/td/Cargo.toml

Cargo.toml

diesel/Cargo.toml

diesel/src/lib.rs

xtask/src/tests/mod.rs

Spxg · 2025-01-10T10:29:33Z

I'm also not happy with the test attribute crate name, sorry about that but I would really like to see a better/longer name there.

I totally agree with using a longer and clearer test attribute crate name. I also think diesel_test_helper needs to be in the root of the workspace, and it's a bit strange to put it in the diesel_tests dir, since the diesel crate also uses it.

Thanks for taking the time to review, I will fix these comments tonight.

Spxg · 2025-01-10T15:40:07Z

All comments fixed, additional changes:

Added several sqlite-wasm vfs tests
Run tests in "Check sqlite wasm" CI directly specify sqlite backend
@weiznich

weiznich

Thanks for the update and thanks again for working on this ❤️

I think it's now ready to be merged. I've adjusted some documentation comments slightly to better fit the over all documentation format. I will just apply the suggestions for that and merge the PR afterwards, so no additional work from your side is required here.

diesel/src/lib.rs

xtask/src/tests/mod.rs

diesel/src/sqlite/connection/mod.rs

insipx · 2025-01-10T17:01:42Z

OPFS is no longer provided by default, because the conditions for using opfs are relatively strict. Currently, MemVFS is the default. The type of vfs to use can be specified through the url, like file:xxxx.db?vfs=[opfs|opfs-sahpool|xxxx]

Love it!

Thanks for your work on this, with the next diesel release it should make sqlite-web-rs redundant since everything is provided in mainline diesel, assuming the sqlite-wasm-rs is kept up to date !

Excited to start experimenting with different rust-native VFS's!

Spxg · 2025-01-10T17:23:18Z

Thanks for your suggestions on this PR and the sqlite-web-rs project ❤️ @weiznich @insipx

Spxg force-pushed the wasm32 branch from e98592a to a8e25ff Compare December 30, 2024 09:30

Spxg marked this pull request as draft January 2, 2025 02:25

weiznich reviewed Jan 3, 2025

View reviewed changes

Spxg force-pushed the wasm32 branch from 8815c18 to aeac2ee Compare January 7, 2025 02:10

Spxg force-pushed the wasm32 branch 4 times, most recently from 54027a0 to c47f627 Compare January 9, 2025 18:40

Spxg marked this pull request as ready for review January 9, 2025 18:49

Spxg marked this pull request as draft January 10, 2025 03:11

Spxg marked this pull request as ready for review January 10, 2025 06:42

Spxg added 11 commits January 10, 2025 17:54

Add wasm32-unknown-unknown target support

36b19f1

Bump sqlite-wasm-rs version to 0.1.1

e1217b9

Update the cfg recommended by rustc book

548911d

Update diesel_compile_tests deps

b186304

No vfs deafult

6ab95a9

Bump sqlite-wasm-rs version to 0.1.2

60bf8b0

Running tests for wasm32

f9b7640

Add sqlite-wasm CI/CD

4f23b76

Fix CI

f84f003

Bump sqlite-wasm-rs version to 0.1.3

33bfda5

Fix CI

4f724c6

Spxg force-pushed the wasm32 branch from 405d5af to 4f724c6 Compare January 10, 2025 09:54

weiznich reviewed Jan 10, 2025

View reviewed changes

Spxg added 8 commits January 10, 2025 22:10

Use diesel_test_helper instead of td

4b9c66b

Update wasm ci config

bfa7f9e

Bound sqlite-wasm-rs version range

0367798

Export minimal possible api from sqlite-wasm-rs

f59e187

Add helpful error message when running wasm tests

5268bb6

Add some comments and tests to SqliteConnection

8b23cb7

Update changelog

868d6f0

Specify sqlite backend wasm test

f38c4cf

Spxg changed the title ~~Add wasm32-unknown-unknown target support~~ Add wasm32-unknown-unknown target support for sqlite Jan 10, 2025

weiznich approved these changes Jan 10, 2025

View reviewed changes

diesel/src/lib.rs Show resolved Hide resolved

xtask/src/tests/mod.rs Outdated Show resolved Hide resolved

diesel/src/sqlite/connection/mod.rs Outdated Show resolved Hide resolved

weiznich enabled auto-merge January 10, 2025 16:33

Improve some documentation comments

a030a2e

weiznich force-pushed the wasm32 branch from 8e4483a to a030a2e Compare January 10, 2025 16:39

weiznich added this pull request to the merge queue Jan 10, 2025

Merged via the queue into diesel-rs:master with commit a24aad1 Jan 10, 2025
49 checks passed

Spxg deleted the wasm32 branch January 10, 2025 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add wasm32-unknown-unknown target support for sqlite #4411

Add wasm32-unknown-unknown target support for sqlite #4411

Spxg commented Dec 29, 2024 •

edited

Loading

insipx commented Jan 2, 2025 •

edited

Loading

Spxg commented Jan 3, 2025 •

edited

Loading

weiznich left a comment

weiznich Jan 3, 2025

Spxg Jan 4, 2025 •

edited

Loading

insipx Jan 6, 2025 •

edited

Loading

Spxg Jan 7, 2025 •

edited

Loading

weiznich Jan 7, 2025

insipx Jan 7, 2025 •

edited

Loading

Spxg Jan 8, 2025 •

edited

Loading

Spxg commented Jan 4, 2025

insipx commented Jan 4, 2025

weiznich commented Jan 7, 2025

Spxg commented Jan 10, 2025 •

edited

Loading

weiznich commented Jan 10, 2025

weiznich left a comment

Spxg commented Jan 10, 2025 •

edited

Loading

Spxg commented Jan 10, 2025

weiznich left a comment

insipx commented Jan 10, 2025

Spxg commented Jan 10, 2025

	/// > In a non-multi-threaded environment, `mysql_init()` invokes
	/// > `mysql_library_init()` automatically as necessary. However,
	/// > `mysql_library_init()` is not thread-safe in a multi-threaded environment,
	/// > and thus neither is `mysql_init()`. Before calling `mysql_init()`, either
	/// > call `mysql_library_init()` prior to spawning any threads, or use a mutex
	/// > to protect the `mysql_library_init()` call. This should be done prior to
	/// > any other client library call.
	///
	/// <https://dev.mysql.com/doc/c-api/8.4/en/mysql-init.html>
	static MYSQL_THREAD_UNSAFE_INIT: Once = Once::new();

	fn perform_thread_unsafe_library_initialization() {
	MYSQL_THREAD_UNSAFE_INIT.call_once(\|\| {
	// mysql_library_init is defined by `#define mysql_library_init mysql_server_init`
	// which isn't picked up by bindgen
	let error_code = unsafe { ffi::mysql_server_init(0, ptr::null_mut(), ptr::null_mut()) };
	if error_code != 0 {
	// FIXME: This is documented as Nonzero if an error occurred.
	// Presumably the value has some sort of meaning that we should
	// reflect in this message. We are going to panic instead of return
	// an error here, since the documentation does not indicate whether
	// it is safe to call this function twice if the first call failed,
	// so I will assume it is not.
	panic!("Unable to perform MySQL global initialization");
	}
	})
	}

Add wasm32-unknown-unknown target support for sqlite #4411

Add wasm32-unknown-unknown target support for sqlite #4411

Conversation

Spxg commented Dec 29, 2024 • edited Loading

insipx commented Jan 2, 2025 • edited Loading

Spxg commented Jan 3, 2025 • edited Loading

weiznich left a comment

Choose a reason for hiding this comment

weiznich Jan 3, 2025

Choose a reason for hiding this comment

Spxg Jan 4, 2025 • edited Loading

Choose a reason for hiding this comment

insipx Jan 6, 2025 • edited Loading

Choose a reason for hiding this comment

Spxg Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

weiznich Jan 7, 2025

Choose a reason for hiding this comment

insipx Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

Spxg Jan 8, 2025 • edited Loading

Choose a reason for hiding this comment

Spxg commented Jan 4, 2025

insipx commented Jan 4, 2025

weiznich commented Jan 7, 2025

Spxg commented Jan 10, 2025 • edited Loading

weiznich commented Jan 10, 2025

weiznich left a comment

Choose a reason for hiding this comment

Spxg commented Jan 10, 2025 • edited Loading

Spxg commented Jan 10, 2025

weiznich left a comment

Choose a reason for hiding this comment

insipx commented Jan 10, 2025

Spxg commented Jan 10, 2025

Spxg commented Dec 29, 2024 •

edited

Loading

insipx commented Jan 2, 2025 •

edited

Loading

Spxg commented Jan 3, 2025 •

edited

Loading

Spxg Jan 4, 2025 •

edited

Loading

insipx Jan 6, 2025 •

edited

Loading

Spxg Jan 7, 2025 •

edited

Loading

insipx Jan 7, 2025 •

edited

Loading

Spxg Jan 8, 2025 •

edited

Loading

Spxg commented Jan 10, 2025 •

edited

Loading

Spxg commented Jan 10, 2025 •

edited

Loading