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

Remove --ninja and --xcode configure options #467

Closed
wants to merge 1 commit into from

Conversation

bnoordhuis
Copy link
Member

@rvagg
Copy link
Member

rvagg commented Jan 16, 2015

/cc @thlorenz are you using the xcode config of from core?

@rvagg
Copy link
Member

rvagg commented Jan 16, 2015

@bnoordhuis does removing Ninja make it difficult at all if we ended up opting to embrace gn?

@rvagg
Copy link
Member

rvagg commented Jan 16, 2015

Actually that's provably a silly question, ignore that.

@chrisdickinson
Copy link
Contributor

LGTM pending @thlorenz's yea or nay re: xcode.

At the very least I don't foresee any tears shed over the loss of ninja support – no matter what, that can probably go.

@caitp
Copy link
Contributor

caitp commented Jan 16, 2015

what exactly is the benefit of killing off ninja here? (and, pro-xcode, it's less of a pain to debug the repl in xcode, no need to worry about capturing SIGTTIN and friends, it's just easier on people)

@bnoordhuis
Copy link
Member Author

@caitp The motivation is described in the commit log. In a nutshell, it's uncertain if people actually use it.

The --xcode flag was added because of nodejs/node-v0.x-archive#4020 (I think.) The goal there was to create fat binaries but the flag isn't used for that. That said, it's not much work to keep it around if people actually use it.

@rvagg
Copy link
Member

rvagg commented Jan 16, 2015

@thlorenz is doing some crazy stuff with xcode & node, e.g. nad, but I'm not sure if his work extends to core, I'm waiting on feedback from him but we may have to wait until next week for that.

@bnoordhuis I'm +1 on the ninja removal but -1 on xcode removal pending @thlorenz' feedback.

bnoordhuis added a commit that referenced this pull request Jan 17, 2015
It is unknown if there are any users of the ninja build and keeping it
around makes refactoring the build system more difficult.  It's partly
broken (or at least, deeply inefficient) because it touches out/Makefile
every time.

PR-URL: #467
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Added in commit ac04716 without explanation or bug number.  Unclear if
there are users.  Remove, reinstate if users complain.
@bnoordhuis bnoordhuis force-pushed the remove-configure-options branch from 875509d to b7400c6 Compare January 17, 2015 00:08
@bnoordhuis
Copy link
Member Author

Thanks, landed the first commit in de224d6. Waiting for @thlorenz's feedback.

@bnoordhuis
Copy link
Member Author

@caitp Forgot to mention, if support for ninja is truly a desired feature, there are better ways to integrate it into the build. The way it was done before just wasn't very great.

@caitp
Copy link
Contributor

caitp commented Jan 17, 2015

as noted by others, since gn isn't really available to people easily without chromium, there's not a very great way to integrate ninja yet apart from what (was) already in the tree.

As for xcode, I'm already using this to simplify llvm debugging (the output in the integrated terminal looks awful, but at least it works without dealing with llvm annoyingness), I expect I'm not the only one

@bnoordhuis
Copy link
Member Author

Okay, let's keep it. Closing the PR.

@bnoordhuis bnoordhuis closed this Jan 17, 2015
@thlorenz
Copy link
Contributor

Maybe too late, but FWIW I'm a big fan of also keeping the ninja generator around.
Building with ninja is just sooo much faster.

Seeing that the --ninja switch got removed now is there another way I can generate a .ninja setup?
Pulling @trevnorris in since he may know of an alternative and as far as I remember actually showed me that option and how fast it is.

@bnoordhuis
Copy link
Member Author

@thlorenz ./configure && python tools/gyp_node.py -f ninja && ninja -C out/Release

@thlorenz
Copy link
Contributor

Thanks.

Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 27, 2016
Ninja is a build backend supported by gyp which is much faster than
Make and is able to parallelize builds across all of the available
cores very well. On my machine, this reduces the average build time
from 5:14 minutes to 4:33 minutes.

Refs: nodejs#467
Refs: de224d6
PR-URL: nodejs#6780
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request May 30, 2016
Ninja is a build backend supported by gyp which is much faster than
Make and is able to parallelize builds across all of the available
cores very well. On my machine, this reduces the average build time
from 5:14 minutes to 4:33 minutes.

Refs: nodejs#467
Refs: de224d6
PR-URL: nodejs#6780
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
Ninja is a build backend supported by gyp which is much faster than
Make and is able to parallelize builds across all of the available
cores very well. On my machine, this reduces the average build time
from 5:14 minutes to 4:33 minutes.

Refs: #467
Refs: de224d6
PR-URL: #6780
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Ninja is a build backend supported by gyp which is much faster than
Make and is able to parallelize builds across all of the available
cores very well. On my machine, this reduces the average build time
from 5:14 minutes to 4:33 minutes.

Refs: #467
Refs: de224d6
PR-URL: #6780
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Ninja is a build backend supported by gyp which is much faster than
Make and is able to parallelize builds across all of the available
cores very well. On my machine, this reduces the average build time
from 5:14 minutes to 4:33 minutes.

Refs: #467
Refs: de224d6
PR-URL: #6780
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Ninja is a build backend supported by gyp which is much faster than
Make and is able to parallelize builds across all of the available
cores very well. On my machine, this reduces the average build time
from 5:14 minutes to 4:33 minutes.

Refs: #467
Refs: de224d6
PR-URL: #6780
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Ninja is a build backend supported by gyp which is much faster than
Make and is able to parallelize builds across all of the available
cores very well. On my machine, this reduces the average build time
from 5:14 minutes to 4:33 minutes.

Refs: #467
Refs: de224d6
PR-URL: #6780
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants