node icon indicating copy to clipboard operation
node copied to clipboard

build: fix -j propagation to ninja

Open tniessen opened this issue 1 year ago • 5 comments

The expression containing MAKEFLAGS must be evaluated in a deferred context for the propagation to work in GNU make. Otherwise, regardless of the -j value passed to make, ninja will spawn a potentially greater number of parallel compilation tasks, which can quickly exhaust all available memory.

cc @nodejs/build

tniessen avatar May 21 '24 19:05 tniessen

Not sure how to test it, but the code logic LGTM

Without this patch, ./configure --ninja followed by make -j4 will display ninja -C .... With this patch, make -j4 will instead show nina -C ... -j4.

tniessen avatar May 23 '24 08:05 tniessen

CI: https://ci.nodejs.org/job/node-test-pull-request/59364/

nodejs-github-bot avatar May 23 '24 08:05 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/59368/

nodejs-github-bot avatar May 23 '24 11:05 nodejs-github-bot

CI: https://ci.nodejs.org/job/node-test-pull-request/59376/

nodejs-github-bot avatar May 23 '24 15:05 nodejs-github-bot

It might make sense to add a comment to prevent it from being reverted to the original version.

lpinca avatar May 23 '24 20:05 lpinca

@lpinca I tend to agree, but I am not sure if it's worth going through CI again (which presumably doesn't even cover this branch).

tniessen avatar May 24 '24 14:05 tniessen

It's your call.

lpinca avatar May 24 '24 14:05 lpinca

Landed in 19f0bcaa621f37023fbd35dc16c7a12f69e48345

nodejs-github-bot avatar May 25 '24 17:05 nodejs-github-bot