diff --git a/CHANGELOG.md b/CHANGELOG.md index e5131d0ff..7c79b83b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj resolve` now notifies the user of remaining conflicts, if any, on success. This can be prevented by the new `--quiet` option. +* Per-repository configuration is now read from `.jj/repo/config.toml`. + ### Fixed bugs * When sharing the working copy with a Git repo, we used to forget to export diff --git a/docs/config.md b/docs/config.md index b021cae7e..40d1c55d0 100644 --- a/docs/config.md +++ b/docs/config.md @@ -2,9 +2,12 @@ These are the config settings available to jj/Jujutsu. -The config settings are located at `~/.jjconfig.toml`. Less common ways +The config settings are loaded from the following locations. Less common ways to specify `jj` config settings are discussed in a later section. +* `~/.jjconfig.toml` (global) +* `.jj/repo/config.toml` (per-repository) + See the [TOML site](https://toml.io/en/) for more on syntax. One thing to remember is that anything under a heading can be dotted e.g. `user.name = "YOUR NAME"` is equivalent to: diff --git a/lib/src/settings.rs b/lib/src/settings.rs index a5991dd97..b9399f3eb 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -61,15 +61,10 @@ impl UserSettings { } } - pub fn with_repo(&self, repo_path: &Path) -> Result { - let config = config::Config::builder() - .add_source(self.config.clone()) - .add_source( - config::File::from(repo_path.join("config")) - .required(false) - .format(config::FileFormat::Toml), - ) - .build()?; + // 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 }) } diff --git a/src/cli_util.rs b/src/cli_util.rs index 4cc3b860d..cdc7e7ab5 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -47,7 +47,7 @@ use jujutsu_lib::tree::{Tree, TreeMergeError}; use jujutsu_lib::working_copy::{ CheckoutStats, LockedWorkingCopy, ResetError, SnapshotError, WorkingCopy, }; -use jujutsu_lib::workspace::{Workspace, WorkspaceInitError, WorkspaceLoadError}; +use jujutsu_lib::workspace::{Workspace, WorkspaceInitError, WorkspaceLoadError, WorkspaceLoader}; use jujutsu_lib::{dag_walk, file_util, git, revset}; use thiserror::Error; use tracing_subscriber::prelude::*; @@ -1650,8 +1650,6 @@ pub fn parse_args( string_args: &[String], layered_configs: &mut LayeredConfigs, ) -> Result<(ArgMatches, Args), CommandError> { - // TODO: read user configs from the repo pointed to by -R. - handle_early_args(ui, app, string_args, layered_configs)?; let matches = app.clone().try_get_matches_from(string_args)?; @@ -1793,7 +1791,15 @@ impl CliRunner { &mut layered_configs, )?; + let workspace_path = cwd.join(args.global_args.repository.as_deref().unwrap_or(".")); + // TODO: reuse loader to load workspace, but we can't report error here, and the + // error isn't clonable. + if let Ok(loader) = WorkspaceLoader::init(&workspace_path) { + // TODO: maybe show error/warning if repo config contained command alias + layered_configs.read_repo_config(loader.repo_path())?; + } let config = layered_configs.merge(); + ui.reset(&config); let settings = UserSettings::from_config(config); let command_helper = CommandHelper::new( self.app, diff --git a/src/config.rs b/src/config.rs index 4a81f04a6..ef05607de 100644 --- a/src/config.rs +++ b/src/config.rs @@ -33,7 +33,7 @@ pub enum ConfigError { /// 1. Default /// 2. Base environment variables /// 3. User config `~/.jjconfig.toml` or `$JJ_CONFIG` -/// 4. TODO: Repo config `.jj/repo/config.toml` +/// 4. Repo config `.jj/repo/config.toml` /// 5. TODO: Workspace config `.jj/config.toml` /// 6. Override environment variables /// 7. Command-line arguments `--config-toml` @@ -42,6 +42,7 @@ pub struct LayeredConfigs { default: config::Config, env_base: config::Config, user: Option, + repo: Option, env_overrides: config::Config, arg_overrides: Option, } @@ -53,6 +54,7 @@ impl LayeredConfigs { default: default_config(), env_base: env_base(), user: None, + repo: None, env_overrides: env_overrides(), arg_overrides: None, } @@ -65,6 +67,11 @@ impl LayeredConfigs { Ok(()) } + pub fn read_repo_config(&mut self, repo_path: &Path) -> Result<(), ConfigError> { + self.repo = Some(read_config_file(&repo_path.join("config.toml"))?); + Ok(()) + } + pub fn parse_config_args(&mut self, toml_strs: &[String]) -> Result<(), ConfigError> { let config = toml_strs .iter() @@ -82,6 +89,7 @@ impl LayeredConfigs { Some(&self.default), Some(&self.env_base), self.user.as_ref(), + self.repo.as_ref(), Some(&self.env_overrides), self.arg_overrides.as_ref(), ]; @@ -192,6 +200,16 @@ 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() { diff --git a/tests/test_config_command.rs b/tests/test_config_command.rs index c57a71884..517457b4f 100644 --- a/tests/test_config_command.rs +++ b/tests/test_config_command.rs @@ -138,6 +138,17 @@ fn test_config_layer_override_default() { merge-tools.vimdiff.program="user" "###); + // Repo + std::fs::write( + repo_path.join(".jj/repo/config.toml"), + format!("{config_key} = {value:?}\n", value = "repo"), + ) + .unwrap(); + let stdout = test_env.jj_cmd_success(&repo_path, &["config", "list", config_key]); + insta::assert_snapshot!(stdout, @r###" + merge-tools.vimdiff.program="repo" + "###); + // Command argument let stdout = test_env.jj_cmd_success( &repo_path, @@ -175,6 +186,17 @@ fn test_config_layer_override_env() { ui.editor="user" "###); + // Repo + std::fs::write( + repo_path.join(".jj/repo/config.toml"), + format!("{config_key} = {value:?}\n", value = "repo"), + ) + .unwrap(); + let stdout = test_env.jj_cmd_success(&repo_path, &["config", "list", config_key]); + insta::assert_snapshot!(stdout, @r###" + ui.editor="repo" + "###); + // Environment override test_env.add_env_var("JJ_EDITOR", "env-override"); let stdout = test_env.jj_cmd_success(&repo_path, &["config", "list", config_key]); @@ -198,6 +220,37 @@ fn test_config_layer_override_env() { "###); } +#[test] +fn test_config_layer_workspace() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "--git", "main"]); + let main_path = test_env.env_root().join("main"); + let secondary_path = test_env.env_root().join("secondary"); + let config_key = "ui.editor"; + + std::fs::write(main_path.join("file"), "contents").unwrap(); + test_env.jj_cmd_success(&main_path, &["new"]); + test_env.jj_cmd_success( + &main_path, + &["workspace", "add", "--name", "second", "../secondary"], + ); + + // Repo + std::fs::write( + main_path.join(".jj/repo/config.toml"), + format!("{config_key} = {value:?}\n", value = "main-repo"), + ) + .unwrap(); + let stdout = test_env.jj_cmd_success(&main_path, &["config", "list", config_key]); + insta::assert_snapshot!(stdout, @r###" + ui.editor="main-repo" + "###); + let stdout = test_env.jj_cmd_success(&secondary_path, &["config", "list", config_key]); + insta::assert_snapshot!(stdout, @r###" + ui.editor="main-repo" + "###); +} + fn find_stdout_lines(keyname_pattern: &str, stdout: &str) -> String { let key_line_re = Regex::new(&format!(r"(?m)^{keyname_pattern}=.*$")).unwrap(); key_line_re diff --git a/tests/test_global_opts.rs b/tests/test_global_opts.rs index a483fcdfe..edc5a3cd2 100644 --- a/tests/test_global_opts.rs +++ b/tests/test_global_opts.rs @@ -190,6 +190,18 @@ color="always""#, @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 o 0000000000000000000000000000000000000000 "###); + + // Test that per-repo config overrides the user config. + std::fs::write( + repo_path.join(".jj/repo/config.toml"), + r#"ui.color = "never""#, + ) + .unwrap(); + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]); + insta::assert_snapshot!(stdout, @r###" + @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 + o 0000000000000000000000000000000000000000 + "###); } #[test] diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs index 39490f491..9191f5d3f 100644 --- a/tests/test_log_command.rs +++ b/tests/test_log_command.rs @@ -524,6 +524,33 @@ fn test_default_revset() { ); } +#[test] +fn test_default_revset_per_repo() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + std::fs::write(repo_path.join("file1"), "foo\n").unwrap(); + test_env.jj_cmd_success(&repo_path, &["describe", "-m", "add a file"]); + + // Set configuration to only show the root commit. + std::fs::write( + repo_path.join(".jj/repo/config.toml"), + r#"ui.default-revset = "root""#, + ) + .unwrap(); + + // Log should only contain one line (for the root commit), and not show the + // commit created above. + assert_eq!( + 1, + test_env + .jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]) + .lines() + .count() + ); +} + #[test] fn test_log_author_timestamp() { let test_env = TestEnvironment::default();