cli: print a message about new and resolved conflicts

When e.g. `jj rebase` results in new conflicts, it's useful for the
user to learn about that without having to run `jj log` right
after. This patch adds reporting of new conflicts created by an
operation. It also add reporting of conflicts that were resolved or
abandoned by the operation.

There was no measurable performance impact when rebasing a single
commit in the Linux kernel repo.

CHANGELOG.md

@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### New features
 
+* Information about new and resolved conflicts is now printed by every command.
+
 ### Fixed bugs
 
 

cli/src/cli_util.rs

@@ -28,7 +28,7 @@ use std::{iter, str};
 
 use clap::builder::{NonEmptyStringValueParser, TypedValueParser, ValueParserFactory};
 use clap::{Arg, ArgAction, ArgMatches, Command, FromArgMatches};
-use indexmap::IndexSet;
+use indexmap::{IndexMap, IndexSet};
 use itertools::Itertools;
 use jj_lib::backend::{BackendError, ChangeId, CommitId, MergedTreeId, ObjectId};
 use jj_lib::commit::Commit;
@@ -52,8 +52,8 @@ use jj_lib::repo::{
 use jj_lib::repo_path::{FsPathParseError, RepoPath, RepoPathBuf};
 use jj_lib::revset::{
     DefaultSymbolResolver, Revset, RevsetAliasesMap, RevsetCommitRef, RevsetEvaluationError,
-    RevsetExpression, RevsetIteratorExt, RevsetParseContext, RevsetParseError,
-    RevsetParseErrorKind, RevsetResolutionError, RevsetWorkspaceContext,
+    RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt, RevsetParseContext,
+    RevsetParseError, RevsetParseErrorKind, RevsetResolutionError, RevsetWorkspaceContext,
 };
 use jj_lib::rewrite::restore_tree;
 use jj_lib::settings::{ConfigResultExt as _, UserSettings};
@@ -1444,8 +1444,9 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
             writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?;
         }
 
-        let maybe_old_wc_commit = tx
-            .base_repo()
+        let old_repo = tx.base_repo().clone();
+
+        let maybe_old_wc_commit = old_repo
             .view()
             .get_wc_commit_id(self.workspace_id())
             .map(|commit_id| tx.base_repo().store().get_commit(commit_id))
@@ -1465,6 +1466,8 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
             print_failed_git_export(ui, &failed_branches)?;
         }
         self.user_repo = ReadonlyUserRepo::new(tx.commit());
+        self.report_repo_changes(ui, &old_repo)?;
+
         if self.may_update_working_copy {
             if let Some(new_commit) = &maybe_new_wc_commit {
                 self.update_working_copy(ui, maybe_old_wc_commit.as_ref(), new_commit)?;
@@ -1484,6 +1487,102 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
         }
         Ok(())
     }
+
+    /// Inform the user about important changes to the repo since the previous
+    /// operation (when `old_repo` was loaded).
+    fn report_repo_changes(
+        &self,
+        ui: &mut Ui,
+        old_repo: &Arc<ReadonlyRepo>,
+    ) -> Result<(), CommandError> {
+        let old_view = old_repo.view();
+        let new_repo = self.repo().as_ref();
+        let new_view = new_repo.view();
+        let added_heads = RevsetExpression::commits(
+            new_view
+                .heads()
+                .iter()
+                .filter(|id| !old_view.heads().contains(id))
+                .cloned()
+                .collect(),
+        );
+        let removed_heads = RevsetExpression::commits(
+            old_view
+                .heads()
+                .iter()
+                .filter(|id| !new_view.heads().contains(id))
+                .cloned()
+                .collect(),
+        );
+        // Filter the revsets by conflicts instead of reading all commits and doing the
+        // filtering here. That way, we can afford to evaluate the revset even if there
+        // are millions of commits added to the repo, assuming the revset engine can
+        // efficiently skip non-conflicting commits. Filter out empty commits mostly so
+        // `jj new <conflicted commit>` doesn't result in a message about new conflicts.
+        let conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict)
+            .intersection(&RevsetExpression::filter(RevsetFilterPredicate::File(None)));
+        let removed_conflicts_expr = added_heads.range(&removed_heads).intersection(&conflicts);
+        let added_conflicts_expr = removed_heads.range(&added_heads).intersection(&conflicts);
+
+        let get_commits = |expr: Rc<RevsetExpression>| -> Result<Vec<Commit>, CommandError> {
+            let commits = expr
+                .evaluate_programmatic(new_repo)?
+                .iter()
+                .commits(new_repo.store())
+                .try_collect()?;
+            Ok(commits)
+        };
+        let removed_conflict_commits = get_commits(removed_conflicts_expr)?;
+        let added_conflict_commits = get_commits(added_conflicts_expr)?;
+
+        fn commits_by_change_id(commits: &[Commit]) -> IndexMap<&ChangeId, Vec<&Commit>> {
+            let mut result: IndexMap<&ChangeId, Vec<&Commit>> = IndexMap::new();
+            for commit in commits {
+                result.entry(commit.change_id()).or_default().push(commit);
+            }
+            result
+        }
+        let removed_conflicts_by_change_id = commits_by_change_id(&removed_conflict_commits);
+        let added_conflicts_by_change_id = commits_by_change_id(&added_conflict_commits);
+        let mut resolved_conflicts_by_change_id = removed_conflicts_by_change_id.clone();
+        resolved_conflicts_by_change_id
+            .retain(|change_id, _commits| !added_conflicts_by_change_id.contains_key(change_id));
+        let mut new_conflicts_by_change_id = added_conflicts_by_change_id.clone();
+        new_conflicts_by_change_id
+            .retain(|change_id, _commits| !removed_conflicts_by_change_id.contains_key(change_id));
+
+        // TODO: Also report new divergence and maybe resolved divergence
+        let mut fmt = ui.stderr_formatter();
+        if !resolved_conflicts_by_change_id.is_empty() {
+            writeln!(
+                fmt,
+                "Existing conflicts were resolved or abandoned from these commits:"
+            )?;
+            for (_, old_commits) in &resolved_conflicts_by_change_id {
+                // TODO: Report which ones were resolved and which ones were abandoned. However,
+                // that involves resolving the change_id among the visible commits in the new
+                // repo, which isn't currently supported by Google's revset engine.
+                for commit in old_commits {
+                    write!(fmt, "  ")?;
+                    self.write_commit_summary(fmt.as_mut(), commit)?;
+                    writeln!(fmt)?;
+                }
+            }
+        }
+        if !new_conflicts_by_change_id.is_empty() {
+            writeln!(fmt, "New conflicts appeared in these commits:")?;
+            for (_, new_commits) in &new_conflicts_by_change_id {
+                for commit in new_commits {
+                    write!(fmt, "  ")?;
+                    self.write_commit_summary(fmt.as_mut(), commit)?;
+                    writeln!(fmt)?;
+                }
+            }
+        }
+
+        // TODO: Hint about doing `jj new <first commit with conflicts>`.
+        Ok(())
+    }
 }
 
 #[must_use]

cli/tests/test_chmod_command.rs

@@ -225,6 +225,8 @@ fn test_chmod_file_dir_deletion_conflicts() {
         test_env.jj_cmd_ok(&repo_path, &["chmod", "x", "file", "-r=file_deletion"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
+    New conflicts appeared in these commits:
+      kmkuslsw 4cc432b5 file_deletion | (conflict) file_deletion
     Working copy now at: kmkuslsw 4cc432b5 file_deletion | (conflict) file_deletion
     Parent commit      : zsuskuln c51c9c55 file | file
     Parent commit      : royxmykx 6b18b3c1 deletion | deletion

cli/tests/test_repo_change_report.rs

@@ -0,0 +1,147 @@
+// Copyright 2023 The Jujutsu Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::common::TestEnvironment;
+
+pub mod common;
+
+#[test]
+fn test_report_conflicts() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    std::fs::write(repo_path.join("file"), "A\n").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m=A"]);
+    std::fs::write(repo_path.join("file"), "B\n").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m=B"]);
+    std::fs::write(repo_path.join("file"), "C\n").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m=C"]);
+
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(B)", "-d=root()"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 3 commits
+    New conflicts appeared in these commits:
+      kkmpptxz a2593769 (conflict) C
+      rlvkpnrz 727244df (conflict) B
+    Working copy now at: zsuskuln 30928080 (conflict) (empty) (no description set)
+    Parent commit      : kkmpptxz a2593769 (conflict) C
+    Added 0 files, modified 1 files, removed 0 files
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 3 commits
+    Existing conflicts were resolved or abandoned from these commits:
+      kkmpptxz hidden a2593769 (conflict) C
+      rlvkpnrz hidden 727244df (conflict) B
+    Working copy now at: zsuskuln 355a2e34 (empty) (no description set)
+    Parent commit      : kkmpptxz ed071401 C
+    Added 0 files, modified 1 files, removed 0 files
+    "###);
+}
+
+#[test]
+fn test_report_conflicts_with_divergent_commits() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    test_env.jj_cmd_ok(&repo_path, &["describe", "-m=A"]);
+    std::fs::write(repo_path.join("file"), "A\n").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m=B"]);
+    std::fs::write(repo_path.join("file"), "B\n").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["new", "-m=C"]);
+    std::fs::write(repo_path.join("file"), "C\n").unwrap();
+    test_env.jj_cmd_ok(&repo_path, &["describe", "-m=C2"]);
+    test_env.jj_cmd_ok(&repo_path, &["describe", "-m=C3", "--at-op=@-"]);
+
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(B)", "-d=root()"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Concurrent modification detected, resolving automatically.
+    Rebased 3 commits
+    New conflicts appeared in these commits:
+      zsuskuln?? 76c40a95 (conflict) C3
+      zsuskuln?? e92329f2 (conflict) C2
+      kkmpptxz aed319ec (conflict) B
+    Working copy now at: zsuskuln?? e92329f2 (conflict) C2
+    Parent commit      : kkmpptxz aed319ec (conflict) B
+    Added 0 files, modified 1 files, removed 0 files
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-d=description(A)"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 3 commits
+    Existing conflicts were resolved or abandoned from these commits:
+      zsuskuln hidden 76c40a95 (conflict) C3
+      zsuskuln hidden e92329f2 (conflict) C2
+      kkmpptxz hidden aed319ec (conflict) B
+    Working copy now at: zsuskuln?? 9c33e9a9 C2
+    Parent commit      : kkmpptxz 9ce42c2a B
+    Added 0 files, modified 1 files, removed 0 files
+    "###);
+
+    // Same thing when rebasing the divergent commits one at a time
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(C2)", "-d=root()"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 1 commits
+    New conflicts appeared in these commits:
+      zsuskuln?? 0d6cb6b7 (conflict) C2
+    Working copy now at: zsuskuln?? 0d6cb6b7 (conflict) C2
+    Parent commit      : zzzzzzzz 00000000 (empty) (no description set)
+    Added 0 files, modified 1 files, removed 0 files
+    "###);
+
+    let (stdout, stderr) =
+        test_env.jj_cmd_ok(&repo_path, &["rebase", "-s=description(C3)", "-d=root()"]);
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 1 commits
+    New conflicts appeared in these commits:
+      zsuskuln?? 9652a362 (conflict) C3
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["rebase", "-s=description(C2)", "-d=description(B)"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 1 commits
+    Existing conflicts were resolved or abandoned from these commits:
+      zsuskuln hidden 0d6cb6b7 (conflict) C2
+    Working copy now at: zsuskuln?? 24f79296 C2
+    Parent commit      : kkmpptxz 9ce42c2a B
+    Added 0 files, modified 1 files, removed 0 files
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["rebase", "-s=description(C3)", "-d=description(B)"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 1 commits
+    Existing conflicts were resolved or abandoned from these commits:
+      zsuskuln hidden 9652a362 (conflict) C3
+    "###);
+}

cli/tests/test_resolve_command.rs

@@ -184,6 +184,8 @@ conflict
     );
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
+    New conflicts appeared in these commits:
+      vruxwmqv ff4e8c6b conflict | (conflict) conflict
     Working copy now at: vruxwmqv ff4e8c6b conflict | (conflict) conflict
     Parent commit      : zsuskuln aa493daf a | a
     Parent commit      : royxmykx db6a4daf b | b
@@ -635,6 +637,8 @@ fn test_multiple_conflicts() {
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["resolve", "another_file"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
+    New conflicts appeared in these commits:
+      vruxwmqv c3c25bce conflict | (conflict) conflict
     Working copy now at: vruxwmqv c3c25bce conflict | (conflict) conflict
     Parent commit      : zsuskuln de7553ef a | a
     Parent commit      : royxmykx f68bc2f0 b | b
@@ -664,6 +668,8 @@ fn test_multiple_conflicts() {
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["resolve", "--quiet", "another_file"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
+    New conflicts appeared in these commits:
+      vruxwmqv fd3874cd conflict | (conflict) conflict
     Working copy now at: vruxwmqv fd3874cd conflict | (conflict) conflict
     Parent commit      : zsuskuln de7553ef a | a
     Parent commit      : royxmykx f68bc2f0 b | b

cli/tests/test_restore_command.rs

@@ -63,6 +63,8 @@ fn test_restore() {
     insta::assert_snapshot!(stderr, @r###"
     Created rlvkpnrz e25100af (empty) (no description set)
     Rebased 1 descendant commits
+    New conflicts appeared in these commits:
+      kkmpptxz e301deb3 (conflict) (no description set)
     Working copy now at: kkmpptxz e301deb3 (conflict) (no description set)
     Parent commit      : rlvkpnrz e25100af (empty) (no description set)
     Added 0 files, modified 1 files, removed 0 files