-
Notifications
You must be signed in to change notification settings - Fork 28.8k
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
build: add MAKEFLAGS="-j1" to node-gyp #9450
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/cc @bnoordhuis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first line of the commit log is slightly too long.
echo "\nRunning addons test $$PWD/$$dirname" ; \ | ||
$(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp --loglevel=$(LOGLEVEL) rebuild \ | ||
echo "\nBuilding addon $$PWD/$$dirname" ; \ | ||
MAKEFLAGS="-j1" $(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
env MAKEFLAGS="-j1" ...
? I believe what you have now is a bash-ism, not a posix-ism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah indeed, I'll correct this. Thanks
ec33fca
to
772be0f
Compare
Sorry, for some reason I though 52 characters was the limit but of course it is 50 (the 2 is for in other lines and then it's 72). Thanks |
@@ -166,8 +166,9 @@ test/addons/.buildstamp: config.gypi \ | |||
# Cannot use $(wildcard test/addons/*/) here, it's evaluated before | |||
# embedded addons have been generated from the documentation. | |||
for dirname in test/addons/*/; do \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about silencing this in the other commit and it landed before I got a chance to get back to it. Can we do it in this? I am okay without that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do it in this?
Absolutely, no problems.
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate.
ce36c39
to
fb4eafd
Compare
Rebased and new CI: https://ci.nodejs.org/job/node-test-pull-request/4789/ |
CI failure does not seem related to this change:
If there are no objections I'll merge this tomorrow. |
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: nodejs#9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Landed in: a504516 |
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: #9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
should this be backported? |
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: nodejs#9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: #9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: #9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: #9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: #9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently, when building the addons the following warning is displayed: make[2]: warning: jobserver unavailable: using -j1. Add `+' to parent make rule. Adding the MAKEFLAGS="-j1" to avoid the warning. Also updated the log message to say that it is building the addon and not running the test as I think that is more accurate. PR-URL: #9450 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
build
Description of change
Currently, when building the addons the following warning is displayed:
Adding the
MAKEFLAGS="-j1"
to avoid the warning.Also updated the log message to say that it is building the addon and
not running the test as I think that is more accurate.