nodejs / nodejs/node

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

Open
#25,135 4 comments 5 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

build confirmed-bug
Dominant language
JavaScript
Stars
122k
Forks
37.3k
Avg merge
4d 2h
Merged PRs (30d)
283

Description

  • Version: master
  • Platform:Linux samtu 4.18.0-12-generic #13-Ubuntu SMP Wed Nov 14 15:17:05 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: gyp/make

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: https://github.com/nodejs/node/blob/eef6504cf7b95ada170ec9878089ce57fde551d8/common.gypi#L549-L560, 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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the listed make and Ninja commands from the repository root, then inspect the generated Makefiles and the dependency settings in common.gypi. Compare the Make and Ninja dependency trees, including js2c and the top-level node symlink. Done means unchanged builds are no-ops, JavaScript changes rerun js2c, parallel test builds are reliable, and Ninja output works with the documented test commands.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
build-system
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.