-
Notifications
You must be signed in to change notification settings - Fork 30k
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: build addons in parallel on Windows #21403
Conversation
Port nodejs#21155 to vcbuild.bat
On my box this reduces addon build time from 100 to 40 seconds. |
setlocal | ||
set npm_config_nodedir=%~dp0 | ||
"%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\addons" | ||
if errorlevel 1 exit /b 1 |
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.
Should this be a neq 0
test like the code block it replaces?
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.
This is more CMD
idiomatic (https://ss64.com/nt/if.html): if errorlevel NUM
checks if %errorlevel%
if equal or greater then NUM
(notice errorlevel
is not dereferenced with %
or !!
)
But AFAIK the last 1
is unnecessary (exit /b
will propagate the errorlevel). https://ss64.com/nt/exit.html
EXIT without an ExitCode acts the same as goto:eof and will not alter the %ERRORLEVEL%
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 errorlevel
be negative?
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, but I think we want the same thing for the N-API addons?
Landed in c403eeb |
Port #21155 to vcbuild.bat
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes/cc @addaleax @nodejs/build-files @nodejs/platform-windows