build: fix gyp configs in debug
Gyp generated build files should be able to be built in either Release/Debug mode.
- make: single entry build file, two configurations by cli:
make -C out BUILDTYPE=Releaseandmake -C out BUILDTYPE=Debug. - msbuild: single entry build file, two configurations by cli:
msbuild node.sln /p:Configuration=Releaseandmsbuild node.sln /p:Configuration=Debug. - ninja: two directories in
out/, build withninja -C out/Releaseorninja -C out/Debug.
Variables that changes with either Release or Debug configuration should be defined in a configuration level, instead of the root level. Example: https://github.com/nodejs/node/blob/8e5d88b6413558b6b4bc127f87fc8122311639ae/common.gypi#L132-L133
This fixes generating invalid build files.
Additionally, v8_gypfiles/toolchain.gypi duplicates defines in
v8_gypfiles/features.gypi. Remove the duplications in
toolchains.gypi
Fixes: https://github.com/nodejs/node/issues/53446
Review requested:
- [ ] @nodejs/gyp
- [ ] @nodejs/v8-update
./configure --debug option is not removed as it sets the default build type as well.
it doesn't change the fact that
./configure --ninja --debug && makestill builds in Release and Debug modes, right?
Yes, it still builds both Release and Debug products.
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6058/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6058/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6058/
CI: https://ci.nodejs.org/job/node-test-pull-request/60043/
CI: https://ci.nodejs.org/job/node-test-pull-request/60184/
CI: https://ci.nodejs.org/job/node-test-pull-request/60187/
Rebasing on to latest main since there were reverts on reproducible snapshot.
CI: https://ci.nodejs.org/job/node-test-pull-request/60193/
CI: https://ci.nodejs.org/job/node-test-pull-request/60195/
Restarting since https://github.com/nodejs/node/pull/53755 landed addressing the snapshot reproducible test.
CI: https://ci.nodejs.org/job/node-test-pull-request/60201/
CI: https://ci.nodejs.org/job/node-test-pull-request/60211/
CI: https://ci.nodejs.org/job/node-test-pull-request/60254/
CI: https://ci.nodejs.org/job/node-test-pull-request/60411/
Tried to reproduce locally and skip the sea test if the debug build is too large: https://github.com/nodejs/node/pull/53918
CI: https://ci.nodejs.org/job/node-test-pull-request/60543/
Commit Queue failed
- Loading data for nodejs/node/pull/53605 ✔ Done loading data for nodejs/node/pull/53605 ----------------------------------- PR info ------------------------------------ Title build: fix gyp configs in debug (#53605) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch legendecas:build/debug-vars -> nodejs:main Labels build, v8 engine, tools, author ready, needs-ci Commits 2 - build: fix conflict gyp configs - fixup! build: fix gyp configs in debug Committers 2 - Chengzhong Wu <[email protected]> - GitHub <[email protected]> PR-URL: https://github.com/nodejs/node/pull/53605 Fixes: https://github.com/nodejs/node/issues/53446 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/53605 Fixes: https://github.com/nodejs/node/issues/53446 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> -------------------------------------------------------------------------------- ℹ This PR was created on Thu, 27 Jun 2024 14:24:05 GMT ✔ Approvals: 2 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/53605#pullrequestreview-2149988265 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/53605#pullrequestreview-2151985821 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2024-07-22T21:41:04Z: https://ci.nodejs.org/job/node-test-pull-request/60543/ - Querying data for job/node-test-pull-request/60543/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 53605 From https://github.com/nodejs/node * branch refs/pull/53605/merge -> FETCH_HEAD ✔ Fetched commits as b19a9501020c..0e1eadd17396 -------------------------------------------------------------------------------- Auto-merging configure.py [main 63ebb41ad3] build: fix conflict gyp configs Author: Chengzhong Wu <[email protected]> Date: Wed Jun 26 15:20:24 2024 +0100 3 files changed, 15 insertions(+), 12 deletions(-) Auto-merging configure.py [main 9f4cabf169] fixup! build: fix gyp configs in debug Author: Chengzhong Wu <[email protected]> Date: Mon Jul 1 12:07:20 2024 +0100 1 file changed, 2 insertions(+), 2 deletions(-) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. Rebasing (2/4)Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- build: fix conflict gyp configs
Gyp generated build files can be built in either Release/Debug mode.
- make: single directory, two configurations by cli:
make -C out BUILDTYPE=Releaseandmake -C out BUILDTYPE=Debug. - msbuild: single directory, two configurations by cli:
msbuild node.sln /p:Configuration=Releaseandmsbuild node.sln /p:Configuration=Debug. - ninja: two directories in
out/, build withninja -C out/Releaseorninja -C out/Debug.
Variables that changes with either Release or Debug configuration should be defined in a configuration level, instead of the root level. This fixes generating invalid build files.
Additionally, v8_gypfiles/toolchain.gypi duplicates defines in
v8_gypfiles/features.gypi. Remove the duplications in
toolchains.gypi
PR-URL: https://github.com/nodejs/node/pull/53605 Fixes: https://github.com/nodejs/node/issues/53446 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
[detached HEAD e2f983717b] build: fix conflict gyp configs Author: Chengzhong Wu <[email protected]> Date: Wed Jun 26 15:20:24 2024 +0100 3 files changed, 15 insertions(+), 12 deletions(-) Rebasing (3/4) Rebasing (4/4)
Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- fixup! build: fix gyp configs in debug
Co-authored-by: Mohammed Keyvanzadeh <[email protected]> PR-URL: https://github.com/nodejs/node/pull/53605 Fixes: https://github.com/nodejs/node/issues/53446 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
[detached HEAD 17303f6e5a] fixup! build: fix gyp configs in debug Author: Chengzhong Wu <[email protected]> Date: Mon Jul 1 12:07:20 2024 +0100 1 file changed, 2 insertions(+), 2 deletions(-)
Successfully rebased and updated refs/heads/main.
ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.https://github.com/nodejs/node/actions/runs/10054618263
Landed in e192a32c27f223e7a3dcb43134fd891dd8ded760