Skip to content
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

Make it possible to build iojs with ninja #2065

Closed
wants to merge 1 commit into from
Closed

Make it possible to build iojs with ninja #2065

wants to merge 1 commit into from

Conversation

yury-s
Copy link

@yury-s yury-s commented Jun 26, 2015

This change fixes link-time error when building iojs with ninja instead of make. The following commands now work fine:

$ ./tools/gyp_node.gyp -f ninja
$ ninja -C out/Release iojs -j100

The patch is going to be merged into gyp (https://codereview.chromium.org/1209553002/). The more details about why this change is necessary can be found in the mentioned codereview.

@brendanashworth brendanashworth added the build Issues and PRs related to build files or the CI. label Jun 26, 2015
@Fishrock123
Copy link
Contributor

It is currently possible to build io.js with ninja (I do it all the time), but make will still do some steps if you run it, which is a little annoying for things like make test.

I have an alias set up to do: ./configure && tools/gyp_node.py -f ninja && ninja -C out/Release && ln -fs out/Release/iojs iojs

@yury-s does this fix that issue where make will still do extra build stuff if you run it, or?

@yury-s
Copy link
Author

yury-s commented Jun 29, 2015

@Fishrock123 which platform are you on (I suspect it may work on Mac because of different toolset used there, did't have chance to check)?

The command line you specified fails for me on linux. ninja -C out/Release fails with tons of 'multiple symbol definition' errors:
.....
/usr/lib/gcc/x86_64-linux-gnu/4.8/libgcc.a(morestack.o):(.text+0x157): first defined here
/usr/lib/gcc/x86_64-linux-gnu/4.8/libgcc.a(morestack.o): In function __morestack_make_guard': (.text+0x161): multiple definition of__morestack_make_guard'
/usr/lib/gcc/x86_64-linux-gnu/4.8/libgcc.a(morestack.o):(.text+0x161): first defined here
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

btw, the gyp patch I mentioned has landed in the gyp repository. I'm not sure how often iojs' copy of gyp is updated though.

@bnoordhuis
Copy link
Member

#2074 - it was about time we upgraded again anyway.

shigeki pushed a commit to shigeki/node that referenced this pull request Jun 29, 2015
Update GYP to HEAD, omit the docs/ and test/ directories.

Fixes: nodejs#2065
@Fishrock123
Copy link
Contributor

@Fishrock123 which platform are you on (I suspect it may work on Mac because of different toolset used there, did't have chance to check)?

Ah, possibly, yes. I use Ninja 1.5.3 on OS X 10.10.3

bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Jun 30, 2015
Includes improved support for VS 2015[0] and makes it possible to build
with ninja again[1].

[0] https://codereview.chromium.org/1112753003
[1] https://codereview.chromium.org/1209553002

Fixes: nodejs#2065
PR-URL: nodejs#2074
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
@bnoordhuis
Copy link
Member

Fixed by 99cbbc0.

@bnoordhuis bnoordhuis closed this Jun 30, 2015
mscdex pushed a commit to mscdex/io.js that referenced this pull request Jul 9, 2015
Includes improved support for VS 2015[0] and makes it possible to build
with ninja again[1].

[0] https://codereview.chromium.org/1112753003
[1] https://codereview.chromium.org/1209553002

Fixes: nodejs#2065
PR-URL: nodejs#2074
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants