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

Version: master
Platform:Linux samtu 4.18.0-12-generic Failed assert in AsyncWrap::MakeCallback(uint32_t, int, Handle) from test-timers-unref.js on ARM #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:

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.

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.

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'

There's been no further discussion or activity here in many years. Is this still relevant or can we go ahead and close this?

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	['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',
	],
	}],
	],

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

Description

Activity

sam-github commented on Dec 20, 2018

sam-github commented on Dec 28, 2018

sam-github commented on Nov 28, 2019

jasnell commented on Jan 22, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions