From 0b049107a2a8de945cd3abc004696afc62d3a24c Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 20 Oct 2023 16:55:24 +0900 Subject: [PATCH] refs: merge tracking state of remote branches Otherwise "jj op undo" can't roll back tracking states (whereas "op restore" can.) --- cli/tests/test_undo.rs | 65 ++++++++++++++++++++++++++++++++++++++++++ lib/src/refs.rs | 19 ++++++++++++ lib/src/repo.rs | 39 ++++++++++++++----------- lib/tests/test_view.rs | 19 ++++++------ 4 files changed, 116 insertions(+), 26 deletions(-) diff --git a/cli/tests/test_undo.rs b/cli/tests/test_undo.rs index 1e50e3be7a..45f9f33397 100644 --- a/cli/tests/test_undo.rs +++ b/cli/tests/test_undo.rs @@ -326,6 +326,71 @@ fn test_git_push_undo_repo_only() { "###); } +#[test] +fn test_branch_track_untrack_undo() { + let test_env = TestEnvironment::default(); + test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); + let git_repo_path = test_env.env_root().join("git-repo"); + git2::Repository::init_bare(git_repo_path).unwrap(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "clone", "git-repo", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + test_env.jj_cmd_ok(&repo_path, &["describe", "-mcommit"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "feature1", "feature2"]); + test_env.jj_cmd_ok(&repo_path, &["git", "push"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "delete", "feature2"]); + insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" + feature1: qpvuntsm 270721f5 (empty) commit + @origin: qpvuntsm 270721f5 (empty) commit + feature2 (deleted) + @origin: qpvuntsm 270721f5 (empty) commit + (this branch will be *deleted permanently* on the remote on the + next `jj git push`. Use `jj branch forget` to prevent this) + "###); + + // Track/untrack can be undone so long as states can be trivially merged. + test_env.jj_cmd_ok( + &repo_path, + &["branch", "untrack", "feature1@origin", "feature2@origin"], + ); + insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" + feature1: qpvuntsm 270721f5 (empty) commit + feature1@origin: qpvuntsm 270721f5 (empty) commit + feature2@origin: qpvuntsm 270721f5 (empty) commit + "###); + + test_env.jj_cmd_ok(&repo_path, &["undo"]); + insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" + feature1: qpvuntsm 270721f5 (empty) commit + @origin: qpvuntsm 270721f5 (empty) commit + feature2 (deleted) + @origin: qpvuntsm 270721f5 (empty) commit + (this branch will be *deleted permanently* on the remote on the + next `jj git push`. Use `jj branch forget` to prevent this) + "###); + + test_env.jj_cmd_ok(&repo_path, &["undo"]); + insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" + feature1: qpvuntsm 270721f5 (empty) commit + feature1@origin: qpvuntsm 270721f5 (empty) commit + feature2@origin: qpvuntsm 270721f5 (empty) commit + "###); + + test_env.jj_cmd_ok(&repo_path, &["branch", "track", "feature1@origin"]); + insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" + feature1: qpvuntsm 270721f5 (empty) commit + @origin: qpvuntsm 270721f5 (empty) commit + feature2@origin: qpvuntsm 270721f5 (empty) commit + "###); + + test_env.jj_cmd_ok(&repo_path, &["undo"]); + insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" + feature1: qpvuntsm 270721f5 (empty) commit + feature1@origin: qpvuntsm 270721f5 (empty) commit + feature2@origin: qpvuntsm 270721f5 (empty) commit + "###); +} + fn get_branch_output(test_env: &TestEnvironment, repo_path: &Path) -> String { test_env.jj_cmd_success(repo_path, &["branch", "list", "--all"]) } diff --git a/lib/src/refs.rs b/lib/src/refs.rs index e75f32b491..946ca9cf74 100644 --- a/lib/src/refs.rs +++ b/lib/src/refs.rs @@ -108,6 +108,25 @@ pub fn merge_ref_targets( } } +pub fn merge_remote_refs( + index: &dyn Index, + left: &RemoteRef, + base: &RemoteRef, + right: &RemoteRef, +) -> RemoteRef { + // Just merge target and state fields separately. Strictly speaking, merging + // target-only change and state-only change shouldn't automatically mark the + // new target as tracking. However, many faulty merges will end up in local + // or remote target conflicts (since fast-forwardable move can be safely + // "tracked"), and the conflicts will require user intervention anyway. So + // there wouldn't be much reason to handle these merges precisely. + let target = merge_ref_targets(index, &left.target, &base.target, &right.target); + // Merged state shouldn't conflict atm since we only have two states, but if + // it does, keep the original state. The choice is arbitrary. + let state = *trivial_merge(&[base.state], &[left.state, right.state]).unwrap_or(&base.state); + RemoteRef { target, state } +} + fn merge_ref_targets_non_trivial( index: &dyn Index, conflict: Merge>, diff --git a/lib/src/repo.rs b/lib/src/repo.rs index e060448d6d..dbeca242df 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -46,7 +46,9 @@ use crate::op_store::{ OpStore, OpStoreError, OperationId, RefTarget, RemoteRef, RemoteRefState, WorkspaceId, }; use crate::operation::Operation; -use crate::refs::{diff_named_ref_targets, merge_ref_targets}; +use crate::refs::{ + diff_named_ref_targets, diff_named_remote_refs, merge_ref_targets, merge_remote_refs, +}; use crate::revset::{self, ChangeIdIndex, Revset, RevsetExpression}; use crate::rewrite::DescendantRebaser; use crate::settings::{RepoSettings, UserSettings}; @@ -1016,6 +1018,20 @@ impl MutableRepo { .set_remote_branch(name, remote_name, remote_ref); } + fn merge_remote_branch( + &mut self, + name: &str, + remote_name: &str, + base_ref: &RemoteRef, + other_ref: &RemoteRef, + ) { + let view = self.view.get_mut(); + let index = self.index.as_index(); + let self_ref = view.get_remote_branch(name, remote_name); + let new_ref = merge_remote_refs(index, self_ref, base_ref, other_ref); + view.set_remote_branch(name, remote_name, new_ref); + } + /// Merges the specified remote branch in to local branch, and starts /// tracking it. pub fn track_remote_branch(&mut self, name: &str, remote_name: &str) { @@ -1131,24 +1147,9 @@ impl MutableRepo { self.view_mut().add_head(added_head); } - // TODO: somehow merge tracking state of remote refs? let changed_refs = itertools::chain!( diff_named_ref_targets(base.local_branches(), other.local_branches()) .map(|(name, diff)| (RefName::LocalBranch(name.to_owned()), diff)), - diff_named_ref_targets( - base.all_remote_branches() - .map(|(full_name, remote_ref)| (full_name, &remote_ref.target)), - other - .all_remote_branches() - .map(|(full_name, remote_ref)| (full_name, &remote_ref.target)), - ) - .map(|((branch, remote), diff)| { - let ref_name = RefName::RemoteBranch { - branch: branch.to_owned(), - remote: remote.to_owned(), - }; - (ref_name, diff) - }), diff_named_ref_targets(base.tags(), other.tags()) .map(|(name, diff)| (RefName::Tag(name.to_owned()), diff)), diff_named_ref_targets(base.git_refs(), other.git_refs()) @@ -1163,6 +1164,12 @@ impl MutableRepo { ); } + let changed_remote_branches = + diff_named_remote_refs(base.all_remote_branches(), other.all_remote_branches()); + for ((name, remote_name), (base_ref, other_ref)) in changed_remote_branches { + self.merge_remote_branch(name, remote_name, base_ref, other_ref); + } + let new_git_head_target = merge_ref_targets( self.index(), self.view().git_head(), diff --git a/lib/tests/test_view.rs b/lib/tests/test_view.rs index b35cd7793d..ff720efc72 100644 --- a/lib/tests/test_view.rs +++ b/lib/tests/test_view.rs @@ -236,15 +236,10 @@ fn test_merge_views_branches() { let mut_repo = tx.mut_repo(); let main_branch_local_tx0 = write_random_commit(mut_repo, &settings); let main_branch_origin_tx0 = write_random_commit(mut_repo, &settings); - let main_branch_origin_tx1 = write_random_commit(mut_repo, &settings); let main_branch_alternate_tx0 = write_random_commit(mut_repo, &settings); let main_branch_origin_tx0_remote_ref = RemoteRef { target: RefTarget::normal(main_branch_origin_tx0.id().clone()), - state: RemoteRefState::Tracking, - }; - let main_branch_origin_tx1_remote_ref = RemoteRef { - target: RefTarget::normal(main_branch_origin_tx1.id().clone()), - state: RemoteRefState::Tracking, + state: RemoteRefState::New, }; let main_branch_alternate_tx0_remote_ref = RemoteRef { target: RefTarget::normal(main_branch_alternate_tx0.id().clone()), @@ -273,8 +268,6 @@ fn test_merge_views_branches() { "main", RefTarget::normal(main_branch_local_tx1.id().clone()), ); - tx1.mut_repo() - .set_remote_branch("main", "origin", main_branch_origin_tx1_remote_ref.clone()); let feature_branch_tx1 = write_random_commit(tx1.mut_repo(), &settings); tx1.mut_repo().set_local_branch_target( "feature", @@ -283,12 +276,17 @@ fn test_merge_views_branches() { let mut tx2 = repo.start_transaction(&settings, "test"); let main_branch_local_tx2 = write_random_commit(tx2.mut_repo(), &settings); + let main_branch_origin_tx2 = write_random_commit(tx2.mut_repo(), &settings); + let main_branch_origin_tx2_remote_ref = RemoteRef { + target: RefTarget::normal(main_branch_origin_tx2.id().clone()), + state: RemoteRefState::Tracking, + }; tx2.mut_repo().set_local_branch_target( "main", RefTarget::normal(main_branch_local_tx2.id().clone()), ); tx2.mut_repo() - .set_remote_branch("main", "origin", main_branch_origin_tx1_remote_ref.clone()); + .set_remote_branch("main", "origin", main_branch_origin_tx2_remote_ref.clone()); let repo = commit_transactions(&settings, vec![tx1, tx2]); let expected_main_branch = BranchTarget { @@ -301,7 +299,8 @@ fn test_merge_views_branches() { ), remote_refs: vec![ ("alternate", &main_branch_alternate_tx0_remote_ref), - ("origin", &main_branch_origin_tx1_remote_ref), + // tx1: unchanged, tx2: new -> tracking + ("origin", &main_branch_origin_tx2_remote_ref), ], }; let expected_feature_branch = BranchTarget {