diff --git a/lib/src/git.rs b/lib/src/git.rs index ee3748c18..9a082d276 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -350,17 +350,17 @@ fn abandon_unreachable_commits( if hidable_git_heads.is_empty() { return vec![]; } - let pinned_heads = itertools::chain!( - changed_remote_refs - .values() - .flat_map(|(_, new_target)| new_target.added_ids()), - pinned_commit_ids(mut_repo.view()), - iter::once(mut_repo.store().root_commit_id()), - ) - .cloned() - .collect_vec(); - let abandoned_expression = RevsetExpression::commits(pinned_heads) + let pinned_expression = RevsetExpression::union_all(&[ + // Local refs are usually visible, no need to filter out hidden + RevsetExpression::commits(pinned_commit_ids(mut_repo.view())), + RevsetExpression::commits(remotely_pinned_commit_ids(mut_repo.view())) + // Hidden remote branches should not contribute to pinning + .intersection(&RevsetExpression::visible_heads().ancestors()), + RevsetExpression::root(), + ]); + let abandoned_expression = pinned_expression .range(&RevsetExpression::commits(hidable_git_heads)) + // Don't include already-abandoned commits in GitImportStats .intersection(&RevsetExpression::visible_heads().ancestors()); let abandoned_commits = abandoned_expression .evaluate_programmatic(mut_repo) @@ -500,13 +500,30 @@ fn default_remote_ref_state_for(ref_name: &RefName, git_settings: &GitSettings) /// On `import_refs()`, this is similar to collecting commits referenced by /// `view.git_refs()`. Main difference is that local branches can be moved by /// tracking remotes, and such mutation isn't applied to `view.git_refs()` yet. -fn pinned_commit_ids(view: &View) -> impl Iterator { +fn pinned_commit_ids(view: &View) -> Vec { itertools::chain!( view.local_branches().map(|(_, target)| target), view.tags().values(), iter::once(view.git_head()), ) .flat_map(|target| target.added_ids()) + .cloned() + .collect() +} + +/// Commits referenced by untracked remote branches including hidden ones. +/// +/// Tracked remote branches aren't included because they should have been merged +/// into the local counterparts, and the changes pulled from one remote should +/// propagate to the other remotes on later push. OTOH, untracked remote +/// branches are considered independent refs. +fn remotely_pinned_commit_ids(view: &View) -> Vec { + view.all_remote_branches() + .filter(|(_, remote_ref)| !remote_ref.is_tracking()) + .map(|(_, remote_ref)| &remote_ref.target) + .flat_map(|target| target.added_ids()) + .cloned() + .collect() } /// Imports `HEAD@git` from the underlying Git repo. diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 5bdf2c097..546b751a2 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -776,6 +776,151 @@ fn test_import_refs_reimport_with_moved_untracked_remote_ref() { ); } +#[test] +fn test_import_refs_reimport_with_deleted_untracked_intermediate_remote_ref() { + let settings = testutils::user_settings(); + let git_settings = GitSettings { + auto_local_branch: false, + ..Default::default() + }; + let test_workspace = TestRepo::init_with_backend(TestRepoBackend::Git); + let repo = &test_workspace.repo; + let git_repo = get_git_repo(repo); + + // Set up linear graph: + // o feature-b@origin + // o feature-a@origin + let remote_ref_name_a = "refs/remotes/origin/feature-a"; + let remote_ref_name_b = "refs/remotes/origin/feature-b"; + let commit_remote_a = empty_git_commit(&git_repo, remote_ref_name_a, &[]); + let commit_remote_b = empty_git_commit(&git_repo, remote_ref_name_b, &[&commit_remote_a]); + let mut tx = repo.start_transaction(&settings); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + let repo = tx.commit("test"); + let view = repo.view(); + + assert_eq!(*view.heads(), hashset! { jj_id(&commit_remote_b) }); + assert_eq!(view.local_branches().count(), 0); + assert_eq!(view.all_remote_branches().count(), 2); + assert_eq!( + view.get_remote_branch("feature-a", "origin"), + &RemoteRef { + target: RefTarget::normal(jj_id(&commit_remote_a)), + state: RemoteRefState::New, + }, + ); + assert_eq!( + view.get_remote_branch("feature-b", "origin"), + &RemoteRef { + target: RefTarget::normal(jj_id(&commit_remote_b)), + state: RemoteRefState::New, + }, + ); + + // Delete feature-a remotely and fetch the changes. + delete_git_ref(&git_repo, remote_ref_name_a); + let mut tx = repo.start_transaction(&settings); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + let repo = tx.commit("test"); + let view = repo.view(); + + // No commits should be abandoned because feature-a is pinned by feature-b. + // Otherwise, feature-b would have to be rebased locally even though the + // user haven't made any modifications to these commits yet. + assert_eq!(*view.heads(), hashset! { jj_id(&commit_remote_b) }); + assert_eq!(view.local_branches().count(), 0); + assert_eq!(view.all_remote_branches().count(), 1); + assert_eq!( + view.get_remote_branch("feature-b", "origin"), + &RemoteRef { + target: RefTarget::normal(jj_id(&commit_remote_b)), + state: RemoteRefState::New, + }, + ); +} + +#[test] +fn test_import_refs_reimport_with_deleted_abandoned_untracked_remote_ref() { + let settings = testutils::user_settings(); + let git_settings = GitSettings { + auto_local_branch: false, + ..Default::default() + }; + let test_workspace = TestRepo::init_with_backend(TestRepoBackend::Git); + let repo = &test_workspace.repo; + let git_repo = get_git_repo(repo); + + // Set up linear graph: + // o feature-b@origin + // o feature-a@origin + let remote_ref_name_a = "refs/remotes/origin/feature-a"; + let remote_ref_name_b = "refs/remotes/origin/feature-b"; + let commit_remote_a = empty_git_commit(&git_repo, remote_ref_name_a, &[]); + let commit_remote_b = empty_git_commit(&git_repo, remote_ref_name_b, &[&commit_remote_a]); + let mut tx = repo.start_transaction(&settings); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + let repo = tx.commit("test"); + let view = repo.view(); + + assert_eq!(*view.heads(), hashset! { jj_id(&commit_remote_b) }); + assert_eq!(view.local_branches().count(), 0); + assert_eq!(view.all_remote_branches().count(), 2); + assert_eq!( + view.get_remote_branch("feature-a", "origin"), + &RemoteRef { + target: RefTarget::normal(jj_id(&commit_remote_a)), + state: RemoteRefState::New, + }, + ); + assert_eq!( + view.get_remote_branch("feature-b", "origin"), + &RemoteRef { + target: RefTarget::normal(jj_id(&commit_remote_b)), + state: RemoteRefState::New, + }, + ); + + // Abandon feature-b locally: + // x feature-b@origin (hidden) + // o feature-a@origin + let mut tx = repo.start_transaction(&settings); + tx.mut_repo() + .record_abandoned_commit(jj_id(&commit_remote_b)); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + let repo = tx.commit("test"); + let view = repo.view(); + assert_eq!(*view.heads(), hashset! { jj_id(&commit_remote_a) }); + assert_eq!(view.local_branches().count(), 0); + assert_eq!(view.all_remote_branches().count(), 2); + + // Delete feature-a remotely and fetch the changes. + delete_git_ref(&git_repo, remote_ref_name_a); + let mut tx = repo.start_transaction(&settings); + git::import_refs(tx.mut_repo(), &git_settings).unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + let repo = tx.commit("test"); + let view = repo.view(); + + // The feature-a commit should be abandoned. Since feature-b has already + // been abandoned, there are no descendant commits to be rebased. + assert_eq!( + *view.heads(), + hashset! { repo.store().root_commit_id().clone() } + ); + assert_eq!(view.local_branches().count(), 0); + assert_eq!(view.all_remote_branches().count(), 1); + assert_eq!( + view.get_remote_branch("feature-b", "origin"), + &RemoteRef { + target: RefTarget::normal(jj_id(&commit_remote_b)), + state: RemoteRefState::New, + }, + ); +} + #[test] fn test_import_refs_reimport_git_head_with_fixed_ref() { // Simulate external `git checkout` in colocated repo, from named branch.