From 9ab54412ea9d6de5d699b6a9e78088e8943f6ef5 Mon Sep 17 00:00:00 2001 From: Michael Bryan Date: Sun, 14 Jan 2018 19:14:27 +0800 Subject: [PATCH] Made it so the CmdRenderer writes directly to the child's stdin (#544) --- Cargo.toml | 1 - src/lib.rs | 1 - src/renderer/mod.rs | 36 ++++++++++++++++++++++-------------- tests/alternate_backends.rs | 18 ++++++++++++++++++ 4 files changed, 40 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b35d6cb2..27cc0f1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,6 @@ open = "1.1" regex = "0.2.1" tempdir = "0.3.4" itertools = "0.7.4" -tempfile = "2.2.0" shlex = "0.1.1" toml-query = "0.6" diff --git a/src/lib.rs b/src/lib.rs index fe1c1253..163f3619 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -111,7 +111,6 @@ extern crate serde_derive; extern crate serde_json; extern crate shlex; extern crate tempdir; -extern crate tempfile; extern crate toml; extern crate toml_query; diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs index 0e145cce..e094b5cd 100644 --- a/src/renderer/mod.rs +++ b/src/renderer/mod.rs @@ -17,9 +17,8 @@ mod html_handlebars; use std::io::Read; use std::path::PathBuf; -use std::process::Command; +use std::process::{Command, Stdio}; use serde_json; -use tempfile; use shlex::Shlex; use errors::*; @@ -153,21 +152,30 @@ impl Renderer for CmdRenderer { fn render(&self, ctx: &RenderContext) -> Result<()> { info!("Invoking the \"{}\" renderer", self.cmd); - // We need to write the RenderContext to a temporary file here instead - // of passing it in via a pipe. This prevents a race condition where - // some quickly executing command (e.g. `/bin/true`) may exit before we - // finish writing the render context (closing the stdin pipe and - // throwing a write error). - let mut temp = tempfile::tempfile().chain_err(|| "Unable to create a temporary file")?; - serde_json::to_writer(&mut temp, &ctx) - .chain_err(|| "Unable to serialize the RenderContext")?; - - let status = self.compose_command()? - .stdin(temp) + let mut child = self.compose_command()? + .stdin(Stdio::piped()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) .current_dir(&ctx.destination) - .status() + .spawn() .chain_err(|| "Unable to start the renderer")?; + { + let mut stdin = child.stdin.take().expect("Child has stdin"); + if let Err(e) = serde_json::to_writer(&mut stdin, &ctx) { + // Looks like the backend hung up before we could finish + // sending it the render context. Log the error and keep going + warn!("Error writing the RenderContext to the backend, {}", e); + } + + // explicitly close the `stdin` file handle + drop(stdin); + } + + let status = child + .wait() + .chain_err(|| "Error waiting for the backend to complete")?; + trace!("{} exited with output: {:?}", self.cmd, status); if !status.success() { diff --git a/tests/alternate_backends.rs b/tests/alternate_backends.rs index 6f69f0ea..c62c7ff7 100644 --- a/tests/alternate_backends.rs +++ b/tests/alternate_backends.rs @@ -3,9 +3,11 @@ extern crate mdbook; extern crate tempdir; +use std::fs::File; use tempdir::TempDir; use mdbook::config::Config; use mdbook::MDBook; +use mdbook::renderer::RenderContext; #[test] fn passing_alternate_backend() { @@ -28,6 +30,22 @@ fn alternate_backend_with_arguments() { md.build().unwrap(); } +#[test] +fn backends_receive_render_context_via_stdin() { + let temp = TempDir::new("output").unwrap(); + let out_file = temp.path().join("out.txt"); + let cmd = format!("tee {}", out_file.display()); + + let (md, _temp) = dummy_book_with_backend("cat-to-file", &cmd); + + assert!(!out_file.exists()); + md.build().unwrap(); + assert!(out_file.exists()); + + let got = RenderContext::from_json(File::open(&out_file).unwrap()); + assert!(got.is_ok()); +} + fn dummy_book_with_backend(name: &str, command: &str) -> (MDBook, TempDir) { let temp = TempDir::new("mdbook").unwrap();