From 9a65c8ab9233c0791d1b0dbf5d684ff196f8ebbe Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 30 Dec 2020 17:26:59 -0800 Subject: [PATCH] Fix relative paths for renderer commands. --- src/renderer/mod.rs | 42 ++++++++++++++++++++++++---- tests/alternative_backends.rs | 52 ++++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs index f14a9f30..8af2fabc 100644 --- a/src/renderer/mod.rs +++ b/src/renderer/mod.rs @@ -20,7 +20,7 @@ mod markdown_renderer; use shlex::Shlex; use std::fs; use std::io::{self, ErrorKind, Read}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use crate::book::Book; @@ -133,14 +133,44 @@ impl CmdRenderer { CmdRenderer { name, cmd } } - fn compose_command(&self) -> Result { + fn compose_command(&self, root: &Path, destination: &Path) -> Result { let mut words = Shlex::new(&self.cmd); - let executable = match words.next() { - Some(e) => e, + let exe = match words.next() { + Some(e) => PathBuf::from(e), None => bail!("Command string was empty"), }; - let mut cmd = Command::new(executable); + let exe = if exe.components().count() == 1 { + // Search PATH for the executable. + exe + } else { + // Relative paths are preferred to be relative to the book root. + let abs_exe = root.join(&exe); + if abs_exe.exists() { + abs_exe + } else { + // Historically paths were relative to the destination, but + // this is not the preferred way. + let legacy_path = destination.join(&exe); + if legacy_path.exists() { + warn!( + "Renderer command `{}` uses a path relative to the \ + renderer output directory `{}`. This was previously \ + accepted, but has been deprecated. Relative executable \ + paths should be relative to the book root.", + exe.display(), + destination.display() + ); + legacy_path + } else { + // Let this bubble through to later be handled by + // handle_render_command_error. + abs_exe.to_path_buf() + } + } + }; + + let mut cmd = Command::new(exe); for arg in words { cmd.arg(arg); @@ -195,7 +225,7 @@ impl Renderer for CmdRenderer { let _ = fs::create_dir_all(&ctx.destination); let mut child = match self - .compose_command()? + .compose_command(&ctx.root, &ctx.destination)? .stdin(Stdio::piped()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) diff --git a/tests/alternative_backends.rs b/tests/alternative_backends.rs index 875b2cfc..cc7bfc7d 100644 --- a/tests/alternative_backends.rs +++ b/tests/alternative_backends.rs @@ -2,7 +2,7 @@ use mdbook::config::Config; use mdbook::MDBook; -#[cfg(not(windows))] +use std::fs; use std::path::Path; use tempfile::{Builder as TempFileBuilder, TempDir}; @@ -71,6 +71,45 @@ fn backends_receive_render_context_via_stdin() { assert!(got.is_ok()); } +#[test] +fn relative_command_path() { + // Checks behavior of relative paths for the `command` setting. + let temp = TempFileBuilder::new().prefix("mdbook").tempdir().unwrap(); + let renderers = temp.path().join("renderers"); + fs::create_dir(&renderers).unwrap(); + rust_exe( + &renderers, + "myrenderer", + r#"fn main() { + std::fs::write("output", "test").unwrap(); + }"#, + ); + let do_test = |cmd_path| { + let mut config = Config::default(); + config + .set("output.html", toml::value::Table::new()) + .unwrap(); + config.set("output.myrenderer.command", cmd_path).unwrap(); + let md = MDBook::init(&temp.path()) + .with_config(config) + .build() + .unwrap(); + let output = temp.path().join("book/myrenderer/output"); + assert!(!output.exists()); + md.build().unwrap(); + assert!(output.exists()); + fs::remove_file(output).unwrap(); + }; + // Legacy paths work, relative to the output directory. + if cfg!(windows) { + do_test("../../renderers/myrenderer.exe"); + } else { + do_test("../../renderers/myrenderer"); + } + // Modern path, relative to the book directory. + do_test("renderers/myrenderer"); +} + fn dummy_book_with_backend( name: &str, command: &str, @@ -112,3 +151,14 @@ fn success_cmd() -> &'static str { "true" } } + +fn rust_exe(temp: &Path, name: &str, src: &str) { + let rs = temp.join(name).with_extension("rs"); + fs::write(&rs, src).unwrap(); + let status = std::process::Command::new("rustc") + .arg(rs) + .current_dir(temp) + .status() + .expect("rustc should run"); + assert!(status.success()); +}