cli: load configs from .jj/repo/config.toml

Since per-repo config may contain CLI settings, it must be visible to CLI.
Therefore, UserSettings::with_repo() -> RepoSettings isn't used, and its
implementation is nullified by this commit.

#616
This commit is contained in:
Yuya Nishihara 2023-01-02 14:18:38 +09:00
parent 810789a830
commit ea96ea3ffe
8 changed files with 130 additions and 14 deletions

View file

@ -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

View file

@ -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:

View file

@ -61,15 +61,10 @@ impl UserSettings {
}
}
pub fn with_repo(&self, repo_path: &Path) -> Result<RepoSettings, config::ConfigError> {
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<RepoSettings, config::ConfigError> {
let config = self.config.clone();
Ok(RepoSettings { _config: config })
}

View file

@ -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,

View file

@ -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<config::Config>,
repo: Option<config::Config>,
env_overrides: config::Config,
arg_overrides: Option<config::Config>,
}
@ -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, config::ConfigError> {
config::Config::builder()
.add_source(
config::File::from(path)
.required(false)
.format(config::FileFormat::Toml),
)
.build()
}
fn read_config_path(config_path: &Path) -> Result<config::Config, config::ConfigError> {
let mut files = vec![];
if config_path.is_dir() {

View file

@ -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

View file

@ -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]

View file

@ -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();