diff --git a/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs b/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs index 0241a081..498b055d 100644 --- a/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs +++ b/crates/mdbook-driver/src/builtin_preprocessors/cmd.rs @@ -2,7 +2,7 @@ use anyhow::{Context, Result, ensure}; use log::{debug, trace, warn}; use mdbook_core::book::Book; use mdbook_preprocessor::{Preprocessor, PreprocessorContext}; -use std::io::{self, Write}; +use std::io::Write; use std::path::PathBuf; use std::process::{Child, Stdio}; @@ -34,12 +34,18 @@ pub struct CmdPreprocessor { name: String, cmd: String, root: PathBuf, + optional: bool, } impl CmdPreprocessor { /// Create a new `CmdPreprocessor`. - pub fn new(name: String, cmd: String, root: PathBuf) -> CmdPreprocessor { - CmdPreprocessor { name, cmd, root } + pub fn new(name: String, cmd: String, root: PathBuf, optional: bool) -> CmdPreprocessor { + CmdPreprocessor { + name, + cmd, + root, + optional, + } } fn write_input_to_child(&self, child: &mut Child, book: &Book, ctx: &PreprocessorContext) { @@ -75,18 +81,29 @@ impl Preprocessor for CmdPreprocessor { fn run(&self, ctx: &PreprocessorContext, book: Book) -> Result { let mut cmd = crate::compose_command(&self.cmd, &ctx.root)?; - let mut child = cmd + let mut child = match cmd .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::inherit()) .current_dir(&self.root) .spawn() - .with_context(|| { - format!( - "Unable to start the \"{}\" preprocessor. Is it installed?", - self.name() - ) - })?; + { + Ok(c) => c, + Err(e) => { + crate::handle_command_error( + e, + self.optional, + "preprocessor", + "preprocessor", + &self.name, + &self.cmd, + )?; + // This should normally not be reached, since the validation + // for NotFound should have already happened when running the + // "supports" command. + return Ok(book); + } + }; self.write_input_to_child(&mut child, &book, ctx); @@ -114,26 +131,16 @@ impl Preprocessor for CmdPreprocessor { }) } - fn supports_renderer(&self, renderer: &str) -> bool { + fn supports_renderer(&self, renderer: &str) -> Result { debug!( "Checking if the \"{}\" preprocessor supports \"{}\"", self.name(), renderer ); - let mut cmd = match crate::compose_command(&self.cmd, &self.root) { - Ok(c) => c, - Err(e) => { - warn!( - "Unable to create the command for the \"{}\" preprocessor, {}", - self.name(), - e - ); - return false; - } - }; + let mut cmd = crate::compose_command(&self.cmd, &self.root)?; - let outcome = cmd + match cmd .arg("supports") .arg(renderer) .stdin(Stdio::null()) @@ -141,19 +148,20 @@ impl Preprocessor for CmdPreprocessor { .stderr(Stdio::inherit()) .current_dir(&self.root) .status() - .map(|status| status.code() == Some(0)); - - if let Err(ref e) = outcome { - if e.kind() == io::ErrorKind::NotFound { - warn!( - "The command wasn't found, is the \"{}\" preprocessor installed?", - self.name - ); - warn!("\tCommand: {}", self.cmd); + { + Ok(status) => Ok(status.code() == Some(0)), + Err(e) => { + crate::handle_command_error( + e, + self.optional, + "preprocessor", + "preprocessor", + &self.name, + &self.cmd, + )?; + Ok(false) } } - - outcome.unwrap_or(false) } } @@ -171,7 +179,12 @@ mod tests { #[test] fn round_trip_write_and_parse_input() { let md = guide(); - let cmd = CmdPreprocessor::new("test".to_string(), "test".to_string(), md.root.clone()); + let cmd = CmdPreprocessor::new( + "test".to_string(), + "test".to_string(), + md.root.clone(), + false, + ); 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 615dedde..b319ff1f 100644 --- a/crates/mdbook-driver/src/builtin_renderers/mod.rs +++ b/crates/mdbook-driver/src/builtin_renderers/mod.rs @@ -6,7 +6,6 @@ use anyhow::{Context, Result, bail}; use log::{error, info, trace, warn}; use mdbook_renderer::{RenderContext, Renderer}; use std::fs; -use std::io::{self, ErrorKind}; use std::process::Stdio; pub use self::markdown_renderer::MarkdownRenderer; @@ -49,41 +48,6 @@ impl CmdRenderer { } } -impl CmdRenderer { - fn handle_render_command_error(&self, ctx: &RenderContext, error: io::Error) -> Result<()> { - if let ErrorKind::NotFound = error.kind() { - // Look for "output.{self.name}.optional". - // If it exists and is true, treat this as a warning. - // Otherwise, fail the build. - - let optional_key = format!("output.{}.optional", self.name); - - let is_optional = match ctx.config.get(&optional_key) { - Ok(Some(value)) => value, - Err(e) => bail!("expected bool for `{optional_key}`: {e}"), - Ok(None) => false, - }; - - if is_optional { - warn!( - "The command `{}` for backend `{}` was not found, \ - but was marked as optional.", - self.cmd, self.name - ); - return Ok(()); - } else { - error!( - "The command `{0}` wasn't found, is the \"{1}\" backend installed? \ - If you want to ignore this error when the \"{1}\" backend is not installed, \ - set `optional = true` in the `[output.{1}]` section of the book.toml configuration file.", - self.cmd, self.name - ); - } - } - Err(error).with_context(|| "Unable to start the backend")? - } -} - impl Renderer for CmdRenderer { fn name(&self) -> &str { &self.name @@ -92,6 +56,13 @@ impl Renderer for CmdRenderer { fn render(&self, ctx: &RenderContext) -> Result<()> { info!("Invoking the \"{}\" renderer", self.name); + let optional_key = format!("output.{}.optional", self.name); + let optional = match ctx.config.get(&optional_key) { + Ok(Some(value)) => value, + Err(e) => bail!("expected bool for `{optional_key}`: {e}"), + Ok(None) => false, + }; + let _ = fs::create_dir_all(&ctx.destination); let mut cmd = crate::compose_command(&self.cmd, &ctx.root)?; @@ -103,7 +74,11 @@ impl Renderer for CmdRenderer { .spawn() { Ok(c) => c, - Err(e) => return self.handle_render_command_error(ctx, e), + Err(e) => { + return crate::handle_command_error( + e, optional, "output", "backend", &self.name, &self.cmd, + ); + } }; let mut stdin = child.stdin.take().expect("Child has stdin"); diff --git a/crates/mdbook-driver/src/lib.rs b/crates/mdbook-driver/src/lib.rs index 8eb85c8b..c2b3123f 100644 --- a/crates/mdbook-driver/src/lib.rs +++ b/crates/mdbook-driver/src/lib.rs @@ -64,7 +64,8 @@ pub mod init; mod load; mod mdbook; -use anyhow::{Result, bail}; +use anyhow::{Context, Result, bail}; +use log::{error, warn}; pub use mdbook::MDBook; pub use mdbook_core::{book, config, errors}; use shlex::Shlex; @@ -95,3 +96,30 @@ fn compose_command(cmd: &str, root: &Path) -> Result { Ok(cmd) } + +/// Handles a failure for a preprocessor or renderer. +fn handle_command_error( + error: std::io::Error, + optional: bool, + key: &str, + what: &str, + name: &str, + cmd: &str, +) -> Result<()> { + if let std::io::ErrorKind::NotFound = error.kind() { + if optional { + warn!( + "The command `{cmd}` for {what} `{name}` was not found, \ + but is marked as optional.", + ); + return Ok(()); + } else { + error!( + "The command `{cmd}` wasn't found, is the `{name}` {what} installed? \ + If you want to ignore this error when the `{name}` {what} is not installed, \ + set `optional = true` in the `[{key}.{name}]` section of the book.toml configuration file.", + ); + } + } + Err(error).with_context(|| format!("Unable to run the {what} `{name}`"))? +} diff --git a/crates/mdbook-driver/src/mdbook.rs b/crates/mdbook-driver/src/mdbook.rs index 54728bae..4edc27c6 100644 --- a/crates/mdbook-driver/src/mdbook.rs +++ b/crates/mdbook-driver/src/mdbook.rs @@ -437,6 +437,8 @@ struct PreprocessorConfig { before: Vec, #[serde(default)] after: Vec, + #[serde(default)] + optional: bool, } /// Look at the `MDBook` and try to figure out what preprocessors to run. @@ -513,7 +515,12 @@ fn determine_preprocessors(config: &Config, root: &Path) -> Result Result { // default preprocessors should be run by default (if supported) if cfg.build.use_default_preprocessors && is_default_preprocessor(preprocessor) { - return Ok(preprocessor.supports_renderer(renderer.name())); + return preprocessor.supports_renderer(renderer.name()); } let key = format!("preprocessor.{}.renderers", preprocessor.name()); @@ -552,7 +559,7 @@ fn preprocessor_should_run( Ok(Some(explicit_renderers)) => { Ok(explicit_renderers.iter().any(|name| name == renderer_name)) } - Ok(None) => Ok(preprocessor.supports_renderer(renderer_name)), + Ok(None) => preprocessor.supports_renderer(renderer_name), Err(e) => bail!("failed to get `{key}`: {e}"), } } diff --git a/crates/mdbook-driver/src/mdbook/tests.rs b/crates/mdbook-driver/src/mdbook/tests.rs index 675bfc25..b7399133 100644 --- a/crates/mdbook-driver/src/mdbook/tests.rs +++ b/crates/mdbook-driver/src/mdbook/tests.rs @@ -247,8 +247,8 @@ impl Preprocessor for BoolPreprocessor { unimplemented!() } - fn supports_renderer(&self, _renderer: &str) -> bool { - self.0 + fn supports_renderer(&self, _renderer: &str) -> Result { + Ok(self.0) } } diff --git a/crates/mdbook-preprocessor/src/lib.rs b/crates/mdbook-preprocessor/src/lib.rs index c1aa35ea..64100500 100644 --- a/crates/mdbook-preprocessor/src/lib.rs +++ b/crates/mdbook-preprocessor/src/lib.rs @@ -39,8 +39,8 @@ pub trait Preprocessor { /// particular renderer. /// /// By default, always returns `true`. - fn supports_renderer(&self, _renderer: &str) -> bool { - true + fn supports_renderer(&self, _renderer: &str) -> Result { + Ok(true) } } diff --git a/examples/nop-preprocessor.rs b/examples/nop-preprocessor.rs index 3423b481..488271dc 100644 --- a/examples/nop-preprocessor.rs +++ b/examples/nop-preprocessor.rs @@ -59,7 +59,7 @@ fn handle_supports(pre: &dyn Preprocessor, sub_args: &ArgMatches) -> ! { let renderer = sub_args .get_one::("renderer") .expect("Required argument"); - let supported = pre.supports_renderer(renderer); + let supported = pre.supports_renderer(renderer).unwrap(); // Signal whether the renderer is supported by exiting with 1 or 0. if supported { @@ -105,8 +105,8 @@ mod nop_lib { Ok(book) } - fn supports_renderer(&self, renderer: &str) -> bool { - renderer != "not-supported" + fn supports_renderer(&self, renderer: &str) -> Result { + Ok(renderer != "not-supported") } } diff --git a/guide/src/format/configuration/preprocessors.md b/guide/src/format/configuration/preprocessors.md index 26a786b2..24745d35 100644 --- a/guide/src/format/configuration/preprocessors.md +++ b/guide/src/format/configuration/preprocessors.md @@ -64,6 +64,18 @@ be overridden by adding a `command` field. command = "python random.py" ``` +### Optional preprocessors + +If you enable a preprocessor that isn't installed, the default behavior is to throw an error. +This behavior can be changed by marking the preprocessor as optional: + +```toml +[preprocessor.example] +optional = true +``` + +This demotes the error to a warning. + ## Require A Certain Order The order in which preprocessors are run can be controlled with the `before` and `after` fields. diff --git a/tests/testsuite/preprocessor.rs b/tests/testsuite/preprocessor.rs index 17f20876..9fa1df1c 100644 --- a/tests/testsuite/preprocessor.rs +++ b/tests/testsuite/preprocessor.rs @@ -76,6 +76,7 @@ fn example() -> CmdPreprocessor { "nop-preprocessor".to_string(), "cargo run --quiet --example nop-preprocessor --".to_string(), std::env::current_dir().unwrap(), + false, ) } @@ -83,7 +84,7 @@ fn example() -> CmdPreprocessor { fn example_supports_whatever() { let cmd = example(); - let got = cmd.supports_renderer("whatever"); + let got = cmd.supports_renderer("whatever").unwrap(); assert_eq!(got, true); } @@ -92,7 +93,7 @@ fn example_supports_whatever() { fn example_doesnt_support_not_supported() { let cmd = example(); - let got = cmd.supports_renderer("not-supported"); + let got = cmd.supports_renderer("not-supported").unwrap(); assert_eq!(got, false); } @@ -149,3 +150,33 @@ fn relative_command_path() { .check_file("support-check", "html") .check_file("preprocessor-ran", "test"); } + +// Preprocessor command is missing. +#[test] +fn missing_preprocessor() { + BookTest::from_dir("preprocessor/missing_preprocessor").run("build", |cmd| { + cmd.expect_failure() + .expect_stdout(str![[""]]) + .expect_stderr(str![[r#" +[TIMESTAMP] [INFO] (mdbook_driver::mdbook): Book building has started +[TIMESTAMP] [ERROR] (mdbook_driver): The command `trduyvbhijnorgevfuhn` wasn't found, is the `missing` preprocessor installed? If you want to ignore this error when the `missing` preprocessor is not installed, set `optional = true` in the `[preprocessor.missing]` section of the book.toml configuration file. +[TIMESTAMP] [ERROR] (mdbook_core::utils): Error: Unable to run the preprocessor `missing` +[TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: [NOT_FOUND] + +"#]]); + }); +} + +// Optional missing is not an error. +#[test] +fn missing_optional_not_fatal() { + BookTest::from_dir("preprocessor/missing_optional_not_fatal").run("build", |cmd| { + cmd.expect_stdout(str![[""]]).expect_stderr(str![[r#" +[TIMESTAMP] [INFO] (mdbook_driver::mdbook): Book building has started +[TIMESTAMP] [WARN] (mdbook_driver): The command `trduyvbhijnorgevfuhn` for preprocessor `missing` was not found, but is marked as optional. +[TIMESTAMP] [INFO] (mdbook_driver::mdbook): Running the html backend +[TIMESTAMP] [INFO] (mdbook_html::html_handlebars::hbs_renderer): HTML book written to `[ROOT]/book` + +"#]]); + }); +} diff --git a/tests/testsuite/preprocessor/missing_optional_not_fatal/book.toml b/tests/testsuite/preprocessor/missing_optional_not_fatal/book.toml new file mode 100644 index 00000000..59bc75cd --- /dev/null +++ b/tests/testsuite/preprocessor/missing_optional_not_fatal/book.toml @@ -0,0 +1,3 @@ +[preprocessor.missing] +command = "trduyvbhijnorgevfuhn" +optional = true diff --git a/tests/testsuite/preprocessor/missing_optional_not_fatal/src/SUMMARY.md b/tests/testsuite/preprocessor/missing_optional_not_fatal/src/SUMMARY.md new file mode 100644 index 00000000..e69de29b diff --git a/tests/testsuite/preprocessor/missing_preprocessor/book.toml b/tests/testsuite/preprocessor/missing_preprocessor/book.toml new file mode 100644 index 00000000..6a5241f3 --- /dev/null +++ b/tests/testsuite/preprocessor/missing_preprocessor/book.toml @@ -0,0 +1,3 @@ +[preprocessor.missing] +command = "trduyvbhijnorgevfuhn" + diff --git a/tests/testsuite/preprocessor/missing_preprocessor/src/SUMMARY.md b/tests/testsuite/preprocessor/missing_preprocessor/src/SUMMARY.md new file mode 100644 index 00000000..e69de29b diff --git a/tests/testsuite/renderer.rs b/tests/testsuite/renderer.rs index 28ff25be..9f2ee010 100644 --- a/tests/testsuite/renderer.rs +++ b/tests/testsuite/renderer.rs @@ -85,9 +85,9 @@ fn missing_renderer() { [TIMESTAMP] [INFO] (mdbook_driver::mdbook): Book building has started [TIMESTAMP] [INFO] (mdbook_driver::mdbook): Running the missing backend [TIMESTAMP] [INFO] (mdbook_driver::builtin_renderers): Invoking the "missing" renderer -[TIMESTAMP] [ERROR] (mdbook_driver::builtin_renderers): The command `trduyvbhijnorgevfuhn` wasn't found, is the "missing" backend installed? If you want to ignore this error when the "missing" backend is not installed, set `optional = true` in the `[output.missing]` section of the book.toml configuration file. +[TIMESTAMP] [ERROR] (mdbook_driver): The command `trduyvbhijnorgevfuhn` wasn't found, is the `missing` backend installed? If you want to ignore this error when the `missing` backend is not installed, set `optional = true` in the `[output.missing]` section of the book.toml configuration file. [TIMESTAMP] [ERROR] (mdbook_core::utils): Error: Rendering failed -[TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: Unable to start the backend +[TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: Unable to run the backend `missing` [TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: [NOT_FOUND] "#]]); @@ -102,7 +102,7 @@ fn missing_optional_not_fatal() { [TIMESTAMP] [INFO] (mdbook_driver::mdbook): Book building has started [TIMESTAMP] [INFO] (mdbook_driver::mdbook): Running the missing backend [TIMESTAMP] [INFO] (mdbook_driver::builtin_renderers): Invoking the "missing" renderer -[TIMESTAMP] [WARN] (mdbook_driver::builtin_renderers): The command `trduyvbhijnorgevfuhn` for backend `missing` was not found, but was marked as optional. +[TIMESTAMP] [WARN] (mdbook_driver): The command `trduyvbhijnorgevfuhn` for backend `missing` was not found, but is marked as optional. "#]]); });