Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

docs(DEVELOPER.md): update to catch up the current behavior #14478

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d6f9db4
docs(DEVELOPER.md): update to catch up the current behavior
spacewander May 10, 2025
9ea418b
update
spacewander May 11, 2025
2676856
Update DEVELOPER.md
spacewander May 12, 2025
65abb2e
tweak a bit
spacewander May 13, 2025
550de29
add missing line back
spacewander May 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions DEVELOPER.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ starting point. It contains the proper file structures, configuration files,
and CI setup to get up and running quickly. This repository seamlessly
integrates with [Pongo](https://github.com/Kong/kong-pongo).

## Build and Install from source
## General guide for development

### Prepare development dependencies and build from source

This is the hard way to build a development environment, and also a good start
for beginners to understand how everything fits together.
Expand All @@ -72,7 +74,7 @@ requires some additional third-party dependencies, some of which are compiled
with tweaked options, and kong runs on a modified version of OpenResty with
patches.

To install from the source, first, we clone the repository:
To build from the source, first, we clone the repository:

```shell
git clone https://github.com/Kong/kong
Expand Down Expand Up @@ -180,8 +182,8 @@ git config --local url.'ssh://[email protected]/'.insteadOf 'https://github.com/'
Finally, we start the build process:

```
# Setup virtual environment, download dependencies and install Kong from source
make install
# Setup virtual environment, download dependencies and build Kong from source
make dev
```

[The build guide](https://github.com/Kong/kong/blob/master/build/README.md) contains a troubleshooting section if
Expand Down Expand Up @@ -219,8 +221,6 @@ kong stop
deactivate
```

### Install Development Dependencies

#### Running for development

By default, the development environment adds current directory to Lua files search path.
Expand All @@ -230,17 +230,14 @@ and [`lua_package_cpath`](https://github.com/openresty/lua-nginx-module#lua_pack
directives will allow Kong to find your custom plugin's source code wherever it
might be in your system.

#### Tests
### Tests

Install the development dependencies ([busted](https://lunarmodules.github.io/busted/),
[luacheck](https://github.com/mpeterv/luacheck)) with:
Before running any tests, please ensure the development dependencies are installed via

```shell
make dev
```

If Rust/Cargo doesn't work, try setting `export KONG_TEST_USER_CARGO_DISABLED=1` first.

Kong relies on three test suites using the [busted](https://lunarmodules.github.io/busted/) testing library:

* Unit tests
Expand Down Expand Up @@ -355,7 +352,7 @@ are available in both versions (i.e. from helpers.lua). The module
the new version into the container of the old version and it can be
used to make new library functionality available to migration tests.

#### Makefile
### Makefile

When developing, you can use the `Makefile` for doing the following operations:

Expand Down
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ install-dev-rocks: build-venv
fi \
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
Copy link
Contributor

@ADD-SP ADD-SP May 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
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?

截屏2025-05-15 11 25 54

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.

Copy link
Contributor Author

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...

Copy link
Contributor

@ADD-SP ADD-SP May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Is there another way to provide the nginx binary? I

The chain is complicated. In short, the make build-venv eventually builds the OpenResty binary.

bazel-build-venv-flow

Copy link
Contributor

@ADD-SP ADD-SP May 16, 2025 •
edited
Loading

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Thanks for your notification. Now I know why:

kong/Makefile

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.


build-release: check-bazel
$(BAZEL) clean --expunge
Expand Down