From e7084e5548b0235d4e9786f4d49cd9021604da10 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 16 Aug 2025 12:25:54 -0700 Subject: [PATCH] Change CmdPreprocessor to use paths relative to the book root This changes preprocessors so that: - Relative paths in the `command` value are relative to the book root. - The process current directory is the book root. This makes it so that it isn't dependent on the directory where `mdbook` is executed. Fixes https://github.com/rust-lang/mdBook/issues/1424 --- .../src/builtin_preprocessors/cmd.rs | 35 ++++++------------- .../src/builtin_renderers/mod.rs | 32 ++--------------- crates/mdbook-driver/src/lib.rs | 29 +++++++++++++++ crates/mdbook-driver/src/mdbook.rs | 10 +++--- crates/mdbook-driver/src/mdbook/tests.rs | 18 +++++----- tests/testsuite/preprocessor.rs | 7 ++-- 6 files changed, 61 insertions(+), 70 deletions(-) diff --git a/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs b/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs index e97c19e2..0241a081 100644 --- a/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs +++ b/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs @@ -1,10 +1,10 @@ -use anyhow::{Context, Result, bail, ensure}; +use anyhow::{Context, Result, ensure}; use log::{debug, trace, warn}; use mdbook_core::book::Book; use mdbook_preprocessor::{Preprocessor, PreprocessorContext}; -use shlex::Shlex; use std::io::{self, Write}; -use std::process::{Child, Command, Stdio}; +use std::path::PathBuf; +use std::process::{Child, Stdio}; /// A custom preprocessor which will shell out to a 3rd-party program. /// @@ -33,12 +33,13 @@ use std::process::{Child, Command, Stdio}; pub struct CmdPreprocessor { name: String, cmd: String, + root: PathBuf, } impl CmdPreprocessor { /// Create a new `CmdPreprocessor`. - pub fn new(name: String, cmd: String) -> CmdPreprocessor { - CmdPreprocessor { name, cmd } + pub fn new(name: String, cmd: String, root: PathBuf) -> CmdPreprocessor { + CmdPreprocessor { name, cmd, root } } fn write_input_to_child(&self, child: &mut Child, book: &Book, ctx: &PreprocessorContext) { @@ -64,22 +65,6 @@ impl CmdPreprocessor { pub fn cmd(&self) -> &str { &self.cmd } - - fn command(&self) -> Result { - let mut words = Shlex::new(&self.cmd); - let executable = match words.next() { - Some(e) => e, - None => bail!("Command string was empty"), - }; - - let mut cmd = Command::new(executable); - - for arg in words { - cmd.arg(arg); - } - - Ok(cmd) - } } impl Preprocessor for CmdPreprocessor { @@ -88,12 +73,13 @@ impl Preprocessor for CmdPreprocessor { } fn run(&self, ctx: &PreprocessorContext, book: Book) -> Result { - let mut cmd = self.command()?; + let mut cmd = crate::compose_command(&self.cmd, &ctx.root)?; let mut child = cmd .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) + .current_dir(&self.root) .spawn() .with_context(|| { format!( @@ -135,7 +121,7 @@ impl Preprocessor for CmdPreprocessor { renderer ); - let mut cmd = match self.command() { + let mut cmd = match crate::compose_command(&self.cmd, &self.root) { Ok(c) => c, Err(e) => { warn!( @@ -153,6 +139,7 @@ impl Preprocessor for CmdPreprocessor { .stdin(Stdio::null()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) + .current_dir(&self.root) .status() .map(|status| status.code() == Some(0)); @@ -183,8 +170,8 @@ mod tests { #[test] fn round_trip_write_and_parse_input() { - let cmd = CmdPreprocessor::new("test".to_string(), "test".to_string()); let md = guide(); + let cmd = CmdPreprocessor::new("test".to_string(), "test".to_string(), md.root.clone()); let ctx = PreprocessorContext::new( md.root.clone(), md.config.clone(), diff --git a/crates/mdbook-driver/src/builtin_renderers/mod.rs b/crates/mdbook-driver/src/builtin_renderers/mod.rs index 980c0a6f..615dedde 100644 --- a/crates/mdbook-driver/src/builtin_renderers/mod.rs +++ b/crates/mdbook-driver/src/builtin_renderers/mod.rs @@ -5,11 +5,9 @@ use anyhow::{Context, Result, bail}; use log::{error, info, trace, warn}; use mdbook_renderer::{RenderContext, Renderer}; -use shlex::Shlex; use std::fs; use std::io::{self, ErrorKind}; -use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::Stdio; pub use self::markdown_renderer::MarkdownRenderer; @@ -49,30 +47,6 @@ impl CmdRenderer { pub fn new(name: String, cmd: String) -> CmdRenderer { CmdRenderer { name, cmd } } - - fn compose_command(&self, root: &Path) -> Result { - let mut words = Shlex::new(&self.cmd); - let exe = match words.next() { - Some(e) => PathBuf::from(e), - None => bail!("Command string was empty"), - }; - - let exe = if exe.components().count() == 1 { - // Search PATH for the executable. - exe - } else { - // Relative path is relative to book root. - root.join(&exe) - }; - - let mut cmd = Command::new(exe); - - for arg in words { - cmd.arg(arg); - } - - Ok(cmd) - } } impl CmdRenderer { @@ -120,8 +94,8 @@ impl Renderer for CmdRenderer { let _ = fs::create_dir_all(&ctx.destination); - let mut child = match self - .compose_command(&ctx.root)? + let mut cmd = crate::compose_command(&self.cmd, &ctx.root)?; + let mut child = match cmd .stdin(Stdio::piped()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) diff --git a/crates/mdbook-driver/src/lib.rs b/crates/mdbook-driver/src/lib.rs index 470fa039..8eb85c8b 100644 --- a/crates/mdbook-driver/src/lib.rs +++ b/crates/mdbook-driver/src/lib.rs @@ -64,5 +64,34 @@ pub mod init; mod load; mod mdbook; +use anyhow::{Result, bail}; pub use mdbook::MDBook; pub use mdbook_core::{book, config, errors}; +use shlex::Shlex; +use std::path::{Path, PathBuf}; +use std::process::Command; + +/// Creates a [`Command`] for command renderers and preprocessors. +fn compose_command(cmd: &str, root: &Path) -> Result { + let mut words = Shlex::new(cmd); + let exe = match words.next() { + Some(e) => PathBuf::from(e), + None => bail!("Command string was empty"), + }; + + let exe = if exe.components().count() == 1 { + // Search PATH for the executable. + exe + } else { + // Relative path is relative to book root. + root.join(&exe) + }; + + let mut cmd = Command::new(exe); + + for arg in words { + cmd.arg(arg); + } + + Ok(cmd) +} diff --git a/crates/mdbook-driver/src/mdbook.rs b/crates/mdbook-driver/src/mdbook.rs index aec7d469..54728bae 100644 --- a/crates/mdbook-driver/src/mdbook.rs +++ b/crates/mdbook-driver/src/mdbook.rs @@ -70,7 +70,7 @@ impl MDBook { let book = load_book(src_dir, &config.build)?; let renderers = determine_renderers(&config)?; - let preprocessors = determine_preprocessors(&config)?; + let preprocessors = determine_preprocessors(&config, &root)?; Ok(MDBook { root, @@ -93,7 +93,7 @@ impl MDBook { let book = load_book_from_disk(&summary, src_dir)?; let renderers = determine_renderers(&config)?; - let preprocessors = determine_preprocessors(&config)?; + let preprocessors = determine_preprocessors(&config, &root)?; Ok(MDBook { root, @@ -258,7 +258,7 @@ impl MDBook { // Index Preprocessor is disabled so that chapter paths // continue to point to the actual markdown files. - self.preprocessors = determine_preprocessors(&self.config)? + self.preprocessors = determine_preprocessors(&self.config, &self.root)? .into_iter() .filter(|pre| pre.name() != IndexPreprocessor::NAME) .collect(); @@ -440,7 +440,7 @@ struct PreprocessorConfig { } /// Look at the `MDBook` and try to figure out what preprocessors to run. -fn determine_preprocessors(config: &Config) -> Result>> { +fn determine_preprocessors(config: &Config, root: &Path) -> Result>> { // Collect the names of all preprocessors intended to be run, and the order // in which they should be run. let mut preprocessor_names = TopologicalSort::::new(); @@ -513,7 +513,7 @@ fn determine_preprocessors(config: &Config) -> Result> .command .to_owned() .unwrap_or_else(|| format!("mdbook-{name}")); - Box::new(CmdPreprocessor::new(name, command)) + Box::new(CmdPreprocessor::new(name, command, root.to_owned())) } }; preprocessors.push(preprocessor); diff --git a/crates/mdbook-driver/src/mdbook/tests.rs b/crates/mdbook-driver/src/mdbook/tests.rs index ba3a3c58..675bfc25 100644 --- a/crates/mdbook-driver/src/mdbook/tests.rs +++ b/crates/mdbook-driver/src/mdbook/tests.rs @@ -47,7 +47,7 @@ fn config_defaults_to_link_and_index_preprocessor_if_not_set() { // make sure we haven't got anything in the `preprocessor` table assert!(cfg.preprocessors::().unwrap().is_empty()); - let got = determine_preprocessors(&cfg); + let got = determine_preprocessors(&cfg, Path::new("")); assert!(got.is_ok()); assert_eq!(got.as_ref().unwrap().len(), 2); @@ -60,7 +60,7 @@ fn use_default_preprocessors_works() { let mut cfg = Config::default(); cfg.build.use_default_preprocessors = false; - let got = determine_preprocessors(&cfg).unwrap(); + let got = determine_preprocessors(&cfg, Path::new("")).unwrap(); assert_eq!(got.len(), 0); } @@ -83,7 +83,7 @@ fn can_determine_third_party_preprocessors() { // make sure the `preprocessor.random` table exists assert!(cfg.get::("preprocessor.random").unwrap().is_some()); - let got = determine_preprocessors(&cfg).unwrap(); + let got = determine_preprocessors(&cfg, Path::new("")).unwrap(); assert!(got.into_iter().any(|p| p.name() == "random")); } @@ -114,7 +114,7 @@ fn preprocessor_before_must_be_array() { let cfg = Config::from_str(cfg_str).unwrap(); - assert!(determine_preprocessors(&cfg).is_err()); + assert!(determine_preprocessors(&cfg, Path::new("")).is_err()); } #[test] @@ -126,7 +126,7 @@ fn preprocessor_after_must_be_array() { let cfg = Config::from_str(cfg_str).unwrap(); - assert!(determine_preprocessors(&cfg).is_err()); + assert!(determine_preprocessors(&cfg, Path::new("")).is_err()); } #[test] @@ -142,7 +142,7 @@ fn preprocessor_order_is_honored() { let cfg = Config::from_str(cfg_str).unwrap(); - let preprocessors = determine_preprocessors(&cfg).unwrap(); + let preprocessors = determine_preprocessors(&cfg, Path::new("")).unwrap(); let index = |name| { preprocessors .iter() @@ -179,7 +179,7 @@ fn cyclic_dependencies_are_detected() { let cfg = Config::from_str(cfg_str).unwrap(); - assert!(determine_preprocessors(&cfg).is_err()); + assert!(determine_preprocessors(&cfg, Path::new("")).is_err()); } #[test] @@ -191,7 +191,7 @@ fn dependencies_dont_register_undefined_preprocessors() { let cfg = Config::from_str(cfg_str).unwrap(); - let preprocessors = determine_preprocessors(&cfg).unwrap(); + let preprocessors = determine_preprocessors(&cfg, Path::new("")).unwrap(); assert!( !preprocessors @@ -212,7 +212,7 @@ fn dependencies_dont_register_builtin_preprocessors_if_disabled() { let cfg = Config::from_str(cfg_str).unwrap(); - let preprocessors = determine_preprocessors(&cfg).unwrap(); + let preprocessors = determine_preprocessors(&cfg, Path::new("")).unwrap(); assert!( !preprocessors diff --git a/tests/testsuite/preprocessor.rs b/tests/testsuite/preprocessor.rs index 5d0df882..17f20876 100644 --- a/tests/testsuite/preprocessor.rs +++ b/tests/testsuite/preprocessor.rs @@ -75,6 +75,7 @@ fn example() -> CmdPreprocessor { CmdPreprocessor::new( "nop-preprocessor".to_string(), "cargo run --quiet --example nop-preprocessor --".to_string(), + std::env::current_dir().unwrap(), ) } @@ -140,11 +141,11 @@ fn relative_command_path() { .expect_stdout(str![""]) .expect_stderr(str![[r#" [TIMESTAMP] [INFO] (mdbook_driver::mdbook): Book building has started -[TIMESTAMP] [WARN] (mdbook_driver::builtin_preprocessors::cmd): The command wasn't found, is the "my-preprocessor" preprocessor installed? -[TIMESTAMP] [WARN] (mdbook_driver::builtin_preprocessors::cmd): [TAB]Command: preprocessors/my-preprocessor [TIMESTAMP] [INFO] (mdbook_driver::mdbook): Running the html backend [TIMESTAMP] [INFO] (mdbook_html::html_handlebars::hbs_renderer): HTML book written to `[ROOT]/src/../book` "#]]); - }); + }) + .check_file("support-check", "html") + .check_file("preprocessor-ran", "test"); }