Skip to content

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Feb 23, 2018

Currently the cctest target depend on the node_core_target_name
target. But it is the node_lib_target_name target that compiles the
sources now which means that if a source file in src is updated the
cctest executable will not be re-linked against it, but will remain
unchanged. The code will still be compiled, just not linked which
means that if you are debugging you'll not see the changes and also a
warning will be displayed about this issue.

This commit changes the cctest target to depend on node_lib_target_name.

PR-URL: #18576
Reviewed-By: Matheus Marchini matheus@sthima.com
Reviewed-By: Yihong Wang yh.wang@ibm.com
Reviewed-By: Gibson Fahnestock gibfahn@gmail.com
Reviewed-By: Ben Noordhuis info@bnoordhuis.nl

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

Currently the cctest target depend on the node_core_target_name
target. But it is the node_lib_target_name target that compiles the
sources now which means that if a source file in src is updated the
cctest executable will not be re-linked against it, but will remain
unchanged. The code will still be compiled, just not linked which
means that if you are debugging you'll not see the changes and also a
warning will be displayed about this issue.

This commit changes the cctest target to depend on node_lib_target_name.

PR-URL: nodejs#18576
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Yihong Wang <yh.wang@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v9.x labels Feb 23, 2018
@danbev
Copy link
Contributor Author

danbev commented Feb 23, 2018

@danbev danbev changed the title build: add node_lib_target_name to cctest deps [v9.x backport] build: add node_lib_target_name to cctest deps Feb 23, 2018
@mmarchini
Copy link
Contributor

@danbev this will break once #18550 lands. Should I add a commit to #18550 with the same fix you did on #18576?

@danbev
Copy link
Contributor Author

danbev commented Feb 26, 2018

Should I add a commit to #18550 with the same fix you did on #18576?

That sounds great! Let me know if we can close this in that case. Thanks

@mmarchini
Copy link
Contributor

Commit added to #18550. @danbev I think we can close this now (no need to resolve the conflict since I'll fix it on #18550)

@danbev
Copy link
Contributor Author

danbev commented Feb 26, 2018

@mmarchini Great, thanks for that. Will close now.

@danbev danbev closed this Feb 26, 2018
@danbev danbev deleted the backport-18576-9.x branch March 27, 2018 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to build files or the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants