ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

view: make get_remote_branch() return RemoteRef instead of RefTarget

Browse source 
I'm going to add tracking state to RemoteRef, and we should compare both
target and state in the tests.
This commit is contained in:
Yuya Nishihara 2023-10-13 04:06:10 +09:00
parent d730b250a0
commit 35ea607abd
7 changed files with 143 additions and 63 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
cli/src/commands/git.rs
View file

@ -491,11 +491,11 @@ fn cmd_git_clone(
let (mut workspace_command, git_repo, stats) = clone_result?;
if let Some(default_branch) = &stats.default_branch {
let default_branch_target = workspace_command
let default_branch_remote_ref = workspace_command
.repo()
.view()
.get_remote_branch(default_branch, "origin");
if let Some(commit_id) = default_branch_target.as_normal().cloned() {
if let Some(commit_id) = default_branch_remote_ref.target.as_normal().cloned() {
let mut checkout_tx =
workspace_command.start_transaction("check out git remote's default branch");
if args.colocate {
@ -730,7 +730,7 @@ fn cmd_git_push(
}
let targets = TrackingRefPair {
local_target: repo.view().get_local_branch(branch_name),
remote_target: repo.view().get_remote_branch(branch_name, &remote),
remote_target: &repo.view().get_remote_branch(branch_name, &remote).target,
};
if targets.local_target.is_absent() && targets.remote_target.is_absent() {
return Err(user_error(format!("Branch {branch_name} doesn't exist")));
@ -784,7 +784,11 @@ fn cmd_git_push(
.set_local_branch_target(&branch_name, RefTarget::normal(commit.id().clone()));
let targets = TrackingRefPair {
local_target: tx.repo().view().get_local_branch(&branch_name),
remote_target: tx.repo().view().get_remote_branch(&branch_name, &remote),
remote_target: &tx
.repo()
.view()
.get_remote_branch(&branch_name, &remote)
.target,
};
match classify_branch_update(&branch_name, &remote, targets) {
Ok(Some(update)) => branch_updates.push((branch_name.clone(), update)),

4
lib/src/repo.rs
View file

@ -42,7 +42,7 @@ use crate::git_backend::GitBackend;
use crate::index::{HexPrefix, Index, IndexStore, MutableIndex, PrefixResolution, ReadonlyIndex};
use crate::local_backend::LocalBackend;
use crate::op_heads_store::{self, OpHeadResolutionError, OpHeadsStore};
use crate::op_store::{OpStore, OpStoreError, OperationId, RefTarget, WorkspaceId};
use crate::op_store::{OpStore, OpStoreError, OperationId, RefTarget, RemoteRef, WorkspaceId};
use crate::operation::Operation;
use crate::refs::{diff_named_refs, merge_ref_targets};
use crate::revset::{self, ChangeIdIndex, Revset, RevsetExpression};
@ -998,7 +998,7 @@ impl MutableRepo {
self.view_mut().set_local_branch_target(name, target);
}
pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> RefTarget {
pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> RemoteRef {
self.view
.with_ref(|v| v.get_remote_branch(name, remote_name).clone())
}

2
lib/src/revset.rs
View file

@ -2018,7 +2018,7 @@ fn resolve_remote_branch(repo: &dyn Repo, name: &str, remote: &str) -> Option<Ve
let view = repo.view();
let target = match (name, remote) {
("HEAD", git::REMOTE_NAME_FOR_LOCAL_GIT_REPO) => view.git_head(),
(name, remote) => view.get_remote_branch(name, remote),
(name, remote) => &view.get_remote_branch(name, remote).target,
};
target
.is_present()

15
lib/src/view.rs
View file

@ -130,7 +130,9 @@ impl View {
pub fn get_ref(&self, name: &RefName) -> &RefTarget {
match &name {
RefName::LocalBranch(name) => self.get_local_branch(name),
RefName::RemoteBranch { branch, remote } => self.get_remote_branch(branch, remote),
RefName::RemoteBranch { branch, remote } => {
&self.get_remote_branch(branch, remote).target
}
RefName::Tag(name) => self.get_tag(name),
RefName::GitRef(name) => self.get_git_ref(name),
}
@ -211,16 +213,11 @@ impl View {
.flatten()
}
pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> &RefTarget {
// TODO: maybe return RemoteRef instead of RefTarget?
pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> &RemoteRef {
if let Some(remote_view) = self.data.remote_views.get(remote_name) {
let maybe_target = remote_view
.branches
.get(name)
.map(|remote_ref| &remote_ref.target);
maybe_target.flatten()
remote_view.branches.get(name).flatten()
} else {
RefTarget::absent_ref()
RemoteRef::absent_ref()
}
}

147
lib/tests/test_git.rs
View file

@ -127,11 +127,15 @@ fn test_import_refs() {
);
assert_eq!(
view.get_remote_branch("main", "git"),
&RefTarget::normal(jj_id(&commit2))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit2)),
},
);
assert_eq!(
view.get_remote_branch("main", "origin"),
&RefTarget::normal(jj_id(&commit1))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit1)),
},
);
assert_eq!(
view.get_local_branch("feature1"),
@ -139,7 +143,9 @@ fn test_import_refs() {
);
assert_eq!(
view.get_remote_branch("feature1", "git"),
&RefTarget::normal(jj_id(&commit3))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit3)),
},
);
assert!(view.get_remote_branch("feature1", "origin").is_absent());
assert_eq!(
@ -148,7 +154,9 @@ fn test_import_refs() {
);
assert_eq!(
view.get_remote_branch("feature2", "git"),
&RefTarget::normal(jj_id(&commit4))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit4)),
},
);
assert!(view.get_remote_branch("feature2", "origin").is_absent());
assert_eq!(
@ -158,7 +166,9 @@ fn test_import_refs() {
assert!(view.get_remote_branch("feature3", "git").is_absent());
assert_eq!(
view.get_remote_branch("feature3", "origin"),
&RefTarget::normal(jj_id(&commit6))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit6)),
},
);
assert_eq!(view.get_tag("v1.0"), &RefTarget::normal(jj_id(&commit5)));
@ -266,16 +276,25 @@ fn test_import_refs_reimport() {
);
assert_eq!(
view.get_remote_branch("main", "git"),
&RefTarget::normal(jj_id(&commit2))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit2)),
},
);
assert_eq!(
view.get_remote_branch("main", "origin"),
&RemoteRef {
target: commit1_target.clone(),
},
);
assert_eq!(view.get_remote_branch("main", "origin"), &commit1_target);
assert_eq!(
view.get_local_branch("feature2"),
&RefTarget::from_legacy_form([jj_id(&commit4)], [commit6.id().clone(), jj_id(&commit5)])
);
assert_eq!(
view.get_remote_branch("feature2", "git"),
&RefTarget::normal(jj_id(&commit5))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit5)),
},
);
assert!(view.get_remote_branch("feature2", "origin").is_absent());
@ -466,7 +485,9 @@ fn test_import_refs_reimport_with_deleted_remote_ref() {
.is_absent());
assert_eq!(
view.get_remote_branch("feature-remote-only", "origin"),
&RefTarget::normal(jj_id(&commit_remote_only))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_only)),
},
);
assert_eq!(
view.get_local_branch("feature-remote-and-local"),
@ -474,11 +495,15 @@ fn test_import_refs_reimport_with_deleted_remote_ref() {
);
assert_eq!(
view.get_remote_branch("feature-remote-and-local", "git"),
&RefTarget::normal(jj_id(&commit_remote_and_local))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_and_local)),
},
);
assert_eq!(
view.get_remote_branch("feature-remote-and-local", "origin"),
&RefTarget::normal(jj_id(&commit_remote_and_local))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_and_local)),
},
);
assert!(view.has_branch("main")); // branch #3 of 3
@ -503,7 +528,9 @@ fn test_import_refs_reimport_with_deleted_remote_ref() {
.is_absent());
assert_eq!(
view.get_remote_branch("feature-remote-and-local", "git"),
&RefTarget::normal(jj_id(&commit_remote_and_local))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_and_local)),
},
);
assert!(view
.get_remote_branch("feature-remote-and-local", "origin")
@ -570,7 +597,9 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
.is_absent());
assert_eq!(
view.get_remote_branch("feature-remote-only", "origin"),
&RefTarget::normal(jj_id(&commit_remote_only))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_only)),
},
);
assert_eq!(
view.get_local_branch("feature-remote-and-local"),
@ -578,11 +607,15 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
);
assert_eq!(
view.get_remote_branch("feature-remote-and-local", "git"),
&RefTarget::normal(jj_id(&commit_remote_and_local))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_and_local)),
},
);
assert_eq!(
view.get_remote_branch("feature-remote-and-local", "origin"),
&RefTarget::normal(jj_id(&commit_remote_and_local))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_and_local)),
},
);
assert!(view.has_branch("main")); // branch #3 of 3
@ -619,7 +652,9 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
.is_absent());
assert_eq!(
view.get_remote_branch("feature-remote-only", "origin"),
&RefTarget::normal(jj_id(&new_commit_remote_only))
&RemoteRef {
target: RefTarget::normal(jj_id(&new_commit_remote_only)),
},
);
assert_eq!(
view.get_local_branch("feature-remote-and-local"),
@ -627,11 +662,15 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
);
assert_eq!(
view.get_remote_branch("feature-remote-and-local", "git"),
&RefTarget::normal(jj_id(&commit_remote_and_local))
&RemoteRef {
target: RefTarget::normal(jj_id(&commit_remote_and_local)),
},
);
assert_eq!(
view.get_remote_branch("feature-remote-and-local", "origin"),
&RefTarget::normal(jj_id(&new_commit_remote_and_local))
&RemoteRef {
target: RefTarget::normal(jj_id(&new_commit_remote_and_local)),
},
);
assert!(view.has_branch("main")); // branch #3 of 3
let expected_heads = hashset! {
@ -733,7 +772,9 @@ fn test_import_refs_reimport_conflicted_remote_branch() {
);
assert_eq!(
repo.view().get_remote_branch("main", "origin"),
&RefTarget::from_legacy_form([], [jj_id(&commit1), jj_id(&commit2)]),
&RemoteRef {
target: RefTarget::from_legacy_form([], [jj_id(&commit1), jj_id(&commit2)]),
},
);
// The conflict can be resolved by importing the current Git state
@ -746,7 +787,9 @@ fn test_import_refs_reimport_conflicted_remote_branch() {
);
assert_eq!(
repo.view().get_remote_branch("main", "origin"),
&RefTarget::normal(jj_id(&commit2)),
&RemoteRef {
target: RefTarget::normal(jj_id(&commit2)),
},
);
}
@ -814,17 +857,25 @@ fn test_import_some_refs() {
// Check that branches feature[1-4] have been locally imported and are known to
// be present on origin as well.
assert_eq!(view.branches().count(), 4);
let commit_feat1_target = RefTarget::normal(jj_id(&commit_feat1));
let commit_feat2_target = RefTarget::normal(jj_id(&commit_feat2));
let commit_feat3_target = RefTarget::normal(jj_id(&commit_feat3));
let commit_feat4_target = RefTarget::normal(jj_id(&commit_feat4));
let commit_feat1_remote_ref = RemoteRef {
target: RefTarget::normal(jj_id(&commit_feat1)),
};
let commit_feat2_remote_ref = RemoteRef {
target: RefTarget::normal(jj_id(&commit_feat2)),
};
let commit_feat3_remote_ref = RemoteRef {
target: RefTarget::normal(jj_id(&commit_feat3)),
};
let commit_feat4_remote_ref = RemoteRef {
target: RefTarget::normal(jj_id(&commit_feat4)),
};
assert_eq!(
view.get_local_branch("feature1"),
&RefTarget::normal(jj_id(&commit_feat1))
);
assert_eq!(
view.get_remote_branch("feature1", "origin"),
&commit_feat1_target
&commit_feat1_remote_ref
);
assert_eq!(
view.get_local_branch("feature2"),
@ -832,7 +883,7 @@ fn test_import_some_refs() {
);
assert_eq!(
view.get_remote_branch("feature2", "origin"),
&commit_feat2_target
&commit_feat2_remote_ref
);
assert_eq!(
view.get_local_branch("feature3"),
@ -840,7 +891,7 @@ fn test_import_some_refs() {
);
assert_eq!(
view.get_remote_branch("feature3", "origin"),
&commit_feat3_target
&commit_feat3_remote_ref
);
assert_eq!(
view.get_local_branch("feature4"),
@ -848,7 +899,7 @@ fn test_import_some_refs() {
);
assert_eq!(
view.get_remote_branch("feature4", "origin"),
&commit_feat4_target
&commit_feat4_remote_ref
);
assert!(!view.has_branch("main"));
assert!(!view.heads().contains(&jj_id(&commit_main)));
@ -1310,7 +1361,9 @@ fn test_import_export_no_auto_local_branch() {
assert!(mut_repo.view().get_local_branch("main").is_absent());
assert_eq!(
mut_repo.view().get_remote_branch("main", "origin"),
&RefTarget::normal(jj_id(&git_commit))
&RemoteRef {
target: RefTarget::normal(jj_id(&git_commit)),
},
);
assert_eq!(
mut_repo.get_git_ref("refs/remotes/origin/main"),
@ -1369,11 +1422,15 @@ fn test_export_conflicts() {
// Conflicted branches shouldn't be copied to the "git" remote
assert_eq!(
mut_repo.get_remote_branch("feature", "git"),
RefTarget::normal(commit_a.id().clone())
RemoteRef {
target: RefTarget::normal(commit_a.id().clone()),
},
);
assert_eq!(
mut_repo.get_remote_branch("main", "git"),
RefTarget::normal(commit_b.id().clone())
RemoteRef {
target: RefTarget::normal(commit_b.id().clone()),
},
);
}
@ -1443,7 +1500,12 @@ fn test_export_partial_failure() {
// Failed branches shouldn't be copied to the "git" remote
assert!(mut_repo.get_remote_branch("", "git").is_absent());
assert!(mut_repo.get_remote_branch("HEAD", "git").is_absent());
assert_eq!(mut_repo.get_remote_branch("main", "git"), target);
assert_eq!(
mut_repo.get_remote_branch("main", "git"),
RemoteRef {
target: target.clone(),
},
);
assert!(mut_repo.get_remote_branch("main/sub", "git").is_absent());
// Now remove the `main` branch and make sure that the `main/sub` gets exported
@ -1471,7 +1533,12 @@ fn test_export_partial_failure() {
assert!(mut_repo.get_remote_branch("", "git").is_absent());
assert!(mut_repo.get_remote_branch("HEAD", "git").is_absent());
assert!(mut_repo.get_remote_branch("main", "git").is_absent());
assert_eq!(mut_repo.get_remote_branch("main/sub", "git"), target);
assert_eq!(
mut_repo.get_remote_branch("main/sub", "git"),
RemoteRef {
target: target.clone(),
},
);
}
#[test]
@ -1634,7 +1701,12 @@ fn test_export_undo_reexport() {
Some(git_id(&commit_a))
);
assert_eq!(mut_repo.get_git_ref("refs/heads/main"), target_a);
assert_eq!(mut_repo.get_remote_branch("main", "git"), target_a);
assert_eq!(
mut_repo.get_remote_branch("main", "git"),
RemoteRef {
target: target_a.clone(),
},
);
// Undo remote changes only
mut_repo.set_remote_branch_target("main", "git", RefTarget::absent());
@ -1646,7 +1718,12 @@ fn test_export_undo_reexport() {
Some(git_id(&commit_a))
);
assert_eq!(mut_repo.get_git_ref("refs/heads/main"), target_a);
assert_eq!(mut_repo.get_remote_branch("main", "git"), target_a);
assert_eq!(
mut_repo.get_remote_branch("main", "git"),
RemoteRef {
target: target_a.clone(),
},
);
}
#[test]

12
lib/tests/test_mut_repo.rs
View file

@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use jj_lib::op_store::{RefTarget, WorkspaceId};
use jj_lib::op_store::{RefTarget, RemoteRef, WorkspaceId};
use jj_lib::repo::Repo;
use maplit::hashset;
use testutils::{
@ -590,12 +590,14 @@ fn test_rename_remote() {
let mut tx = repo.start_transaction(&settings, "test");
let mut_repo = tx.mut_repo();
let commit = write_random_commit(mut_repo, &settings);
let target = RefTarget::normal(commit.id().clone());
mut_repo.set_remote_branch_target("main", "origin", target.clone());
let remote_ref = RemoteRef {
target: RefTarget::normal(commit.id().clone()),
};
mut_repo.set_remote_branch_target("main", "origin", remote_ref.target.clone());
mut_repo.rename_remote("origin", "upstream");
assert_eq!(mut_repo.get_remote_branch("main", "upstream"), target);
assert_eq!(mut_repo.get_remote_branch("main", "upstream"), remote_ref);
assert_eq!(
mut_repo.get_remote_branch("main", "origin"),
RefTarget::absent()
RemoteRef::absent()
);
}

14
lib/tests/test_rewrite.rs
View file

@ -13,7 +13,7 @@
// limitations under the License.
use itertools::Itertools as _;
use jj_lib::op_store::{RefTarget, WorkspaceId};
use jj_lib::op_store::{RefTarget, RemoteRef, WorkspaceId};
use jj_lib::repo::Repo;
use jj_lib::repo_path::RepoPath;
use jj_lib::rewrite::DescendantRebaser;
@ -1000,13 +1000,13 @@ fn test_rebase_descendants_basic_branch_update_with_non_local_branch() {
let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
let commit_a = graph_builder.initial_commit();
let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
let commit_b_remote_ref = RemoteRef {
target: RefTarget::normal(commit_b.id().clone()),
};
tx.mut_repo()
.set_local_branch_target("main", RefTarget::normal(commit_b.id().clone()));
tx.mut_repo().set_remote_branch_target(
"main",
"origin",
RefTarget::normal(commit_b.id().clone()),
);
tx.mut_repo()
.set_remote_branch_target("main", "origin", commit_b_remote_ref.target.clone());
tx.mut_repo()
.set_tag_target("v1", RefTarget::normal(commit_b.id().clone()));
let repo = tx.commit();
@ -1025,7 +1025,7 @@ fn test_rebase_descendants_basic_branch_update_with_non_local_branch() {
// The remote branch and tag should not get updated
assert_eq!(
tx.mut_repo().get_remote_branch("main", "origin"),
RefTarget::normal(commit_b.id().clone())
commit_b_remote_ref,
);
assert_eq!(
tx.mut_repo().get_tag("v1"),