-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Do not rely on file extensions after path canonicalization. #32828
Conversation
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
Also, I'd like to nominate this fix for backporting into 1.8. |
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
f.write_str(match *self { | ||
CrateFlavor::Rlib => "rlib", | ||
CrateFlavor::Dylib => "dylib" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comma at the end of this line please?
This seems pretty similar to #32317 in scope, but perhaps somewhat different in terms of problem solved? Could you also be sure to add a test for this as well? |
No, I think it's the same issue. Wasn't aware #32317 existed. |
I think that #32317 is arguably more general, but I somewhat prefer this tactic as it solves the problem at hand without changing the semantics that much |
cc @taralx |
Added a test in case you want to go with this one. |
This fixes the problem that I have specifically, although #32317 is probably also worth doing long-term (once I work out what's broken about Windows with it). |
Do not rely on file extensions after path canonicalization. Rustc does not recognize libraries which are symlinked to files having extension other than .rlib. The problem is that find_library_crate calls fs::canonicalize on found library paths, but then the resulting path is passed to get_metadata_section, which assumes it will end in ".rlib" if it's an rlib (from https://internals.rust-lang.org/t/is-library-path-canonicalization-worth-it/3206). cc #29433
Nominating for backport on behalf of @taralx. |
Discussed in @rust-lang/compiler meeting. Is this fixing a regression or just some longstanding bug? |
This is technically a regression due to #12474. Before that, symlinks named *.rlib worked, regardless of the name of the thing they pointed to. |
@nikomatsakis I believe, (ias @taralx pointed out) that this is just fixing a longstanding bug |
In that case, I believe (based on consensus from previous @rust-lang/compiler meeting) we would prefer not to backport. |
@rust-lang/compiler: Can you please reconsider? This prevents Rust builds from working on our build cluster. I believe this is a pretty non-invasive change, and the beta cycle has just started.... |
Rustc does not recognize libraries which are symlinked to files having extension other than .rlib. The problem is that find_library_crate calls fs::canonicalize on found library paths, but then the resulting path is passed to get_metadata_section, which assumes it will end in ".rlib" if it's an rlib (from https://internals.rust-lang.org/t/is-library-path-canonicalization-worth-it/3206).
cc #29433