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

fix(build): use NGX_WASM_MODULE_BRANCH environment variable #12241

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

locao
Copy link
Contributor

@locao locao commented Dec 22, 2023

Summary

To improve the development and testing experience of the integration between Kong and Wasmx, the NGX_WASM_MODULE_BRANCH environment variable should be used to overwrite the ngx_wasm_module branch/commit in the .requirements file, as it is documented in build/README.md. But this variable was being ignored by the build system.

Checklist

Issue reference

KAG-3396

Copy link
Contributor

@flrgh flrgh left a comment

Choose a reason for hiding this comment

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

Looks good to me as long as it's working!

Are there any caveats to this with regards to bazel's caching that should be documented? i.e. Do I need to manually purge the cache when changing NGX_WASM_MODULE_BRANCH?

@locao locao force-pushed the fix/build_use_wasm_branch_env branch 2 times, most recently from e866264 to 0ccf54c Compare January 5, 2024 14:03
@locao
Copy link
Contributor Author

locao commented Jan 5, 2024

Are there any caveats to this with regards to bazel's caching that should be documented?

Yes, after the first build NGX_WASM_MODULE_REMOTE and NGX_WASM_MODULE_BRANCH changes are ignored by the build system, but this issue is already documented.

@locao locao force-pushed the fix/build_use_wasm_branch_env branch from 0ccf54c to 94a054e Compare January 8, 2024 14:42
@hishamhm
Copy link
Contributor

hishamhm commented Jan 8, 2024

Yes, after the first build NGX_WASM_MODULE_REMOTE and NGX_WASM_MODULE_BRANCH changes are ignored by the build system, but this issue is already documented.

If I keep those variables unchanged but change the actual code in the branch between runs, does it require a purge as well?

@fffonion
Copy link
Contributor

fffonion commented Jan 9, 2024

the environ variable should be added to https://github.com/Kong/kong/pull/12241/files#diff-2cc0f6ace9a475cc6da6de926e8eabbe69e8b7e6e5c4fb721ec83d59f3f04725R113 here to evict cache (of the kong_bindings.bzl file/repository only)

@hishamhm
Copy link
Contributor

@locao could we get @fffonion's suggested addition in as well, to make the cache work correctly? 🙏

@locao locao force-pushed the fix/build_use_wasm_branch_env branch 2 times, most recently from 9017aa0 to 327529d Compare January 15, 2024 20:16
@RobSerafini
Copy link
Contributor

@fffonion - can you re-review this now?

@locao locao force-pushed the fix/build_use_wasm_branch_env branch from 327529d to e4465e9 Compare January 18, 2024 18:39
@@ -0,0 +1,3 @@
message: use NGX_WASM_MODULE_BRANCH environment variable to set ngx_wasm_module repository branch when building Kong.
type: bugfix
scope: Core
Copy link
Member

Choose a reason for hiding this comment

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

"chore"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it would be a better option, but it is not one of the types available in the new changelog format.

@locao locao force-pushed the fix/build_use_wasm_branch_env branch 4 times, most recently from 4678671 to dcb0478 Compare January 22, 2024 15:40
@locao locao force-pushed the fix/build_use_wasm_branch_env branch from dcb0478 to 23e6aa0 Compare January 22, 2024 20:03
@kikito kikito merged commit d2cf328 into master Jan 23, 2024
24 checks passed
@kikito kikito deleted the fix/build_use_wasm_branch_env branch January 23, 2024 17:53
@locao locao added the cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee label Jan 23, 2024
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-12241-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-12241-to-master-to-upstream
git checkout -b cherry-pick-12241-to-master-to-upstream
ancref=$(git merge-base 36c3836326d29444b400d45d7e6cdcba67f065a6 23e6aa078b29ddb325d57d7905f1a68c47fa489f)
git cherry-pick -x $ancref..23e6aa078b29ddb325d57d7905f1a68c47fa489f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/bazel cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/docs size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants