-
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
Open
spacewander
wants to merge
5
commits into
Kong:master
Choose a base branch
from
spacewander:0510
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+15
−11
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
|
@@ -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 | ||
|
@@ -180,8 +182,8 @@ git config --local url.'ssh://[email protected]/'.insteadOf 'https://github.com/' | |
Finally, we start the build process: | ||
|
||
``` | ||
# Build the virtual environment for developing Kong | ||
make build-venv | ||
# 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 | ||
|
@@ -204,18 +206,21 @@ Now you can start Kong: | |
# Use the pre-defined docker-compose file to bring up databases etc | ||
start_services | ||
|
||
# bootstrap database | ||
kong migrations bootstrap | ||
# Start Kong! | ||
kong start | ||
|
||
# Verify if Kong is started by querying the Admin endpoint | ||
curl 127.0.0.1:8001 -I | ||
|
||
# Stop Kong | ||
kong stop | ||
|
||
# Cleanup | ||
deactivate | ||
``` | ||
|
||
### Install Development Dependencies | ||
|
||
#### Running for development | ||
|
||
By default, the development environment adds current directory to Lua files search path. | ||
|
@@ -225,10 +230,9 @@ 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 | ||
|
@@ -350,7 +354,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: | ||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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 thedev
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
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.
The
build-openresty
builds the OpenResty, which is required by Kong. Is there another way to provide thenginx
binary? I take a look at the CI, and it usesbazel build //build:install-openresty
under the hood in thebuild
job: https://github.com/Kong/kong/actions/runs/15016755778/job/42253855998?pr=14478The 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 anincr build
and afull build
? I am not so sure...Uh oh!
There was an error while loading. Please reload this page.
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.
The chain is complicated. In short, the
make build-venv
eventually builds the OpenResty binary.Uh oh!
There was an error while loading. Please reload this page.
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.
I didn't fully deep dive it.
There are some hacks inside the
//build:dev-make-openresty
, itrm
the binary in the working directory and re-run themake && 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.
Thanks for your notification. Now I know why:
kong/Makefile
Line 100 in 5885ea4
the
build-venv
will check the generated file before running the actual command. When running for the first time, the command generated thevenv.sh
file, but run failed later when installing luarocks. (because of missinglibyaml
). When I rerun the command after the fix, thebuild-venv
doesn't nothing, only venv is generated, so I thought this task only generates the venv.