Use an UTF-8 locale for the linker. #74416
Conversation
|
r? @oli-obk (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| linker.env("LC_ALL", "C"); | ||
| // We use an UTF-8 locale, as the generic C locale disables support for non-ASCII | ||
| // bytes in filenames on some platforms. | ||
| linker.env("LC_ALL", "en_US.UTF-8"); |
There was a problem hiding this comment.
Wouldn't C.UTF-8 be better here?
There was a problem hiding this comment.
C.UTF-8 is a non-standard locale invented by Debian: commercialhaskell/stack#856, https://sources.debian.org/src/glibc/2.31-1/debian/patches/localedata/locale-C.diff/
There was a problem hiding this comment.
C.UTF-8 is making its way upstream.
And you can't count on en_US.UTF-8 existing.
|
is it possible to add a test for this? Maybe in https://github.com/rust-lang/rust/tree/master/src/test/codegen ? |
|
On Fri, Jul 17, 2020 at 03:38:39AM -0700, Oliver Scherer wrote:
is it possible to add a test for this? Maybe in https://github.com/rust-lang/rust/tree/master/src/test/codegen ?
I don't see any way to meaningfully test this. UTF-8 filenames
themselves are already tested, but this issue only shows up on specific
platforms.
Would it really be worth it to assert that the linker is ran with the
LC_ALL value being set here?
|
And I'm guessing we have no CI for these platforms, so it was never noticed?
No that's ok, since this PR doesn't regress existing platforms and is tested on these platforms, the correct passing of the env var is essentially already tested. |
Indeed. The platform I am aware of is Guix - I noticed this issue when I tried packaging the new stable release, and cargo's |
|
@bors r+ rollup |
|
📌 Commit 2ff13d9 has been approved by |
bors
added
S-waiting-on-bors
|
Thanks for the thorough explanations! |
…i-obk Use an UTF-8 locale for the linker. Using a `C` locale breaks unicode filenames on Guix, where the linker is wrapped by a Guile program.
…arth Rollup of 18 pull requests Successful merges: - rust-lang#71670 (Enforce even more the code blocks attributes check through rustdoc) - rust-lang#73930 (Make some Option methods const) - rust-lang#74009 (Fix MinGW `run-make-fulldeps` tests) - rust-lang#74056 (Add Arguments::as_str().) - rust-lang#74169 (Stop processing unreachable blocks when solving dataflow) - rust-lang#74251 (Teach bootstrap about target files vs target triples) - rust-lang#74288 (Fix src/test/run-make/static-pie/test-aslr.rs) - rust-lang#74300 (Use intra-doc links in core::iter module) - rust-lang#74364 (add lazy normalization regression tests) - rust-lang#74368 (Add CSS tidy check) - rust-lang#74394 (Remove leftover from emscripten fastcomp support) - rust-lang#74411 (Don't assign `()` to `!` MIR locals) - rust-lang#74416 (Use an UTF-8 locale for the linker.) - rust-lang#74424 (Move hir::Place to librustc_middle/hir) - rust-lang#74428 (docs: better demonstrate that None values are skipped as many times a…) - rust-lang#74438 (warn about uninitialized multi-variant enums) - rust-lang#74440 (Fix Arc::as_ptr docs) - rust-lang#74452 (intra-doc links: resolve modules in the type namespace) Failed merges: r? @ghost
|
This seems to have caused a significant compiler performance regression, so I've opened a PR that reverts this at #74478 |
…e-utf8, r=Mark-Simulacrum Revert "Use an UTF-8 locale for the linker." Reverts rust-lang#74416 This is suspected to have caused significant compile time regressions: https://perf.rust-lang.org/compare.html?start=39d5a61f2e4e237123837f5162cc275c2fd7e625&end=d3df8512d2c2afc6d2e7d8b5b951dd7f2ad77b02&stat=instructions:u
Original message: Rollup merge of rust-lang#74478 - rust-lang:revert-74416-linker-locale-utf8, r=Mark-Simulacrum Revert "Use an UTF-8 locale for the linker." Reverts rust-lang#74416 This is suspected to have caused significant compile time regressions: https://perf.rust-lang.org/compare.html?start=39d5a61f2e4e237123837f5162cc275c2fd7e625&end=d3df8512d2c2afc6d2e7d8b5b951dd7f2ad77b02&stat=instructions:u
Using a
Clocale breaks unicode filenames on Guix, where the linker is wrapped by a Guile program.