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.

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

Jump to bottom

Add remark for missing llvm-tools component re. rustc_private linker failures related to not finding LLVM libraries #137931

Merged
merged 1 commit into from
Mar 10, 2025
Merged

Add remark for missing llvm-tools component re. rustc_private linker failures related to not finding LLVM libraries #137931

merged 1 commit into from
Mar 10, 2025

Conversation

xizheyin
Copy link
Contributor

@xizheyin xizheyin commented Mar 3, 2025

Fixes #137421

@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2025

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 3, 2025
Noratrieb
Noratrieb reviewed Mar 3, 2025
View reviewed changes
compiler/rustc_codegen_ssa/src/errors.rs Outdated
@@ -469,6 +469,9 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for LinkingFailed<'_> {
diag.note(fluent::codegen_ssa_use_cargo_directive);
}
}
if self.linker_path.display().to_string().contains("cc") {
Copy link
Member

Choose a reason for hiding this comment

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

this will show up for every linker error, when though almost all linker errors are not caused by this.

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 change the condition

if self.escaped_output.contains("unable to find library -lLLVM-") {

Is that correct?

Copy link
Member

Choose a reason for hiding this comment

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

Not really, this only applies to rustc_private, not all links against LLVM.
And doing this kind of linker string error parsing is very unreliable in general and should only be done very carefully if at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, this does require careful consideration. Do you have any good suggestions? Do I need to check the rustc_private feature?

@jieyouxu
Copy link
Member

jieyouxu commented Mar 3, 2025

I do not think we should be making this suggestion for a few reasons:

  1. #![feature(rustc_private)] is a perma-unstable feature to "allow[..] access to unstable internal compiler crates such as rustc_driver." I think it's reasonable to expect the user to know how to "operate" this feature correctly if they want to rely on perma-unstable compiler details1.
  2. Matching on linker error messages is very fragile, and there may be many potential causes for "unable to find library -lLLVM-", not just llvm-tools not being installed. For instance, you may have messed with library search paths.
  3. I would maybe consider adding that as an remark somewhere in rustc-dev-guide or maybe on the #![feature(rustc_private)] doc itself, but this has less to do with rustc itself and more with how rustc-dev vs llvm-tools component are shipped via rustup.

Footnotes

  1. The docs for this feature is quite sparse (at https://doc.rust-lang.org/nightly/unstable-book/language-features/rustc-private.html), partly because it's perma-unstable and expected to change even in breaking fashion. ↩

@xizheyin
Copy link
Contributor Author

xizheyin commented Mar 3, 2025

Good point. However, could this suggestion serve as a reminder of a potential solution? It doesn't seem to get timely feedback if it's written in the documentation.

@fmease fmease assigned jieyouxu and unassigned fmease Mar 3, 2025
@Noratrieb
Copy link
Member

Noratrieb commented Mar 3, 2025 •
edited
Loading

#![feature(rustc_private)] is a perma-unstable feature to "allow[..] access to unstable internal compiler crates such as rustc_driver." I think it's reasonable to expect the user to know how to "operate" this feature correctly if they want to rely on perma-unstable compiler details

i disagree with this point. we should still help people with this confusing thing if possible. i've seen people that were very confused by this (and so was I trying to help them!). but I yes, I don't like linker string matching so I'm not very inclined to merge this PR's solution (especially because as implemented today it will likely be incorrect in many cases where it happens).

@jieyouxu
Copy link
Member

jieyouxu commented Mar 3, 2025 •
edited
Loading

That seems fair. I think I would rather still see this as docs in rustc-dev-guide or docs on the unstable feature itself. I don't really want to merge the string matching here either (I don't really have a good heuristic alternative, because AFAIK rustc won't have enough info here).

@xizheyin maybe change this PR into documenting this in unstable-features book and/or rustc-dev-guide, I'd r+ that.

@xizheyin
Copy link
Contributor Author

xizheyin commented Mar 3, 2025

I got it, if compiler can't provide sufficient information, we have to record it in the document. But, could we provide a web link to the docs where record some potential solutions?

@Noratrieb
Copy link
Member

the problem here is that it's basically impossible to detect whether this specific problem is occurring. any detection algorithm will be wrong in many cases, which we don't want.

@jieyouxu
Copy link
Member

jieyouxu commented Mar 3, 2025

To further elaborate, I don't want to add this as a diagnostics because it's also quite misleading if the cause isn't that llvm-tools isn't installed.

@xizheyin
Copy link
Contributor Author

xizheyin commented Mar 4, 2025

Ok, I'll close it. Maybe a note in the documentation would be a better choice.

@xizheyin xizheyin closed this Mar 4, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Mar 4, 2025

Ok, I'll close it. Maybe a note in the documentation would be a better choice.

Sure. Feel free to r/? me if you submit a doc PR.

@xizheyin
Copy link
Contributor Author

xizheyin commented Mar 5, 2025

Ok, I have submit a PR in rustc-dev-guide. By the way, what about alloc a error-code for this error? @jieyouxu @Noratrieb

@xizheyin xizheyin reopened this Mar 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@rustbot rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2025
@rustbot

This comment has been minimized.

Sign in to view
@xizheyin
Copy link
Contributor Author

@rustbot label -S-waiting-on-review

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 10, 2025
@rustbot rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2025
@xizheyin
Copy link
Contributor Author

r? @jieyouxu

@rustbot
Copy link
Collaborator

rustbot commented Mar 10, 2025

Could not assign reviewer from: jieyouxu.
User(s) jieyouxu are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@jieyouxu jieyouxu changed the title Add Required Components for rustc_private in unstable book Add remark for missing llvm-tools component re. rustc_private linker failures related to not finding LLVM libraries Mar 10, 2025
jieyouxu
jieyouxu approved these changes Mar 10, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks. Can you squash your commits into one?

@xizheyin
Copy link
Contributor Author

It's done. @jieyouxu

@jieyouxu
Copy link
Member

Cheers.
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 10, 2025

📌 Commit adb94ce has been approved by jieyouxu

It is now in the queue for this repository.

@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 Mar 10, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jieyouxu
Copy link
Member

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 10, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Mar 10, 2025 •
edited
Loading

You'll have to change the code blocks to use

```text
```

to avoid the book interpreting them as valid Rust code and trying to run them as doctests.

@jieyouxu jieyouxu assigned jieyouxu and unassigned nnethercote Mar 10, 2025
@xizheyin
Copy link
Contributor Author

I modified and squashed the commit. Thank you! @jieyouxu

@xizheyin
Add remark for missing llvm-tools component re. rustc_private linker …
dc576cb 
…failures related to not finding LLVM libraries

Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
@jieyouxu
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 10, 2025

📌 Commit dc576cb has been approved by jieyouxu

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 10, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2025
@bors
Auto merge of rust-lang#138310 - matthiaskrgr:rollup-zvbpuei, r=matth…
9fb94b3 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#137931 (Add remark for missing `llvm-tools` component re. `rustc_private` linker failures related to not finding LLVM libraries)
 - rust-lang#138138 (Pass `InferCtxt` to `InlineAsmCtxt` to properly taint on error)
 - rust-lang#138223 (Fix post-merge workflow)
 - rust-lang#138268 (Handle empty test suites in GitHub job summary report)
 - rust-lang#138278 (Delegation: fix ICE with invalid `MethodCall` generation)
 - rust-lang#138281 (Fix O(tests) stack usage in edition 2024 mergeable doctests)
 - rust-lang#138305 (Subtree update of `rust-analyzer`)
 - rust-lang#138306 (Revert "Use workspace lints for crates in `compiler/` rust-lang#138084")

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 150e88c into rust-lang:master Mar 10, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 10, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2025
@rust-timer
Unrolled build for rust-lang#137931
d09b229 
Rollup merge of rust-lang#137931 - xizheyin:issue-137421, r=jieyouxu

Add remark for missing `llvm-tools` component re. `rustc_private` linker failures related to not finding LLVM libraries

Fixes rust-lang#137421
@xizheyin xizheyin deleted the issue-137421 branch March 11, 2025 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Noratrieb Noratrieb Noratrieb left review comments

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.

rustc_private linker error when llvm-tools is not installed
8 participants
@xizheyin @rustbot @jieyouxu @Noratrieb @bors @rust-log-analyzer @nnethercote @fmease