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

llvm, rt: build using the Ninja generator if available #34606

Merged
merged 2 commits into from
Jul 17, 2016

Conversation

mathstuf
Copy link
Contributor

@mathstuf mathstuf commented Jul 2, 2016

The Ninja generator generally builds much faster than make. It may also
be used on Windows to have a vast speed improvement over the Visual
Studio generators.

Currently hidden behind an --enable-ninja flag because it does not
obey the top-level -j or -l flags given to make.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@mathstuf mathstuf force-pushed the llvm-with-ninja branch 2 times, most recently from 838cea8 to 7c56d19 Compare July 2, 2016 00:17
@brson
Copy link
Contributor

brson commented Jul 2, 2016

@alexcrichton Do you think we should also add this to rustbuild at the same time?

@TimNN
Copy link
Contributor

TimNN commented Jul 2, 2016

@brson Rustbuild already has support for building llvm using ninja, if I remember correctly,

@alexcrichton
Copy link
Member

Thanks for the PR @mathstuf! And yeah @brson I think it'd be good to implement this there as well. Right now rustbuild has support for ninja basically in the same form as this, but we probably want to hook them up.

@mathstuf Could you update this match statement so rustbuild can read off the --enable-ninja option as well?

Other than that though this looks good to me!

@@ -68,6 +68,7 @@ pub struct Config {
pub rustc: Option<PathBuf>,
pub cargo: Option<PathBuf>,
pub local_rebuild: bool,
pub ninja: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathstuf: There already is a ninja field defined in this struct (see line 41)

@alexcrichton
Copy link
Member

Thanks @mathstuf! Could you also squash the commits together? Other than that r=me

@nikomatsakis
Copy link
Contributor

@bors r=acrichto

@bors
Copy link
Contributor

bors commented Jul 7, 2016

📌 Commit 33d4594 has been approved by acrichto

@bors
Copy link
Contributor

bors commented Jul 7, 2016

☔ The latest upstream changes (presumably #34677) made this pull request unmergeable. Please resolve the merge conflicts.

mathstuf added 2 commits July 7, 2016 21:10
The Ninja generator generally builds much faster than make. It may also
be used on Windows to have a vast speed improvement over the Visual
Studio generators.

Currently hidden behind an `--enable-ninja` flag because it does not
obey the top-level `-j` or `-l` flags given to `make`.
The Visual Studio generators create a `clean` target that we can use.
@mathstuf
Copy link
Contributor Author

Ping?

@alexcrichton
Copy link
Member

Oh oops, sorry @mathstuf! Feel free to ping a PR though whenever it's updated, github only recently started sending out notifications and I feel like I may not always get them...

@bors: r+ b9a3590

@mathstuf
Copy link
Contributor Author

Ah, I've been thrilled to not have to do that anymore, so I've been trying to kick that habit.

@bors
Copy link
Contributor

bors commented Jul 16, 2016

⌛ Testing commit b9a3590 with merge 257f0de...

@bors
Copy link
Contributor

bors commented Jul 16, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@alexcrichton
Copy link
Member

@bors: retry

  • Accidental merge, now reverted

On Sat, Jul 16, 2016 at 1:14 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-gnu-32-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt-rustbuild/builds/1776


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34606 (comment), or mute
the thread
/~https://github.com/notifications/unsubscribe-auth/AAD95CyV1fjBtDn71HjtnZie3t17S_Scks5qWTuigaJpZM4JDjtn
.

@bors
Copy link
Contributor

bors commented Jul 17, 2016

⌛ Testing commit b9a3590 with merge 34d7f7e...

bors added a commit that referenced this pull request Jul 17, 2016
llvm, rt: build using the Ninja generator if available

The Ninja generator generally builds much faster than make. It may also
be used on Windows to have a vast speed improvement over the Visual
Studio generators.

Currently hidden behind an `--enable-ninja` flag because it does not
obey the top-level `-j` or `-l` flags given to `make`.
@bors bors merged commit b9a3590 into rust-lang:master Jul 17, 2016
@mathstuf mathstuf deleted the llvm-with-ninja branch July 18, 2016 15:41
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.

7 participants