diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 1206a7cf3..0527b9c7c 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -10,7 +10,9 @@ use std::{fs, io}; use clap::{ArgGroup, Subcommand}; use itertools::Itertools; use jj_lib::backend::{CommitId, ObjectId, TreeValue}; -use jj_lib::git::{self, parse_gitmodules, GitFetchError, GitPushError, GitRefUpdate}; +use jj_lib::git::{ + self, parse_gitmodules, GitFetchError, GitFetchStats, GitPushError, GitRefUpdate, +}; use jj_lib::git_backend::GitBackend; use jj_lib::op_store::{BranchTarget, RefTarget}; use jj_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate}; @@ -480,11 +482,12 @@ fn cmd_git_clone( } } - if let (mut workspace_command, git_repo, Some(default_branch)) = clone_result? { + let (mut workspace_command, git_repo, stats) = clone_result?; + if let Some(default_branch) = &stats.default_branch { let default_branch_target = workspace_command .repo() .view() - .get_remote_branch(&default_branch, "origin"); + .get_remote_branch(default_branch, "origin"); if let Some(commit_id) = default_branch_target.as_normal().cloned() { let mut checkout_tx = workspace_command.start_transaction("check out git remote's default branch"); @@ -509,7 +512,7 @@ fn do_git_clone( colocate: bool, source: &str, wc_path: &Path, -) -> Result<(WorkspaceCommandHelper, git2::Repository, Option), CommandError> { +) -> Result<(WorkspaceCommandHelper, git2::Repository, GitFetchStats), CommandError> { let (workspace, repo) = if colocate { let git_repo = git2::Repository::init(wc_path)?; add_to_git_exclude(ui, &git_repo)?; @@ -524,7 +527,7 @@ fn do_git_clone( git_repo.remote(remote_name, source).unwrap(); let mut fetch_tx = workspace_command.start_transaction("fetch from git remote into empty repo"); - let maybe_default_branch = with_remote_callbacks(ui, |cb| { + let stats = with_remote_callbacks(ui, |cb| { git::fetch( fetch_tx.mut_repo(), &git_repo, @@ -545,7 +548,7 @@ fn do_git_clone( } })?; fetch_tx.finish(ui)?; - Ok((workspace_command, git_repo, maybe_default_branch)) + Ok((workspace_command, git_repo, stats)) } fn with_remote_callbacks(ui: &mut Ui, f: impl FnOnce(git::RemoteCallbacks<'_>) -> T) -> T { diff --git a/lib/src/git.rs b/lib/src/git.rs index 643f7c489..f48b5960e 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -59,6 +59,13 @@ pub enum GitImportError { InternalGitError(#[from] git2::Error), } +/// Describes changes made by `import_refs()` or `fetch()`. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GitImportStats { + /// Commits superseded by newly imported commits. + pub abandoned_commits: Vec, +} + fn parse_git_ref(ref_name: &str) -> Option { if let Some(branch_name) = ref_name.strip_prefix("refs/heads/") { // Git CLI says 'HEAD' is not a valid branch name @@ -180,7 +187,7 @@ pub fn import_refs( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, git_settings: &GitSettings, -) -> Result<(), GitImportError> { +) -> Result { import_some_refs(mut_repo, git_repo, git_settings, |_| true) } @@ -193,7 +200,7 @@ pub fn import_some_refs( git_repo: &git2::Repository, git_settings: &GitSettings, git_ref_filter: impl Fn(&RefName) -> bool, -) -> Result<(), GitImportError> { +) -> Result { // TODO: Should this be a separate function? We may not always want to import // the Git HEAD (and add it to our set of heads). let old_git_head = mut_repo.view().git_head(); @@ -286,9 +293,17 @@ pub fn import_some_refs( .cloned() .collect_vec(); if hidable_git_heads.is_empty() { - return Ok(()); + let stats = GitImportStats { + abandoned_commits: vec![], + }; + return Ok(stats); } - let pinned_heads = pinned_commit_ids(mut_repo.view()).cloned().collect_vec(); + let pinned_heads = itertools::chain( + pinned_commit_ids(mut_repo.view()), + iter::once(mut_repo.store().root_commit_id()), + ) + .cloned() + .collect_vec(); // We could use mut_repo.record_rewrites() here but we know we only need to care // about abandoned commits for now. We may want to change this if we ever // add a way of preserving change IDs across rewrites by `git` (e.g. by @@ -297,14 +312,12 @@ pub fn import_some_refs( .unwrap() .iter() .collect_vec(); - let root_commit_id = mut_repo.store().root_commit_id().clone(); - for abandoned_commit in abandoned_commits { - if abandoned_commit != root_commit_id { - mut_repo.record_abandoned_commit(abandoned_commit); - } + for abandoned_commit in &abandoned_commits { + mut_repo.record_abandoned_commit(abandoned_commit.clone()); } - Ok(()) + let stats = GitImportStats { abandoned_commits }; + Ok(stats) } /// Calculates diff of git refs to be imported. @@ -797,6 +810,15 @@ pub enum GitFetchError { InternalGitError(#[from] git2::Error), } +/// Describes successful `fetch()` result. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GitFetchStats { + /// Remote's default branch. + pub default_branch: Option, + /// Changes made by the import. + pub import_stats: GitImportStats, +} + #[tracing::instrument(skip(mut_repo, git_repo, callbacks))] pub fn fetch( mut_repo: &mut MutableRepo, @@ -805,7 +827,7 @@ pub fn fetch( branch_name_globs: Option<&[&str]>, callbacks: RemoteCallbacks<'_>, git_settings: &GitSettings, -) -> Result, GitFetchError> { +) -> Result { let branch_name_filter = { let regex = if let Some(globs) = branch_name_globs { let result = regex::RegexSet::new( @@ -913,12 +935,16 @@ pub fn fetch( // local branches. We also import local tags since remote tags should have // been merged by Git. tracing::debug!("import_refs"); - import_some_refs(mut_repo, git_repo, git_settings, |ref_name| { + let import_stats = import_some_refs(mut_repo, git_repo, git_settings, |ref_name| { to_remote_branch(ref_name, remote_name) .map(&branch_name_filter) .unwrap_or_else(|| matches!(ref_name, RefName::Tag(_))) })?; - Ok(default_branch) + let stats = GitFetchStats { + default_branch, + import_stats, + }; + Ok(stats) } #[derive(Error, Debug, PartialEq)] diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 17925990b..2035dcc7b 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -105,11 +105,12 @@ fn test_import_refs() { let git_repo = get_git_repo(repo); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); let view = repo.view(); + assert!(stats.abandoned_commits.is_empty()); let expected_heads = hashset! { jj_id(&commit3), jj_id(&commit4), @@ -201,10 +202,11 @@ fn test_import_refs_reimport() { .unwrap(); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); + assert!(stats.abandoned_commits.is_empty()); let expected_heads = hashset! { jj_id(&commit3), jj_id(&commit4), @@ -228,10 +230,18 @@ fn test_import_refs_reimport() { let repo = tx.commit(); let mut tx = repo.start_transaction(&settings, "test"); - git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); + let stats = git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); let repo = tx.commit(); + assert_eq!( + // The order is unstable just because we import heads from Git repo. + HashSet::from_iter(stats.abandoned_commits), + hashset! { + jj_id(&commit4), + jj_id(&commit3), + }, + ); let view = repo.view(); let expected_heads = hashset! { jj_id(&commit5), @@ -1599,7 +1609,7 @@ fn test_fetch_empty_repo() { let mut tx = test_data .repo .start_transaction(&test_data.settings, "test"); - let default_branch = git::fetch( + let stats = git::fetch( tx.mut_repo(), &test_data.git_repo, "origin", @@ -1609,7 +1619,8 @@ fn test_fetch_empty_repo() { ) .unwrap(); // No default branch and no refs - assert_eq!(default_branch, None); + assert_eq!(stats.default_branch, None); + assert!(stats.import_stats.abandoned_commits.is_empty()); assert_eq!(*tx.mut_repo().view().git_refs(), btreemap! {}); assert_eq!(*tx.mut_repo().view().branches(), btreemap! {}); } @@ -1623,7 +1634,7 @@ fn test_fetch_initial_commit() { let mut tx = test_data .repo .start_transaction(&test_data.settings, "test"); - let default_branch = git::fetch( + let stats = git::fetch( tx.mut_repo(), &test_data.git_repo, "origin", @@ -1633,7 +1644,8 @@ fn test_fetch_initial_commit() { ) .unwrap(); // No default branch because the origin repo's HEAD wasn't set - assert_eq!(default_branch, None); + assert_eq!(stats.default_branch, None); + assert!(stats.import_stats.abandoned_commits.is_empty()); let repo = tx.commit(); // The initial commit is visible after git::fetch(). let view = repo.view(); @@ -1692,7 +1704,7 @@ fn test_fetch_success() { let mut tx = test_data .repo .start_transaction(&test_data.settings, "test"); - let default_branch = git::fetch( + let stats = git::fetch( tx.mut_repo(), &test_data.git_repo, "origin", @@ -1702,7 +1714,8 @@ fn test_fetch_success() { ) .unwrap(); // The default branch is "main" - assert_eq!(default_branch, Some("main".to_string())); + assert_eq!(stats.default_branch, Some("main".to_string())); + assert!(stats.import_stats.abandoned_commits.is_empty()); let repo = tx.commit(); // The new commit is visible after we fetch again let view = repo.view(); @@ -1738,7 +1751,7 @@ fn test_fetch_success() { fn test_fetch_prune_deleted_ref() { let test_data = GitRepoData::create(); let git_settings = GitSettings::default(); - empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); + let commit = empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]); let mut tx = test_data .repo @@ -1762,7 +1775,7 @@ fn test_fetch_prune_deleted_ref() { .delete() .unwrap(); // After re-fetching, the branch should be deleted - git::fetch( + let stats = git::fetch( tx.mut_repo(), &test_data.git_repo, "origin", @@ -1771,6 +1784,7 @@ fn test_fetch_prune_deleted_ref() { &git_settings, ) .unwrap(); + assert_eq!(stats.import_stats.abandoned_commits, vec![jj_id(&commit)]); assert!(tx.mut_repo().get_branch("main").is_none()); } @@ -1806,7 +1820,7 @@ fn test_fetch_no_default_branch() { .set_head_detached(initial_git_commit.id()) .unwrap(); - let default_branch = git::fetch( + let stats = git::fetch( tx.mut_repo(), &test_data.git_repo, "origin", @@ -1816,7 +1830,7 @@ fn test_fetch_no_default_branch() { ) .unwrap(); // There is no default branch - assert_eq!(default_branch, None); + assert_eq!(stats.default_branch, None); } #[test]