-
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
download-rustc: Fix x test core
on MacOS
#112382
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
let filtered_extensions = [ | ||
OsStr::new("rmeta"), | ||
OsStr::new("rlib"), | ||
OsStr::new(std::env::consts::DLL_EXTENSION), |
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.
technically this will be wrong when cross-compiling, but we don't support that today: https://github.com/rust-lang/rust/blob/50c80eb69e2f99cd431e97f080f38dfbf8bb3d9c/src/bootstrap/compile.rs#L1322-L1325
cc #110411
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.
Either we could list the whole list of extensions (dll
, so
, <whatever is used on macos>
, <whatever is used on other platforms>
), or at lease leave a FIXME.
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.
Either way r=me with the choice you make. This could also fix things in Windows IIUC.
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.
👍 FIXME sounds good, i'll do that. i don't want to try and support this without testing it, there's a dozen other things in bootstrap that are fundamentally broken when --host
doesn't match --build
.
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.
I meant that in the list, instead of using the platform-specific constant, using the list of extensions for the platforms that are supported.
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.
yeah i just don't think it's worth worrying about i guess - it's broken in so many other ways, and i don't know why you'd even want to use it.
before, this hardcoded `.so` as the extension for dynamically linked objects, which is incorrect everywhere except linux
50c80eb
to
a2ab47f
Compare
Thanks for the PR! |
Rollup of 6 pull requests Successful merges: - rust-lang#112076 (Fall back to bidirectional normalizes-to if no subst-relate candidate in alias-relate goal) - rust-lang#112122 (Add `-Ztrait-solver=next-coherence`) - rust-lang#112251 (rustdoc: convert `if let Some()` that always matches to variable) - rust-lang#112345 (fix(expand): prevent infinity loop in macro containing only "///") - rust-lang#112359 (Respect `RUST_BACKTRACE` for delayed bugs) - rust-lang#112382 (download-rustc: Fix `x test core` on MacOS) r? `@ghost` `@rustbot` modify labels: rollup
before, this hardcoded
.so
as the extension for dynamically linked objects, which is incorrect everywhere except linux.