Skip to content
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

Packages: Pass CFLAGS to compile wasm modules on all packaging targets #1153

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

thresheek
Copy link
Member

This extends the approach used for debian-based packages in 3f805bc to rpm as well. Notable change for both deb and rpm packaging is to use CFLAGS as defined in the build/Makefile, and not pass them from the environment which might not be there (as is the case for rpm).

While at it, stop passing CFLAGS in the install phase, as it should no longer invoke builds (see d54af16).

The rpm part was overlooked in 7a64055, since testing was not done on the platforms where problem manifested itself, notably Amazon Linux 2023 and Fedora 38+.

@ac000 ac000 self-requested a review February 22, 2024 23:07
This extends the approach used for debian-based packages in 3f805bc
to rpm as well.  Notable change for both deb and rpm packaging is to use
CFLAGS as defined in the build/Makefile, and not pass them from the
environment which might not be there (as is the case for rpm).

While at it, stop passing CFLAGS in the install phase, as it should no
longer invoke builds (see d54af16).

The rpm part was overlooked in 7a64055, since testing was not done
on the platforms where problem manifested itself, notably Amazon Linux
2023 and Fedora 38+.
@thresheek
Copy link
Member Author

Fixed wrong target in rpm for make install (wasm-install instead of wasm)

git range-diff --no-color 17a2618...52f5a7e
1:  17a26184 ! 1:  52f5a7e2 Packages: Pass CFLAGS to compile wasm modules on all packaging targets
    @@ pkg/rpm/Makefile.wasm: MODULE_VERSION_wasm=      $(VERSION)

      MODULE_CONFARGS_wasm=     wasm --include-path=\`pwd\`/pkg/contrib/wasmtime/crates/c-api/include --lib-path=\`pwd\`/pkg/contrib/wasmtime/target/release \&\& ./configure wasm-wasi-component
     -MODULE_MAKEARGS_wasm=     wasm wasm-wasi-component
    --MODULE_INSTARGS_wasm=     wasm-install wasm-wasi-component-install
     +MODULE_MAKEARGS_wasm=     wasm wasm-wasi-component CFLAGS=\"\$$(grep ^CFLAGS build/Makefile | cut -d' ' -f 3-) -Wno-missing-prototypes\"
    -+MODULE_INSTARGS_wasm=     wasm wasm-wasi-component-install
    + MODULE_INSTARGS_wasm=     wasm-install wasm-wasi-component-install

      MODULE_SOURCES_wasm=
    -

@ac000
Copy link
Member

ac000 commented Feb 22, 2024

Heh, all the wasm's merge together... ;)

@thresheek thresheek merged commit faa7e79 into nginx:master Feb 23, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants