Skip to content

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

Merged
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:not-all-utf8
Apr 20, 2016
Merged

Don't require all build script output to be utf-8#2560
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:not-all-utf8

Conversation

@alexcrichton

Copy link
Copy Markdown
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
Copy Markdown
Member Author

r? @brson

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

Copy link
Copy Markdown

r? @huonw

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

@brson

brson commented Apr 20, 2016

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Apr 20, 2016

Copy link
Copy Markdown
Contributor

📌 Commit 020db0d has been approved by brson

@bors

bors commented Apr 20, 2016

Copy link
Copy Markdown
Contributor

⌛ 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

bors commented Apr 20, 2016

Copy link
Copy Markdown
Contributor

@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

luser commented Jul 27, 2016

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

luser commented Jul 27, 2016

Copy link
Copy Markdown
Contributor

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

@alexcrichton

Copy link
Copy Markdown
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