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.

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

Jump to bottom

build: only generate specified build type ninja files #53511

Merged
merged 1 commit into from
Jun 22, 2024
Merged

build: only generate specified build type ninja files #53511

merged 1 commit into from
Jun 22, 2024

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Jun 19, 2024 •
edited
Loading

Release and Debug build configurations can not be shared, only
generate specified configuration in configure with the ninja
generator. This avoids generating invalid DEBUG ninja build files
when configure was not run with --debug.

Fixes: #53446

@legendecas
build: only generate specified build type files
95a97e1 
Release and Debug build configurations can not be shared, only
generate specified configuration in `configure`.
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Jun 19, 2024
@legendecas
Copy link
Member Author

legendecas commented Jun 19, 2024 •
edited
Loading

Specifically, setting v8_enable_v8_checks in generated config.gypi to 1 and regenerate ninja files could fix the issue. Anyway, https://github.com/nodejs/node/blob/main/doc/contributing/building-node-with-ninja.md#producing-a-debug-build suggested that build with ./configure --ninja --debug && make.

@legendecas legendecas requested a review from targos June 19, 2024 15:24
@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59892/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/59905/

@legendecas legendecas added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 21, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 22, 2024
@nodejs-github-bot nodejs-github-bot merged commit 6cb940a into nodejs:main Jun 22, 2024
62 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 6cb940a

@legendecas legendecas deleted the build/ninja branch June 22, 2024 08:41
@targos
Copy link
Member

targos commented Jun 25, 2024

I should have tried it before approving. I'm not able to work with ninja and make after this change:

$ python3 configure.py --ninja --debug --node-builtin-modules-path=`pwd`
Warning! Loading builtin modules from disk is for development
INFO: configure completed successfully

$ make -j6
gmake: *** No rule to make target 'out/Release/build.ninja', needed by 'node'.  Stop.

$ make -j6 BUILDTYPE=Debug
gmake: *** No rule to make target 'out/Release/build.ninja', needed by 'node'.  Stop.

targos pushed a commit that referenced this pull request Jun 25, 2024
@legendecas @targos
build: only generate specified build type files
1a3ccba 
Release and Debug build configurations can not be shared, only
generate specified configuration in `configure`.

PR-URL: #53511
Fixes: #53446
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@legendecas legendecas mentioned this pull request Jun 25, 2024
Merged
@legendecas
Copy link
Member Author

Yeah, make BUILDTYPE=Debug still builds in release mode so I think an alternative should be making the generated config.gypi works in both Release mode and Debug mode. Reverting since this is breaking existing work flow.

@targos targos added the dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. label Jun 25, 2024
@aduh95 aduh95 mentioned this pull request Oct 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joyeecheung joyeecheung joyeecheung approved these changes

@targos targos targos approved these changes

@anonrig anonrig anonrig approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

node_mksnapshot debug build segfault
7 participants
@legendecas @nodejs-github-bot @targos @jasnell @lpinca @anonrig @joyeecheung