diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b6abbd72f..270be32916 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed bugs +* Error on `trunk()` revset resolution is now handled gracefully. + [#4616](https://github.com/martinvonz/jj/issues/4616) ## [0.22.0] - 2024-10-02 diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index d93665404e..011b655792 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -382,6 +382,7 @@ impl CommandHelper { let op_head = self.resolve_operation(ui, workspace.repo_loader())?; let repo = workspace.repo_loader().load_at(&op_head)?; let env = self.workspace_environment(ui, &workspace)?; + revset_util::warn_unresolvable_trunk(ui, repo.as_ref(), &env.revset_parse_context())?; WorkspaceCommandHelper::new(ui, workspace, repo, env, self.is_at_head_operation()) } diff --git a/cli/src/config-schema.json b/cli/src/config-schema.json index b3faa02d78..bcb2631437 100644 --- a/cli/src/config-schema.json +++ b/cli/src/config-schema.json @@ -389,7 +389,7 @@ "log": { "type": "string", "description": "Default set of revisions to show when no explicit revset is given for jj log and similar commands", - "default": "present(@) | ancestors(immutable_heads().., 2) | trunk()" + "default": "present(@) | ancestors(immutable_heads().., 2) | present(trunk())" }, "short-prefixes": { "type": "string", @@ -408,7 +408,7 @@ "immutable_heads()": { "type": "string", "description": "Revisions to consider immutable. Ancestors of these are also considered immutable. The root commit is always considered immutable.", - "default": "trunk() | tags() | untracked_remote_branches()" + "default": "present(trunk()) | tags() | untracked_remote_branches()" } }, "additionalProperties": { diff --git a/cli/src/config/revsets.toml b/cli/src/config/revsets.toml index b07b3cdfbf..e036670dcb 100644 --- a/cli/src/config/revsets.toml +++ b/cli/src/config/revsets.toml @@ -3,9 +3,14 @@ [revsets] fix = "reachable(@, mutable())" -log = "present(@) | ancestors(immutable_heads().., 2) | trunk()" +# log revset is also used as the default short-prefixes. If it failed to +# evaluate, lengthy warning messages would be printed. Use present(expr) to +# suppress symbol resolution error. +log = "present(@) | ancestors(immutable_heads().., 2) | present(trunk())" [revset-aliases] +# trunk() can be overridden as '@'. Use present(trunk()) if +# symbol resolution error should be suppressed. 'trunk()' = ''' latest( remote_bookmarks(exact:"main", exact:"origin") | @@ -18,7 +23,9 @@ latest( ) ''' -'builtin_immutable_heads()' = 'trunk() | tags() | untracked_remote_bookmarks()' +# If immutable_heads() failed to evaluate, many jj commands wouldn't work. Use +# present(expr) to suppress symbol resolution error. +'builtin_immutable_heads()' = 'present(trunk()) | tags() | untracked_remote_bookmarks()' 'immutable_heads()' = 'builtin_immutable_heads()' 'immutable()' = '::(immutable_heads() | root())' 'mutable()' = '~immutable()' diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index cdfe140e83..9391bc4e0e 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::io; use std::rc::Rc; use std::sync::Arc; @@ -213,6 +214,37 @@ pub fn parse_immutable_heads_expression( Ok(heads.union(&RevsetExpression::root())) } +/// Prints warning if `trunk()` alias cannot be resolved. This alias could be +/// generated by `jj git init`/`clone`. +pub(super) fn warn_unresolvable_trunk( + ui: &Ui, + repo: &dyn Repo, + context: &RevsetParseContext, +) -> io::Result<()> { + let (_, _, revset_str) = context + .aliases_map() + .get_function("trunk", 0) + .expect("trunk() should be defined by default"); + let Ok(expression) = revset::parse(&mut RevsetDiagnostics::new(), revset_str, context) else { + // Parse error would have been reported. + return Ok(()); + }; + // Not using IdPrefixContext since trunk() revset shouldn't contain short + // prefixes. + let symbol_resolver = DefaultSymbolResolver::new(repo, context.symbol_resolvers()); + if let Err(err) = expression.resolve_user_expression(repo, &symbol_resolver) { + writeln!( + ui.warning_default(), + "Failed to resolve `revset-aliases.trunk()`: {err}" + )?; + writeln!( + ui.hint_default(), + "Use `jj config edit --repo` to adjust the `trunk()` alias." + )?; + } + Ok(()) +} + pub(super) fn evaluate_revset_to_single_commit<'a>( revision_str: &str, expression: &RevsetExpressionEvaluator<'_>, diff --git a/cli/tests/test_git_clone.rs b/cli/tests/test_git_clone.rs index a9022346b2..0ce982e47b 100644 --- a/cli/tests/test_git_clone.rs +++ b/cli/tests/test_git_clone.rs @@ -522,6 +522,41 @@ fn test_git_clone_with_remote_name() { "#); } +#[test] +fn test_git_clone_trunk_deleted() { + let test_env = TestEnvironment::default(); + let git_repo_path = test_env.env_root().join("source"); + let git_repo = git2::Repository::init(git_repo_path).unwrap(); + set_up_non_empty_git_repo(&git_repo); + let clone_path = test_env.env_root().join("clone"); + + let (stdout, stderr) = + test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "source", "clone"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r#" + Fetching into new repo in "$TEST_ENV/clone" + bookmark: main@origin [new] untracked + Setting the revset alias "trunk()" to "main@origin" + Working copy now at: sqpuoqvx cad212e1 (empty) (no description set) + Parent commit : mzyxwzks 9f01a0e0 main | message + Added 1 files, modified 0 files, removed 0 files + "#); + + test_env.jj_cmd_ok(&clone_path, &["bookmark", "forget", "main"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["log"]); + insta::assert_snapshot!(stdout, @r#" + @ sqpuoqvx test.user@example.com 2001-02-03 08:05:07 cad212e1 + │ (empty) (no description set) + ○ mzyxwzks some.one@example.com 1970-01-01 11:00:00 9f01a0e0 + │ message + ◆ zzzzzzzz root() 00000000 + "#); + insta::assert_snapshot!(stderr, @r#" + Warning: Failed to resolve `revset-aliases.trunk()`: Revision "main@origin" doesn't exist + Hint: Use `jj config edit --repo` to adjust the `trunk()` alias. + "#); +} + fn get_bookmark_output(test_env: &TestEnvironment, repo_path: &Path) -> String { test_env.jj_cmd_success(repo_path, &["bookmark", "list", "--all-remotes"]) } diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index cdfd0019d4..793f6fb0be 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -789,10 +789,6 @@ fn test_op_diff() { test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "git-repo", "repo"]); let repo_path = test_env.env_root().join("repo"); - // Suppress warning in the commit summary template. The configured "trunk()" - // can't be found in earlier operations. - test_env.add_config("template-aliases.'format_short_id(id)' = 'id.short(8)'"); - // Overview of op log. let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log"]); insta::assert_snapshot!(&stdout, @r#" @@ -1586,10 +1582,6 @@ fn test_op_show() { test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "git-repo", "repo"]); let repo_path = test_env.env_root().join("repo"); - // Suppress warning in the commit summary template. The configured "trunk()" - // can't be found in earlier operations. - test_env.add_config("template-aliases.'format_short_id(id)' = 'id.short(8)'"); - // Overview of op log. let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log"]); insta::assert_snapshot!(&stdout, @r#" diff --git a/docs/config.md b/docs/config.md index c1cf2155eb..e4fa406613 100644 --- a/docs/config.md +++ b/docs/config.md @@ -266,8 +266,8 @@ diff-invocation-mode = "file-by-file" You can configure the set of immutable commits via `revset-aliases."immutable_heads()"`. The default set of immutable heads is -`trunk() | tags() | untracked_remote_bookmarks()`. For example, to prevent -rewriting commits on `main@origin` and commits authored by other users: +`present(trunk()) | tags() | untracked_remote_bookmarks()`. For example, to +prevent rewriting commits on `main@origin` and commits authored by other users: ```toml # The `main.. &` bit is an optimization to scan for non-`mine()` commits only @@ -290,7 +290,7 @@ revsets.log = "main@origin.." ``` The default value for `revsets.log` is -`'present(@) | ancestors(immutable_heads().., 2) | trunk()'`. +`'present(@) | ancestors(immutable_heads().., 2) | present(trunk())'`. ### Graph style diff --git a/docs/revsets.md b/docs/revsets.md index efd2c2d98d..bb9e900468 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -429,15 +429,15 @@ for a comprehensive list. 'trunk()' = 'your-bookmark@your-remote' ``` -* `builtin_immutable_heads()`: Resolves to `trunk() | tags() | untracked_remote_bookmarks()`. - It is used as the default definition for `immutable_heads()` below. it is not - recommended to redefined this alias. Prefer to redefine `immutable_heads()` - instead. - -* `immutable_heads()`: Resolves to `trunk() | tags() | untracked_remote_bookmarks()` - by default. It is actually defined as `builtin_immutable_heads()`, and can be - overridden as required. See [here](config.md#set-of-immutable-commits) for - details. +* `builtin_immutable_heads()`: Resolves to + `present(trunk()) | tags() | untracked_remote_bookmarks()`. It is used as the + default definition for `immutable_heads()` below. it is not recommended to + redefined this alias. Prefer to redefine `immutable_heads()` instead. + +* `immutable_heads()`: Resolves to + `present(trunk()) | tags() | untracked_remote_bookmarks()` by default. It is + actually defined as `builtin_immutable_heads()`, and can be overridden as + required. See [here](config.md#set-of-immutable-commits) for details. * `immutable()`: The set of commits that `jj` treats as immutable. This is equivalent to `::(immutable_heads() | root())`. It is not recommended to redefine