liblzma: Support wasi-sdk with `ENABLE_THREADS=OFF` option

ChanTsune commented 1 year ago

Hi! tukaani-project members!

In the process of my personal project, I made it possible to compile liblzma with WebAssembly as the target, so please use it if you like.

Pull request checklist

Please check if your PR fulfills the following requirements:

[x] Tests for the changes have been added (for bug fixes / features)
[x] Docs have been reviewed and added / updated if needed (for bug fixes / features)
[x] Build was run locally and without warnings or errors
[x] All previous and new tests pass

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?

Build failed with rust+wasi-sdk.

"clang" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "--target=wasm32-wasi" "--sysroot" "/wasi-sdk-20.0/share/wasi-sysroot" "-D_WASI_EMULATED_SIGNAL" "-I" "xz-5.2/src/liblzma/api" "-I" "xz-5.2/src/liblzma/lzma" "-I" "xz-5.2/src/liblzma/lz" "-I" "xz-5.2/src/liblzma/check" "-I" "xz-5.2/src/liblzma/simple" "-I" "xz-5.2/src/liblzma/delta" "-I" "xz-5.2/src/liblzma/common" "-I" "xz-5.2/src/liblzma/rangecoder" "-I" "xz-5.2/src/common" "-I" "/xz2-rs/lzma-sys" "-std=c99" "-pthread" "-DHAVE_CONFIG_H=1" "-o" "/xz2-rs/target/wasm32-wasi/debug/build/lzma-sys-7bbeecf3b4119da3/out/xz-5.2/src/liblzma/check/check.o" "-c" "xz-5.2/src/liblzma/check/check.c"
  cargo:warning=In file included from xz-5.2/src/liblzma/check/check.c:13:
  cargo:warning=In file included from xz-5.2/src/liblzma/check/check.h:16:
  cargo:warning=In file included from xz-5.2/src/liblzma/common/common.h:17:
  cargo:warning=xz-5.2/src/common/mythread.h:146:12: warning: call to undeclared function 'pthread_sigmask'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  cargo:warning=        int ret = pthread_sigmask(how, set, oset);
  cargo:warning=                  ^
  cargo:warning=xz-5.2/src/common/mythread.h:160:2: warning: call to undeclared function 'sigfillset'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  cargo:warning=        sigfillset(&all);
  cargo:warning=        ^
  cargo:warning=xz-5.2/src/common/mythread.h:162:19: error: use of undeclared identifier 'SIG_SETMASK'
  cargo:warning=        mythread_sigmask(SIG_SETMASK, &all, &old);
  cargo:warning=                         ^
  cargo:warning=xz-5.2/src/common/mythread.h:164:19: error: use of undeclared identifier 'SIG_SETMASK'
  cargo:warning=        mythread_sigmask(SIG_SETMASK, &old, NULL);
  cargo:warning=                         ^
  cargo:warning=2 warnings and 2 errors generated.
  exit status: 1

What is the new behavior?

Optimize to include mythread.h only where necessary
Build liblzma with ENABLE_THREADS=OFF now passes with the latest wasi-sdk

Does this introduce a breaking change?

[ ] Yes
[x] No

Other information

JiaT75 commented 1 year ago

Hi! Thanks for the PR. Unfortunatly, I do not think this PR solves a problem. I am guessing the issue is with your build setup instead. If you look at mythread.h, the functions referenced in your error message are in the #elif defined(MYTHREAD_POSIX) block and should be removed by the preprocessor.

If you are using our CMakeLists.txt, then setting ENABLE_THREADS=OFF will ensure MYTHREAD_POSIX is never added to the compile definitions. Its possible a make clean or removing the CMakeCache.txt could solve your problem.

Compiling liblzma with WebAssembly sounds like a great project though!

ChanTsune commented 1 year ago

Thanks for review!

Sorry. The error message I pasted was the wrong one

This is correct

  running: "clang" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "--target=wasm32-wasi" "--sysroot" "/wasi-sdk-20.0/share/wasi-sysroot" "-D_WASI_EMULATED_SIGNAL" "-I" "xz-5.2/src/liblzma/api" "-I" "xz-5.2/src/liblzma/lzma" "-I" "xz-5.2/src/liblzma/lz" "-I" "xz-5.2/src/liblzma/check" "-I" "xz-5.2/src/liblzma/simple" "-I" "xz-5.2/src/liblzma/delta" "-I" "xz-5.2/src/liblzma/common" "-I" "xz-5.2/src/liblzma/rangecoder" "-I" "xz-5.2/src/common" "-I" "/xz2-rs/lzma-sys" "-std=c99" "-pthread" "-DHAVE_CONFIG_H=1" "-o" "/xz2-rs/target/wasm32-wasi/debug/build/lzma-sys-7bbeecf3b4119da3/out/xz-5.2/src/liblzma/check/check.o" "-c" "xz-5.2/src/liblzma/check/check.c"
  cargo:warning=In file included from xz-5.2/src/liblzma/check/check.c:13:
  cargo:warning=In file included from xz-5.2/src/liblzma/check/check.h:16:
  cargo:warning=In file included from xz-5.2/src/liblzma/common/common.h:17:
  cargo:warning=xz-5.2/src/common/mythread.h:87:33: error: unknown type name 'sigset_t'
  cargo:warning=mythread_sigmask(int how, const sigset_t *restrict set,
  cargo:warning=                                ^
  cargo:warning=xz-5.2/src/common/mythread.h:88:3: error: unknown type name 'sigset_t'
  cargo:warning=                sigset_t *restrict oset)
  cargo:warning=                ^
  cargo:warning=xz-5.2/src/common/mythread.h:90:12: warning: call to undeclared function 'sigprocmask'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  cargo:warning=        int ret = sigprocmask(how, set, oset);
  cargo:warning=                  ^
  cargo:warning=1 warning and 2 errors generated.
  exit status: 1

JiaT75 commented 1 year ago

Thanks for the updated error message. Can I ask more about your build environment? The <signal.h> header file should provide definitions for sigset_t and sigprocmask() in a POSIX compliant system and the preprocessor should filter out Windows builds that do not set the __CYGWIN__ macro.

The reason I want to avoid removing #include "mythread.h" from common.h is

Avoid breaking something unexpectedly
It allows referencing MYTHREAD_ENABLED just by including common.h. Our common.h header files includes config.h (when building with autotools) and that contains all of the configurations. So it makes the liblzma files simpler since they only need to include common.h to get all configurations.

Usually only including the header files you actually need is a good idea. In this case, there are few other files that would need to include mythread.h for it to work. But I would like to avoid this if possible.

ChanTsune commented 1 year ago

I am using the latest WASI-SDK. WASI is trying to provide a project that provides a POSIX compatible API as WASIX but it seems it's not perfect yet.

There is no deep meaning in using it via Rust, so if you can use Docker, I think you can reproduce the equivalent environment with the following Dockerfile

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

RUN apt update && apt install -y git

RUN git clone https://github.com/tukaani-project/xz.git

RUN ./xz/build-aux/ci_build.sh -b cmake -d threads,shared -p all

$ docker build -t xz .

WASI-SDK can download from https://github.com/WebAssembly/wasi-sdk/releases/tag/wasi-sdk-20

ChanTsune commented 1 year ago

It's a little redundant description, but how about adding the following changes

common.h

-#include "mythread.h"
+// If any type of threading is enabled, #include "mythread.h".
+#if defined(MYTHREAD_POSIX) || defined(MYTHREAD_WIN95) \
+       || defined(MYTHREAD_VISTA)
+#  include "mythread.h"
+#endif

I think this will solve 2.

tukaani-project / xz