-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
Comments
An additional issue:
|
This comment has been minimized.
This comment has been minimized.
This is pretty painful on machines with slow link times while testing CI builds (so, not allowed to use ninja), because 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
|
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, butmake node
relinks bothnode
andcctest
(!?) every time it is run.make -C out node
is faster,butand 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 abovemake -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
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 leasttest-doc
relies on this), so aln -s out/Release/node node
is required formake test-doc
. As formake test
, it won't use the ninja build output, because it relies on .PHONY targets, so it always (effectively) does amake -C out all
.The text was updated successfully, but these errors were encountered: