mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-02-09 22:16:32 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

view: store Option<RefTarget> in maps, add extension trait to flatten Option

Browse source 
Alternatively, we can wrap BTreeMap<String, Option<RefTarget>> to flatten
Option<&Option<..>> internally, but doing that would be tedious. It would
also be unclear if map.remove(name) should construct an absent RefTarget if
the ref doesn't exist.
This commit is contained in:
Yuya Nishihara 2023-07-12 23:41:38 +09:00
parent 7461370f6e
commit 443391bf8f
12 changed files with 161 additions and 120 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
lib/src/git.rs
View file

@ -26,7 +26,7 @@ use thiserror::Error;
use crate::backend::{CommitId, ObjectId};
use crate::git_backend::NO_GC_REF_NAMESPACE;
use crate::op_store::{BranchTarget, RefTarget, RefTargetExt as _};
use crate::op_store::{BranchTarget, RefTarget, RefTargetExt as _, RefTargetOptionExt};
use crate::repo::{MutableRepo, Repo};
use crate::revset;
use crate::settings::GitSettings;
@ -82,7 +82,7 @@ fn to_remote_branch<'a>(parsed_ref: &'a RefName, remote_name: &str) -> Option<&'
/// should be faster than `git_ref.peel_to_commit()`.
fn resolve_git_ref_to_commit_id(
git_ref: &git2::Reference<'_>,
known_target: Option<&RefTarget>,
known_target: &Option<RefTarget>,
) -> Option<CommitId> {
// Try fast path if we have a candidate id which is known to be a commit object.
if let Some(id) = known_target.as_normal() {
@ -137,7 +137,7 @@ pub fn build_unified_branches_map(view: &View) -> (BTreeMap<String, BranchTarget
(all_branches, bad_branch_names)
}
fn local_branch_git_tracking_refs(view: &View) -> impl Iterator<Item = (&str, &RefTarget)> {
fn local_branch_git_tracking_refs(view: &View) -> impl Iterator<Item = (&str, &Option<RefTarget>)> {
view.git_refs().iter().filter_map(|(ref_name, target)| {
ref_name
.strip_prefix("refs/heads/")
@ -145,8 +145,10 @@ fn local_branch_git_tracking_refs(view: &View) -> impl Iterator<Item = (&str, &R
})
}
pub fn get_local_git_tracking_branch<'a>(view: &'a View, branch: &str) -> Option<&'a RefTarget> {
view.git_refs().get(&format!("refs/heads/{branch}"))
pub fn get_local_git_tracking_branch<'a>(view: &'a View, branch: &str) -> &'a Option<RefTarget> {
view.git_refs()
.get(&format!("refs/heads/{branch}"))
.flatten()
}
fn prevent_gc(git_repo: &git2::Repository, id: &CommitId) -> Result<(), git2::Error> {
@ -221,7 +223,8 @@ pub fn import_some_refs(
// Skip other refs (such as notes) and symbolic refs.
continue;
};
let Some(id) = resolve_git_ref_to_commit_id(&git_repo_ref, jj_view_git_refs.get(full_name))
let Some(id) =
resolve_git_ref_to_commit_id(&git_repo_ref, jj_view_git_refs.get(full_name).flatten())
else {
// Skip invalid refs.
continue;
@ -232,7 +235,7 @@ pub fn import_some_refs(
}
// TODO: Make it configurable which remotes are publishing and update public
// heads here.
let old_target = jj_view_git_refs.remove(full_name);
let old_target = jj_view_git_refs.remove(full_name).flatten();
let new_target = RefTarget::normal(id.clone());
if new_target != old_target {
prevent_gc(git_repo, &id)?;
@ -247,7 +250,7 @@ pub fn import_some_refs(
let ref_name = parse_git_ref(&full_name).expect("stored git ref should be parsable");
if git_ref_filter(&ref_name) {
mut_repo.set_git_ref_target(&full_name, RefTarget::absent());
changed_git_refs.insert(ref_name, (Some(target), None));
changed_git_refs.insert(ref_name, (target, RefTarget::absent()));
} else {
pinned_git_heads.insert(ref_name, target.added_ids().cloned().collect());
}
@ -562,7 +565,7 @@ pub fn rename_remote(
.collect_vec();
for (old, new, target) in git_refs {
mut_repo.set_git_ref_target(&old, RefTarget::absent());
mut_repo.set_git_ref_target(&new, Some(target));
mut_repo.set_git_ref_target(&new, target);
}
Ok(())
}

49
lib/src/op_store.rs
View file

@ -152,9 +152,9 @@ impl RefTarget {
/// Returns non-conflicting target pointing to no commit.
///
/// This will typically be used in place of `None` returned by map lookup.
pub fn absent_ref() -> Option<&'static Self> {
pub fn absent_ref() -> &'static Option<Self> {
// TODO: This will be static ref to Conflict::resolved(None).
None
&None
}
/// Creates non-conflicting target pointing to a commit.
@ -273,9 +273,34 @@ impl RefTargetExt for Option<&RefTarget> {
}
}
/// Helper to strip redundant `Option<T>` from `RefTarget` lookup result.
pub trait RefTargetOptionExt {
type Value;
fn flatten(self) -> Self::Value;
}
// This isn't needed right now, but Option<RefTarget> will be replaced with new
// Conflict-based RefTarget type, and it will no longer be Option<Option<T>>.
impl RefTargetOptionExt for Option<Option<RefTarget>> {
type Value = Option<RefTarget>;
fn flatten(self) -> Self::Value {
self.unwrap_or_else(RefTarget::absent)
}
}
impl<'a> RefTargetOptionExt for Option<&'a Option<RefTarget>> {
type Value = &'a Option<RefTarget>;
fn flatten(self) -> Self::Value {
self.unwrap_or_else(|| RefTarget::absent_ref())
}
}
/// Wrapper to exclude absent `RefTarget` entries from `ContentHash`.
#[derive(Default, PartialEq, Eq, Clone, Debug)]
pub struct RefTargetMap(pub BTreeMap<String, RefTarget>);
pub struct RefTargetMap(pub BTreeMap<String, Option<RefTarget>>);
impl RefTargetMap {
pub fn new() -> Self {
@ -290,8 +315,12 @@ impl ContentHash for RefTargetMap {
// Derived from content_hash.rs. It's okay for this to produce a different hash
// value than the inner map, but the value must not be equal to the map which
// preserves absent RefTarget entries.
state.update(&(self.0.len() as u64).to_le_bytes());
for (k, v) in self.0.iter() {
let iter = self
.0
.iter()
.filter_map(|(k, v)| v.as_ref().map(|u| (k, u)));
state.update(&(iter.clone().count() as u64).to_le_bytes());
for (k, v) in iter {
k.hash(state);
v.hash(state);
}
@ -300,7 +329,7 @@ impl ContentHash for RefTargetMap {
// Abuse Deref as this is a temporary workaround. See the comment above.
impl Deref for RefTargetMap {
type Target = BTreeMap<String, RefTarget>;
type Target = BTreeMap<String, Option<RefTarget>>;
fn deref(&self) -> &Self::Target {
&self.0
@ -314,8 +343,8 @@ impl DerefMut for RefTargetMap {
}
impl IntoIterator for RefTargetMap {
type Item = (String, RefTarget);
type IntoIter = btree_map::IntoIter<String, RefTarget>;
type Item = (String, Option<RefTarget>);
type IntoIter = btree_map::IntoIter<String, Option<RefTarget>>;
fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
@ -323,8 +352,8 @@ impl IntoIterator for RefTargetMap {
}
impl<'a> IntoIterator for &'a RefTargetMap {
type Item = (&'a String, &'a RefTarget);
type IntoIter = btree_map::Iter<'a, String, RefTarget>;
type Item = (&'a String, &'a Option<RefTarget>);
type IntoIter = btree_map::Iter<'a, String, Option<RefTarget>>;
fn into_iter(self) -> Self::IntoIter {
self.0.iter()

16
lib/src/refs.rs
View file

@ -18,7 +18,7 @@ use crate::backend::CommitId;
use crate::conflicts::Conflict;
use crate::index::Index;
use crate::merge::trivial_merge;
use crate::op_store::{BranchTarget, RefTarget, RefTargetExt as _};
use crate::op_store::{BranchTarget, RefTarget, RefTargetExt as _, RefTargetOptionExt};
pub fn merge_ref_targets(
index: &dyn Index,
@ -129,8 +129,8 @@ pub fn classify_branch_push_action(
branch_target: &BranchTarget,
remote_name: &str,
) -> BranchPushAction {
let local_target = branch_target.local_target.as_ref();
let remote_target = branch_target.remote_targets.get(remote_name);
let local_target = &branch_target.local_target;
let remote_target = branch_target.remote_targets.get(remote_name).flatten();
if local_target == remote_target {
BranchPushAction::AlreadyMatches
} else if local_target.is_conflict() {
@ -159,7 +159,7 @@ mod tests {
let branch = BranchTarget {
local_target: RefTarget::normal(commit_id1.clone()),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1).unwrap(),
"origin".to_string() => RefTarget::normal(commit_id1),
}),
};
assert_eq!(
@ -190,7 +190,7 @@ mod tests {
let branch = BranchTarget {
local_target: RefTarget::absent(),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1.clone()).unwrap(),
"origin".to_string() => RefTarget::normal(commit_id1.clone()),
}),
};
assert_eq!(
@ -209,7 +209,7 @@ mod tests {
let branch = BranchTarget {
local_target: RefTarget::normal(commit_id2.clone()),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1.clone()).unwrap(),
"origin".to_string() => RefTarget::normal(commit_id1.clone()),
}),
};
assert_eq!(
@ -228,7 +228,7 @@ mod tests {
let branch = BranchTarget {
local_target: RefTarget::from_legacy_form([], [commit_id1.clone(), commit_id2]),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(commit_id1).unwrap(),
"origin".to_string() => RefTarget::normal(commit_id1),
}),
};
assert_eq!(
@ -247,7 +247,7 @@ mod tests {
"origin".to_string() => RefTarget::from_legacy_form(
[],
[commit_id1, commit_id2],
).unwrap(),
),
}),
};
assert_eq!(

8
lib/src/repo.rs
View file

@ -1011,7 +1011,7 @@ impl MutableRepo {
}
pub fn git_head(&self) -> Option<RefTarget> {
self.view.with_ref(|v| v.git_head().cloned())
self.view.with_ref(|v| v.git_head().clone())
}
pub fn set_git_head_target(&mut self, target: Option<RefTarget>) {
@ -1141,9 +1141,9 @@ impl MutableRepo {
let new_git_head_target = merge_ref_targets(
self.index(),
self.view().git_head(),
base.git_head(),
other.git_head(),
self.view().git_head().as_ref(),
base.git_head().as_ref(),
other.git_head().as_ref(),
);
self.set_git_head_target(new_git_head_target);
}

4
lib/src/revset.rs
View file

@ -1673,12 +1673,12 @@ fn collect_branch_symbols(repo: &dyn Repo, include_synced_remotes: bool) -> Vec<
all_branches
.iter()
.flat_map(|(name, branch_target)| {
let local_target = branch_target.local_target.as_ref();
let local_target = &branch_target.local_target;
let local_symbol = local_target.is_present().then(|| name.to_owned());
let remote_symbols = branch_target
.remote_targets
.iter()
.filter(move |&(_, target)| include_synced_remotes || Some(target) != local_target)
.filter(move |&(_, target)| include_synced_remotes || target != local_target)
.map(move |(remote_name, _)| format!("{name}@{remote_name}"));
local_symbol.into_iter().chain(remote_symbols)
})

33
lib/src/simple_op_store.rs
View file

@ -218,13 +218,13 @@ fn view_to_proto(view: &View) -> crate::protos::op_store::View {
..Default::default()
};
branch_proto.name = name.clone();
branch_proto.local_target = ref_target_to_proto(target.local_target.as_ref());
branch_proto.local_target = ref_target_to_proto(&target.local_target);
for (remote_name, target) in &target.remote_targets {
branch_proto
.remote_branches
.push(crate::protos::op_store::RemoteBranch {
remote_name: remote_name.clone(),
target: ref_target_to_proto(Some(target)),
target: ref_target_to_proto(target),
});
}
proto.branches.push(branch_proto);
@ -233,19 +233,19 @@ fn view_to_proto(view: &View) -> crate::protos::op_store::View {
for (name, target) in &view.tags {
proto.tags.push(crate::protos::op_store::Tag {
name: name.clone(),
target: ref_target_to_proto(Some(target)),
target: ref_target_to_proto(target),
});
}
for (git_ref_name, target) in &view.git_refs {
proto.git_refs.push(crate::protos::op_store::GitRef {
name: git_ref_name.clone(),
target: ref_target_to_proto(Some(target)),
target: ref_target_to_proto(target),
..Default::default()
});
}
proto.git_head = ref_target_to_proto(view.git_head.as_ref());
proto.git_head = ref_target_to_proto(&view.git_head);
proto
}
@ -277,7 +277,7 @@ fn view_from_proto(proto: crate::protos::op_store::View) -> View {
for remote_branch in branch_proto.remote_branches {
remote_targets.insert(
remote_branch.remote_name,
ref_target_from_proto(remote_branch.target).unwrap(),
ref_target_from_proto(remote_branch.target),
);
}
@ -291,10 +291,8 @@ fn view_from_proto(proto: crate::protos::op_store::View) -> View {
}
for tag_proto in proto.tags {
view.tags.insert(
tag_proto.name,
ref_target_from_proto(tag_proto.target).unwrap(),
);
view.tags
.insert(tag_proto.name, ref_target_from_proto(tag_proto.target));
}
for git_ref in proto.git_refs {
@ -304,7 +302,7 @@ fn view_from_proto(proto: crate::protos::op_store::View) -> View {
// Legacy format
RefTarget::normal(CommitId::new(git_ref.commit_id))
};
view.git_refs.insert(git_ref.name, target.unwrap());
view.git_refs.insert(git_ref.name, target);
}
#[allow(deprecated)]
@ -317,7 +315,7 @@ fn view_from_proto(proto: crate::protos::op_store::View) -> View {
view
}
fn ref_target_to_proto(value: Option<&RefTarget>) -> Option<crate::protos::op_store::RefTarget> {
fn ref_target_to_proto(value: &Option<RefTarget>) -> Option<crate::protos::op_store::RefTarget> {
if let Some(id) = value.as_normal() {
let proto = crate::protos::op_store::RefTarget {
value: Some(crate::protos::op_store::ref_target::Value::CommitId(
@ -326,6 +324,7 @@ fn ref_target_to_proto(value: Option<&RefTarget>) -> Option<crate::protos::op_st
};
Some(proto)
} else if value.is_conflict() {
// TODO: Preserve "absent" targets, and remove op_store::RefTargetMap hack.
let ref_conflict_proto = crate::protos::op_store::RefConflict {
removes: value.removed_ids().map(|id| id.to_bytes()).collect(),
adds: value.added_ids().map(|id| id.to_bytes()).collect(),
@ -391,22 +390,22 @@ mod tests {
"main".to_string() => BranchTarget {
local_target: branch_main_local_target,
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => branch_main_origin_target.unwrap(),
"origin".to_string() => branch_main_origin_target,
}),
},
"deleted".to_string() => BranchTarget {
local_target: RefTarget::absent(),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => branch_deleted_origin_target.unwrap(),
"origin".to_string() => branch_deleted_origin_target,
}),
},
},
tags: RefTargetMap(btreemap! {
"v1.0".to_string() => tag_v1_target.unwrap(),
"v1.0".to_string() => tag_v1_target,
}),
git_refs: RefTargetMap(btreemap! {
"refs/heads/main".to_string() => git_refs_main_target.unwrap(),
"refs/heads/feature".to_string() => git_refs_feature_target.unwrap(),
"refs/heads/main".to_string() => git_refs_main_target,
"refs/heads/feature".to_string() => git_refs_feature_target,
}),
git_head: RefTarget::normal(CommitId::from_hex("fff111")),
wc_commit_ids: hashmap! {

51
lib/src/view.rs
View file

@ -22,7 +22,9 @@ use itertools::Itertools;
use crate::backend::CommitId;
use crate::index::Index;
use crate::op_store;
use crate::op_store::{BranchTarget, RefTarget, RefTargetExt as _, WorkspaceId};
use crate::op_store::{
BranchTarget, RefTarget, RefTargetExt as _, RefTargetOptionExt as _, WorkspaceId,
};
use crate::refs::merge_ref_targets;
#[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Hash, Debug)]
@ -90,16 +92,16 @@ impl View {
&self.data.branches
}
pub fn tags(&self) -> &BTreeMap<String, RefTarget> {
pub fn tags(&self) -> &BTreeMap<String, Option<RefTarget>> {
&self.data.tags
}
pub fn git_refs(&self) -> &BTreeMap<String, RefTarget> {
pub fn git_refs(&self) -> &BTreeMap<String, Option<RefTarget>> {
&self.data.git_refs
}
pub fn git_head(&self) -> Option<&RefTarget> {
self.data.git_head.as_ref()
pub fn git_head(&self) -> &Option<RefTarget> {
&self.data.git_head
}
pub fn set_wc_commit(&mut self, workspace_id: WorkspaceId, commit_id: CommitId) {
@ -170,15 +172,16 @@ impl View {
/// Sets local branch to point to the given target. If the target is absent,
/// and if no associated remote branches exist, the branch will be removed.
pub fn set_local_branch_target(&mut self, name: &str, target: Option<RefTarget>) {
if let Some(target) = target {
if target.is_present() {
self.insert_local_branch(name.to_owned(), target);
} else {
self.remove_local_branch(name);
}
}
fn insert_local_branch(&mut self, name: String, target: RefTarget) {
self.data.branches.entry(name).or_default().local_target = Some(target);
fn insert_local_branch(&mut self, name: String, target: Option<RefTarget>) {
assert!(target.is_present());
self.data.branches.entry(name).or_default().local_target = target;
}
fn remove_local_branch(&mut self, name: &str) {
@ -191,10 +194,13 @@ impl View {
}
pub fn get_remote_branch(&self, name: &str, remote_name: &str) -> Option<RefTarget> {
self.data
.branches
.get(name)
.and_then(|branch_target| branch_target.remote_targets.get(remote_name).cloned())
self.data.branches.get(name).and_then(|branch_target| {
branch_target
.remote_targets
.get(remote_name)
.flatten()
.clone()
})
}
/// Sets remote-tracking branch to point to the given target. If the target
@ -205,14 +211,20 @@ impl View {
remote_name: &str,
target: Option<RefTarget>,
) {
if let Some(target) = target {
if target.is_present() {
self.insert_remote_branch(name.to_owned(), remote_name.to_owned(), target);
} else {
self.remove_remote_branch(name, remote_name);
}
}
fn insert_remote_branch(&mut self, name: String, remote_name: String, target: RefTarget) {
fn insert_remote_branch(
&mut self,
name: String,
remote_name: String,
target: Option<RefTarget>,
) {
assert!(target.is_present());
self.data
.branches
.entry(name)
@ -232,20 +244,21 @@ impl View {
pub fn rename_remote(&mut self, old: &str, new: &str) {
for branch in self.data.branches.values_mut() {
if let Some(target) = branch.remote_targets.remove(old) {
let target = branch.remote_targets.remove(old).flatten();
if target.is_present() {
branch.remote_targets.insert(new.to_owned(), target);
}
}
}
pub fn get_tag(&self, name: &str) -> Option<RefTarget> {
self.data.tags.get(name).cloned()
self.data.tags.get(name).flatten().clone()
}
/// Sets tag to point to the given target. If the target is absent, the tag
/// will be removed.
pub fn set_tag_target(&mut self, name: &str, target: Option<RefTarget>) {
if let Some(target) = target {
if target.is_present() {
self.data.tags.insert(name.to_owned(), target);
} else {
self.data.tags.remove(name);
@ -253,13 +266,13 @@ impl View {
}
pub fn get_git_ref(&self, name: &str) -> Option<RefTarget> {
self.data.git_refs.get(name).cloned()
self.data.git_refs.get(name).flatten().clone()
}
/// Sets the last imported Git ref to point to the given target. If the
/// target is absent, the reference will be removed.
pub fn set_git_ref_target(&mut self, name: &str, target: Option<RefTarget>) {
if let Some(target) = target {
if target.is_present() {
self.data.git_refs.insert(name.to_owned(), target);
} else {
self.data.git_refs.remove(name);

67
lib/tests/test_git.rs
View file

@ -27,7 +27,7 @@ use jj_lib::commit_builder::CommitBuilder;
use jj_lib::git;
use jj_lib::git::{GitFetchError, GitPushError, GitRefUpdate, SubmoduleConfig};
use jj_lib::git_backend::GitBackend;
use jj_lib::op_store::{BranchTarget, RefTarget, RefTargetMap};
use jj_lib::op_store::{BranchTarget, RefTarget, RefTargetMap, RefTargetOptionExt as _};
use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
use jj_lib::settings::{GitSettings, UserSettings};
use jj_lib::view::RefName;
@ -111,7 +111,7 @@ fn test_import_refs() {
let expected_main_branch = BranchTarget {
local_target: RefTarget::normal(jj_id(&commit2)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&commit1)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&commit1)),
}),
};
assert_eq!(
@ -137,7 +137,7 @@ fn test_import_refs() {
let expected_feature3_branch = BranchTarget {
local_target: RefTarget::normal(jj_id(&commit6)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&commit6)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&commit6)),
}),
};
assert_eq!(
@ -146,8 +146,8 @@ fn test_import_refs() {
);
assert_eq!(
view.tags().get("v1.0"),
RefTarget::normal(jj_id(&commit5)).as_ref()
view.tags().get("v1.0").flatten(),
&RefTarget::normal(jj_id(&commit5))
);
assert_eq!(view.git_refs().len(), 6);
@ -175,7 +175,7 @@ fn test_import_refs() {
view.get_git_ref("refs/tags/v1.0"),
RefTarget::normal(jj_id(&commit5))
);
assert_eq!(view.git_head(), RefTarget::normal(jj_id(&commit2)).as_ref());
assert_eq!(view.git_head(), &RefTarget::normal(jj_id(&commit2)));
}
#[test]
@ -241,7 +241,7 @@ fn test_import_refs_reimport() {
let expected_main_branch = BranchTarget {
local_target: RefTarget::normal(jj_id(&commit2)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => commit1_target.clone().unwrap(),
"origin".to_string() => commit1_target.clone(),
}),
};
assert_eq!(
@ -439,7 +439,7 @@ fn test_import_refs_reimport_with_deleted_remote_ref() {
// creates one. This follows the model explained in docs/branches.md.
local_target: RefTarget::normal(jj_id(&commit_remote_only)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_only)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_only)),
}),
}),
);
@ -448,7 +448,7 @@ fn test_import_refs_reimport_with_deleted_remote_ref() {
Some(&BranchTarget {
local_target: RefTarget::normal(jj_id(&commit_remote_and_local)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_and_local)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_and_local)),
}),
}),
);
@ -528,7 +528,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
// creates one. This follows the model explained in docs/branches.md.
local_target: RefTarget::normal(jj_id(&commit_remote_only)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_only)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_only)),
}),
}),
);
@ -537,7 +537,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
Some(&BranchTarget {
local_target: RefTarget::normal(jj_id(&commit_remote_and_local)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_and_local)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&commit_remote_and_local)),
}),
}),
);
@ -572,7 +572,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
Some(&BranchTarget {
local_target: RefTarget::normal(jj_id(&new_commit_remote_only)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&new_commit_remote_only)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&new_commit_remote_only)),
}),
}),
);
@ -581,7 +581,7 @@ fn test_import_refs_reimport_with_moved_remote_ref() {
Some(&BranchTarget {
local_target: RefTarget::normal(jj_id(&new_commit_remote_and_local)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&new_commit_remote_and_local)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&new_commit_remote_and_local)),
}),
}),
);
@ -713,7 +713,7 @@ fn test_import_some_refs() {
let expected_feature1_branch = BranchTarget {
local_target: RefTarget::normal(jj_id(&commit_feat1)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => commit_feat1_target.unwrap(),
"origin".to_string() => commit_feat1_target,
}),
};
assert_eq!(
@ -723,7 +723,7 @@ fn test_import_some_refs() {
let expected_feature2_branch = BranchTarget {
local_target: RefTarget::normal(jj_id(&commit_feat2)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => commit_feat2_target.unwrap(),
"origin".to_string() => commit_feat2_target,
}),
};
assert_eq!(
@ -733,7 +733,7 @@ fn test_import_some_refs() {
let expected_feature3_branch = BranchTarget {
local_target: RefTarget::normal(jj_id(&commit_feat3)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => commit_feat3_target.unwrap(),
"origin".to_string() => commit_feat3_target,
}),
};
assert_eq!(
@ -743,7 +743,7 @@ fn test_import_some_refs() {
let expected_feature4_branch = BranchTarget {
local_target: RefTarget::normal(jj_id(&commit_feat4)),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => commit_feat4_target.unwrap(),
"origin".to_string() => commit_feat4_target,
}),
};
assert_eq!(
@ -926,10 +926,7 @@ fn test_import_refs_detached_head() {
let expected_heads = hashset! { jj_id(&commit1) };
assert_eq!(*repo.view().heads(), expected_heads);
assert_eq!(repo.view().git_refs().len(), 0);
assert_eq!(
repo.view().git_head(),
RefTarget::normal(jj_id(&commit1)).as_ref()
);
assert_eq!(repo.view().git_head(), &RefTarget::normal(jj_id(&commit1)));
}
#[test]
@ -1153,7 +1150,7 @@ fn test_import_export_no_auto_local_branch() {
let expected_branch = BranchTarget {
local_target: RefTarget::absent(),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(jj_id(&git_commit)).unwrap(),
"origin".to_string() => RefTarget::normal(jj_id(&git_commit)),
}),
};
assert_eq!(
@ -1403,15 +1400,15 @@ fn test_export_reexport_transitions() {
assert_eq!(
*mut_repo.view().git_refs(),
btreemap! {
"refs/heads/AAX".to_string() => RefTarget::normal(commit_a.id().clone()).unwrap(),
"refs/heads/AAB".to_string() => RefTarget::normal(commit_a.id().clone()).unwrap(),
"refs/heads/ABA".to_string() => RefTarget::normal(commit_b.id().clone()).unwrap(),
"refs/heads/ABB".to_string() => RefTarget::normal(commit_b.id().clone()).unwrap(),
"refs/heads/ABC".to_string() => RefTarget::normal(commit_a.id().clone()).unwrap(),
"refs/heads/ABX".to_string() => RefTarget::normal(commit_a.id().clone()).unwrap(),
"refs/heads/AXB".to_string() => RefTarget::normal(commit_a.id().clone()).unwrap(),
"refs/heads/XAA".to_string() => RefTarget::normal(commit_a.id().clone()).unwrap(),
"refs/heads/XAX".to_string() => RefTarget::normal(commit_a.id().clone()).unwrap(),
"refs/heads/AAX".to_string() => RefTarget::normal(commit_a.id().clone()),
"refs/heads/AAB".to_string() => RefTarget::normal(commit_a.id().clone()),
"refs/heads/ABA".to_string() => RefTarget::normal(commit_b.id().clone()),
"refs/heads/ABB".to_string() => RefTarget::normal(commit_b.id().clone()),
"refs/heads/ABC".to_string() => RefTarget::normal(commit_a.id().clone()),
"refs/heads/ABX".to_string() => RefTarget::normal(commit_a.id().clone()),
"refs/heads/AXB".to_string() => RefTarget::normal(commit_a.id().clone()),
"refs/heads/XAA".to_string() => RefTarget::normal(commit_a.id().clone()),
"refs/heads/XAX".to_string() => RefTarget::normal(commit_a.id().clone()),
}
);
}
@ -1496,7 +1493,7 @@ fn test_fetch_initial_commit() {
assert_eq!(
*view.git_refs(),
btreemap! {
"refs/remotes/origin/main".to_string() => initial_commit_target.clone().unwrap(),
"refs/remotes/origin/main".to_string() => initial_commit_target.clone(),
}
);
assert_eq!(
@ -1505,7 +1502,7 @@ fn test_fetch_initial_commit() {
"main".to_string() => BranchTarget {
local_target: initial_commit_target.clone(),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => initial_commit_target.unwrap(),
"origin".to_string() => initial_commit_target,
})
},
}
@ -1561,7 +1558,7 @@ fn test_fetch_success() {
assert_eq!(
*view.git_refs(),
btreemap! {
"refs/remotes/origin/main".to_string() => new_commit_target.clone().unwrap(),
"refs/remotes/origin/main".to_string() => new_commit_target.clone(),
}
);
assert_eq!(
@ -1570,7 +1567,7 @@ fn test_fetch_success() {
"main".to_string() => BranchTarget {
local_target: new_commit_target.clone(),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => new_commit_target.unwrap(),
"origin".to_string() => new_commit_target,
}),
},
}

14
lib/tests/test_view.rs
View file

@ -310,8 +310,8 @@ fn test_merge_views_branches() {
],
),
remote_targets: RefTargetMap(btreemap! {
"origin".to_string() => RefTarget::normal(main_branch_origin_tx1.id().clone()).unwrap(),
"alternate".to_string() => RefTarget::normal(main_branch_alternate_tx0.id().clone()).unwrap(),
"origin".to_string() => RefTarget::normal(main_branch_origin_tx1.id().clone()),
"alternate".to_string() => RefTarget::normal(main_branch_alternate_tx0.id().clone()),
}),
};
let expected_feature_branch = BranchTarget {
@ -367,8 +367,8 @@ fn test_merge_views_tags() {
assert_eq!(
repo.view().tags(),
&btreemap! {
"v1.0".to_string() => expected_v1.unwrap(),
"v2.0".to_string() => expected_v2.unwrap(),
"v1.0".to_string() => expected_v1,
"v2.0".to_string() => expected_v2,
}
);
}
@ -425,8 +425,8 @@ fn test_merge_views_git_refs() {
assert_eq!(
repo.view().git_refs(),
&btreemap! {
"refs/heads/main".to_string() => expected_main_branch.unwrap(),
"refs/heads/feature".to_string() => expected_feature_branch.unwrap(),
"refs/heads/main".to_string() => expected_main_branch,
"refs/heads/feature".to_string() => expected_feature_branch,
}
);
}
@ -460,7 +460,7 @@ fn test_merge_views_git_heads() {
[tx0_head.id().clone()],
[tx1_head.id().clone(), tx2_head.id().clone()],
);
assert_eq!(repo.view().git_head(), expected_git_head.as_ref());
assert_eq!(repo.view().git_head(), &expected_git_head);
}
fn commit_transactions(settings: &UserSettings, txs: Vec<Transaction>) -> Arc<ReadonlyRepo> {

4
src/cli_util.rs
View file

@ -733,8 +733,8 @@ impl WorkspaceCommandHelper {
let mut tx = self.start_transaction("import git refs").into_inner();
git::import_refs(tx.mut_repo(), git_repo, &self.settings.git_settings())?;
if tx.mut_repo().has_changes() {
let old_git_head = self.repo().view().git_head().cloned();
let new_git_head = tx.mut_repo().view().git_head().cloned();
let old_git_head = self.repo().view().git_head().clone();
let new_git_head = tx.mut_repo().view().git_head().clone();
// If the Git HEAD has changed, abandon our old checkout and check out the new
// Git HEAD.
match new_git_head.as_normal() {

8
src/commands/branch.rs
View file

@ -369,7 +369,7 @@ fn cmd_branch_list(
}
let print_branch_target =
|formatter: &mut dyn Formatter, target: &RefTarget| -> Result<(), CommandError> {
|formatter: &mut dyn Formatter, target: &Option<RefTarget>| -> Result<(), CommandError> {
if let Some(id) = target.as_normal() {
write!(formatter, ": ")?;
let commit = repo.store().get_commit(id)?;
@ -406,8 +406,8 @@ fn cmd_branch_list(
};
write!(formatter.labeled("branch"), "{name}")?;
if let Some(target) = branch_target.local_target.as_ref() {
print_branch_target(formatter, target)?;
if branch_target.local_target.is_present() {
print_branch_target(formatter, &branch_target.local_target)?;
} else if found_non_git_remote {
writeln!(formatter, " (deleted)")?;
} else {
@ -415,7 +415,7 @@ fn cmd_branch_list(
}
for (remote, remote_target) in branch_target.remote_targets.iter() {
if Some(remote_target) == branch_target.local_target.as_ref() {
if remote_target == &branch_target.local_target {
continue;
}
write!(formatter, " ")?;

6
src/commit_templater.rs
View file

@ -354,11 +354,11 @@ fn build_branches_index(repo: &dyn Repo) -> RefNamesIndex {
let mut index = RefNamesIndex::default();
let (all_branches, _) = git::build_unified_branches_map(repo.view());
for (branch_name, branch_target) in &all_branches {
let local_target = branch_target.local_target.as_ref();
let local_target = &branch_target.local_target;
let mut unsynced_remote_targets = branch_target
.remote_targets
.iter()
.filter(|&(_, target)| Some(target) != local_target)
.filter(|&(_, target)| target != local_target)
.peekable();
if local_target.is_present() {
let decorated_name = if local_target.is_conflict() {
@ -383,7 +383,7 @@ fn build_branches_index(repo: &dyn Repo) -> RefNamesIndex {
}
fn build_ref_names_index<'a>(
ref_pairs: impl IntoIterator<Item = (&'a String, &'a RefTarget)>,
ref_pairs: impl IntoIterator<Item = (&'a String, &'a Option<RefTarget>)>,
) -> RefNamesIndex {
let mut index = RefNamesIndex::default();
for (name, target) in ref_pairs {