Skip to content

build: fix -j propagation to ninja - #53088

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:build-fix-ninja-jobs
May 25, 2024
Merged

build: fix -j propagation to ninja#53088
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:build-fix-ninja-jobs

Conversation

@tniessen

Copy link
Copy Markdown
Member

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

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.
@tniessentniessen added the build Issues and PRs related to build files or the CI. label May 21, 2024
@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label May 21, 2024

@aduh95aduh95 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@aduh95aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels May 23, 2024
@tniessen

Copy link
Copy Markdown
MemberAuthor

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.

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2024
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@lpinca

Copy link
Copy Markdown
Member

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

@tniessen

Copy link
Copy Markdown
MemberAuthor

@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).

@lpinca

Copy link
Copy Markdown
Member

It's your call.

@aduh95aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label May 25, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label May 25, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 19f0bca into nodejs:mainMay 25, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 19f0bca

targos pushed a commit that referenced this pull request Jun 1, 2024
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.
PR-URL: #53088
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request Jun 7, 2024
eliphazbouye pushed a commit to eliphazbouye/node that referenced this pull request Jun 20, 2024
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.
PR-URL: nodejs#53088
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
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.
PR-URL: #53088
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@marco-ippolitomarco-ippolito mentioned this pull request Jul 19, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.buildIssues and PRs related to build files or the CI.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@tniessen@nodejs-github-bot@lpinca@jasnell@aduh95