tukaani-project / xz

XZ Utils
https://tukaani.org/xz/
Other
623 stars 115 forks source link

Support build target `wasm32-unknown-unknown` in clang when `ENABLE_THREADS=OFF` #57

Closed ChanTsune closed 1 year ago

ChanTsune commented 1 year ago

Thanks for the review #56 yesterday!

I tried another approach, could you please review this one?

Pull request checklist

Please check if your PR fulfills the following requirements:

Pull request type

Please check the type of change your PR introduces: - [ ] Bugfix - [ ] Feature - [ ] Code style update (formatting, renaming, typo fix) - [x] Refactoring (no functional changes, no api changes) - [x] Build related changes - [ ] Documentation content changes - [ ] Other (please describe): ## What is the current behavior?
$ mkdir build && cd build
$ cmake .. -DENABLE_THREADS=OFF
$ make liblzma
[  0%] Building C object CMakeFiles/liblzma.dir/src/common/tuklib_physmem.c.obj                                       
[  1%] Building C object CMakeFiles/liblzma.dir/src/liblzma/check/check.c.obj                                         
In file included from /xz/src/liblzma/check/check.c:13:                                                               
In file included from /xz/src/liblzma/check/check.h:16:                                                               
In file included from /xz/src/liblzma/common/common.h:17:
In file included from /xz/src/common/mythread.h:84:
/wasi-sysroot/include/signal.h:2:2: error: "wasm lacks signal support; to enable minimal signal emulation, compile with -D_WASI_EMULATED_SIGNAL and link with -lwasi-emulated-signal"
#error "wasm lacks signal support; to enable minimal signal emulation, \
 ^
In file included from /xz/src/liblzma/check/check.c:13:
In file included from /xz/src/liblzma/check/check.h:16:
In file included from /xz/src/liblzma/common/common.h:17:
/xz/src/common/mythread.h:87:33: error: unknown type name 'sigset_t'
mythread_sigmask(int how, const sigset_t *restrict set,
                                ^
/xz/src/common/mythread.h:88:3: error: unknown type name 'sigset_t'
                sigset_t *restrict oset)
                ^
/xz/src/common/mythread.h:90:12: error: call to undeclared function 'sigprocmask'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        int ret = sigprocmask(how, set, oset);
                  ^
4 errors generated.
make[3]: *** [CMakeFiles/liblzma.dir/build.make:90: CMakeFiles/liblzma.dir/src/liblzma/check/check.c.obj] Error 1
make[2]: *** [CMakeFiles/Makefile2:139: CMakeFiles/liblzma.dir/all] Error 2
make[1]: *** [CMakeFiles/Makefile2:146: CMakeFiles/liblzma.dir/rule] Error 2
make: *** [Makefile:179: liblzma] Error 2

Related Issue URL:

56

What is the new behavior?

Does this introduce a breaking change?

Other information

Build tested on docker

Dockerfile

FROM ghcr.io/webassembly/wasi-sdk:latest

RUN apt update && apt install -y git

RUN git clone https://github.com/ChanTsune/xz.git

RUN mkdir -p ./xz/build

RUN cd xz && git fetch && git switch feature/liblzma/wasm

WORKDIR /xz/build

RUN cmake .. -DENABLE_THREADS=OFF

RUN CFLAGS="-target wasm32-unknown-unknown" make liblzma
$ docker build -t xz .

If you need, you can see the predefined macros when targeting wasm32 in clang by following commands

$ clang -E -dM -target wasm32-unknown-unknown -x c /dev/null

or if you want to check with the latest wasi-sdk clang

$ docker image pull ghcr.io/webassembly/wasi-sdk
$ docker run --rm -i ghcr.io/webassembly/wasi-sdk clang-16 -E -dM -target wasm32-unknown-unknown -x c /dev/null
JiaT75 commented 1 year ago

Hi! Thanks again for the very detailed PR. I looked more into wasm signal support since at first I thought it was some sort of bug that the signal emulation did not define sigset_t or sigprocmask(). This seems intentional however so your PR is certainly needed for a successful port to web assembly.

What you have so far seems like it is enough for liblzma to build, but we also should support an xz port. This should be easy to add just by following the example of VMS in src/xz/signal.*. Since the only functions in xz that use mythread_sigmask() it should be enough to define signals_block() and signals_unblock() as no-ops in signal.h (and remove implementation from signal.c).

Let me know if you have questions or if something else is preventing us from building xz with wasi-sdk

JiaT75 commented 1 year ago

On second thought, will be more complicated that I initially thought since signals_init() needs to be disabled too. I will merge what you have once I make a fix for the xz side. Thanks for you contributions!

ChanTsune commented 1 year ago

Thank you for your kind review!

I am honored to contribute to your project!

JiaT75 commented 1 year ago

@ChanTsune We would like to add you to our THANKS file in the repository but were not sure what your name was. If you are interested in being credited in the THANKS file please let us know the name you would like credited since we could not find it on your GitHub profile.

Thanks again for your contribution!

ChanTsune commented 1 year ago

Thank you! I updated my profile name. I would appreciate it if you could add this name to the THANKS file!