-
Notifications
You must be signed in to change notification settings - Fork 305
secp256k1-sys: Add memmove to the wasm-sysroot header #818
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
Conversation
WASM build is currently failing because we don't include `memmove` in our custom WASM header.
|
What versions do you want to backport this to @apoelstra? |
|
Does this really fix it? Under what conditions does it fail? Can you reenable the CI job after this? I don't care to backport it. It worked for years without this change and at some point the wasm people broke their own tooling. I don't mind patching stuff going forward but if people want their builds to work with wasm they shouldn't be upgrading their tooling. |
|
Oh, I see, this is because we introduced But can we attempt to reenable CI with this change? |
|
I didn't check I just got it to build locally. I can take a look tomorrow though. |
|
Gonna ACK and merge this, since the OP confirmed that it fixes his issue, and I believe it was our fault (sorta) rather than wasm's. But it would be nice, separately, to get the CI job working again.. |
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.
ACK c74c086; successfully ran local tests
1800e73 CI: Re-enable WASM job (Tobin C. Harding) Pull request description: On top of #818, this was quicker than writing an issue. Maybe it will just work. ACKs for top commit: apoelstra: ACK 1800e73; successfully ran local tests Tree-SHA512: 29787ca03f5f3354d635d5cb24a033299385119fdc40d98dbe063aefd04319147c4b72ced5a5b5a411b513138b91cb55e4d4627bc17e1f7c202f3f485a0952e0
…the wasm-sysroot header
c74c086f4d27b0d939945971bb8d0f498b7cfdf0 secp256k1-sys: Add memmove to the wasm-sysroot header (Tobin C. Harding)
Pull request description:
WASM build is currently failing because we don't include `memmove` in our custom WASM header.
Close: #817
ACKs for top commit:
apoelstra:
ACK c74c086f4d27b0d939945971bb8d0f498b7cfdf0; successfully ran local tests
Tree-SHA512: 24249fc61337ae77c0d2928dc57bb72aebb6f0876b29fe5b2e865e06586e2c2841c921ecbdb13a35eae264b1575fe9140a0e2d667d00a6af4703450c952a9e93
…the wasm-sysroot header
c74c086f4d27b0d939945971bb8d0f498b7cfdf0 secp256k1-sys: Add memmove to the wasm-sysroot header (Tobin C. Harding)
Pull request description:
WASM build is currently failing because we don't include `memmove` in our custom WASM header.
Close: #817
ACKs for top commit:
apoelstra:
ACK c74c086f4d27b0d939945971bb8d0f498b7cfdf0; successfully ran local tests
Tree-SHA512: 24249fc61337ae77c0d2928dc57bb72aebb6f0876b29fe5b2e865e06586e2c2841c921ecbdb13a35eae264b1575fe9140a0e2d667d00a6af4703450c952a9e93
WASM build is currently failing because we don't include
memmovein our custom WASM header.Close: #817