cli: warn if trunk() alias cannot be resolved, fall back to none()

This patch replaces all call sites with present(trunk()), and adds an explicit
check for unresolvable trunk(). If we add coalesce() expression, maybe it can
be rewritten to coalesce(present(trunk()), builtin_trunk()).

Fixes #4616

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
 

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())
     }
 

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": {

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 '<bookmark>@<remote>'. 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()'

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<'_>,

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"])
 }

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#"

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
 

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