diff --git a/cli/examples/custom-backend/main.rs b/cli/examples/custom-backend/main.rs index c2c805236..4d56c546d 100644 --- a/cli/examples/custom-backend/main.rs +++ b/cli/examples/custom-backend/main.rs @@ -177,7 +177,7 @@ impl Backend for JitBackend { fn get_copy_records( &self, - paths: &[RepoPathBuf], + paths: Option<&[RepoPathBuf]>, root: &CommitId, head: &CommitId, ) -> BackendResult>> { diff --git a/cli/src/commands/debug/copy_detection.rs b/cli/src/commands/debug/copy_detection.rs index 9ed117a82..99c89fb5e 100644 --- a/cli/src/commands/debug/copy_detection.rs +++ b/cli/src/commands/debug/copy_detection.rs @@ -43,7 +43,7 @@ pub fn cmd_debug_copy_detection( let commit = ws.resolve_single_rev(&args.revision)?; for parent_id in commit.parent_ids() { for CopyRecord { target, source, .. } in - block_on_stream(git.get_copy_records(&[], parent_id, commit.id())?) + block_on_stream(git.get_copy_records(None, parent_id, commit.id())?) .filter_map(|r| r.ok()) { writeln!( diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index a643ee86e..5d1524a94 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -12,9 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools; +use jj_lib::backend::CopyRecords; +use jj_lib::commit::Commit; +use jj_lib::repo::Repo; +use jj_lib::rewrite::merge_commit_trees; use tracing::instrument; -use crate::cli_util::{print_unmatched_explicit_paths, CommandHelper, RevisionArg}; +use crate::cli_util::{ + print_unmatched_explicit_paths, CommandHelper, RevisionArg, WorkspaceCommandHelper, +}; use crate::command_error::CommandError; use crate::diff_util::DiffFormatArgs; use crate::ui::Ui; @@ -59,31 +66,50 @@ pub(crate) fn cmd_diff( args: &DiffArgs, ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; + let resolve_revision = |r: &Option| { + workspace_command.resolve_single_rev(r.as_ref().unwrap_or(&RevisionArg::AT)) + }; + let from_tree; let to_tree; + let mut copy_records = CopyRecords::default(); if args.from.is_some() || args.to.is_some() { - let from = - workspace_command.resolve_single_rev(args.from.as_ref().unwrap_or(&RevisionArg::AT))?; + let from = resolve_revision(&args.from)?; + let to = resolve_revision(&args.to)?; from_tree = from.tree()?; - let to = - workspace_command.resolve_single_rev(args.to.as_ref().unwrap_or(&RevisionArg::AT))?; to_tree = to.tree()?; + + copy_records.add_records(workspace_command.repo().store().get_copy_records( + None, + from.id(), + to.id(), + )?)?; } else { - let commit = workspace_command - .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?; - from_tree = commit.parent_tree(workspace_command.repo().as_ref())?; - to_tree = commit.tree()? + let to = resolve_revision(&args.revision)?; + let parents: Vec<_> = to.parents().try_collect()?; + from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &parents)?; + to_tree = to.tree()?; + + for p in &parents { + copy_records.add_records(workspace_command.repo().store().get_copy_records( + None, + p.id(), + to.id(), + )?)?; + } } + + let diff_renderer = workspace_command.diff_renderer_for(&args.format)?; let fileset_expression = workspace_command.parse_file_patterns(&args.paths)?; let matcher = fileset_expression.to_matcher(); - let diff_renderer = workspace_command.diff_renderer_for(&args.format)?; ui.request_pager(); diff_renderer.show_diff( ui, ui.stdout_formatter().as_mut(), &from_tree, &to_tree, - matcher.as_ref(), + &matcher, + ©_records, ui.term_width(), )?; print_unmatched_explicit_paths( diff --git a/cli/src/commands/interdiff.rs b/cli/src/commands/interdiff.rs index a4fc517fd..717e645ff 100644 --- a/cli/src/commands/interdiff.rs +++ b/cli/src/commands/interdiff.rs @@ -72,6 +72,7 @@ pub(crate) fn cmd_interdiff( &from_tree, &to_tree, matcher.as_ref(), + &Default::default(), ui.term_width(), )?; Ok(()) diff --git a/cli/src/commands/obslog.rs b/cli/src/commands/obslog.rs index 32424dfe9..b645f0c7b 100644 --- a/cli/src/commands/obslog.rs +++ b/cli/src/commands/obslog.rs @@ -199,6 +199,7 @@ fn show_predecessor_patch( &predecessor_tree, &tree, &EverythingMatcher, + &Default::default(), width, )?; Ok(()) diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs index 7b9b32a5b..17d8a714f 100644 --- a/cli/src/commands/operation/diff.rs +++ b/cli/src/commands/operation/diff.rs @@ -567,6 +567,7 @@ fn show_change_diff( &predecessor_tree, &tree, &EverythingMatcher, + &Default::default(), width, )?; } diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs index 7869f9f57..bb025a3a2 100644 --- a/cli/src/commands/status.rs +++ b/cli/src/commands/status.rs @@ -67,7 +67,15 @@ pub(crate) fn cmd_status( writeln!(formatter, "Working copy changes:")?; let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]); let width = ui.term_width(); - diff_renderer.show_diff(ui, formatter, &parent_tree, &tree, &matcher, width)?; + diff_renderer.show_diff( + ui, + formatter, + &parent_tree, + &tree, + &matcher, + &Default::default(), + width, + )?; } // TODO: Conflicts should also be filtered by the `matcher`. See the related diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 159e187c3..5010e40e6 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -20,7 +20,7 @@ use std::{io, mem}; use futures::StreamExt; use itertools::Itertools; -use jj_lib::backend::{BackendError, TreeValue}; +use jj_lib::backend::{BackendError, CopyRecords, TreeValue}; use jj_lib::commit::Commit; use jj_lib::conflicts::{materialized_diff_stream, MaterializedTreeValue}; use jj_lib::diff::{Diff, DiffHunk}; @@ -245,10 +245,19 @@ impl<'a> DiffRenderer<'a> { from_tree: &MergedTree, to_tree: &MergedTree, matcher: &dyn Matcher, + copy_records: &CopyRecords, width: usize, ) -> Result<(), DiffRenderError> { formatter.with_label("diff", |formatter| { - self.show_diff_inner(ui, formatter, from_tree, to_tree, matcher, width) + self.show_diff_inner( + ui, + formatter, + from_tree, + to_tree, + matcher, + copy_records, + width, + ) }) } @@ -259,6 +268,7 @@ impl<'a> DiffRenderer<'a> { from_tree: &MergedTree, to_tree: &MergedTree, matcher: &dyn Matcher, + _copy_records: &CopyRecords, width: usize, ) -> Result<(), DiffRenderError> { let store = self.repo.store(); @@ -324,7 +334,15 @@ impl<'a> DiffRenderer<'a> { ) -> Result<(), DiffRenderError> { let from_tree = commit.parent_tree(self.repo)?; let to_tree = commit.tree()?; - self.show_diff(ui, formatter, &from_tree, &to_tree, matcher, width) + self.show_diff( + ui, + formatter, + &from_tree, + &to_tree, + matcher, + &Default::default(), + width, + ) } } diff --git a/lib/src/backend.rs b/lib/src/backend.rs index 78406a3de..75c75b069 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -15,12 +15,13 @@ #![allow(missing_docs)] use std::any::Any; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt::Debug; use std::io::Read; use std::time::SystemTime; use async_trait::async_trait; +use futures::executor::block_on_stream; use futures::stream::BoxStream; use thiserror::Error; @@ -180,6 +181,44 @@ pub struct CopyRecord { pub source_commit: CommitId, } +/// A collection of CopyRecords. +#[derive(Default, Debug)] +pub struct CopyRecords { + records: Vec, + // Maps from `target` to the index of the target in `records`. Conflicts + // are excluded by keeping an out of range value. + targets: HashMap, +} + +impl CopyRecords { + /// Adds information about a stream of CopyRecords to `self`. A target with + /// multiple conflicts is discarded and treated as not having an origin. + pub fn add_records( + &mut self, + stream: BoxStream>, + ) -> BackendResult<()> { + for record in block_on_stream(stream) { + let r = record?; + let value = self + .targets + .entry(r.target.clone()) + .or_insert(self.records.len()); + + if *value != self.records.len() { + // TODO: handle conflicts instead of ignoring both sides. + *value = usize::MAX; + } + self.records.push(r); + } + Ok(()) + } + + /// Gets any copy record associated with a target path. + pub fn for_target(&self, target: &RepoPath) -> Option<&CopyRecord> { + self.targets.get(target).and_then(|&i| self.records.get(i)) + } +} + /// Error that may occur during backend initialization. #[derive(Debug, Error)] #[error(transparent)] @@ -458,7 +497,7 @@ pub trait Backend: Send + Sync + Debug { /// unnecessary resources. fn get_copy_records( &self, - paths: &[RepoPathBuf], + paths: Option<&[RepoPathBuf]>, root: &CommitId, head: &CommitId, ) -> BackendResult>>; diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 419adb4de..2ae56a566 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -1255,7 +1255,7 @@ impl Backend for GitBackend { fn get_copy_records( &self, - paths: &[RepoPathBuf], + paths: Option<&[RepoPathBuf]>, root_id: &CommitId, head_id: &CommitId, ) -> BackendResult>> { @@ -1285,7 +1285,7 @@ impl Backend for GitBackend { .map_err(|err| to_invalid_utf8_err(err, head_id))?; let target = RepoPathBuf::from_internal_string(dest); - if !paths.is_empty() && !paths.contains(&target) { + if !paths.map_or(true, |paths| paths.contains(&target)) { return Ok(None); } diff --git a/lib/src/local_backend.rs b/lib/src/local_backend.rs index 618d81c55..db12f2795 100644 --- a/lib/src/local_backend.rs +++ b/lib/src/local_backend.rs @@ -304,7 +304,7 @@ impl Backend for LocalBackend { fn get_copy_records( &self, - _paths: &[RepoPathBuf], + _paths: Option<&[RepoPathBuf]>, _root: &CommitId, _head: &CommitId, ) -> BackendResult>> { diff --git a/lib/src/secret_backend.rs b/lib/src/secret_backend.rs index 77d8c4606..7c457c24b 100644 --- a/lib/src/secret_backend.rs +++ b/lib/src/secret_backend.rs @@ -170,7 +170,7 @@ impl Backend for SecretBackend { fn get_copy_records( &self, - paths: &[RepoPathBuf], + paths: Option<&[RepoPathBuf]>, root: &CommitId, head: &CommitId, ) -> BackendResult>> { diff --git a/lib/src/store.rs b/lib/src/store.rs index 8c485ed5a..e73e76601 100644 --- a/lib/src/store.rs +++ b/lib/src/store.rs @@ -85,7 +85,7 @@ impl Store { pub fn get_copy_records( &self, - paths: &[RepoPathBuf], + paths: Option<&[RepoPathBuf]>, root: &CommitId, head: &CommitId, ) -> BackendResult>> { diff --git a/lib/tests/test_git_backend.rs b/lib/tests/test_git_backend.rs index 7daedc3ac..07011a905 100644 --- a/lib/tests/test_git_backend.rs +++ b/lib/tests/test_git_backend.rs @@ -51,7 +51,7 @@ fn collect_no_gc_refs(git_repo_path: &Path) -> HashSet { fn get_copy_records( store: &Store, - paths: &[RepoPathBuf], + paths: Option<&[RepoPathBuf]>, a: &Commit, b: &Commit, ) -> HashMap { @@ -262,23 +262,27 @@ fn test_copy_detection() { let store = repo.store(); assert_eq!( - get_copy_records(store, paths, &commit_a, &commit_b), + get_copy_records(store, Some(paths), &commit_a, &commit_b), HashMap::from([("file1".to_string(), "file0".to_string())]) ); assert_eq!( - get_copy_records(store, paths, &commit_b, &commit_c), + get_copy_records(store, Some(paths), &commit_b, &commit_c), HashMap::from([("file2".to_string(), "file1".to_string())]) ); assert_eq!( - get_copy_records(store, paths, &commit_a, &commit_c), + get_copy_records(store, Some(paths), &commit_a, &commit_c), HashMap::from([("file2".to_string(), "file0".to_string())]) ); assert_eq!( - get_copy_records(store, &[paths[1].clone()], &commit_a, &commit_c), + get_copy_records(store, None, &commit_a, &commit_c), + HashMap::from([("file2".to_string(), "file0".to_string())]) + ); + assert_eq!( + get_copy_records(store, Some(&[paths[1].clone()]), &commit_a, &commit_c), HashMap::default(), ); assert_eq!( - get_copy_records(store, paths, &commit_c, &commit_c), + get_copy_records(store, Some(paths), &commit_c, &commit_c), HashMap::default(), ); } diff --git a/lib/testutils/src/test_backend.rs b/lib/testutils/src/test_backend.rs index 6d6999f05..5a109ab76 100644 --- a/lib/testutils/src/test_backend.rs +++ b/lib/testutils/src/test_backend.rs @@ -303,7 +303,7 @@ impl Backend for TestBackend { fn get_copy_records( &self, - _paths: &[RepoPathBuf], + _paths: Option<&[RepoPathBuf]>, _root: &CommitId, _head: &CommitId, ) -> BackendResult>> {