Merge pull request #1418 from ehuss/relative-renderer-command

Fix relative paths for renderer commands.

2 files changed, 87 insertions, 7 deletions

diff --git a/src/renderer/mod.rs b/src/renderer/mod.rs
index 2650a2e0..a3350c57 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;
@@ -130,14 +130,44 @@ impl CmdRenderer {
         CmdRenderer { name, cmd }
     }
 
-    fn compose_command(&self) -> Result<Command> {
+    fn compose_command(&self, root: &Path, destination: &Path) -> Result<Command> {
         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);
@@ -192,7 +222,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());
+}