-
Notifications
You must be signed in to change notification settings - Fork 315
feat(wasm): add feature support for indexedb and sqlite to matrix-sdk-ffi #5245
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
1ea1398
bd2de9c
6c5cfeb
a148409
629b610
5dc20db
4cde428
15ea884
fe98e31
ebebe34
fc92feb
4543b1b
69538a5
7987cc2
1d844de
ce33b96
90048a8
5c93af9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ Given the number of platforms targeted, we have broken out a number of features | |
### Functionality | ||
- `sentry`: Enable error monitoring using Sentry, not supports on Wasm platforms. | ||
- `bundled-sqlite`: Use an embedded version of sqlite instead of the system provided one. | ||
- `sqlite`: Use sqlite for session storage, not available on Wasm platforms. | ||
- `indexeddb`: Use IndexedDb for session storage, only available on Wasm platforms. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same suggestions as in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know what this means There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These sentences are copied-pasted from the |
||
|
||
### Unstable specs | ||
- `unstable-msc4274`: Adds support for gallery message types, which contain multiple media elements. | ||
|
@@ -22,7 +24,7 @@ Each supported target should use features to select the relevant TLS system. He | |
|
||
- Android: `"bundled-sqlite,unstable-msc4274,rustls-tls,sentry"` | ||
- iOS: `"bundled-sqlite,unstable-msc4274,native-tls,sentry"` | ||
- Javascript/Wasm: `"unstable-msc4274,native-tls"` | ||
- Javascript/Wasm: `"indexeddb,unstable-msc4274,native-tls"` | ||
|
||
### Swift/iOS sync | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,7 @@ use std::{ | |||||||||||||||||
env, | ||||||||||||||||||
error::Error, | ||||||||||||||||||
path::{Path, PathBuf}, | ||||||||||||||||||
process::Command, | ||||||||||||||||||
process::{self, Command}, | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
use vergen::EmitBuilder; | ||||||||||||||||||
|
@@ -56,7 +56,31 @@ fn get_clang_major_version(clang_path: &Path) -> String { | |||||||||||||||||
clang_version.split('.').next().expect("could not parse clang output").to_owned() | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
fn env_is_set(var_name: &str) -> bool { | ||||||||||||||||||
env::var_os(var_name).is_some() | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
fn ensure(cond: bool, err: &str) { | ||||||||||||||||||
if !cond { | ||||||||||||||||||
eprintln!( | ||||||||||||||||||
"\n\ | ||||||||||||||||||
┏━━━━━━━━{pad}━┓\n\ | ||||||||||||||||||
┃ error: {err} ┃\n\ | ||||||||||||||||||
┗━━━━━━━━{pad}━┛\n\ | ||||||||||||||||||
", | ||||||||||||||||||
pad = "━".repeat(err.len()), | ||||||||||||||||||
); | ||||||||||||||||||
process::exit(1); | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
fn main() -> Result<(), Box<dyn Error>> { | ||||||||||||||||||
let sqlite_set = env_is_set("CARGO_FEATURE_SQLITE"); | ||||||||||||||||||
let indexeddb_set = env_is_set("CARGO_FEATURE_INDEXEDDB"); | ||||||||||||||||||
ensure( | ||||||||||||||||||
sqlite_set || indexeddb_set, | ||||||||||||||||||
"one of the features 'sqlite' or 'indexeddb' must be enabled", | ||||||||||||||||||
); | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You must also ensure that they are not both activated at the same time, it's a XOR:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think they are necessarily exclusive. At the moment they are because of platform limitations, but the code to enable IndexeDB + SQLite at the same time would work perfectly fine someday when there is a bundled sqlite solution on web (not an impossibility). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I doubt the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the compilation layer, you can definitely have two stores enabled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have updated this as you requested, although I do still disagree with your reasoning. The builder system written here is explicitly designed to choose between the two, nothing is mutually exclusive about the feature as written. The fact that indexedb and sqlite have platform restrictions is an orthogonal issue entirely, and doesn't seem worth guarding against at the feature level. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the compilation layer, you can have IDB and SQLite if SQLite can be compiled to Wasm. Right now: $ cargo build --target wasm32-unknown-unknown --package matrix-sdk-sqlite does not compile. I prefer to be safe and to prevent someone doing the wrong thing. If one day a user is asking for SQLite+Wasm support, I'll be happy to revisit this. For the moment, I prefer to provide a satisfying developer experience. |
||||||||||||||||||
setup_x86_64_android_workaround(); | ||||||||||||||||||
uniffi::generate_scaffolding("./src/api.udl").expect("Building the UDL file failed"); | ||||||||||||||||||
EmitBuilder::builder().git_sha(true).emit()?; | ||||||||||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.