diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 0454b78909..91fb6ed49f 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -281,7 +281,6 @@ struct CommandHelperData { global_args: GlobalArgs, config_env: ConfigEnv, settings: UserSettings, - stacked_config: StackedConfig, revset_extensions: Arc, commit_template_extensions: Vec>, operation_template_extensions: Vec>, @@ -323,11 +322,6 @@ impl CommandHelper { &self.data.settings } - // TODO: will be moved to UserSettings - pub fn stacked_config(&self) -> &StackedConfig { - &self.data.stacked_config - } - pub fn revset_extensions(&self) -> &Arc { &self.data.revset_extensions } @@ -337,7 +331,7 @@ impl CommandHelper { /// For most commands that depend on a loaded repo, you should use /// `WorkspaceCommandHelper::template_aliases_map()` instead. fn load_template_aliases(&self, ui: &Ui) -> Result { - load_template_aliases(ui, &self.data.stacked_config) + load_template_aliases(ui, self.settings().stacked_config()) } /// Parses template of the given language into evaluation tree. @@ -724,7 +718,7 @@ impl WorkspaceCommandEnvironment { #[instrument(skip_all)] fn new(ui: &Ui, command: &CommandHelper, workspace: &Workspace) -> Result { let revset_aliases_map = - revset_util::load_revset_aliases(ui, &command.data.stacked_config)?; + revset_util::load_revset_aliases(ui, command.settings().stacked_config())?; let template_aliases_map = command.load_template_aliases(ui)?; let path_converter = RepoPathUiConverter::Fs { cwd: command.cwd().to_owned(), @@ -3627,7 +3621,7 @@ impl CliRunner { } } - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(stacked_config); let command_helper_data = CommandHelperData { app: self.app, cwd, @@ -3636,7 +3630,6 @@ impl CliRunner { global_args: args.global_args, config_env, settings, - stacked_config, revset_extensions: self.revset_extensions.into(), commit_template_extensions: self.commit_template_extensions, operation_template_extensions: self.operation_template_extensions, diff --git a/cli/src/commands/config/list.rs b/cli/src/commands/config/list.rs index a7de617df5..984871933c 100644 --- a/cli/src/commands/config/list.rs +++ b/cli/src/commands/config/list.rs @@ -79,7 +79,7 @@ pub fn cmd_config_list( let mut formatter = ui.stdout_formatter(); let name_path = args.name.clone().unwrap_or_else(ConfigNamePathBuf::root); let mut wrote_values = false; - for annotated in resolved_config_values(command.stacked_config(), &name_path)? { + for annotated in resolved_config_values(command.settings().stacked_config(), &name_path)? { // Remove overridden values. if annotated.is_overridden && !args.include_overridden { continue; diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index d7be4f55fa..fd1a77d19a 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -359,15 +359,21 @@ impl MergeEditor { #[cfg(test)] mod tests { + use jj_lib::config::ConfigLayer; + use jj_lib::config::ConfigSource; + use jj_lib::config::StackedConfig; + use super::*; - fn config_from_string(text: &str) -> config::Config { - config::Config::builder() - // Load defaults to test the default args lookup - .add_source(crate::config::default_config()) - .add_source(config::File::from_str(text, config::FileFormat::Toml)) - .build() - .unwrap() + fn config_from_string(text: &str) -> StackedConfig { + let mut config = StackedConfig::empty(); + // Load defaults to test the default args lookup + config.add_layer(ConfigLayer::with_data( + ConfigSource::Default, + crate::config::default_config(), + )); + config.add_layer(ConfigLayer::parse(ConfigSource::User, text).unwrap()); + config } #[test] @@ -440,7 +446,7 @@ mod tests { fn test_get_diff_editor_from_settings() { let get = |text| { let config = config_from_string(text); - let ui = Ui::with_config(&config).unwrap(); + let ui = Ui::with_config(&config.merge()).unwrap(); let settings = UserSettings::from_config(config); DiffEditor::from_settings( &ui, @@ -657,7 +663,7 @@ mod tests { fn test_get_merge_editor_from_settings() { let get = |text| { let config = config_from_string(text); - let ui = Ui::with_config(&config).unwrap(); + let ui = Ui::with_config(&config.merge()).unwrap(); let settings = UserSettings::from_config(config); MergeEditor::from_settings(&ui, &settings, ConflictMarkerStyle::Diff) .map(|editor| editor.tool) diff --git a/lib/src/config.rs b/lib/src/config.rs index 250645de5c..923852bd75 100644 --- a/lib/src/config.rs +++ b/lib/src/config.rs @@ -171,6 +171,14 @@ impl ConfigLayer { } } + /// Parses TOML document `text` into new layer. + pub fn parse(source: ConfigSource, text: &str) -> Result { + let data = config::Config::builder() + .add_source(config::File::from_str(text, config::FileFormat::Toml)) + .build()?; + Ok(Self::with_data(source, 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() diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 5fab1e978e..083f8fa713 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -1536,6 +1536,7 @@ mod tests { use test_case::test_case; use super::*; + use crate::config::StackedConfig; use crate::content_hash::blake2b_hash; #[test_case(false; "legacy tree format")] @@ -2220,7 +2221,7 @@ mod tests { // UserSettings type. testutils returns jj_lib (2)'s UserSettings, whereas // our UserSettings type comes from jj_lib (1). fn user_settings() -> UserSettings { - let config = config::Config::default(); + let config = StackedConfig::empty(); UserSettings::from_config(config) } } diff --git a/lib/src/settings.rs b/lib/src/settings.rs index f8b12c5ebd..74ab8a6b9a 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -28,6 +28,7 @@ use crate::backend::Commit; use crate::backend::Signature; use crate::backend::Timestamp; use crate::config::ConfigError; +use crate::config::StackedConfig; use crate::conflicts::ConflictMarkerStyle; use crate::fmt_util::binary_prefix; use crate::fsmonitor::FsmonitorSettings; @@ -35,6 +36,8 @@ use crate::signing::SignBehavior; #[derive(Debug, Clone)] pub struct UserSettings { + // TODO: merged "config" will be replaced by StackedConfig + stacked_config: StackedConfig, config: config::Config, timestamp: Option, rng: Arc, @@ -42,7 +45,7 @@ pub struct UserSettings { #[derive(Debug, Clone)] pub struct RepoSettings { - _config: config::Config, + _stacked_config: StackedConfig, } #[derive(Debug, Clone)] @@ -135,10 +138,12 @@ fn get_rng_seed_config(config: &config::Config) -> Option { } impl UserSettings { - pub fn from_config(config: config::Config) -> Self { + pub fn from_config(stacked_config: StackedConfig) -> Self { + let config = stacked_config.merge(); let timestamp = get_timestamp_config(&config, "debug.commit-timestamp"); let rng_seed = get_rng_seed_config(&config); UserSettings { + stacked_config, config, timestamp, rng: Arc::new(JJRng::new(rng_seed)), @@ -148,8 +153,10 @@ impl UserSettings { // TODO: Reconsider UserSettings/RepoSettings abstraction. See // https://github.com/martinvonz/jj/issues/616#issuecomment-1345170699 pub fn with_repo(&self, _repo_path: &Path) -> Result { - let config = self.config.clone(); - Ok(RepoSettings { _config: config }) + let stacked_config = self.stacked_config.clone(); + Ok(RepoSettings { + _stacked_config: stacked_config, + }) } pub fn get_rng(&self) -> Arc { @@ -224,6 +231,10 @@ impl UserSettings { self.get_bool("ui.allow-init-native").unwrap_or(false) } + pub fn stacked_config(&self) -> &StackedConfig { + &self.stacked_config + } + // TODO: remove pub fn raw_config(&self) -> &config::Config { &self.config diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index c90e47e5ae..682351dbdb 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -13,11 +13,15 @@ // limitations under the License. use futures::StreamExt as _; +use indoc::indoc; use itertools::Itertools; use jj_lib::backend::ChangeId; use jj_lib::backend::MillisSinceEpoch; use jj_lib::backend::Signature; use jj_lib::backend::Timestamp; +use jj_lib::config::ConfigLayer; +use jj_lib::config::ConfigSource; +use jj_lib::config::StackedConfig; use jj_lib::matchers::EverythingMatcher; use jj_lib::merged_tree::MergedTree; use jj_lib::repo::Repo; @@ -32,6 +36,19 @@ use testutils::CommitGraphBuilder; use testutils::TestRepo; use testutils::TestRepoBackend; +fn config_with_commit_timestamp(timestamp: &str) -> StackedConfig { + let mut config = testutils::base_user_config(); + config.add_layer(ConfigLayer::with_data( + ConfigSource::User, + config::Config::builder() + .set_override("debug.commit-timestamp", timestamp) + .unwrap() + .build() + .unwrap(), + )); + config +} + fn diff_paths(from_tree: &MergedTree, to_tree: &MergedTree) -> Vec { from_tree .diff_stream(to_tree, &EverythingMatcher) @@ -152,13 +169,17 @@ fn test_rewrite(backend: TestRepoBackend) { ], ); - let config = config::Config::builder() - .set_override("user.name", "Rewrite User") - .unwrap() - .set_override("user.email", "rewrite.user@example.com") - .unwrap() - .build() - .unwrap(); + let mut config = StackedConfig::empty(); + config.add_layer( + ConfigLayer::parse( + ConfigSource::User, + indoc! {" + user.name = 'Rewrite User' + user.email = 'rewrite.user@example.com' + "}, + ) + .unwrap(), + ); let rewrite_settings = UserSettings::from_config(config); let mut tx = repo.start_transaction(&settings); let rewritten_commit = tx @@ -203,8 +224,7 @@ fn test_rewrite(backend: TestRepoBackend) { #[test_case(TestRepoBackend::Local ; "local backend")] #[test_case(TestRepoBackend::Git ; "git backend")] fn test_rewrite_update_missing_user(backend: TestRepoBackend) { - let missing_user_settings = - UserSettings::from_config(config::Config::builder().build().unwrap()); + let missing_user_settings = UserSettings::from_config(StackedConfig::empty()); let test_repo = TestRepo::init_with_backend(backend); let repo = &test_repo.repo; @@ -223,13 +243,17 @@ fn test_rewrite_update_missing_user(backend: TestRepoBackend) { assert_eq!(initial_commit.committer().name, ""); assert_eq!(initial_commit.committer().email, ""); - let config = config::Config::builder() - .set_override("user.name", "Configured User") - .unwrap() - .set_override("user.email", "configured.user@example.com") - .unwrap() - .build() - .unwrap(); + let mut config = StackedConfig::empty(); + config.add_layer( + ConfigLayer::parse( + ConfigSource::User, + indoc! {" + user.name = 'Configured User' + user.email = 'configured.user@example.com' + "}, + ) + .unwrap(), + ); let settings = UserSettings::from_config(config); let rewritten_commit = tx .repo_mut() @@ -257,12 +281,7 @@ fn test_rewrite_resets_author_timestamp(backend: TestRepoBackend) { // Create discardable commit let initial_timestamp = "2001-02-03T04:05:06+07:00"; - let config = testutils::base_config() - .set_override("debug.commit-timestamp", initial_timestamp) - .unwrap() - .build() - .unwrap(); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config_with_commit_timestamp(initial_timestamp)); let mut tx = repo.start_transaction(&settings); let initial_commit = tx .repo_mut() @@ -281,12 +300,7 @@ fn test_rewrite_resets_author_timestamp(backend: TestRepoBackend) { // Rewrite discardable commit to no longer be discardable let new_timestamp_1 = "2002-03-04T05:06:07+08:00"; - let config = testutils::base_config() - .set_override("debug.commit-timestamp", new_timestamp_1) - .unwrap() - .build() - .unwrap(); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config_with_commit_timestamp(new_timestamp_1)); let rewritten_commit_1 = tx .repo_mut() .rewrite_commit(&settings, &initial_commit) @@ -304,12 +318,7 @@ fn test_rewrite_resets_author_timestamp(backend: TestRepoBackend) { // Rewrite non-discardable commit let new_timestamp_2 = "2003-04-05T06:07:08+09:00"; - let config = testutils::base_config() - .set_override("debug.commit-timestamp", new_timestamp_2) - .unwrap() - .build() - .unwrap(); - let settings = UserSettings::from_config(config); + let settings = UserSettings::from_config(config_with_commit_timestamp(new_timestamp_2)); let rewritten_commit_2 = tx .repo_mut() .rewrite_commit(&settings, &rewritten_commit_1) diff --git a/lib/tests/test_init.rs b/lib/tests/test_init.rs index 0e9d33cf4b..6546622bff 100644 --- a/lib/tests/test_init.rs +++ b/lib/tests/test_init.rs @@ -15,6 +15,7 @@ use std::path::Path; use std::path::PathBuf; +use jj_lib::config::StackedConfig; use jj_lib::git_backend::GitBackend; use jj_lib::op_store::WorkspaceId; use jj_lib::repo::Repo; @@ -140,7 +141,9 @@ fn test_init_external_git() { #[test_case(TestRepoBackend::Git ; "git backend")] fn test_init_no_config_set(backend: TestRepoBackend) { // Test that we can create a repo without setting any config - let settings = UserSettings::from_config(config::Config::default()); + // TODO: Perhaps, StackedConfig::empty() will be replaced with ::default() + // or something that includes the minimal configuration variables. + let settings = UserSettings::from_config(StackedConfig::empty()); let test_workspace = TestWorkspace::init_with_backend(&settings, backend); let repo = &test_workspace.repo; let wc_commit_id = repo diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index d12bec6ac3..750a218be4 100644 --- a/lib/tests/test_operations.rs +++ b/lib/tests/test_operations.rs @@ -20,6 +20,8 @@ use std::time::SystemTime; use assert_matches::assert_matches; use itertools::Itertools as _; use jj_lib::backend::CommitId; +use jj_lib::config::ConfigLayer; +use jj_lib::config::ConfigSource; use jj_lib::object_id::ObjectId; use jj_lib::op_walk; use jj_lib::op_walk::OpsetEvaluationError; @@ -421,15 +423,15 @@ fn test_reparent_range_bookmarky() { } fn stable_op_id_settings() -> UserSettings { - UserSettings::from_config( - testutils::base_config() - .add_source(config::File::from_str( - "debug.operation-timestamp = '2001-02-03T04:05:06+07:00'", - config::FileFormat::Toml, - )) - .build() - .unwrap(), - ) + let mut config = testutils::base_user_config(); + config.add_layer( + ConfigLayer::parse( + ConfigSource::User, + "debug.operation-timestamp = '2001-02-03T04:05:06+07:00'", + ) + .unwrap(), + ); + UserSettings::from_config(config) } #[test] diff --git a/lib/tests/test_signing.rs b/lib/tests/test_signing.rs index 1361bfd7ad..cc999be21d 100644 --- a/lib/tests/test_signing.rs +++ b/lib/tests/test_signing.rs @@ -1,6 +1,8 @@ use jj_lib::backend::MillisSinceEpoch; use jj_lib::backend::Signature; use jj_lib::backend::Timestamp; +use jj_lib::config::ConfigLayer; +use jj_lib::config::ConfigSource; use jj_lib::repo::Repo; use jj_lib::settings::UserSettings; use jj_lib::signing::SigStatus; @@ -15,18 +17,19 @@ use testutils::TestRepoBackend; use testutils::TestWorkspace; fn user_settings(sign_all: bool) -> UserSettings { - let config = testutils::base_config() - .add_source(config::File::from_str( + let mut config = testutils::base_user_config(); + config.add_layer( + ConfigLayer::parse( + ConfigSource::User, &format!( r#" signing.key = "impeccable" signing.sign-all = {sign_all} "# ), - config::FileFormat::Toml, - )) - .build() - .unwrap(); + ) + .unwrap(), + ); UserSettings::from_config(config) } diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index a6afeea061..9cdfb1ecb8 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -37,6 +37,9 @@ use jj_lib::backend::Timestamp; use jj_lib::backend::TreeValue; use jj_lib::commit::Commit; use jj_lib::commit_builder::CommitBuilder; +use jj_lib::config::ConfigLayer; +use jj_lib::config::ConfigSource; +use jj_lib::config::StackedConfig; use jj_lib::git_backend::GitBackend; use jj_lib::local_backend::LocalBackend; use jj_lib::merged_tree::MergedTree; @@ -101,22 +104,25 @@ pub fn new_temp_dir() -> TempDir { .unwrap() } -pub fn base_config() -> config::ConfigBuilder { - config::Config::builder().add_source(config::File::from_str( - r#" - user.name = "Test User" - user.email = "test.user@example.com" - operation.username = "test-username" - operation.hostname = "host.example.com" - debug.randomness-seed = "42" - "#, - config::FileFormat::Toml, - )) +/// Returns new low-level config object that includes fake user configuration +/// needed to run basic operations. +pub fn base_user_config() -> StackedConfig { + let config_text = r#" + user.name = "Test User" + user.email = "test.user@example.com" + operation.username = "test-username" + operation.hostname = "host.example.com" + debug.randomness-seed = "42" + "#; + let mut config = StackedConfig::empty(); + config.add_layer(ConfigLayer::parse(ConfigSource::User, config_text).unwrap()); + config } +/// Returns new immutable settings object that includes fake user configuration +/// needed to run basic operations. pub fn user_settings() -> UserSettings { - let config = base_config().build().unwrap(); - UserSettings::from_config(config) + UserSettings::from_config(base_user_config()) } #[derive(Debug)]