Skip to content

Commit

Permalink
refs: merge tracking state of remote branches
Browse files Browse the repository at this point in the history
Otherwise "jj op undo" can't roll back tracking states (whereas "op restore"
can.)
  • Loading branch information
yuja committed Oct 23, 2023
1 parent e9e715e commit 0b04910
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 26 deletions.
65 changes: 65 additions & 0 deletions cli/tests/test_undo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
}
19 changes: 19 additions & 0 deletions lib/src/refs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Option<CommitId>>,
Expand Down
39 changes: 23 additions & 16 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
Expand All @@ -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(),
Expand Down
19 changes: 9 additions & 10 deletions lib/tests/test_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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",
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down

0 comments on commit 0b04910

Please sign in to comment.