diff --git a/cli/src/config.rs b/cli/src/config.rs index 621d49c0fc..08367ab458 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -23,8 +23,10 @@ use std::process::Command; use itertools::Itertools; use jj_lib::config::ConfigError; +use jj_lib::config::ConfigLayer; use jj_lib::config::ConfigNamePathBuf; use jj_lib::config::ConfigSource; +use jj_lib::config::StackedConfig; use jj_lib::settings::ConfigResultExt as _; use regex::Captures; use regex::Regex; @@ -105,33 +107,32 @@ pub struct AnnotatedValue { /// 7. Command-line arguments `--config-toml` #[derive(Clone, Debug)] pub struct LayeredConfigs { - default: config::Config, - env_base: config::Config, - // TODO: Track explicit file paths, especially for when user config is a dir. - user: Option, - repo: Option, - env_overrides: config::Config, - arg_overrides: Option, + inner: StackedConfig, } impl LayeredConfigs { /// Initializes configs with infallible sources. pub fn from_environment(default: config::Config) -> Self { - LayeredConfigs { - default, - env_base: env_base(), - user: None, - repo: None, - env_overrides: env_overrides(), - arg_overrides: None, - } + let mut inner = StackedConfig::empty(); + inner.add_layer(ConfigLayer::with_data(ConfigSource::Default, default)); + inner.add_layer(ConfigLayer::with_data(ConfigSource::EnvBase, env_base())); + inner.add_layer(ConfigLayer::with_data( + ConfigSource::EnvOverrides, + env_overrides(), + )); + LayeredConfigs { inner } } #[instrument] pub fn read_user_config(&mut self) -> Result<(), ConfigEnvError> { - self.user = existing_config_path()? - .map(|path| read_config_path(&path)) - .transpose()?; + self.inner.remove_layers(ConfigSource::User); + if let Some(path) = existing_config_path()? { + if path.is_dir() { + self.inner.load_dir(ConfigSource::User, path)?; + } else { + self.inner.load_file(ConfigSource::User, path)?; + } + } Ok(()) } @@ -141,7 +142,11 @@ impl LayeredConfigs { #[instrument] pub fn read_repo_config(&mut self, repo_path: &Path) -> Result<(), ConfigEnvError> { - self.repo = Some(read_config_file(&self.repo_config_path(repo_path))?); + self.inner.remove_layers(ConfigSource::Repo); + let path = self.repo_config_path(repo_path); + if path.exists() { + self.inner.load_file(ConfigSource::Repo, path)?; + } Ok(()) } @@ -150,41 +155,28 @@ impl LayeredConfigs { } pub fn parse_config_args(&mut self, toml_strs: &[String]) -> Result<(), ConfigEnvError> { + self.inner.remove_layers(ConfigSource::CommandArg); let config = toml_strs .iter() .fold(config::Config::builder(), |builder, s| { builder.add_source(config::File::from_str(s, config::FileFormat::Toml)) }) .build()?; - self.arg_overrides = Some(config); + self.inner + .add_layer(ConfigLayer::with_data(ConfigSource::CommandArg, config)); Ok(()) } /// Creates new merged config. pub fn merge(&self) -> config::Config { - self.sources() - .into_iter() - .map(|(_, config)| config) - .fold(config::Config::builder(), |builder, source| { - builder.add_source(source.clone()) - }) - .build() - .expect("loaded configs should be merged without error") + self.inner.merge() } - pub fn sources(&self) -> Vec<(ConfigSource, &config::Config)> { - let config_sources = [ - (ConfigSource::Default, Some(&self.default)), - (ConfigSource::EnvBase, Some(&self.env_base)), - (ConfigSource::User, self.user.as_ref()), - (ConfigSource::Repo, self.repo.as_ref()), - (ConfigSource::EnvOverrides, Some(&self.env_overrides)), - (ConfigSource::CommandArg, self.arg_overrides.as_ref()), - ]; - config_sources - .into_iter() - .filter_map(|(source, config)| config.map(|c| (source, c))) - .collect_vec() + pub fn sources(&self) -> impl DoubleEndedIterator { + self.inner + .layers() + .iter() + .map(|layer| (layer.source, &layer.data)) } pub fn resolved_config_values( @@ -432,46 +424,6 @@ fn env_overrides() -> config::Config { builder.build().unwrap() } -fn read_config_file(path: &Path) -> Result { - config::Config::builder() - .add_source( - config::File::from(path) - .required(false) - .format(config::FileFormat::Toml), - ) - .build() -} - -fn read_config_path(config_path: &Path) -> Result { - let mut files = vec![]; - if config_path.is_dir() { - if let Ok(read_dir) = config_path.read_dir() { - // TODO: Walk the directory recursively? - for dir_entry in read_dir.flatten() { - let path = dir_entry.path(); - if path.is_file() { - files.push(path); - } - } - } - files.sort(); - } else { - files.push(config_path.to_owned()); - } - - files - .iter() - .fold(config::Config::builder(), |builder, path| { - // TODO: Accept other formats and/or accept only certain file extensions? - builder.add_source( - config::File::from(path.as_ref()) - .required(false) - .format(config::FileFormat::Toml), - ) - }) - .build() -} - fn read_config(path: &Path) -> Result, CommandError> { let config_toml = std::fs::read_to_string(path).or_else(|err| { match err.kind() { @@ -781,14 +733,8 @@ mod tests { #[test] fn test_layered_configs_resolved_config_values_empty() { - let empty_config = config::Config::default(); let layered_configs = LayeredConfigs { - default: empty_config.clone(), - env_base: empty_config.clone(), - user: None, - repo: None, - env_overrides: empty_config, - arg_overrides: None, + inner: StackedConfig::empty(), }; assert_eq!( layered_configs @@ -800,7 +746,6 @@ mod tests { #[test] fn test_layered_configs_resolved_config_values_single_key() { - let empty_config = config::Config::default(); let env_base_config = config::Config::builder() .set_override("user.name", "base-user-name") .unwrap() @@ -813,14 +758,13 @@ mod tests { .unwrap() .build() .unwrap(); - let layered_configs = LayeredConfigs { - default: empty_config.clone(), - env_base: env_base_config, - user: None, - repo: Some(repo_config), - env_overrides: empty_config, - arg_overrides: None, - }; + let mut inner = StackedConfig::empty(); + inner.add_layer(ConfigLayer::with_data( + ConfigSource::EnvBase, + env_base_config, + )); + inner.add_layer(ConfigLayer::with_data(ConfigSource::Repo, repo_config)); + let layered_configs = LayeredConfigs { inner }; // Note: "email" is alphabetized, before "name" from same layer. insta::assert_debug_snapshot!( layered_configs.resolved_config_values(&ConfigNamePathBuf::root()).unwrap(), @@ -947,7 +891,6 @@ mod tests { #[test] fn test_layered_configs_resolved_config_values_filter_path() { - let empty_config = config::Config::default(); let user_config = config::Config::builder() .set_override("test-table1.foo", "user-FOO") .unwrap() @@ -960,14 +903,10 @@ mod tests { .unwrap() .build() .unwrap(); - let layered_configs = LayeredConfigs { - default: empty_config.clone(), - env_base: empty_config.clone(), - user: Some(user_config), - repo: Some(repo_config), - env_overrides: empty_config, - arg_overrides: None, - }; + let mut inner = StackedConfig::empty(); + inner.add_layer(ConfigLayer::with_data(ConfigSource::User, user_config)); + inner.add_layer(ConfigLayer::with_data(ConfigSource::Repo, repo_config)); + let layered_configs = LayeredConfigs { inner }; insta::assert_debug_snapshot!( layered_configs .resolved_config_values(&ConfigNamePathBuf::from_iter(["test-table1"])) diff --git a/cli/tests/test_builtin_aliases.rs b/cli/tests/test_builtin_aliases.rs index e20bd89a71..3ece226076 100644 --- a/cli/tests/test_builtin_aliases.rs +++ b/cli/tests/test_builtin_aliases.rs @@ -151,9 +151,9 @@ fn test_builtin_user_redefines_builtin_immutable_heads() { │ (empty) description 1 ~ "###); - insta::assert_snapshot!(stderr, @r###" + insta::assert_snapshot!(stderr, @r" Warning: Redefining `revset-aliases.builtin_immutable_heads()` is not recommended; redefine `immutable_heads()` instead - Warning: Redefining `revset-aliases.immutable()` is not recommended; redefine `immutable_heads()` instead Warning: Redefining `revset-aliases.mutable()` is not recommended; redefine `immutable_heads()` instead - "###); + Warning: Redefining `revset-aliases.immutable()` is not recommended; redefine `immutable_heads()` instead + "); } diff --git a/lib/src/config.rs b/lib/src/config.rs index 20595ddb4d..250645de5c 100644 --- a/lib/src/config.rs +++ b/lib/src/config.rs @@ -16,12 +16,17 @@ use std::borrow::Cow; use std::fmt; +use std::ops::Range; +use std::path::Path; +use std::path::PathBuf; use std::slice; use std::str::FromStr; use config::Source as _; use itertools::Itertools as _; +use crate::file_util::IoResultExt as _; + /// Error that can occur when accessing configuration. // TODO: will be replaced with our custom error type pub type ConfigError = config::ConfigError; @@ -145,6 +150,158 @@ pub enum ConfigSource { CommandArg, } +/// Set of configuration variables with source information. +#[derive(Clone, Debug)] +pub struct ConfigLayer { + /// Source type of this layer. + pub source: ConfigSource, + /// Source file path of this layer if any. + pub path: Option, + /// Configuration variables. + pub data: config::Config, +} + +impl ConfigLayer { + /// Creates new layer with the configuration variables `data`. + pub fn with_data(source: ConfigSource, data: config::Config) -> Self { + ConfigLayer { + source, + path: None, + data, + } + } + + fn load_from_file(source: ConfigSource, path: PathBuf) -> Result { + // TODO: will be replaced with toml_edit::DocumentMut or ImDocument + let data = config::Config::builder() + .add_source( + config::File::from(path.clone()) + // TODO: The path should exist, but the config crate refuses + // to read a special file (e.g. /dev/null) as TOML. + .required(false) + .format(config::FileFormat::Toml), + ) + .build()?; + Ok(ConfigLayer { + source, + path: Some(path), + data, + }) + } + + fn load_from_dir(source: ConfigSource, path: &Path) -> Result, ConfigError> { + // TODO: Walk the directory recursively? + let mut file_paths: Vec<_> = path + .read_dir() + .and_then(|dir_entries| { + dir_entries + .map(|entry| Ok(entry?.path())) + // TODO: Accept only certain file extensions? + .filter_ok(|path| path.is_file()) + .try_collect() + }) + .context(path) + .map_err(|err| ConfigError::Foreign(err.into()))?; + file_paths.sort_unstable(); + file_paths + .into_iter() + .map(|path| Self::load_from_file(source, path)) + .try_collect() + } +} + +/// Stack of configuration layers which can be merged as needed. +#[derive(Clone, Debug)] +pub struct StackedConfig { + /// Layers sorted by `source` (the lowest precedence one first.) + layers: Vec, +} + +impl StackedConfig { + /// Creates an empty stack of configuration layers. + pub fn empty() -> Self { + StackedConfig { layers: vec![] } + } + + /// Loads config file from the specified `path`, inserts it at the position + /// specified by `source`. The file should exist. + pub fn load_file( + &mut self, + source: ConfigSource, + path: impl Into, + ) -> Result<(), ConfigError> { + let layer = ConfigLayer::load_from_file(source, path.into())?; + self.add_layer(layer); + Ok(()) + } + + /// Loads config files from the specified directory `path`, inserts them at + /// the position specified by `source`. The directory should exist. + pub fn load_dir( + &mut self, + source: ConfigSource, + path: impl AsRef, + ) -> Result<(), ConfigError> { + let layers = ConfigLayer::load_from_dir(source, path.as_ref())?; + let index = self.insert_point(source); + self.layers.splice(index..index, layers); + Ok(()) + } + + /// Inserts new layer at the position specified by `layer.source`. + pub fn add_layer(&mut self, layer: ConfigLayer) { + let index = self.insert_point(layer.source); + self.layers.insert(index, layer); + } + + /// Removes layers of the specified `source`. + pub fn remove_layers(&mut self, source: ConfigSource) { + self.layers.drain(self.layer_range(source)); + } + + fn layer_range(&self, source: ConfigSource) -> Range { + // Linear search since the size of Vec wouldn't be large. + let start = self + .layers + .iter() + .take_while(|layer| layer.source < source) + .count(); + let count = self.layers[start..] + .iter() + .take_while(|layer| layer.source == source) + .count(); + start..(start + count) + } + + fn insert_point(&self, source: ConfigSource) -> usize { + // Search from end since layers are usually added in order, and the size + // of Vec wouldn't be large enough to do binary search. + let skip = self + .layers + .iter() + .rev() + .take_while(|layer| layer.source > source) + .count(); + self.layers.len() - skip + } + + /// Layers sorted by precedence. + pub fn layers(&self) -> &[ConfigLayer] { + &self.layers + } + + /// Creates new merged config. + pub fn merge(&self) -> config::Config { + self.layers + .iter() + .fold(config::Config::builder(), |builder, layer| { + builder.add_source(layer.data.clone()) + }) + .build() + .expect("loaded configs should be merged without error") + } +} + #[cfg(test)] mod tests { use super::*; @@ -185,4 +342,71 @@ mod tests { (Some("foo.bar".into()), [key("baz()")].as_slice()) ); } + + #[test] + fn test_stacked_config_layer_order() { + let empty_data = || config::Config::builder().build().unwrap(); + let layer_sources = |config: &StackedConfig| { + config + .layers() + .iter() + .map(|layer| layer.source) + .collect_vec() + }; + + // Insert in reverse order + let mut config = StackedConfig::empty(); + config.add_layer(ConfigLayer::with_data(ConfigSource::Repo, empty_data())); + config.add_layer(ConfigLayer::with_data(ConfigSource::User, empty_data())); + config.add_layer(ConfigLayer::with_data(ConfigSource::Default, empty_data())); + assert_eq!( + layer_sources(&config), + vec![ + ConfigSource::Default, + ConfigSource::User, + ConfigSource::Repo, + ] + ); + + // Insert some more + config.add_layer(ConfigLayer::with_data( + ConfigSource::CommandArg, + empty_data(), + )); + config.add_layer(ConfigLayer::with_data(ConfigSource::EnvBase, empty_data())); + config.add_layer(ConfigLayer::with_data(ConfigSource::User, empty_data())); + assert_eq!( + layer_sources(&config), + vec![ + ConfigSource::Default, + ConfigSource::EnvBase, + ConfigSource::User, + ConfigSource::User, + ConfigSource::Repo, + ConfigSource::CommandArg, + ] + ); + + // Remove last, first, middle + config.remove_layers(ConfigSource::CommandArg); + config.remove_layers(ConfigSource::Default); + config.remove_layers(ConfigSource::User); + assert_eq!( + layer_sources(&config), + vec![ConfigSource::EnvBase, ConfigSource::Repo] + ); + + // Remove unknown + config.remove_layers(ConfigSource::Default); + config.remove_layers(ConfigSource::EnvOverrides); + assert_eq!( + layer_sources(&config), + vec![ConfigSource::EnvBase, ConfigSource::Repo] + ); + + // Remove remainders + config.remove_layers(ConfigSource::EnvBase); + config.remove_layers(ConfigSource::Repo); + assert_eq!(layer_sources(&config), vec![]); + } }