Auto merge of #1109 - fitzgen:rustfmt-stdout, r=fitzgen
lib: rustfmt output to stdout
*(Recreated + slightly touched up version of #1042 + a version bump)*
Simplify the rustfmt and write mechanism. Use rustfmt generated string to allow writing to stdout or to rustfmt a file.
diff --git a/Cargo.lock b/Cargo.lock
index 8c2399f..ce443cc 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1,6 +1,6 @@
[root]
name = "bindgen"
-version = "0.30.1"
+version = "0.31.0"
dependencies = [
"cexpr 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"cfg-if 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/Cargo.toml b/Cargo.toml
index d1e9332..50653af 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -13,7 +13,7 @@
readme = "README.md"
repository = "https://github.com/rust-lang-nursery/rust-bindgen"
documentation = "https://docs.rs/bindgen"
-version = "0.30.1"
+version = "0.31.0"
build = "build.rs"
include = [
diff --git a/src/lib.rs b/src/lib.rs
index deadf47..fccee7b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1652,18 +1652,13 @@
/// Write these bindings as source text to a file.
pub fn write_to_file<P: AsRef<Path>>(&self, path: P) -> io::Result<()> {
- {
- let file = try!(
- OpenOptions::new()
- .write(true)
- .truncate(true)
- .create(true)
- .open(path.as_ref())
- );
- self.write(Box::new(file))?;
- }
-
- self.rustfmt_generated_file(path.as_ref())
+ let file = OpenOptions::new()
+ .write(true)
+ .truncate(true)
+ .create(true)
+ .open(path.as_ref())?;
+ self.write(Box::new(file))?;
+ Ok(())
}
/// Write these bindings as source text to the given `Write`able.
@@ -1676,31 +1671,52 @@
writer.write(line.as_bytes())?;
writer.write("\n".as_bytes())?;
}
+
if !self.options.raw_lines.is_empty() {
writer.write("\n".as_bytes())?;
}
- writer.write(self.module.as_str().as_bytes())?;
+ let bindings = self.module.as_str().to_string();
+
+ match self.rustfmt_generated_string(bindings) {
+ Ok(rustfmt_bindings) => {
+ writer.write(rustfmt_bindings.as_str().as_bytes())?;
+ },
+ Err(err) => eprintln!("{:?}", err),
+ }
Ok(())
}
- /// Checks if rustfmt_bindings is set and runs rustfmt on the file
- fn rustfmt_generated_file(&self, file: &Path) -> io::Result<()> {
- let _t = time::Timer::new("rustfmt_generated_file")
+ /// Checks if rustfmt_bindings is set and runs rustfmt on the string
+ fn rustfmt_generated_string(&self, source: String) -> io::Result<String> {
+ let _t = time::Timer::new("rustfmt_generated_string")
.with_output(self.options.time_phases);
if !self.options.rustfmt_bindings {
- return Ok(());
+ return Ok(source);
}
let rustfmt = if let Ok(rustfmt) = which::which("rustfmt") {
rustfmt
} else {
- warn!("Not running rustfmt because it does not exist in PATH");
- return Ok(());
+ eprintln!("warning: could not find usable rustfmt to pretty print bindings");
+ return Ok(source);
};
- let mut cmd = Command::new(rustfmt);
+ // Prefer using the `rustfmt-nightly` version of `rustmft`, if
+ // possible. It requires being run via `rustup run nightly ...`.
+ let mut cmd = if let Ok(rustup) = which::which("rustup") {
+ let mut cmd = Command::new(rustup);
+ cmd.args(&["run", "nightly", "rustfmt", "--"]);
+ cmd
+ } else {
+ Command::new(rustfmt)
+ };
+
+ cmd
+ .args(&["--write-mode=display"])
+ .stdin(Stdio::piped())
+ .stdout(Stdio::piped());
if let Some(path) = self.options
.rustfmt_configuration_file
@@ -1710,34 +1726,52 @@
cmd.args(&["--config-path", path]);
}
- if let Ok(output) = cmd.arg(file).output() {
- if !output.status.success() {
- let stderr = String::from_utf8_lossy(&output.stderr);
- match output.status.code() {
- Some(2) => Err(io::Error::new(
- io::ErrorKind::Other,
- format!("Rustfmt parsing errors:\n{}", stderr),
- )),
- Some(3) => {
- warn!(
- "Rustfmt could not format some lines:\n{}",
- stderr
- );
- Ok(())
- }
- _ => Err(io::Error::new(
- io::ErrorKind::Other,
- format!("Internal rustfmt error:\n{}", stderr),
- )),
+ match cmd.spawn() {
+ Ok(mut child) => {
+ let mut child_stdin = child.stdin.take().unwrap();
+ let mut child_stdout = child.stdout.take().unwrap();
+
+ // Write to stdin in a new thread, so that we can read from stdout on this
+ // thread. This keeps the child from blocking on writing to its stdout which
+ // might block us from writing to its stdin.
+ let stdin_handle = ::std::thread::spawn(move || {
+ let _ = child_stdin.write_all(source.as_bytes());
+ source
+ });
+
+ let mut output = vec![];
+ io::copy(&mut child_stdout, &mut output)?;
+
+ let status = child.wait()?;
+ let source = stdin_handle.join()
+ .expect("The thread writing to rustfmt's stdin doesn't do \
+ anything that could panic");
+
+ match String::from_utf8(output) {
+ Ok(bindings) => {
+ match status.code() {
+ Some(0) => Ok(bindings),
+ Some(2) => Err(io::Error::new(
+ io::ErrorKind::Other,
+ "Rustfmt parsing errors.".to_string(),
+ )),
+ Some(3) => {
+ warn!("Rustfmt could not format some lines.");
+ Ok(bindings)
+ }
+ _ => Err(io::Error::new(
+ io::ErrorKind::Other,
+ "Internal rustfmt error".to_string(),
+ )),
+ }
+ },
+ _ => Ok(source)
}
- } else {
- Ok(())
}
- } else {
- Err(io::Error::new(
- io::ErrorKind::Other,
- "Error executing rustfmt!",
- ))
+ Err(e) => {
+ eprintln!("Error spawning rustfmt: {}", e);
+ Ok(source)
+ }
}
}
}
diff --git a/tests/expectations/struct_with_anon_struct_array_float.rs b/tests/expectations/struct_with_anon_struct_array_float.rs
index e69de29..8b13789 100644
--- a/tests/expectations/struct_with_anon_struct_array_float.rs
+++ b/tests/expectations/struct_with_anon_struct_array_float.rs
@@ -0,0 +1 @@
+
diff --git a/tests/tests.rs b/tests/tests.rs
index 36c872d..5b12861 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -256,6 +256,9 @@
let prepend = [
"bindgen",
+ // We format in `compare_generated_header` ourselves to have a little
+ // more control.
+ "--no-rustfmt-bindings",
"--with-derive-default",
header_str,
"--raw-line",