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.

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

Don't require all build script output to be utf-8 #2560

Merged
merged 1 commit into from
Apr 20, 2016

Conversation

alexcrichton
Copy link
Member

Build scripts often stream output of native build systems like cmake/make and
those aren't always guaranteed to produce utf-8 output. For example German
MSVC cmake build has been reported to print non-utf-8 umlauts.

This commit instead only attempts to interpret each line as utf-8 rather than
the entire build script output. All non-utf-8 output is ignored.

Closes #2556

Build scripts often stream output of native build systems like cmake/make and
those aren't always guaranteed to produce utf-8 output. For example  German
MSVC cmake build has been reported to print non-utf-8 umlauts.

This commit instead only attempts to interpret each line as utf-8 rather than
the entire build script output. All non-utf-8 output is ignored.

Closes rust-lang#2556
@alexcrichton
Copy link
Member Author

r? @brson

@rust-highfive rust-highfive assigned brson and unassigned huonw Apr 11, 2016
@rust-highfive
Copy link

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@brson
Copy link
Contributor

brson commented Apr 20, 2016

@bors r+

@bors
Copy link
Contributor

bors commented Apr 20, 2016

📌 Commit 020db0d has been approved by brson

@bors
Copy link
Contributor

bors commented Apr 20, 2016

⌛ Testing commit 020db0d with merge 1c9a1ef...

bors added a commit that referenced this pull request Apr 20, 2016
Don't require all build script output to be utf-8

Build scripts often stream output of native build systems like cmake/make and
those aren't always guaranteed to produce utf-8 output. For example  German
MSVC cmake build has been reported to print non-utf-8 umlauts.

This commit instead only attempts to interpret each line as utf-8 rather than
the entire build script output. All non-utf-8 output is ignored.

Closes #2556
@bors
Copy link
Contributor

bors commented Apr 20, 2016

@bors bors merged commit 020db0d into rust-lang:master Apr 20, 2016
@alexcrichton alexcrichton deleted the not-all-utf8 branch April 21, 2016 16:30
@luser
Copy link
Contributor

luser commented Jul 27, 2016

Random comment on an old PR, but I hit this same issue with my sccache2 tool that wraps compiler execution. For the MSVC case, what you really want to do is convert from the system codepage, which works fine. (In general the problem is sort of unsolvable unless you want to just take the bytes from the program's stdout and write them using WriteConsoleA.) The code I used is here:
https://github.com/luser/sccache2/blob/ece0aee662dfd095093ef20d884461db22de0614/src/compiler/msvc.rs#L40

@alexcrichton
Copy link
Member Author

@luser just to clarify, did you hit this assertion or see any bad behavior because of this? I was hoping that this would cause Cargo to avoid all non-utf8 output and only look at lines which are utf-8, which in theory all cargo:foo=bar lines should be.

@luser
Copy link
Contributor

luser commented Jul 27, 2016

No, sorry, I mean "my project had the same problem dealing with non-UTF-8 output from MSVC".

@alexcrichton
Copy link
Member Author

Ah ok, thanks for the clarification! Also, thanks for the link!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants