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.

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

Makefile dependency tree is incomplete, makefiles do not build only as necessary #25135

Open
sam-github opened this issue Dec 19, 2018 · 3 comments
Labels
build Issues and PRs related to build files or the CI. confirmed-bug Issues with confirmed bugs.

Comments

@sam-github
Copy link
Contributor

sam-github commented Dec 19, 2018

The makefiles generated by gyp are not working for development (they work better for single-shot CI builds). Perhaps I'm missing something, please tell me if I am, but I've done some asking around to people who know the gyp system, and have some agreement on these points.

  • make node; make node Make should do nothing if no source code has been changed, but make node relinks both node and cctest (!?) every time it is run.

  • make -C out node is faster, but and builds just node, not cctest, but it does it even if no source has changed. However, its build/dependency tree is incomplete! It doesn't run js2c so if any js source files are changed they will not be built-in with js2c. Fast and wrong is worse than slow.

  • make -C out/Release node as above

  • make -j4 node; make -j1 test often required. parallel make is not working for test. Various symptoms of brokenness occur (irc has some discussion). Sounds like this could be related to the dependency relationships not being correctly specified, causing build rules to run in parallel when they cannot, and thus be racy.

  • ./configure --shared-openssl-includes will cause openssl include paths to be passed to dependencies that don't use openssl. This might appear harmless, but it (for example) causes ccache to rebuild icu, even though icu doesn't use openssl. Possible fix:

    node/common.gypi

    Lines 549 to 560 in eef6504

    ['node_shared_openssl!="true"', {
    # `OPENSSL_THREADS` is defined via GYP for openSSL for all architectures.
    'defines': [
    'OPENSSL_THREADS',
    ],
    }],
    ['node_shared_openssl!="true" and openssl_no_asm==1', {
    'defines': [
    'OPENSSL_NO_ASM',
    ],
    }],
    ],
    , add OPENSSL_THREADS to the else clause (I haven't tried yet).

Ninja notes:

Ninja (./configure --ninja) doesn't have the above problems for building, its dependency tree is correct (it builds nothing when nothing has changed, and it reruns js2c when js files have changed).

However, it is either incomplete or not integrated with the rest of the build (I'm not sure how its intended to work). It doesn't make the top-level node symlink (at least test-doc relies on this), so a ln -s out/Release/node node is required for make test-doc. As for make test, it won't use the ninja build output, because it relies on .PHONY targets, so it always (effectively) does a make -C out all.

@addaleax addaleax added confirmed-bug Issues with confirmed bugs. build Issues and PRs related to build files or the CI. labels Dec 19, 2018
@sam-github
Copy link
Contributor Author

An additional issue:

  • js2c outputs a single file from multiple inputs, this is not emenable to fast re-compilation. It should output each js file as a single .h file, so that when only one is changed, only one needs to be converted. Its quite slow ATM.

@sam-github

This comment has been minimized.

@sam-github
Copy link
Contributor Author

This is pretty painful on machines with slow link times while testing CI builds (so, not allowed to use ninja), because make test does a relink every time.

I have been poking at this a bit, and it appears to be a bug in the V8 gyp files (@nodejs/v8-update), they seem to do some actions without considering whether deps have changed. Following is a rebuild (after just having done 5 clean builds). Is the root cause of this that Generating inspector protocol sources from protocol json has no deps?

% make -C out -j1
make: Entering directory '/home/sam/w/core/lts/out'
  TOUCH dc2a6a8f2bf222b5a02a9df3d748ab02b9f73ccf.intermediate
  ACTION _home_sam_w_core_lts_tools_v8_gypfiles_v8_gyp_run_torque_target_run_torque_action dc2a6a8f2bf222b5a02a9df3d748ab02b9f73ccf.intermediate
  TOUCH 8ce4ae39fa160c52d3938e994929629c6a3c704e.intermediate
  ACTION Generating inspector protocol sources from protocol json 8ce4ae39fa160c52d3938e994929629c6a3c704e.intermediate
  TOUCH f12aef6ca9be3b6903ccca9065c46321958e5d57.intermediate
  ACTION generating: "/home/sam/w/core/lts/out/Release/obj.target/v8_version/geni/snapshot.cc" "/home/sam/w/core/lts/out/Release/obj.target/v8_version/geni/embedded.S" f12aef
6ca9be3b6903ccca9065c46321958e5d57.intermediate
  CXX(target) /home/sam/w/core/lts/out/Release/obj.target/v8_snapshot/geni/snapshot.o
  CC(target) /home/sam/w/core/lts/out/Release/obj.target/v8_snapshot/geni/embedded.o
  AR(target) /home/sam/w/core/lts/out/Release/obj.target/tools/v8_gypfiles/libv8_snapshot.a
  TOUCH /home/sam/w/core/lts/out/Release/obj.target/tools/v8_gypfiles/v8_maybe_snapshot.stamp
  TOUCH d91d7401cc24d87f844dbc275fc67fb1cbbc9681.intermediate
  ACTION Generating node protocol sources from protocol json d91d7401cc24d87f844dbc275fc67fb1cbbc9681.intermediate
  LINK(target) /home/sam/w/core/lts/out/Release/cctest
  LINK(target) /home/sam/w/core/lts/out/Release/mkcodecache
  LINK(target) /home/sam/w/core/lts/out/Release/node_mksnapshot
  ACTION _home_sam_w_core_lts_node_gyp_node_target_run_mkcodecache /home/sam/w/core/lts/out/Release/obj/gen/node_code_cache.cc
  ACTION _home_sam_w_core_lts_node_gyp_node_target_node_mksnapshot /home/sam/w/core/lts/out/Release/obj/gen/node_snapshot.cc
  CXX(target) /home/sam/w/core/lts/out/Release/obj.target/node/gen/node_code_cache.o
  CXX(target) /home/sam/w/core/lts/out/Release/obj.target/node/gen/node_snapshot.o
  LINK(target) /home/sam/w/core/lts/out/Release/node
rm f12aef6ca9be3b6903ccca9065c46321958e5d57.intermediate dc2a6a8f2bf222b5a02a9df3d748ab02b9f73ccf.intermediate 8ce4ae39fa160c52d3938e994929629c6a3c704e.intermediate d91d7401c
c24d87f844dbc275fc67fb1cbbc9681.intermediate
make: Leaving directory '/home/sam/w/core/lts/out'

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. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

No branches or pull requests

2 participants