diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d34664d..49fda4dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,10 @@ This release also includes many new features described below. We have prepared a [0.5 Migration Guide](#05-migration-guide) to help existing authors switch from 0.4. -The final 0.5.0 release does not contain any changes since [0.5.0-beta.2](#mdbook-050-beta2). +The final 0.5.0 release only contains the following changes since [0.5.0-beta.2](#mdbook-050-beta2): + +- Added error handling to environment config handling. This checks that environment variables starting with `MDBOOK_` are correctly specified instead of silently ignoring. This also fixed being able to replace entire top-level tables like `MDBOOK_OUTPUT`. + [#2942](https://github.com/rust-lang/mdBook/pull/2942) ## 0.5 Migration Guide @@ -56,6 +59,10 @@ The following is a summary of the changes that may require your attention when u [#2775](https://github.com/rust-lang/mdBook/pull/2775) - Removed the very old legacy config support. Warnings have been displayed in previous versions on how to migrate. [#2783](https://github.com/rust-lang/mdBook/pull/2783) +- Top-level config values set from the environment like `MDBOOK_BOOK` now *replace* the contents of the top-level table instead of merging into it. + [#2942](https://github.com/rust-lang/mdBook/pull/2942) +- Invalid environment variables are now rejected. Previously unknown keys like `MDBOOK_FOO` would be ignored, or keys or invalid values inside objects like the `[book]` table would be ignored. + [#2942](https://github.com/rust-lang/mdBook/pull/2942) ### Theme changes diff --git a/crates/mdbook-core/src/config.rs b/crates/mdbook-core/src/config.rs index 166e921e..3368e5f6 100644 --- a/crates/mdbook-core/src/config.rs +++ b/crates/mdbook-core/src/config.rs @@ -123,11 +123,10 @@ impl Config { /// /// For example: /// - /// - `MDBOOK_foo` -> `foo` - /// - `MDBOOK_FOO` -> `foo` - /// - `MDBOOK_FOO__BAR` -> `foo.bar` - /// - `MDBOOK_FOO_BAR` -> `foo-bar` - /// - `MDBOOK_FOO_bar__baz` -> `foo-bar.baz` + /// - `MDBOOK_book` -> `book` + /// - `MDBOOK_BOOK` -> `book` + /// - `MDBOOK_BOOK__TITLE` -> `book.title` + /// - `MDBOOK_BOOK__TEXT_DIRECTION` -> `book.text-direction` /// /// So by setting the `MDBOOK_BOOK__TITLE` environment variable you can /// override the book's title without needing to touch your `book.toml`. @@ -147,7 +146,7 @@ impl Config { /// The latter case may be useful in situations where `mdbook` is invoked /// from a script or CI, where it sometimes isn't possible to update the /// `book.toml` before building. - pub fn update_from_env(&mut self) { + pub fn update_from_env(&mut self) -> Result<()> { debug!("Updating the config from environment variables"); let overrides = @@ -162,19 +161,9 @@ impl Config { let parsed_value = serde_json::from_str(&value) .unwrap_or_else(|_| serde_json::Value::String(value.to_string())); - if key == "book" || key == "build" { - if let serde_json::Value::Object(ref map) = parsed_value { - // To `set` each `key`, we wrap them as `prefix.key` - for (k, v) in map { - let full_key = format!("{key}.{k}"); - self.set(&full_key, v).expect("unreachable"); - } - return; - } - } - - self.set(key, parsed_value).expect("unreachable"); + self.set(key, parsed_value)?; } + Ok(()) } /// Get a value from the configuration. @@ -266,24 +255,39 @@ impl Config { /// `output.html.playground` will set the "playground" in the html output /// table). /// - /// The only way this can fail is if we can't serialize `value` into a - /// `toml::Value`. + /// # Errors + /// + /// This will fail if: + /// + /// - The value cannot be represented as TOML. + /// - The value is not a correct type. + /// - The key is an unknown configuration option. pub fn set>(&mut self, index: I, value: S) -> Result<()> { let index = index.as_ref(); let value = Value::try_from(value) .with_context(|| "Unable to represent the item as a JSON Value")?; - if let Some(key) = index.strip_prefix("book.") { - self.book.update_value(key, value); + if index == "book" { + self.book = value.try_into()?; + } else if index == "build" { + self.build = value.try_into()?; + } else if index == "rust" { + self.rust = value.try_into()?; + } else if index == "output" { + self.output = value; + } else if index == "preprocessor" { + self.preprocessor = value; + } else if let Some(key) = index.strip_prefix("book.") { + self.book.update_value(key, value)?; } else if let Some(key) = index.strip_prefix("build.") { - self.build.update_value(key, value); + self.build.update_value(key, value)?; } else if let Some(key) = index.strip_prefix("rust.") { - self.rust.update_value(key, value); + self.rust.update_value(key, value)?; } else if let Some(key) = index.strip_prefix("output.") { - self.output.update_value(key, value); + self.output.update_value(key, value)?; } else if let Some(key) = index.strip_prefix("preprocessor.") { - self.preprocessor.update_value(key, value); + self.preprocessor.update_value(key, value)?; } else { bail!("invalid key `{index}`"); } @@ -703,18 +707,13 @@ pub struct SearchChapterSettings { /// This is definitely not the most performant way to do things, which means you /// should probably keep it away from tight loops... trait Updateable<'de>: Serialize + Deserialize<'de> { - fn update_value(&mut self, key: &str, value: S) { + fn update_value(&mut self, key: &str, value: S) -> Result<()> { let mut raw = Value::try_from(&self).expect("unreachable"); - - if let Ok(value) = Value::try_from(value) { - raw.insert(key, value); - } else { - return; - } - - if let Ok(updated) = raw.try_into() { - *self = updated; - } + let value = Value::try_from(value)?; + raw.insert(key, value); + let updated = raw.try_into()?; + *self = updated; + Ok(()) } } diff --git a/crates/mdbook-driver/src/mdbook.rs b/crates/mdbook-driver/src/mdbook.rs index 80ae692b..e5dac902 100644 --- a/crates/mdbook-driver/src/mdbook.rs +++ b/crates/mdbook-driver/src/mdbook.rs @@ -56,7 +56,7 @@ impl MDBook { Config::default() }; - config.update_from_env(); + config.update_from_env()?; if tracing::enabled!(tracing::Level::TRACE) { for line in format!("Config: {config:#?}").lines() { diff --git a/guide/src/format/configuration/environment-variables.md b/guide/src/format/configuration/environment-variables.md index 0170e759..bef0431f 100644 --- a/guide/src/format/configuration/environment-variables.md +++ b/guide/src/format/configuration/environment-variables.md @@ -12,11 +12,10 @@ underscore (`_`) is replaced with a dash (`-`). For example: -- `MDBOOK_foo` -> `foo` -- `MDBOOK_FOO` -> `foo` -- `MDBOOK_FOO__BAR` -> `foo.bar` -- `MDBOOK_FOO_BAR` -> `foo-bar` -- `MDBOOK_FOO_bar__baz` -> `foo-bar.baz` +- `MDBOOK_book` -> `book` +- `MDBOOK_BOOK` -> `book` +- `MDBOOK_BOOK__TITLE` -> `book.title` +- `MDBOOK_BOOK__TEXT_DIRECTION` -> `book.text-direction` So by setting the `MDBOOK_BOOK__TITLE` environment variable you can override the book's title without needing to touch your `book.toml`. diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 253cc0f7..e44649a8 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -216,10 +216,7 @@ fn env_invalid_config_key() { .expect_failure() .expect_stdout(str![[""]]) .expect_stderr(str![[r#" - -thread 'main' ([..]) panicked at [..] -unreachable: invalid key `foo` -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +ERROR invalid key `foo` "#]]); }); @@ -231,25 +228,26 @@ fn env_invalid_value() { BookTest::from_dir("config/empty") .run("build", |cmd| { cmd.env("MDBOOK_BOOK", r#"{"titlez": "typo"}"#) + .expect_failure() .expect_stdout(str![[""]]) .expect_stderr(str![[r#" - INFO Book building has started - INFO Running the html backend - INFO HTML book written to `[ROOT]/book` +ERROR unknown field `titlez`, expected one of `title`, `authors`, `description`, `src`, `language`, `text-direction` + "#]]); }) .run("build", |cmd| { cmd.env("MDBOOK_BOOK__TITLE", r#"{"looks like obj": "abc"}"#) + .expect_failure() .expect_stdout(str![[""]]) .expect_stderr(str![[r#" - INFO Book building has started - INFO Running the html backend - INFO HTML book written to `[ROOT]/book` +ERROR invalid type: map, expected a string +in `title` + "#]]); }) - .check_file_contains("book/index.html", "Chapter 1") + // This is not valid JSON, so falls back to be interpreted as a string. .run("build", |cmd| { cmd.env("MDBOOK_BOOK__TITLE", r#"{braces}"#) .expect_stdout(str![[""]]) @@ -276,7 +274,8 @@ fn env_entire_book_table() { .run("build", |cmd| { cmd.env("MDBOOK_BOOK", r#"{"description": "custom description"}"#); }) - .check_file_contains("book/index.html", "Chapter 1 - config title") + // The book.toml title is removed. + .check_file_contains("book/index.html", "Chapter 1") .check_file_contains( "book/index.html", r#""#, @@ -311,6 +310,7 @@ fn env_entire_output_preprocessor_table() { let mut s = String::new(); std::io::stdin().read_to_string(&mut s).unwrap(); assert!(s.contains("custom output config")); + eprintln!("preprocessor saw custom config"); } "#, ) @@ -329,14 +329,15 @@ fn env_entire_output_preprocessor_table() { r#"{"my-preprocessor": {"foo": "custom preprocessor config"}}"#, ) .env("PATH", path) - .expect_failure() .expect_stdout(str![[""]]) .expect_stderr(str![[r#" - -thread 'main' ([..]) panicked at [..] -unreachable: invalid key `output` -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + INFO Book building has started + INFO Running the my-output backend + INFO Invoking the "my-output" renderer +preprocessor saw custom config "#]]); - }); + }) + // No HTML output + .check_file_list("book", str![[""]]); }