Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix relative paths for renderer commands. #1418

Merged
merged 1 commit into from
Jan 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions src/renderer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -133,14 +133,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);
Expand Down Expand Up @@ -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())
Expand Down
52 changes: 51 additions & 1 deletion tests/alternative_backends.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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());
}