FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build: fix -j propagation to ninja by tniessen · Pull Request #53088 · nodejs/node · GitHub

/ node Public

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

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

aduh95 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

aduh95 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels May 23, 2024

Copy link
Copy Markdown
Member Author

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-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 23, 2024

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Copy Markdown
Collaborator

lpinca commented May 23, 2024

Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member Author

@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 commented May 24, 2024

Copy link
Copy Markdown
Member

It's your call.

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

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>
RafaelGSS mentioned this pull request Jun 7, 2024
soophoo pushed a commit to soophoo/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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL