Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upDo not rely on file extensions after path canonicalization. #32828
Conversation
rust-highfive
assigned
nrc
Apr 8, 2016
This comment has been minimized.
This comment has been minimized.
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
|
Also, I'd like to nominate this fix for backporting into 1.8. |
GuillaumeGomez
reviewed
Apr 8, 2016
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | ||
| f.write_str(match *self { | ||
| CrateFlavor::Rlib => "rlib", | ||
| CrateFlavor::Dylib => "dylib" |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
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? |
alexcrichton
assigned
alexcrichton
and unassigned
nrc
Apr 8, 2016
This comment has been minimized.
This comment has been minimized.
|
No, I think it's the same issue. Wasn't aware #32317 existed. |
This comment has been minimized.
This comment has been minimized.
|
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 |
This comment has been minimized.
This comment has been minimized.
|
cc @taralx |
This comment has been minimized.
This comment has been minimized.
|
Added a test in case you want to go with this one. |
This comment has been minimized.
This comment has been minimized.
|
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). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Apr 13, 2016
bors
referenced this pull request
Apr 13, 2016
Merged
Doc fix: Update Cargo.toml in book/getting-started #32931
This comment has been minimized.
This comment has been minimized.
bors
merged commit cc3b6f2
into
rust-lang:master
Apr 13, 2016
bors
referenced this pull request
Apr 13, 2016
Merged
Deduplicate libraries on hash instead of filename. #32317
eddyb
added
the
beta-nominated
label
Apr 13, 2016
This comment has been minimized.
This comment has been minimized.
|
Nominating for backport on behalf of @taralx. |
This comment has been minimized.
This comment has been minimized.
|
Discussed in @rust-lang/compiler meeting. Is this fixing a regression or just some longstanding bug? |
This comment has been minimized.
This comment has been minimized.
|
This is technically a regression due to #12474. Before that, symlinks named *.rlib worked, regardless of the name of the thing they pointed to. |
This comment has been minimized.
This comment has been minimized.
|
@nikomatsakis I believe, (ias @taralx pointed out) that this is just fixing a longstanding bug |
This comment has been minimized.
This comment has been minimized.
In that case, I believe (based on consensus from previous @rust-lang/compiler meeting) we would prefer not to backport. |
nikomatsakis
removed
the
beta-nominated
label
Apr 27, 2016
This comment has been minimized.
This comment has been minimized.
|
@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.... |
vadimcn commentedApr 8, 2016
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