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

chore(ci): add wasm to release builds #11039

Merged
merged 6 commits into from
Jun 30, 2023
Merged

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented Jun 9, 2023

This turns on ngx_wasm_module by default, adding it to all of our amd64 builds in the release workflow.

Right now we only build and package with wasmtime, because that is what is planned to ship, but there is partial support for wasmer and v8 in here as well, should we decide to make them available in the future.

Additionally, with this change the wasm runtime is now statically linked to ngx_wasm_module, so there are fewer shared object files that need to be copied around into our common lib directory at build time.

@github-actions github-actions bot added build/bazel chore Not part of the core functionality of kong, but still needed labels Jun 9, 2023
@flrgh flrgh force-pushed the chore/ci-wasm-releases branch 2 times, most recently from 5162537 to 9bd5881 Compare June 26, 2023 16:17
@flrgh flrgh changed the title WIP: add wasmx to release builds chore(ci): add wasm to release builds Jun 28, 2023
@flrgh flrgh marked this pull request as ready for review June 28, 2023 16:43
@@ -197,6 +234,12 @@ CONFIGURE_OPTIONS = [
}) + select({
"@kong//:wasmx_flag": [
"--add-module=$$EXT_BUILD_ROOT$$/external/ngx_wasm_module",
# this is a workaround for rhel/centos builds
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: @curiositycasualty @fffonion

I don't know enough to judge the level of risk/impact of this change. If necessary I'm sure it could be limited to just centos/rhel-based builds, but I opted to start with the simple approach for now.

@flrgh flrgh added this to the 3.4.0 milestone Jun 28, 2023
Copy link
Contributor

@locao locao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few nitpicks

BUILD.bazel Outdated Show resolved Hide resolved
BUILD.bazel Outdated Show resolved Hide resolved
build/openresty/wasmx/wasmx_repositories.bzl Outdated Show resolved Hide resolved
flrgh and others added 3 commits June 29, 2023 16:49
Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>
Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>
Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>
@flrgh flrgh requested a review from locao June 29, 2023 23:51
Copy link
Contributor

@locao locao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The build is green and everything seems OK. I am not pressing the merge button right now because the build is failing on my local environment (unrelated to the *dylib change). I am trying to find out if it's my environment that is broken or not.

Copy link
Contributor

@locao locao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looking for the root-cause, but @gszr confirmed this change breaks the build on macos.

the commented line breaks the build for macos, probably the flag is not
supported by clang
@locao locao merged commit 08af019 into feat/wasmx Jun 30, 2023
21 checks passed
@locao locao deleted the chore/ci-wasm-releases branch June 30, 2023 20:20
locao added a commit that referenced this pull request Jul 13, 2023
* chore(ci): add wasm to release builds

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(build): change the v8 runtime URL

* chore(build): workaround for macos

the commented line breaks the build for macos, probably the flag is not
supported by clang

---------

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>
locao added a commit that referenced this pull request Jul 14, 2023
* chore(ci): add wasm to release builds

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(build): change the v8 runtime URL

* chore(build): workaround for macos

the commented line breaks the build for macos, probably the flag is not
supported by clang

---------

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>
flrgh added a commit that referenced this pull request Jul 17, 2023
* chore(ci): add wasm to release builds

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(*): apply review suggestions

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>

* chore(build): change the v8 runtime URL

* chore(build): workaround for macos

the commented line breaks the build for macos, probably the flag is not
supported by clang

---------

Co-authored-by: Vinicius Mignot <vinicius.mignot@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/bazel chore Not part of the core functionality of kong, but still needed size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants