From 020db0de96aa1956007c4c1571593bbf63dc2740 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 11 Apr 2016 10:06:47 -0700
Subject: [PATCH] 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
---
 src/cargo/ops/cargo_rustc/custom_build.rs | 17 ++++++------
 src/cargo/util/paths.rs                   | 13 ++++++++-
 tests/test_cargo_compile_custom_build.rs  | 33 +++++++++++++++++++++++
 3 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs
index 929e54b932a..85ace677b10 100644
--- a/src/cargo/ops/cargo_rustc/custom_build.rs
+++ b/src/cargo/ops/cargo_rustc/custom_build.rs
@@ -6,7 +6,7 @@ use std::str;
 use std::sync::{Mutex, Arc};
 
 use core::PackageId;
-use util::{CargoResult, human, Human};
+use util::{CargoResult, Human};
 use util::{internal, ChainError, profile, paths};
 use util::Freshness;
 
@@ -213,10 +213,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
         // This is also the location where we provide feedback into the build
         // state informing what variables were discovered via our script as
         // well.
-        let output = try!(str::from_utf8(&output.stdout).map_err(|_| {
-            human("build script output was not valid utf-8")
-        }));
-        let parsed_output = try!(BuildOutput::parse(output, &pkg_name));
+        let parsed_output = try!(BuildOutput::parse(&output.stdout, &pkg_name));
         build_state.insert(id, kind, parsed_output);
         Ok(())
     });
@@ -270,13 +267,13 @@ impl BuildState {
 
 impl BuildOutput {
     pub fn parse_file(path: &Path, pkg_name: &str) -> CargoResult<BuildOutput> {
-        let contents = try!(paths::read(path));
+        let contents = try!(paths::read_bytes(path));
         BuildOutput::parse(&contents, pkg_name)
     }
 
     // Parses the output of a script.
     // The `pkg_name` is used for error messages.
-    pub fn parse(input: &str, pkg_name: &str) -> CargoResult<BuildOutput> {
+    pub fn parse(input: &[u8], pkg_name: &str) -> CargoResult<BuildOutput> {
         let mut library_paths = Vec::new();
         let mut library_links = Vec::new();
         let mut cfgs = Vec::new();
@@ -284,7 +281,11 @@ impl BuildOutput {
         let mut rerun_if_changed = Vec::new();
         let whence = format!("build script of `{}`", pkg_name);
 
-        for line in input.lines() {
+        for line in input.split(|b| *b == b'\n') {
+            let line = match str::from_utf8(line) {
+                Ok(line) => line.trim(),
+                Err(..) => continue,
+            };
             let mut iter = line.splitn(2, ':');
             if iter.next() != Some("cargo") {
                 // skip this line since it doesn't start with "cargo:"
diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs
index 5770b80ea6f..8444c3fd209 100644
--- a/src/cargo/util/paths.rs
+++ b/src/cargo/util/paths.rs
@@ -68,7 +68,7 @@ pub fn without_prefix<'a>(a: &'a Path, b: &'a Path) -> Option<&'a Path> {
 }
 
 pub fn read(path: &Path) -> CargoResult<String> {
-    (|| -> CargoResult<String> {
+    (|| -> CargoResult<_> {
         let mut ret = String::new();
         let mut f = try!(File::open(path));
         try!(f.read_to_string(&mut ret));
@@ -78,6 +78,17 @@ pub fn read(path: &Path) -> CargoResult<String> {
     })
 }
 
+pub fn read_bytes(path: &Path) -> CargoResult<Vec<u8>> {
+    (|| -> CargoResult<_> {
+        let mut ret = Vec::new();
+        let mut f = try!(File::open(path));
+        try!(f.read_to_end(&mut ret));
+        Ok(ret)
+    })().map_err(human).chain_error(|| {
+        human(format!("failed to read `{}`", path.display()))
+    })
+}
+
 pub fn write(path: &Path, contents: &[u8]) -> CargoResult<()> {
     (|| -> CargoResult<()> {
         let mut f = try!(File::create(path));
diff --git a/tests/test_cargo_compile_custom_build.rs b/tests/test_cargo_compile_custom_build.rs
index a9c69ce8144..a49f7543b3e 100644
--- a/tests/test_cargo_compile_custom_build.rs
+++ b/tests/test_cargo_compile_custom_build.rs
@@ -1866,3 +1866,36 @@ test!(please_respect_the_dag {
 {running} `rustc [..] -L native=foo -L native=bar[..]`
 ", running = RUNNING)));
 });
+
+test!(non_utf8_output {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.5.0"
+            authors = []
+            build = "build.rs"
+        "#)
+        .file("build.rs", r#"
+            use std::io::prelude::*;
+
+            fn main() {
+                let mut out = std::io::stdout();
+                // print something that's not utf8
+                out.write_all(b"\xff\xff\n").unwrap();
+
+                // now print some cargo metadata that's utf8
+                println!("cargo:rustc-cfg=foo");
+
+                // now print more non-utf8
+                out.write_all(b"\xff\xff\n").unwrap();
+            }
+        "#)
+        .file("src/main.rs", r#"
+            #[cfg(foo)]
+            fn main() {}
+        "#);
+
+    assert_that(p.cargo_process("build").arg("-v"),
+                execs().with_status(0));
+});