From 6ec1c8e7d516742e0387e3285e92bf48a2e2ff99 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 8 Dec 2025 12:17:51 +0900 Subject: [PATCH] settings: parse auto-track-bookmarks as string matcher expressions The settings field is now wrapped in Option because [remotes.] may have multiple fields, and some of them can be omitted. --- CHANGELOG.md | 3 + cli/src/commands/bookmark/create.rs | 6 +- cli/src/commands/bookmark/set.rs | 6 +- cli/src/config-schema.json | 2 +- cli/src/git_util.rs | 8 +-- cli/src/revset_util.rs | 39 +++++++++++++ cli/tests/test_bookmark_command.rs | 85 ++++++++++++++++++++++++++--- cli/tests/test_git_clone.rs | 5 +- cli/tests/test_git_fetch.rs | 4 +- cli/tests/test_git_init.rs | 4 +- cli/tests/test_git_push.rs | 2 +- lib/src/settings.rs | 26 ++------- 12 files changed, 145 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ccb3fa70..7b7ddd139 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Breaking changes +* `remotes..auto-track-bookmarks` is now parsed the same way they + are in revsets and can be combined with logical operators. + * On Windows, symlinks that point to a path with `/` won't be supported. This path is [invalid on Windows]. diff --git a/cli/src/commands/bookmark/create.rs b/cli/src/commands/bookmark/create.rs index 5777e0105..51d5ed23b 100644 --- a/cli/src/commands/bookmark/create.rs +++ b/cli/src/commands/bookmark/create.rs @@ -82,12 +82,14 @@ pub fn cmd_bookmark_create( let mut tx = workspace_command.start_transaction(); let remote_settings = tx.settings().remote_settings()?; + let remote_auto_track_matchers = + revset_util::parse_remote_auto_track_bookmarks_map(ui, &remote_settings)?; let readonly_repo = tx.base_repo().clone(); for name in bookmark_names { tx.repo_mut() .set_local_bookmark_target(name, RefTarget::normal(target_commit.id().clone())); - for (remote_name, settings) in &remote_settings { - if !settings.auto_track_bookmarks.is_match(name.as_str()) { + for (remote_name, matcher) in &remote_auto_track_matchers { + if !matcher.is_match(name.as_str()) { continue; } let Some(view) = readonly_repo.view().get_remote_view(remote_name) else { diff --git a/cli/src/commands/bookmark/set.rs b/cli/src/commands/bookmark/set.rs index b2bfb02ad..8db53c842 100644 --- a/cli/src/commands/bookmark/set.rs +++ b/cli/src/commands/bookmark/set.rs @@ -93,13 +93,15 @@ pub fn cmd_bookmark_set( let mut tx = workspace_command.start_transaction(); let remote_settings = tx.settings().remote_settings()?; + let remote_auto_track_matchers = + revset_util::parse_remote_auto_track_bookmarks_map(ui, &remote_settings)?; let readonly_repo = tx.base_repo().clone(); for name in bookmark_names { tx.repo_mut() .set_local_bookmark_target(name, RefTarget::normal(target_commit.id().clone())); if new_bookmarks.contains(name) { - for (remote_name, settings) in &remote_settings { - if !settings.auto_track_bookmarks.is_match(name.as_str()) { + for (remote_name, matcher) in &remote_auto_track_matchers { + if !matcher.is_match(name.as_str()) { continue; } let Some(view) = readonly_repo.view().get_remote_view(remote_name) else { diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index 80faeedd2..aeba71dd1 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -495,7 +495,7 @@ "auto-track-bookmarks": { "type": "string", "description": "A string pattern describing the bookmarks to automatically track with this remote. It will be applied to any new bookmark, created or fetched. See https://docs.jj-vcs.dev/latest/config/#automatic-tracking-of-bookmarks", - "default": "" + "default": "~glob:*" } } } diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index d30a8e6b6..446efb618 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -53,6 +53,7 @@ use crate::command_error::cli_error; use crate::command_error::user_error; use crate::formatter::Formatter; use crate::formatter::FormatterExt as _; +use crate::revset_util::parse_remote_auto_track_bookmarks_map; use crate::ui::ProgressOutput; use crate::ui::Ui; @@ -284,17 +285,14 @@ pub fn with_remote_git_callbacks(ui: &Ui, f: impl FnOnce(git::RemoteCallbacks } pub fn load_git_import_options( - _ui: &Ui, // TODO: write parser warnings to ui + ui: &Ui, git_settings: &GitSettings, remote_settings: &RemoteSettingsMap, ) -> Result { Ok(GitImportOptions { auto_local_bookmark: git_settings.auto_local_bookmark, abandon_unreachable_commits: git_settings.abandon_unreachable_commits, - remote_auto_track_bookmarks: remote_settings - .iter() - .map(|(name, settings)| (name.clone(), settings.auto_track_bookmarks.to_matcher())) - .collect(), + remote_auto_track_bookmarks: parse_remote_auto_track_bookmarks_map(ui, remote_settings)?, }) } diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index e8dcdabf2..b56a39a63 100644 --- a/cli/src/revset_util.rs +++ b/cli/src/revset_util.rs @@ -14,6 +14,7 @@ //! Utility for parsing and evaluating user-provided revset expressions. +use std::collections::HashMap; use std::io; use std::sync::Arc; @@ -26,6 +27,7 @@ use jj_lib::config::ConfigSource; use jj_lib::config::StackedConfig; use jj_lib::id_prefix::IdPrefixContext; use jj_lib::ref_name::RefNameBuf; +use jj_lib::ref_name::RemoteNameBuf; use jj_lib::repo::Repo; use jj_lib::revset; use jj_lib::revset::ResolvedRevsetExpression; @@ -42,10 +44,13 @@ use jj_lib::revset::RevsetResolutionError; use jj_lib::revset::SymbolResolver; use jj_lib::revset::SymbolResolverExtension; use jj_lib::revset::UserRevsetExpression; +use jj_lib::settings::RemoteSettingsMap; use jj_lib::str_util::StringExpression; +use jj_lib::str_util::StringMatcher; use thiserror::Error; use crate::command_error::CommandError; +use crate::command_error::config_error_with_message; use crate::command_error::print_parse_diagnostics; use crate::command_error::revset_parse_error_hint; use crate::command_error::user_error; @@ -370,3 +375,37 @@ where print_parse_diagnostics(ui, "In name pattern", &diagnostics)?; Ok(StringExpression::union_all(expressions)) } + +/// Parses the given `remotes..auto-track-bookmarks` settings into a map +/// of string matchers. +pub fn parse_remote_auto_track_bookmarks_map( + ui: &Ui, + remote_settings: &RemoteSettingsMap, +) -> Result, CommandError> { + let mut matchers = HashMap::new(); + for (name, settings) in remote_settings { + let Some(text) = &settings.auto_track_bookmarks else { + continue; + }; + let mut diagnostics = RevsetDiagnostics::new(); + let expr = revset::parse_string_expression(&mut diagnostics, text).map_err(|err| { + // From, but with different message and error kind + let hint = revset_parse_error_hint(&err); + let message = format!( + "Invalid `remotes.{}.auto-track-bookmarks`: {}", + name.as_symbol(), + err.kind() + ); + let mut cmd_err = config_error_with_message(message, err); + cmd_err.extend_hints(hint); + cmd_err + })?; + print_parse_diagnostics( + ui, + &format!("In `remotes.{}.auto-track-bookmarks`", name.as_symbol()), + &diagnostics, + )?; + matchers.insert(name.clone(), expr.to_matcher()); + } + Ok(matchers) +} diff --git a/cli/tests/test_bookmark_command.rs b/cli/tests/test_bookmark_command.rs index 5c6da26b2..1129995c7 100644 --- a/cli/tests/test_bookmark_command.rs +++ b/cli/tests/test_bookmark_command.rs @@ -1221,7 +1221,7 @@ fn test_bookmark_track_untrack() { "refs/heads/feature2", ], ); - test_env.add_config("remotes.origin.auto-track-bookmarks = ''"); + test_env.add_config("remotes.origin.auto-track-bookmarks = '~glob:*'"); let output = work_dir.run_jj(["git", "fetch"]); insta::assert_snapshot!(output, @r" ------- stderr ------- @@ -1501,7 +1501,7 @@ fn test_bookmark_track_untrack_patterns() { ); // Fetch new commit without auto tracking - test_env.add_config("remotes.origin.auto-track-bookmarks = ''"); + test_env.add_config("remotes.origin.auto-track-bookmarks = '~glob:*'"); let output = work_dir.run_jj(["git", "fetch"]); insta::assert_snapshot!(output, @r" ------- stderr ------- @@ -1673,7 +1673,7 @@ fn test_bookmark_list() { .success(); local_dir .run_jj([ - "--config=remotes.origin.auto-track-bookmarks=''", + "--config=remotes.origin.auto-track-bookmarks='~glob:*'", "bookmark", "create", "local-only", @@ -1926,7 +1926,7 @@ fn test_bookmark_list_filtered() { .success(); local_dir .run_jj([ - "--config=remotes.origin.auto-track-bookmarks=''", + "--config=remotes.origin.auto-track-bookmarks='~glob:*'", "bookmark", "create", "local-keep", @@ -2213,7 +2213,7 @@ fn test_bookmark_list_much_remote_divergence() { } local_dir .run_jj([ - "--config=remotes.origin.auto-track-bookmarks=''", + "--config=remotes.origin.auto-track-bookmarks='~glob:*'", "bookmark", "create", "local-only", @@ -2313,8 +2313,8 @@ fn test_bookmark_list_tracked() { .success(); local_dir .run_jj([ - "--config=remotes.origin.auto-track-bookmarks=''", - "--config=remotes.upstream.auto-track-bookmarks=''", + "--config=remotes.origin.auto-track-bookmarks='~glob:*'", + "--config=remotes.upstream.auto-track-bookmarks='~glob:*'", "bookmark", "create", "local-only", @@ -2588,7 +2588,7 @@ fn test_create_and_set_auto_track_bookmarks() { [remotes.origin] auto-track-bookmarks = 'glob:mine/*' [remotes.fork] - auto-track-bookmarks = 'glob:*' + auto-track-bookmarks = 'glob:mine/* | glob:not-mine/*' ", ); @@ -2660,6 +2660,75 @@ fn test_create_and_set_auto_track_bookmarks() { "); } +#[test] +fn test_bad_auto_track_bookmarks() { + let test_env = TestEnvironment::default(); + test_env.run_jj_in(".", ["git", "init", "repo"]).success(); + let work_dir = test_env.work_dir("repo"); + + // silence "Target revision is empty" warning + work_dir.write_file("file", ""); + + let output = work_dir.run_jj([ + "bookmark", + "create", + "a", + "--config=remotes.origin.auto-track-bookmarks=''", + ]); + insta::assert_snapshot!(output, @r" + ------- stderr ------- + Config error: Invalid `remotes.origin.auto-track-bookmarks`: Syntax error + Caused by: --> 1:1 + | + 1 | + | ^--- + | + = expected + Hint: See https://docs.jj-vcs.dev/latest/revsets/ or use `jj help -k revsets` for revsets syntax and how to quote symbols. + For help, see https://docs.jj-vcs.dev/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "); + + let output = work_dir.run_jj([ + "bookmark", + "create", + "a", + "--config=remotes.origin.auto-track-bookmarks='foo &'", + ]); + insta::assert_snapshot!(output, @r" + ------- stderr ------- + Config error: Invalid `remotes.origin.auto-track-bookmarks`: Syntax error + Caused by: --> 1:6 + | + 1 | foo & + | ^--- + | + = expected `::`, `..`, `~`, or + Hint: See https://docs.jj-vcs.dev/latest/revsets/ or use `jj help -k revsets` for revsets syntax and how to quote symbols. + For help, see https://docs.jj-vcs.dev/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "); + + let output = work_dir.run_jj([ + "bookmark", + "create", + "a", + "--config=remotes.origin.auto-track-bookmarks=[{}]", + ]); + insta::assert_snapshot!(output, @r" + ------- stderr ------- + Config error: Invalid type or value for remotes.origin + Caused by: invalid type: sequence, expected a string + in `auto-track-bookmarks` + + For help, see https://docs.jj-vcs.dev/latest/config/ or use `jj help -k config`. + [EOF] + [exit status: 1] + "); +} + #[must_use] fn get_log_output(work_dir: &TestWorkDir) -> CommandOutput { let template = r#"bookmarks ++ " " ++ commit_id.short()"#; diff --git a/cli/tests/test_git_clone.rs b/cli/tests/test_git_clone.rs index ea687b92c..b45e2833a 100644 --- a/cli/tests/test_git_clone.rs +++ b/cli/tests/test_git_clone.rs @@ -612,8 +612,9 @@ fn test_git_clone_remote_default_bookmark() { [EOF] "#); - // Only the default bookmark will be imported if auto-track-bookmarks = '' - test_env.add_config("remotes.origin.auto-track-bookmarks = ''"); + // Only the default bookmark will be imported if auto-track-bookmarks = + // '~glob:*' + test_env.add_config("remotes.origin.auto-track-bookmarks = '~glob:*'"); let output = root_dir.run_jj(["git", "clone", "source", "clone2"]); insta::assert_snapshot!(output, @r#" ------- stderr ------- diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs index 994f652ce..6e8144a83 100644 --- a/cli/tests/test_git_fetch.rs +++ b/cli/tests/test_git_fetch.rs @@ -1794,8 +1794,8 @@ fn test_git_fetch_remote_only_bookmark() { &[], ); - // Fetch using remotes.origin.auto-track-bookmarks = '' - test_env.add_config("remotes.origin.auto-track-bookmarks = ''"); + // Fetch using remotes.origin.auto-track-bookmarks = '~glob:*' + test_env.add_config("remotes.origin.auto-track-bookmarks = '~glob:*'"); work_dir .run_jj(["git", "fetch", "--remote=origin"]) .success(); diff --git a/cli/tests/test_git_init.rs b/cli/tests/test_git_init.rs index 145acb747..27836ffe1 100644 --- a/cli/tests/test_git_init.rs +++ b/cli/tests/test_git_init.rs @@ -652,8 +652,8 @@ fn test_git_init_colocated_via_git_repo_path_imported_refs() { [EOF] "); - // With remotes.origin.auto-track-bookmarks = '' - test_env.add_config("remotes.origin.auto-track-bookmarks = ''"); + // With remotes.origin.auto-track-bookmarks = '~glob:*' + test_env.add_config("remotes.origin.auto-track-bookmarks = '~glob:*'"); let local_dir = test_env.work_dir("local2"); set_up_local_repo(local_dir.root()); let output = local_dir.run_jj(["git", "init", "--git-repo=."]); diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs index 44e60cf54..3e36c4ddd 100644 --- a/cli/tests/test_git_push.rs +++ b/cli/tests/test_git_push.rs @@ -581,7 +581,7 @@ fn test_git_push_locally_created_and_rewritten() { set_up(&test_env); let work_dir = test_env.work_dir("local"); // Ensure that remote bookmarks aren't tracked automatically - test_env.add_config("remotes.origin.auto-track-bookmarks = ''"); + test_env.add_config("remotes.origin.auto-track-bookmarks = '~glob:*'"); // Push locally-created bookmark work_dir.run_jj(["new", "root()", "-mlocal 1"]).success(); diff --git a/lib/src/settings.rs b/lib/src/settings.rs index 85c846dcd..f7e55d0ee 100644 --- a/lib/src/settings.rs +++ b/lib/src/settings.rs @@ -38,7 +38,6 @@ use crate::config::ToConfigNamePath; use crate::fmt_util::binary_prefix; use crate::ref_name::RemoteNameBuf; use crate::signing::SignBehavior; -use crate::str_util::StringPattern; #[derive(Debug, Clone)] pub struct UserSettings { @@ -61,9 +60,12 @@ struct UserSettingsData { pub type RemoteSettingsMap = HashMap; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] pub struct RemoteSettings { - pub auto_track_bookmarks: StringPattern, + /// String matcher expression whether to track bookmarks automatically. + #[serde(default)] + pub auto_track_bookmarks: Option, } impl RemoteSettings { @@ -72,23 +74,7 @@ impl RemoteSettings { ) -> Result { settings .table_keys("remotes") - .map(|name| { - Ok(( - name.into(), - Self { - auto_track_bookmarks: settings.get_value_with( - ["remotes", name, "auto-track-bookmarks"], - |value| -> Result<_, Box> { - Ok(StringPattern::parse( - value - .as_str() - .ok_or_else(|| "expected a string".to_string())?, - )?) - }, - )?, - }, - )) - }) + .map(|name| Ok((name.into(), settings.get(["remotes", name])?))) .try_collect() } }