Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 22, 2021
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a627b4f
[bootstrap] Improve the error message when `ninja` is not found to li…
daira Sep 19, 2021
23d6437
Restore the suggestion to set `ninja = false`.
daira Sep 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/bootstrap/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1494,8 +1494,12 @@ impl Build {
{
eprintln!(
"
Couldn't find required command: ninja
You should install ninja, or set `ninja=false` in config.toml in the `[llvm]` section.
Couldn't find required command: ninja (or ninja-build)

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.

);
std::process::exit(1);
Expand Down