nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.25k stars 320 forks source link

Unit is not compiled with php, njs and wasm-wasi-component #1320

Closed kvelaro closed 1 week ago

kvelaro commented 1 week ago

Steps to reproduce

/configure --prefix=/opt/unit --njs --openssl --cc-opt="-I../njs/src/ -I../njs/build/" --ld-opt="-L../njs/build/" --modules=/opt/unit/lib/unit/modules
./configure wasm-wasi-component
./configure php --config=/opt/php81/bin/php-config --lib-path=/opt/php81/lib/ --module=/opt/php81
make

Expected to see, something like: Good work, ready to install

Actual result:

   Compiling wasm-wasi-component v0.1.0 (/opt/unit-src/src/wasm-wasi-component)
error: failed to run custom build command for `wasm-wasi-component v0.1.0 (/opt/unit-src/src/wasm-wasi-component)`

Caused by:
  process didn't exit successfully: `/opt/unit-src/src/wasm-wasi-component/target/release/build/wasm-wasi-component-c0e33c6cbea2b957/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-changed=wrapper.h
  cargo:rerun-if-env-changed=TARGET
  cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS_aarch64-unknown-linux-gnu
  cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS_aarch64_unknown_linux_gnu
  cargo:rerun-if-env-changed=BINDGEN_EXTRA_CLANG_ARGS

  --- stderr
  ../nxt_js.h:11:10: fatal error: 'njs_main.h' file not found
  thread 'main' panicked at build.rs:23:10:
  Unable to generate bindings: ClangDiagnostic("../nxt_js.h:11:10: fatal error: 'njs_main.h' file not found\n")
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
make: *** [build/Makefile:1983: src/wasm-wasi-component/target/release/libwasm_wasi_component.so] Error 101

my comment : njs_main.h exist in /opt/njs/src

Environment: Ubuntu 22.04 under MacOs (Docker), Unit 1.32.1, php 8.1.15, njs: 0.8.2 Dockerfile (stripped):

# Variables: 
$WORKDIR="/opt"
$PHPPATH="/opt/php81"

# Njs (Nginx Unit module)
RUN <<EOR
  cd $WORKDIR
  git clone https://github.com/nginx/njs.git
  cd njs
  git checkout tags/$NJSVERSION
  ./configure --no-zlib --no-libxml2 && make
EOR

# Unit itself
RUN <<EOR
  cd $WORKDIR
  git clone https://github.com/nginx/unit.git unit-src
  cd unit-src
  git checkout tags/$UNITVERSION
  ./configure --prefix=$WORKDIR/unit --njs --openssl --cc-opt="-I../njs/src/ -I../njs/build/" --ld-opt="-L../njs/build/" --modules=$WORKDIR/unit/lib/unit/modules
  ./configure wasm-wasi-component
  ./configure php --config=$PHPPATH/bin/php-config --lib-path=$PHPPATH/lib/ --module=$PHPNAME
  make && make install clean
EOR

If I configure and compile only wasm-wasi-component (without njs and php) everything ends good If I configure and compile php + njs everything also goes well Wasm module breaks everything (but we need it in order to maintain SSI)

ac000 commented 1 week ago

It's the building with njs that breaks the wasm-wasi-component module, due to

src/wasm-wasi-component/wrapper.h containing nxt_main.h which includes nxt_tstr.h which includes nxt_js.h which when building with njs support includes njs_main.h

cargo/rust has an include path that includes -I../ -I../../build/include neither of which contains njs_main.h (njs_auto_config.h is also then required) which is included in the njs source...

As a workaround you could use this patch (to unit)

diff --git ./src/wasm-wasi-component/build.rs ./src/wasm-wasi-component/build.rs
index 5ea74f17..ce546856 100644
--- ./src/wasm-wasi-component/build.rs
+++ ./src/wasm-wasi-component/build.rs
@@ -8,6 +8,8 @@ fn main() {
     let bindings = bindgen::Builder::default()
         .clang_args(["-I", "../"])
         .clang_args(["-I", "../../build/include"])
+        .clang_args(["-I", "../../../njs/src"])
+        .clang_args(["-I", "../../../njs/build"])
         .header("./wrapper.h")
         // only generate bindings for `nxt_*` header files
         .allowlist_file(".*nxt_.*.h")

Adjust the paths to the njs repository as necessary, the above assumes it's in the same directory that the unit repository is in (i.e the paths are relative to ./src/wasm-wasi-component).

kvelaro commented 1 week ago

Thank you for fast reply, really appreciate it

Some questions about patch:

ac000 commented 1 week ago

Thank you for fast reply, really appreciate it

Some questions about patch:

* Why this is not documented anywhere? Is it a bug, or something like argument passing is an issue to future releases?

I suppose technically it's a bug.

It's a problem of trying to marry two different worlds together, the C & make world with the rust world with its own build system...

* As complete solution unit has ready to use binaries with modules included. Did they apply this patch to build successfully? Does this official solution for now?
  P.S: Sorry maybe for silly questions, just curious, had never faced applying patches.

This is true.

You see, to actually build the language modules, you don't really need to fully configure Unit itself. E.g to build the wasm-wasi-component language module, it's sufficient to do just

$ ./configure
$ ./configure wasm-wasi-component
$ make

That will build src/wasm-wasi-component/target/release/libwasm_wasi_component.so

That's essentially how the language module packages are built.

kvelaro commented 1 week ago

Thank you!

ac000 commented 1 week ago

In fact if you do make wasm-wasi-component instead of just make it wont even bother building unit...