-
Notifications
You must be signed in to change notification settings - Fork 35
Enable Windows Emscripten build #362
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
Enable Windows Emscripten build #362
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #362 +/- ##
=======================================
Coverage 81.78% 81.78%
=======================================
Files 20 20
Lines 950 950
Branches 87 87
=======================================
Hits 777 777
Misses 173 173 🚀 New features to boost your workflow:
|
Do we want to work in the windows ci here, too? |
@vgvassilev xeus-cpp uses Emscripten forges emsdk, which is only available for very particular platforms. When we asked previously for them to make emsdk available on Windows before the request was rejected. I could add a Windows Emscripten build here too, but we would need to get the upstream emsdk to achieve this. What would you like me to do? |
d31dfa4
to
5391dcd
Compare
5391dcd
to
8e910d3
Compare
I am not sure what's our path forward. We can merge that PR as it is if it helps with anything. However, it'd probably regress without any way of testing even small parts of this code for that platform.. |
@vgvassilev This platform would be tested from CppInterOp initially through the ci in this PR compiler-research/CppInterOp#647 . Once this PR is in I will add browser tests. I think its ok to work under the principle that if this workflow fails, then we implement a basic workflow in xeus-cpp to debug. Will that suffice? Without this PR going in, that PR in CppInterOp is dead in the water without any way to move forward. To be fair xeus-cpp already doesn't test on platforms that CppInterOp does for the Emscripten xeus-cpp, e.g. osx x86 and Ubuntu arm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Description
Please include a summary of changes, motivation and context for this PR.
This PR enable xeus-cpp to be cross compiled to Emscripten on a Windows platform (following Emscripten build instructions in CppInterOp). The 2 changes are as follows
To see these changing allow an Emscripten build of xeus-cpp to work and pass all Emscripten tests, see this PR in CppInterOp compiler-research/CppInterOp#647
Fixes # (issue)
Type of change
Please tick all options which are relevant.