Add wasm32-unknown-emscripten support#43
Conversation
lovasoa
left a comment
lovasoa
left a comment
There was a problem hiding this comment.
Thank you for the fix ! I added a few comments below
.github/workflows/rust.yml
Outdated
There was a problem hiding this comment.
Can we ensure emsdk is properly cached ? Otherwise it'll slow down ci. Maybe use https://github.com/mymindstorm/setup-emsdk ?
There was a problem hiding this comment.
fixed. i also pins emsdk version now
There was a problem hiding this comment.
mymindstorm is abandoned. mymindstorm/setup-emsdk#48 i am using fork pyodide/setup-emsdk@v15
build.rs
Outdated
There was a problem hiding this comment.
This isn't specific to emscripten is it ?
There was a problem hiding this comment.
91-92: issue opened in cc crate.
93-94: moved out from if statement
build.rs
Outdated
There was a problem hiding this comment.
Why do we need to re-detect emscripten here ? Isn't cargo supposed to set the right sysroot automatically ?
There was a problem hiding this comment.
after some digging, this issue still opens cross builds where bindgen picks up host headers instead of target headers
a few related PRs in this issue adds BINDGEN_EXTRA_CLANG_ARGS, BINDGEN_EXTRA_CLANG_ARGS_<TARGET> and ParseCallbacks::read_env_var.
afaik, BINDGEN_EXTRA_CLANG_ARGS_<TARGET> seems for user-side overriding, so I guess current code makes more sense
|
CI in my fork is green: https://github.com/sssxks/highs-sys/actions/runs/22031807613 |
For emscripten targets, parse headers as HOST in bindgen and disable layout tests.\n\nThis avoids missing Highs_* function declarations in generated bindings, while keeping the existing CMake exception flags fix.
2 participants
closes #42