Skip to content

Commit

Permalink
cli: integrate config migration
Browse files Browse the repository at this point in the history
The list of migration rules is managed by CliRunner. I don't know if that's
needed, but in theory, an extension may insert migration rules as well as
default config layers.

Migration could be handled by ConfigEnv::resolve_config(), but it seemed rather
complicated because Vec<ConfigMigrationRule> cannot be cloned, and the scope of
these variables are a bit different.
  • Loading branch information
yuja committed Jan 8, 2025
1 parent 8d3f2fb commit 1fc1851
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
30 changes: 29 additions & 1 deletion cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use jj_lib::commit::Commit;
use jj_lib::config::ConfigGetError;
use jj_lib::config::ConfigGetResultExt as _;
use jj_lib::config::ConfigLayer;
use jj_lib::config::ConfigMigrationRule;
use jj_lib::config::ConfigNamePathBuf;
use jj_lib::config::ConfigSource;
use jj_lib::config::StackedConfig;
Expand Down Expand Up @@ -290,6 +291,7 @@ struct CommandHelperData {
matches: ArgMatches,
global_args: GlobalArgs,
config_env: ConfigEnv,
config_migrations: Vec<ConfigMigrationRule>,
raw_config: RawConfig,
settings: UserSettings,
revset_extensions: Arc<RevsetExtensions>,
Expand Down Expand Up @@ -356,7 +358,10 @@ impl CommandHelper {
let repo_path = workspace_root.join(".jj").join("repo");
config_env.reset_repo_path(&repo_path);
config_env.reload_repo_config(&mut raw_config)?;
let config = config_env.resolve_config(&raw_config)?;
let mut config = config_env.resolve_config(&raw_config)?;
// No migration messages here, which would usually be emitted before.
jj_lib::config::migrate(&mut config, &self.data.config_migrations)
.map_err(|err| config_error_with_message("Migration failed", err))?;
Ok(self.data.settings.with_new_config(config)?)
}

Expand Down Expand Up @@ -3451,6 +3456,7 @@ pub struct CliRunner {
tracing_subscription: TracingSubscription,
app: Command,
config_layers: Vec<ConfigLayer>,
config_migrations: Vec<ConfigMigrationRule>,
store_factories: StoreFactories,
working_copy_factories: WorkingCopyFactories,
workspace_loader_factory: Box<dyn WorkspaceLoaderFactory>,
Expand All @@ -3476,6 +3482,7 @@ impl CliRunner {
tracing_subscription,
app: crate::commands::default_app(),
config_layers: crate::config::default_config_layers(),
config_migrations: crate::config::default_config_migrations(),
store_factories: StoreFactories::default(),
working_copy_factories: default_working_copy_factories(),
workspace_loader_factory: Box::new(DefaultWorkspaceLoaderFactory),
Expand Down Expand Up @@ -3516,6 +3523,12 @@ impl CliRunner {
self
}

/// Adds config migration rule in addition to the default rules.
pub fn add_extra_config_migration(mut self, rule: ConfigMigrationRule) -> Self {
self.config_migrations.push(rule);
self
}

/// Adds `StoreFactories` to be used.
pub fn add_store_factories(mut self, store_factories: StoreFactories) -> Self {
self.store_factories.merge(store_factories);
Expand Down Expand Up @@ -3627,6 +3640,13 @@ impl CliRunner {
)
})?;
let mut config_env = ConfigEnv::from_environment()?;
let mut last_config_migration_descriptions = Vec::new();
let mut migrate_config = |config: &mut StackedConfig| -> Result<(), CommandError> {
last_config_migration_descriptions =
jj_lib::config::migrate(config, &self.config_migrations)
.map_err(|err| config_error_with_message("Migration failed", err))?;
Ok(())
};
// Use cwd-relative workspace configs to resolve default command and
// aliases. WorkspaceLoader::init() won't do any heavy lifting other
// than the path resolution.
Expand All @@ -3640,6 +3660,7 @@ impl CliRunner {
config_env.reload_repo_config(&mut raw_config)?;
}
let mut config = config_env.resolve_config(&raw_config)?;
migrate_config(&mut config)?;
ui.reset(&config)?;

if env::var_os("COMPLETE").is_some() {
Expand All @@ -3651,6 +3672,7 @@ impl CliRunner {
if !config_layers.is_empty() {
raw_config.as_mut().extend_layers(config_layers);
config = config_env.resolve_config(&raw_config)?;
migrate_config(&mut config)?;
ui.reset(&config)?;
}
if !args.config_toml.is_empty() {
Expand All @@ -3677,13 +3699,18 @@ impl CliRunner {
config_env.reset_repo_path(loader.repo_path());
config_env.reload_repo_config(&mut raw_config)?;
config = config_env.resolve_config(&raw_config)?;
migrate_config(&mut config)?;
Ok(loader)
} else {
maybe_cwd_workspace_loader
};

// Apply workspace configs and --config arguments.
ui.reset(&config)?;
// Print only the last migration messages to omit duplicates.
for desc in &last_config_migration_descriptions {
writeln!(ui.warning_default(), "Deprecated config: {desc}")?;
}

// If -R or --config* is specified, check if the expanded arguments differ.
if args.global_args.repository.is_some() || args.global_args.early_args.has_config_args() {
Expand All @@ -3705,6 +3732,7 @@ impl CliRunner {
matches,
global_args: args.global_args,
config_env,
config_migrations: self.config_migrations,
raw_config,
settings,
revset_extensions: self.revset_extensions.into(),
Expand Down
1 change: 1 addition & 0 deletions cli/src/complete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@ fn get_jj_command() -> Result<(JjBuilder, UserSettings), CommandError> {
let cwd = std::env::current_dir()
.and_then(dunce::canonicalize)
.map_err(user_error)?;
// No config migration for completion. Simply ignore deprecated variables.
let mut config_env = ConfigEnv::from_environment()?;
let maybe_cwd_workspace_loader = DefaultWorkspaceLoaderFactory.create(find_workspace_dir(&cwd));
let _ = config_env.reload_user_config(&mut raw_config);
Expand Down
6 changes: 6 additions & 0 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use jj_lib::config::ConfigFile;
use jj_lib::config::ConfigGetError;
use jj_lib::config::ConfigLayer;
use jj_lib::config::ConfigLoadError;
use jj_lib::config::ConfigMigrationRule;
use jj_lib::config::ConfigNamePathBuf;
use jj_lib::config::ConfigResolutionContext;
use jj_lib::config::ConfigSource;
Expand Down Expand Up @@ -590,6 +591,11 @@ fn parse_config_arg_item(item_str: &str) -> Result<(ConfigNamePathBuf, ConfigVal
Ok((name, value))
}

/// List of rules to migrate deprecated config variables.
pub fn default_config_migrations() -> Vec<ConfigMigrationRule> {
vec![] // TODO
}

/// Command name and arguments specified by config.
#[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize)]
#[serde(untagged)]
Expand Down

0 comments on commit 1fc1851

Please sign in to comment.