Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upbuild: add CONFIG_FLAGS to with-code-cache target #22207
Conversation
This comment has been minimized.
This comment has been minimized.
nodejs-github-bot
added
the
build
label
Aug 9, 2018
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
richardlau
reviewed
Aug 9, 2018
Makefile
Outdated
| .PHONY: with-code-cache | ||
| with-code-cache: | ||
| $(PYTHON) ./configure | ||
| $(PYTHON) ./configure $(debug_flag) |
This comment has been minimized.
This comment has been minimized.
richardlau
Aug 9, 2018
Member
Perhaps it would be better to make the ./configure invocations in this recipe take $(CONFIG_FLAGS) (like the other invocations of ./configure in this file) and then the ifeq statement a few lines up could append --debug to that?
This comment has been minimized.
This comment has been minimized.
danbev
Aug 10, 2018
Author
Member
Ah, I was not aware of the CONFIG_FLAGS, I'll take a look. Thanks!
danbev
force-pushed the
danbev:build_codecache_debug_flag
branch
from
cd54fa3
to
2345a6c
Aug 10, 2018
danbev
changed the title
build: add debug flag to with-code-cache target
build: add CONFIG_FLAGS to with-code-cache target
Aug 10, 2018
This comment has been minimized.
This comment has been minimized.
|
Updated and rebased CI: https://ci.nodejs.org/job/node-test-pull-request/16333/ |
richardlau
approved these changes
Aug 10, 2018
joyeecheung
approved these changes
Aug 10, 2018
refack
approved these changes
Aug 10, 2018
maclover7
approved these changes
Aug 11, 2018
maclover7
added
the
author ready
label
Aug 11, 2018
This comment has been minimized.
This comment has been minimized.
|
(One failure on CI is due to a SmartOS 17 machine that has been having issues, this should be safe to land) |
This comment has been minimized.
This comment has been minimized.
|
Landed in ec8f31d |
maclover7
closed this
Aug 12, 2018
maclover7
added a commit
that referenced
this pull request
Aug 12, 2018
danbev
deleted the
danbev:build_codecache_debug_flag
branch
Aug 12, 2018
targos
added a commit
that referenced
this pull request
Aug 12, 2018
This was referenced Aug 16, 2018
beevelop
referenced this pull request
Aug 16, 2018
Closed
2018-08-15 Version 10.9.0 (Current) @rvagg #219
renovate
bot
referenced this pull request
Feb 19, 2019
Merged
chore(deps): update node.js to v10.9.0 #58
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
danbev commentedAug 9, 2018
•
edited
This commit adds CONFIG_FLAGS to allow the
with-code-cachetarget to beused with a debug build. The motivation for this is to make it easier to
debug a build with the code cache enabled.
The suggested usage:
The
BUILDTYPEoption is not needed if ./configure was alreadyconfigured with --debug.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes