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

hotfix(config) fix external ngx-wasm-rs builds #389

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

thibaultcha
Copy link
Member

@thibaultcha thibaultcha commented Aug 17, 2023

First this fixes the test cases and documentation as they were merged, which wrongly assumed NGX_CC_OPT and NGX_LD_OPT were digested by Nginx's auto scripts, they are not. They are derived from --with-{cc,ld}-opt arguments.

Second, it moves defines and mandatory checks to config since we need both even when NGX_WASM_CARGO=0 and auto/cargo does not run.

The above fixes NGX_WASM_CARGO=0 as a mean to completely disable ngx-wasm-rs, as supported by Wasmer. This "mandatory={yes,no}" behavior is tested for both Wasmer and V8 as well.

Finally, we also consider NGX_WASM_CARGO in the local development environment freshness build script.

@thibaultcha thibaultcha force-pushed the hotfix/externalized-cargo branch 3 times, most recently from a6e9413 to 8b11f83 Compare August 17, 2023 23:24
First this fixes the test cases and documentation as they were merged
(with edits), which wrongly assumed `NGX_CC_OPT` and `NGX_LD_OPT` were
digested by Nginx's `auto` scripts, they are not. They are derived from
`--with-{cc,ld}-opt` arguments.

Second, it moves defines and mandatory checks to `config` since we need
both even when `NGX_WASM_CARGO=0` and `auto/cargo` does not run.

The above fixes `NGX_WASM_CARGO=0` as a mean to *completely* disable
ngx-wasm-rs, as supported by Wasmer. This "mandatory={yes,no}" behavior
is tested for both Wasmer and V8 as well.

Finally, we also consider `NGX_WASM_CARGO` in the local development
environment freshness build script.
@thibaultcha thibaultcha merged commit 2e28b7a into main Aug 18, 2023
30 of 31 checks passed
@thibaultcha thibaultcha deleted the hotfix/externalized-cargo branch August 18, 2023 00:40
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.

1 participant