diff --git a/CHANGELOG.md b/CHANGELOG.md index cb870e71e0..6b48d6c378 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 5c95fb78b0..221c90e0de 100644 --- a/cli/src/cli_util.rs +++ b/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, + ) -> 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 ` 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| -> Result, 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 `. + Ok(()) + } } #[must_use] diff --git a/cli/tests/test_chmod_command.rs b/cli/tests/test_chmod_command.rs index 2d523142a8..64875b6e92 100644 --- a/cli/tests/test_chmod_command.rs +++ b/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 diff --git a/cli/tests/test_repo_change_report.rs b/cli/tests/test_repo_change_report.rs new file mode 100644 index 0000000000..e6f7b32e3e --- /dev/null +++ b/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 + "###); +} diff --git a/cli/tests/test_resolve_command.rs b/cli/tests/test_resolve_command.rs index 42c44f5a5d..069d7c035e 100644 --- a/cli/tests/test_resolve_command.rs +++ b/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 diff --git a/cli/tests/test_restore_command.rs b/cli/tests/test_restore_command.rs index aafff60697..335ca8213e 100644 --- a/cli/tests/test_restore_command.rs +++ 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