Fix wasm build

[tcharding]

Total re-write ... again :)

Currently we are defining the WASM integer size and alignments in the stdio.h header file, this is wrong because this file is included in the build by way of build.rs as well as by upstream libsecp256k1.

Move WASM integer definitions to a C source file and build the file into the binary if target is WASM.

Fixes the first part of #419 (#422 does the second part).

Note to reviewers

I'm not exactly sure why the directory was-sysroot is named as it is or if the name is significant to cargo , please review carefully the directory tree changes.

cd secp256k1-sys
tree wasm
wasm
├── wasm.c
└── wasm-sysroot
           ├── stdio.h
           ├── stdlib.h
           └── string.h

[TheBlueMatt]

What happens if we just drop all the WASM32_* consts in stdio.h? Otherwise we'll need to not define them, just declare the extern and add another .c file which actually defines them and only build it in when building for wasm.

[tcharding]

I'll digest that more thoroughly tomorrow and have a go. Cheers.

[cryptoquick]

Just wanted to leave a word of encouragement, @tcharding, the work you're doing here is very valuable.

[tcharding]

Thanks man! For the record, I absolutely love working on this project :)

[reuvenpo]

Hi! Passing by because I'm having compilation issues with v0.22.1 and clang v10, but letting you know that earlier versions (namely v0.20.3) only worked with clang v10 when targeting wasm.

[tcharding]

Noted! Thanks for the info @reuvenpo.

[thomaseizinger]

We can delete the entire file I guess?

[tcharding]

I left it because I don't know why these files are there, stdlib.h is empty as well.

[thomaseizinger]

AFAIK @TheBlueMatt added them when he worked on wasm compatibility. They are included in the build-script but serve otherwise no purpose.

This may be an instance of Chesterton’s fence but I would honestly just try and delete them and if nothing breaks, you are good to go :D

[tcharding]

I tried removing it and build fails, FTR here is a portion of the error:

cargo:warning=In file included from depend/secp256k1/src/field.h:25:
cargo:warning=depend/secp256k1/src/util.h:16:10: fatal error: 'stdio.h' file not found
cargo:warning=#include <stdio.h>
cargo:warning=         ^~~~~~~~~
cargo:warning=1 error generated.

[thomaseizinger]

Ah interesting. Yeah that makes sense!

[TheBlueMatt]

Yea, if nothing breaks these files (or any parts of them you can) should definitely be removed!

[apoelstra]

Lol, I am pretty sure that #include should not be present upstream. cc @real-or-random

[real-or-random]

I've created bitcoin-core/secp256k1#1095 to track this.

[elichai]

We added these align/size checks because wasm32 doesn't have a well-defined ABI.
Are we sure we want to remove those?
See #208

[apoelstra]

Agreed, let's not remove them. But they should be in their own include, not stdio.h, which is included only from secp256k1.c and not the other binaries.

[tcharding]

Total re-write, please see re-written PR description.

[thomaseizinger]

You should likely patch this by means of a .patch file that is automatically applied by vendor.sh otherwise this change will be gone with the next update of libsecp256k1.

[tcharding]

oh coool, that will fix my unease mentioned in the commit message about patching vendored code :) Thanks man.

[real-or-random]

In this case you can also append -include ../../../wasm-sysroot/wasm.h to the command line of the C compiler (https://docs.rs/cc/latest/cc/struct.Build.html#method.flag), this works on gcc and clang at least.

[elichai]

Should this maybe just be a c file that we compile and link to? that way we don't need to include it anywhere.
and we anyway just use it for the symbols

[tcharding]

Force push is total re-write, please see updated PR description.

[thomaseizinger]

Very clean, thanks!
Can't see any blockers in this.

ACK bfd88db

[apoelstra]

ACK bfd88db

Huzzah! My local CI script passes once again.

[apoelstra]

Thank you @elichai for explaining wtf was going on here.

[elichai]

post merge ACK bfd88db