From a0a01ecd606d8b5173592d4e39f2a1d289c58a2f Mon Sep 17 00:00:00 2001 From: Hollow Man Date: Mon, 18 Aug 2025 13:49:12 +0300 Subject: [PATCH 1/2] Keep preprocessors/backends execution order deterministic There's a regression caused by recent refactor work, as it used to execute preprocessors/backends in a deterministic way, but now this is not the case, which causes trouble when some backends implicitly depend on the result from another backend and happen to work (e.g. mdbook-pdf). The root cause is that a HashMap has no order, so this PR switches this into `BTreeMap` instead. Signed-off-by: Hollow Man --- crates/mdbook-core/src/config.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/mdbook-core/src/config.rs b/crates/mdbook-core/src/config.rs index 1a49f02e..c53d03a3 100644 --- a/crates/mdbook-core/src/config.rs +++ b/crates/mdbook-core/src/config.rs @@ -48,7 +48,7 @@ use crate::utils::log_backtrace; use anyhow::{Context, Error, Result, bail}; use log::{debug, trace}; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::env; use std::fs::File; use std::io::Read; @@ -214,7 +214,7 @@ impl Config { } /// Returns the configuration for all preprocessors. - pub fn preprocessors<'de, T: Deserialize<'de>>(&self) -> Result> { + pub fn preprocessors<'de, T: Deserialize<'de>>(&self) -> Result> { self.preprocessor .clone() .try_into() @@ -222,7 +222,7 @@ impl Config { } /// Returns the configuration for all renderers. - pub fn outputs<'de, T: Deserialize<'de>>(&self) -> Result> { + pub fn outputs<'de, T: Deserialize<'de>>(&self) -> Result> { self.output .clone() .try_into() From 6746df7ce9cc6d726edf5458a73b76214cefe824 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 18 Aug 2025 11:43:30 -0700 Subject: [PATCH 2/2] Add tests for preprocessor/output default sort order --- crates/mdbook-driver/src/mdbook/tests.rs | 40 +++++++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/crates/mdbook-driver/src/mdbook/tests.rs b/crates/mdbook-driver/src/mdbook/tests.rs index 2914324c..f4a37072 100644 --- a/crates/mdbook-driver/src/mdbook/tests.rs +++ b/crates/mdbook-driver/src/mdbook/tests.rs @@ -47,12 +47,10 @@ 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, Path::new("")); + let got = determine_preprocessors(&cfg, Path::new("")).unwrap(); - assert!(got.is_ok()); - assert_eq!(got.as_ref().unwrap().len(), 2); - assert_eq!(got.as_ref().unwrap()[0].name(), "index"); - assert_eq!(got.as_ref().unwrap()[1].name(), "links"); + let names: Vec<_> = got.values().map(|p| p.name()).collect(); + assert_eq!(names, ["index", "links"]); } #[test] @@ -252,3 +250,35 @@ fn preprocessor_should_run_falls_back_to_supports_renderer_method() { let got = preprocessor_should_run(&BoolPreprocessor(should_be), &html, &cfg).unwrap(); assert_eq!(got, should_be); } + +// Default is to sort preprocessors alphabetically. +#[test] +fn preprocessor_sorted_by_name() { + let cfg_str = r#" + [preprocessor.xyz] + [preprocessor.abc] + "#; + + let cfg = Config::from_str(cfg_str).unwrap(); + + let got = determine_preprocessors(&cfg, Path::new("")).unwrap(); + + let names: Vec<_> = got.values().map(|p| p.name()).collect(); + assert_eq!(names, ["abc", "index", "links", "xyz"]); +} + +// Default is to sort renderers alphabetically. +#[test] +fn renderers_sorted_by_name() { + let cfg_str = r#" + [output.xyz] + [output.abc] + "#; + + let cfg = Config::from_str(cfg_str).unwrap(); + + let got = determine_renderers(&cfg).unwrap(); + + let names: Vec<_> = got.values().map(|p| p.name()).collect(); + assert_eq!(names, ["abc", "xyz"]); +}