-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: fix building with ninja on linux #11348
Conversation
/cc @Fishrock123, @gibfahn (to test on OS X?). @sam-github (who also had this issue) |
4825a7b
to
043a16f
Compare
On Linux, `ninja` appears to place `libv8_base.a` inside `OBJ_DIR`, as opposed to `ninja` on OS X which places it outside of that directory. Furthermore, the expected `OBJ_DIR` value (`obj.target/`) is actually just `obj/` for `ninja`. This patch solves both of these issues by setting `OBJ_DIR` and `V8_BASE` to the correct values for `ninja` on Linux specifically. Fixes: nodejs#9861
Built on Linux for me. Doing some rebuilding to see if ninja is faster than make, LGTM |
Seems to work fine for me on macOS 10.10.5 |
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.
Works for me on macOS 10.12 (Sierra).
CI only to make sure there's no impact on regular build: https://ci.nodejs.org/job/node-test-pull-request/6454/ |
On Linux, `ninja` appears to place `libv8_base.a` inside `OBJ_DIR`, as opposed to `ninja` on OS X which places it outside of that directory. Furthermore, the expected `OBJ_DIR` value (`obj.target/`) is actually just `obj/` for `ninja`. This patch solves both of these issues by setting `OBJ_DIR` and `V8_BASE` to the correct values for `ninja` on Linux specifically. PR-URL: #11348 Fixes: #9861 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in 70afe9d |
On Linux, `ninja` appears to place `libv8_base.a` inside `OBJ_DIR`, as opposed to `ninja` on OS X which places it outside of that directory. Furthermore, the expected `OBJ_DIR` value (`obj.target/`) is actually just `obj/` for `ninja`. This patch solves both of these issues by setting `OBJ_DIR` and `V8_BASE` to the correct values for `ninja` on Linux specifically. PR-URL: nodejs#11348 Fixes: nodejs#9861 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]>
On Linux, `ninja` appears to place `libv8_base.a` inside `OBJ_DIR`, as opposed to `ninja` on OS X which places it outside of that directory. Furthermore, the expected `OBJ_DIR` value (`obj.target/`) is actually just `obj/` for `ninja`. This patch solves both of these issues by setting `OBJ_DIR` and `V8_BASE` to the correct values for `ninja` on Linux specifically. PR-URL: #11348 Fixes: #9861 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: James M Snell <[email protected]>
needs a backport PR to land on v6 or v4 |
On Linux,
ninja
appears to placelibv8_base.a
insideOBJ_DIR
, as opposed toninja
on OS X which places it outside of that directory. Furthermore, the expectedOBJ_DIR
value (obj.target/
) is actually justobj/
forninja
. This patch solves both of these issues by settingOBJ_DIR
andV8_BASE
to thecorrect values for
ninja
on Linux specifically.Fixes: #9861
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem
build