Reduced dependencies from confi-table and enabled wasm on io_print feature.
#276
Conversation
Codecov Report
@@ Coverage Diff @@
## main #276 +/- ##
=======================================
Coverage 77.14% 77.15%
=======================================
Files 254 254
Lines 20648 20648
=======================================
+ Hits 15929 15931 +2
+ Misses 4719 4717 -2
Continue to review full report at Codecov.
|
| @@ -271,7 +271,7 @@ jobs: | |||
| export CARGO_HOME="/github/home/.cargo" | |||
| export CARGO_TARGET_DIR="/github/home/target" | |||
| # no need | |||
| cargo build --no-default-features --features=merge_sort,io_ipc,io_csv,io_json,io_parquet --target wasm32-unknown-unknown | |||
| cargo build --no-default-features --features=merge_sort,io_ipc,io_csv,io_print,io_json,io_parquet --target wasm32-unknown-unknown | |||
There was a problem hiding this comment.
I doubt wasm32-unknown-unknown will work. Only wasm32-wasi works with stdio.
Edit: I stand corrected it seems to pass CI, no idea how but I'll check it later.
There was a problem hiding this comment.
It appears like the approach in rust is to throw unsupported operation errors. See /~https://github.com/rust-lang/rust/tree/master/library/std/src/sys.
So it compiles but any code path that actually uses it will fail unless the user does some extra low level work (unlike in wasm32-wasi where it's actually implemented out of the box).
My personal opinion it that it's perfectly fine to just ensure that it compiles and users would need to ensure that their specific usage is using only valid execution paths. It's just that having this specific feature in the CI targeting wasm32-unknown-unknown might mislead users. That's minor though.
There was a problem hiding this comment.
Thanks a lot for the investigation!
What is the specific path that would fail? the usage of println!?
There was a problem hiding this comment.
So I wrote a little sample and it all works perfectly fine:
- The
arrow2::io::print::writefunction works as expected - The
arrow2::io::print::printfunction doesn't print anything inwasm32-unknown-unknownbut it doesn't fail nor panics.
We're good here. Thanks for the opportunity to learn something new ;)
There was a problem hiding this comment.
Thank you. I really do not understand wasm, so every opportunity for a technical discussion about it is awesome :)
maybe we should deprecate arrow2::io::print::print, since it is basically just println!("{}", write(...)?), anyways.
jorgecarleitao
changed the title
wasm on io_print feature.
Hat tip to @roee88 for the ping :)