From 26c9f1b7a221decfea2911f51eb102e796b362c0 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 17 Dec 2024 16:53:38 +0900 Subject: [PATCH] cli: resolve conditional config scopes This is an alternative way to achieve includeIf of Git without adding "include" directive. Conditional include (or include in general) is a bit trickier to implement than loading all files and filtering the contents. Closes #616 --- CHANGELOG.md | 3 + cli/src/cli_util.rs | 6 +- cli/src/complete.rs | 6 +- cli/src/config.rs | 28 ++++++++- cli/tests/test_config_command.rs | 101 +++++++++++++++++++++++++++++++ cli/tests/test_global_opts.rs | 52 ++++++++++++++++ docs/config.md | 24 ++++++++ 7 files changed, 213 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c94e2cbe4c..09adc28ae3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). `snapshot.max-new-file-size` config option. It will print a warning and large files will be left untracked. +* Configuration files now support [conditional + variables](docs/config.md#conditional-variables). + * New command options `--config=NAME=VALUE` and `--config-file=PATH` to set string value without quoting and to load additional configuration from files. diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index b625d8ba40..a3282722b5 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -3650,7 +3650,7 @@ impl CliRunner { config_env.reset_repo_path(loader.repo_path()); config_env.reload_repo_config(&mut raw_config)?; } - let mut config = raw_config.as_ref().clone(); // TODO + let mut config = config_env.resolve_config(&raw_config)?; ui.reset(&config)?; if env::var_os("COMPLETE").is_some() { @@ -3661,7 +3661,7 @@ impl CliRunner { let (args, config_layers) = parse_early_args(&self.app, &string_args)?; if !config_layers.is_empty() { raw_config.as_mut().extend_layers(config_layers); - config = raw_config.as_ref().clone(); // TODO + config = config_env.resolve_config(&raw_config)?; ui.reset(&config)?; } if !args.config_toml.is_empty() { @@ -3687,7 +3687,7 @@ impl CliRunner { .map_err(|err| map_workspace_load_error(err, Some(path)))?; config_env.reset_repo_path(loader.repo_path()); config_env.reload_repo_config(&mut raw_config)?; - config = raw_config.as_ref().clone(); // TODO + config = config_env.resolve_config(&raw_config)?; Ok(loader) } else { maybe_cwd_workspace_loader diff --git a/cli/src/complete.rs b/cli/src/complete.rs index e92578b9a4..c10278a59f 100644 --- a/cli/src/complete.rs +++ b/cli/src/complete.rs @@ -693,7 +693,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { config_env.reset_repo_path(loader.repo_path()); let _ = config_env.reload_repo_config(&mut raw_config); } - let mut config = raw_config.as_ref().clone(); /* TODO */ + let mut config = config_env.resolve_config(&raw_config)?; // skip 2 because of the clap_complete prelude: jj -- jj let args = std::env::args_os().skip(2); let args = expand_args(&ui, &app, args, &config)?; @@ -710,7 +710,9 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> { if let Ok(loader) = DefaultWorkspaceLoaderFactory.create(&cwd.join(&repository)) { config_env.reset_repo_path(loader.repo_path()); let _ = config_env.reload_repo_config(&mut raw_config); - config = raw_config.as_ref().clone(); // TODO + if let Ok(new_config) = config_env.resolve_config(&raw_config) { + config = new_config; + } } cmd_args.push("--repository".into()); cmd_args.push(repository); diff --git a/cli/src/config.rs b/cli/src/config.rs index 1a83e3ebab..1e8a652c0f 100644 --- a/cli/src/config.rs +++ b/cli/src/config.rs @@ -23,9 +23,11 @@ use std::process::Command; use itertools::Itertools; use jj_lib::config::ConfigFile; +use jj_lib::config::ConfigGetError; use jj_lib::config::ConfigLayer; use jj_lib::config::ConfigLoadError; use jj_lib::config::ConfigNamePathBuf; +use jj_lib::config::ConfigResolutionContext; use jj_lib::config::ConfigSource; use jj_lib::config::ConfigValue; use jj_lib::config::StackedConfig; @@ -247,6 +249,8 @@ impl UnresolvedConfigEnv { #[derive(Clone, Debug)] pub struct ConfigEnv { + home_dir: Option, + repo_path: Option, user_config_path: ConfigPath, repo_config_path: ConfigPath, } @@ -254,12 +258,18 @@ pub struct ConfigEnv { impl ConfigEnv { /// Initializes configuration loader based on environment variables. pub fn from_environment() -> Result { + // Canonicalize home as we do canonicalize cwd in CliRunner. + // TODO: Maybe better to switch to dunce::canonicalize() and remove + // canonicalization of cwd, home, etc.? + let home_dir = dirs::home_dir().map(|path| path.canonicalize().unwrap_or(path)); let env = UnresolvedConfigEnv { config_dir: dirs::config_dir(), - home_dir: dirs::home_dir(), + home_dir: home_dir.clone(), jj_config: env::var("JJ_CONFIG").ok(), }; Ok(ConfigEnv { + home_dir, + repo_path: None, user_config_path: env.resolve()?, repo_config_path: ConfigPath::Unavailable, }) @@ -324,6 +334,7 @@ impl ConfigEnv { /// Sets the directory where repo-specific config file is stored. The path /// is usually `.jj/repo`. pub fn reset_repo_path(&mut self, path: &Path) { + self.repo_path = Some(path.to_owned()); self.repo_config_path = ConfigPath::new(Some(path.join("config.toml"))); } @@ -371,6 +382,16 @@ impl ConfigEnv { } Ok(()) } + + /// Resolves conditional scopes within the current environment. Returns new + /// resolved config. + pub fn resolve_config(&self, config: &RawConfig) -> Result { + let context = ConfigResolutionContext { + home_dir: self.home_dir.as_deref(), + repo_path: self.repo_path.as_deref(), + }; + jj_lib::config::resolve(config.as_ref(), &context) + } } fn config_files_for( @@ -1188,9 +1209,10 @@ mod tests { impl TestCase { fn resolve(&self, root: &Path) -> Result { + let home_dir = self.env.home_dir.as_ref().map(|p| root.join(p)); let env = UnresolvedConfigEnv { config_dir: self.env.config_dir.as_ref().map(|p| root.join(p)), - home_dir: self.env.home_dir.as_ref().map(|p| root.join(p)), + home_dir: home_dir.clone(), jj_config: self .env .jj_config @@ -1198,6 +1220,8 @@ mod tests { .map(|p| root.join(p).to_str().unwrap().to_string()), }; Ok(ConfigEnv { + home_dir, + repo_path: None, user_config_path: env.resolve()?, repo_config_path: ConfigPath::Unavailable, }) diff --git a/cli/tests/test_config_command.rs b/cli/tests/test_config_command.rs index 566da658db..3abdef3140 100644 --- a/cli/tests/test_config_command.rs +++ b/cli/tests/test_config_command.rs @@ -1047,6 +1047,107 @@ fn test_config_path_syntax() { "###); } +#[test] +#[cfg(not(windows))] // TODO: dirs::home_dir() can't be overridden by $HOME +fn test_config_conditional() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo1"]); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo2"]); + let repo1_path = test_env.home_dir().join("repo1"); + let repo2_path = test_env.home_dir().join("repo2"); + // Test with fresh new config file + let user_config_path = test_env.env_root().join("config.toml"); + test_env.set_config_path(user_config_path.clone()); + std::fs::write( + &user_config_path, + indoc! {" + foo = 'global' + [[--scope]] + --when.repositories = ['~/repo1'] + foo = 'repo1' + [[--scope]] + --when.repositories = ['~/repo2'] + foo = 'repo2' + "}, + ) + .unwrap(); + + // get and list should refer to the resolved config + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "get", "foo"]); + insta::assert_snapshot!(stdout, @"global"); + let stdout = test_env.jj_cmd_success(&repo1_path, &["config", "get", "foo"]); + insta::assert_snapshot!(stdout, @"repo1"); + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "list", "--user"]); + insta::assert_snapshot!(stdout, @"foo = 'global'"); + let stdout = test_env.jj_cmd_success(&repo1_path, &["config", "list", "--user"]); + insta::assert_snapshot!(stdout, @"foo = 'repo1'"); + let stdout = test_env.jj_cmd_success(&repo2_path, &["config", "list", "--user"]); + insta::assert_snapshot!(stdout, @"foo = 'repo2'"); + + // relative workspace path + let stdout = test_env.jj_cmd_success(&repo2_path, &["config", "list", "--user", "-R../repo1"]); + insta::assert_snapshot!(stdout, @"foo = 'repo1'"); + + // set and unset should refer to the source config + // (there's no option to update scoped table right now.) + let (_stdout, stderr) = test_env.jj_cmd_ok( + test_env.env_root(), + &["config", "set", "--user", "bar", "new value"], + ); + insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(std::fs::read_to_string(&user_config_path).unwrap(), @r#" + foo = 'global' + bar = "new value" + [[--scope]] + --when.repositories = ['~/repo1'] + foo = 'repo1' + [[--scope]] + --when.repositories = ['~/repo2'] + foo = 'repo2' + "#); + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo1_path, &["config", "unset", "--user", "foo"]); + insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(std::fs::read_to_string(&user_config_path).unwrap(), @r#" + bar = "new value" + [[--scope]] + --when.repositories = ['~/repo1'] + foo = 'repo1' + [[--scope]] + --when.repositories = ['~/repo2'] + foo = 'repo2' + "#); +} + +// Minimal test for Windows where the home directory can't be switched. +// (Can be removed if test_config_conditional() is enabled on Windows.) +#[test] +fn test_config_conditional_without_home_dir() { + let mut test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + // Test with fresh new config file + let user_config_path = test_env.env_root().join("config.toml"); + test_env.set_config_path(user_config_path.clone()); + std::fs::write( + &user_config_path, + format!( + indoc! {" + foo = 'global' + [[--scope]] + --when.repositories = [{repo_path}] + foo = 'repo' + "}, + repo_path = toml_edit::Value::from(repo_path.to_str().unwrap()) + ), + ) + .unwrap(); + + let stdout = test_env.jj_cmd_success(test_env.env_root(), &["config", "get", "foo"]); + insta::assert_snapshot!(stdout, @"global"); + let stdout = test_env.jj_cmd_success(&repo_path, &["config", "get", "foo"]); + insta::assert_snapshot!(stdout, @"repo"); +} + #[test] fn test_config_show_paths() { let test_env = TestEnvironment::default(); diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index cc6ffa3ae4..bef3c1768b 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -721,6 +721,58 @@ fn test_invalid_config_value() { "); } +#[test] +#[cfg(not(windows))] // TODO: dirs::home_dir() can't be overridden by $HOME +fn test_conditional_config() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo1"]); + test_env.jj_cmd_ok(test_env.home_dir(), &["git", "init", "repo2"]); + test_env.add_config(indoc! {" + aliases.foo = ['new', 'root()', '-mglobal'] + [[--scope]] + --when.repositories = ['~'] + aliases.foo = ['new', 'root()', '-mhome'] + [[--scope]] + --when.repositories = ['~/repo1'] + aliases.foo = ['new', 'root()', '-mrepo1'] + "}); + + let stdout = test_env.jj_cmd_success( + test_env.env_root(), + &["config", "list", "--include-overridden", "aliases"], + ); + insta::assert_snapshot!(stdout, @"aliases.foo = ['new', 'root()', '-mglobal']"); + let stdout = test_env.jj_cmd_success( + &test_env.home_dir().join("repo1"), + &["config", "list", "--include-overridden", "aliases"], + ); + insta::assert_snapshot!(stdout, @r" + # aliases.foo = ['new', 'root()', '-mglobal'] + # aliases.foo = ['new', 'root()', '-mhome'] + aliases.foo = ['new', 'root()', '-mrepo1'] + "); + let stdout = test_env.jj_cmd_success( + &test_env.home_dir().join("repo2"), + &["config", "list", "--include-overridden", "aliases"], + ); + insta::assert_snapshot!(stdout, @r" + # aliases.foo = ['new', 'root()', '-mglobal'] + aliases.foo = ['new', 'root()', '-mhome'] + "); + + // Aliases can be expanded by using the conditional tables + let (_stdout, stderr) = test_env.jj_cmd_ok(&test_env.home_dir().join("repo1"), &["foo"]); + insta::assert_snapshot!(stderr, @r" + Working copy now at: royxmykx 82899b03 (empty) repo1 + Parent commit : zzzzzzzz 00000000 (empty) (no description set) + "); + let (_stdout, stderr) = test_env.jj_cmd_ok(&test_env.home_dir().join("repo2"), &["foo"]); + insta::assert_snapshot!(stderr, @r" + Working copy now at: yqosqzyt 3bd315a9 (empty) home + Parent commit : zzzzzzzz 00000000 (empty) (no description set) + "); +} + #[test] fn test_no_user_configured() { // Test that the user is reminded if they haven't configured their name or email diff --git a/docs/config.md b/docs/config.md index be9292d3d9..6ca5e62461 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1219,3 +1219,27 @@ To load an entire TOML document, use `--config-file`: ```shell jj --config-file=extra-config.toml log ``` + +### Conditional variables + +You can conditionally enable config variables by using `--when` and +`[[--scope]]` tables. Variables defined in the `[[--scope]]` table are expanded +to the root table. `--when` specifies the condition to enable the scope table. + +```toml +[user] +name = "YOUR NAME" +email = "YOUR_DEFAULT_EMAIL@example.com" + +# override user.email if the repository is located under ~/oss +[[--scope]] +--when.repositories = ["~/oss"] +[--scope.user] +email = "YOUR_OSS_EMAIL@example.org" +``` + +Condition keys: + +* `--when.repositories`: List of paths to match the repository path prefix. + +If no condition is specified, the table is always enabled.