-
Notifications
You must be signed in to change notification settings - Fork 5k
docs(DEVELOPER.md): update to catch up the current behavior #14478
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
base: master
Are you sure you want to change the base?
Conversation
`make build-venv` only setup the venv. It doesn't build the kong. Signed-off-by: spacewander <spacewanderlzx@gmail.com>
Co-authored-by: Qi <add_sp@outlook.com>
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall LGTM, just have a small question.
Signed-off-by: spacewander <spacewanderlzx@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks you!
| done; | ||
|
|
||
| dev: install-rust-toolchain build-venv install-dev-rocks bin/grpcurl bin/h2client | ||
| dev: install-rust-toolchain build-venv build-openresty install-dev-rocks bin/grpcurl bin/h2client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| dev: install-rust-toolchain build-venv build-openresty install-dev-rocks bin/grpcurl bin/h2client | |
| dev: install-rust-toolchain build-venv install-dev-rocks bin/grpcurl bin/h2client |
This fixed the CI failures. Did the missing build-openresty (as the deps of the dev target) cause issues while developing locally?
Background
The target build-openresty is only for local nginx module/patch development, so there are some assumptions in the Bazel script.
In simple words, the Makefile target invokes the Bazel target @openresty//:dev-just-make to rebuild the OpenResty, but this Bazel target tries to reuse the current Bazel working directory.
kong/build/openresty/BUILD.openresty.bazel
Line 369 in 5885ea4
| pushd $(RULEDIR)/openresty.build_tmpdir >/dev/null |
The Bazel working directory is not cached across the CI jobs, only the Bazel output directory is cached, so the Makefile target build-openresty is failing on CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixed the CI failures. Did the missing build-openresty (as the deps of the dev target) cause issues while developing locally?
The build-openresty builds the OpenResty, which is required by Kong. Is there another way to provide the nginx binary? I take a look at the CI, and it uses bazel build //build:install-openresty under the hood in the build job: https://github.com/Kong/kong/actions/runs/15016755778/job/42253855998?pr=14478
The Makefile task build-openresty will call //build:dev-make-openresty instead of //build:install-openresty, which causes the CI to fail. Is there a difference between an incr build and a full build? I am not so sure...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is make build-venv not generating OpenResty binary on dev box? If so, this should be a bug and need to be fixed in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a difference between an incr build and a full build?
I didn't fully deep dive it.
There are some hacks inside the //build:dev-make-openresty, it rm the binary in the working directory and re-run the make && make install under the OpenResty source.
And for the full build, at least, it doesn't re-run the configure script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
make build-venvnot generating OpenResty binary on dev box? If so, this should be a bug and need to be fixed in another PR.
Thanks for your notification. Now I know why:
Line 100 in 5885ea4
| @if [ ! -e bazel-bin/build/$(BUILD_NAME)-venv.sh ]; then \ |
the build-venv will check the generated file before running the actual command. When running for the first time, the command generated the venv.sh file, but run failed later when installing luarocks. (because of missing libyaml). When I rerun the command after the fix, the build-venv doesn't nothing, only venv is generated, so I thought this task only generates the venv.

make build-venvonly setup the venv. It doesn't build the kong.