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

[bootstrap] Improve the error message when ninja is not found to link to installation instructions #89096

Merged
merged 2 commits into from
Sep 22, 2021

Conversation

daira
Copy link
Contributor

@daira daira commented Sep 19, 2021

fixes #89091

Signed-off-by: Daira Hopwood daira@jacaranda.org

…nk to installation instructions.

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@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 @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2021
You should install ninja as described at
<https://github.com/ninja-build/ninja/wiki/Pre-built-Ninja-packages>,
or set `download-ci-llvm = true` in the `[llvm]` section of `config.toml`
to download LLVM rather than building it.
"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this last suggestion is great - it might be useful in addition to suggesting ninja = false, but it shouldn't replace it altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding "ninja = false" didn't actually do anything as far as I could see. Is it working properly?

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be working ok. I think an issue is that if you run x.py once with the wrong ninja, then that gets cached in CMakeCache.txt, and then if you set ninja=false, the cached setup remains. Did you delete the llvm directory after changing the configuration? If not, then I suspect that is the cause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's consistent with what I saw. What can we do to improve that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to include both suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

What can we do to improve that?

Sorry, I'm not too familiar with cmake. I would maybe look at removing CMakeCache.txt at the beginning of the build, but would need to check with the regular llvm developers if that would cause a problem.

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 19, 2021
src/bootstrap/lib.rs Outdated Show resolved Hide resolved
@jyn514
Copy link
Member

jyn514 commented Sep 19, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 19, 2021

📌 Commit 23d6437 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2021
Rollup of 12 pull requests

Successful merges:

 - rust-lang#88795 (Print a note if a character literal contains a variation selector)
 - rust-lang#89015 (core::ascii::escape_default: reduce struct size)
 - rust-lang#89078 (Cleanup: Remove needless reference in ParentHirIterator)
 - rust-lang#89086 (Stabilize `Iterator::map_while`)
 - rust-lang#89096 ([bootstrap] Improve the error message when `ninja` is not found to link to installation instructions)
 - rust-lang#89113 (dont `.ensure()` the `thir_abstract_const` query call in `mir_build`)
 - rust-lang#89114 (Fixes a technicality regarding the size of C's `char` type)
 - rust-lang#89115 (:arrow_up: rust-analyzer)
 - rust-lang#89126 (Fix ICE when `indirect_structural_match` is allowed)
 - rust-lang#89141 (Impl `Error` for `FromSecsError` without foreign type)
 - rust-lang#89142 (Fix match for placeholder region)
 - rust-lang#89147 (add case for checking const refs in check_const_value_eq)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a8633eb into rust-lang:master Sep 22, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 22, 2021
flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 28, 2021
Rollup of 12 pull requests

Successful merges:

 - rust-lang#88795 (Print a note if a character literal contains a variation selector)
 - rust-lang#89015 (core::ascii::escape_default: reduce struct size)
 - rust-lang#89078 (Cleanup: Remove needless reference in ParentHirIterator)
 - rust-lang#89086 (Stabilize `Iterator::map_while`)
 - rust-lang#89096 ([bootstrap] Improve the error message when `ninja` is not found to link to installation instructions)
 - rust-lang#89113 (dont `.ensure()` the `thir_abstract_const` query call in `mir_build`)
 - rust-lang#89114 (Fixes a technicality regarding the size of C's `char` type)
 - rust-lang#89115 (:arrow_up: rust-analyzer)
 - rust-lang#89126 (Fix ICE when `indirect_structural_match` is allowed)
 - rust-lang#89141 (Impl `Error` for `FromSecsError` without foreign type)
 - rust-lang#89142 (Fix match for placeholder region)
 - rust-lang#89147 (add case for checking const refs in check_const_value_eq)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
7 participants