From 4db3d8d3a63573e63ad067275989f6714d7c8c2b Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 16 Jan 2021 10:42:22 -0800 Subject: [PATCH] view: add tracking of "public" heads (copying Mercurial's phase concept) Mercurial's "phase" concept is important for evolution, and it's also useful for filtering out uninteresting commits from log output. Commits are typically marked "public" when they are pushed to a remote. The CLI prevents public commits from being rewritten. Public commits cannot be obsolete (even if they have a successor, they won't be considered obsolete like non-public commits would). This commits just makes space for tracking the public heads in the View. --- lib/protos/op_store.proto | 1 + lib/src/op_store.rs | 3 ++ lib/src/simple_op_store.rs | 7 +++ lib/src/transaction.rs | 12 ++++++ lib/src/view.rs | 29 ++++++++++++- lib/tests/test_transaction.rs | 80 +++++++++++++++++++++++++++++++++++ 6 files changed, 131 insertions(+), 1 deletion(-) diff --git a/lib/protos/op_store.proto b/lib/protos/op_store.proto index f8a735072..018aac20f 100644 --- a/lib/protos/op_store.proto +++ b/lib/protos/op_store.proto @@ -23,6 +23,7 @@ message GitRef { message View { repeated bytes head_ids = 1; + repeated bytes public_head_ids = 4; bytes checkout = 2; // Only a subset of the refs. For example, does not include refs/notes/. repeated GitRef git_refs = 3; diff --git a/lib/src/op_store.rs b/lib/src/op_store.rs index 9f2c5defc..9307ab281 100644 --- a/lib/src/op_store.rs +++ b/lib/src/op_store.rs @@ -52,6 +52,8 @@ impl OperationId { pub struct View { /// All head commits pub head_ids: HashSet, + /// Heads of the set of public commits. + pub public_head_ids: HashSet, pub git_refs: BTreeMap, // The commit that *should be* checked out in the (default) working copy. Note that the // working copy (.jj/working_copy/) has the source of truth about which commit *is* checked out @@ -64,6 +66,7 @@ impl View { pub fn new(checkout: CommitId) -> Self { Self { head_ids: HashSet::new(), + public_head_ids: HashSet::new(), git_refs: BTreeMap::new(), checkout, } diff --git a/lib/src/simple_op_store.rs b/lib/src/simple_op_store.rs index c285fb408..93e9b6e0b 100644 --- a/lib/src/simple_op_store.rs +++ b/lib/src/simple_op_store.rs @@ -198,6 +198,9 @@ fn view_to_proto(view: &View) -> crate::protos::op_store::View { for head_id in &view.head_ids { proto.head_ids.push(head_id.0.clone()); } + for head_id in &view.public_head_ids { + proto.public_head_ids.push(head_id.0.clone()); + } for (git_ref_name, commit_id) in &view.git_refs { let mut git_ref_proto = crate::protos::op_store::GitRef::new(); git_ref_proto.set_name(git_ref_name.clone()); @@ -212,6 +215,10 @@ fn view_from_proto(proto: &crate::protos::op_store::View) -> View { for head_id_bytes in proto.head_ids.iter() { view.head_ids.insert(CommitId(head_id_bytes.to_vec())); } + for head_id_bytes in proto.public_head_ids.iter() { + view.public_head_ids + .insert(CommitId(head_id_bytes.to_vec())); + } for git_ref in proto.git_refs.iter() { view.git_refs .insert(git_ref.name.clone(), CommitId(git_ref.commit_id.to_vec())); diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index 3b3570dde..e1de2c3d8 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -171,6 +171,18 @@ impl<'r> Transaction<'r> { mut_repo.evolution.as_mut().unwrap().invalidate(); } + pub fn add_public_head(&mut self, head: &Commit) { + let mut_repo = Arc::get_mut(self.repo.as_mut().unwrap()).unwrap(); + mut_repo.view.as_mut().unwrap().add_public_head(head); + mut_repo.evolution.as_mut().unwrap().add_commit(head); + } + + pub fn remove_public_head(&mut self, head: &Commit) { + let mut_repo = Arc::get_mut(self.repo.as_mut().unwrap()).unwrap(); + mut_repo.view.as_mut().unwrap().remove_public_head(head); + mut_repo.evolution.as_mut().unwrap().invalidate(); + } + pub fn insert_git_ref(&mut self, name: String, commit_id: CommitId) { let mut_repo = Arc::get_mut(self.repo.as_mut().unwrap()).unwrap(); mut_repo diff --git a/lib/src/view.rs b/lib/src/view.rs index 11e9c540f..c201c2261 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -32,6 +32,7 @@ use crate::store_wrapper::StoreWrapper; pub trait View { fn checkout(&self) -> &CommitId; fn heads<'a>(&'a self) -> Box + 'a>; + fn public_heads<'a>(&'a self) -> Box + 'a>; fn git_refs(&self) -> &BTreeMap; fn op_store(&self) -> Arc; fn base_op_head_id(&self) -> &OperationId; @@ -66,6 +67,8 @@ fn enforce_invariants(store: &StoreWrapper, view: &mut op_store::View) { // TODO: This is surely terribly slow on large repos, at least in its current // form. We should make it faster (using the index) and avoid calling it in // most cases (avoid adding a head that's already reachable in the view). + view.public_head_ids = heads_of_set(store, view.public_head_ids.iter().cloned()); + view.head_ids.extend(view.public_head_ids.iter().cloned()); view.head_ids.extend(view.git_refs.values().cloned()); view.head_ids = heads_of_set(store, view.head_ids.iter().cloned()); } @@ -150,6 +153,14 @@ pub fn merge_views( // and emit a warning? } + for removed_head in base.public_head_ids.difference(&right.public_head_ids) { + result.public_head_ids.remove(removed_head); + } + for added_head in right.public_head_ids.difference(&base.public_head_ids) { + result.public_head_ids.insert(added_head.clone()); + } + result.public_head_ids = heads_of_set(store, result.public_head_ids.into_iter()); + for removed_head in base.head_ids.difference(&right.head_ids) { result.head_ids.remove(removed_head); } @@ -316,6 +327,10 @@ impl View for ReadonlyView { Box::new(self.data.head_ids.iter()) } + fn public_heads<'a>(&'a self) -> Box + 'a> { + Box::new(self.data.public_head_ids.iter()) + } + fn git_refs(&self) -> &BTreeMap { &self.data.git_refs } @@ -407,6 +422,10 @@ impl View for MutableView { Box::new(self.data.head_ids.iter()) } + fn public_heads<'a>(&'a self) -> Box + 'a> { + Box::new(self.data.public_head_ids.iter()) + } + fn git_refs(&self) -> &BTreeMap { &self.data.git_refs } @@ -435,10 +454,18 @@ impl MutableView { pub fn remove_head(&mut self, head: &Commit) { self.data.head_ids.remove(head.id()); - // To potentially add back heads based on git refs enforce_invariants(&self.store, &mut self.data); } + pub fn add_public_head(&mut self, head: &Commit) { + self.data.public_head_ids.insert(head.id().clone()); + enforce_invariants(&self.store, &mut self.data); + } + + pub fn remove_public_head(&mut self, head: &Commit) { + self.data.public_head_ids.remove(head.id()); + } + pub fn insert_git_ref(&mut self, name: String, commit_id: CommitId) { self.data.git_refs.insert(name, commit_id); } diff --git a/lib/tests/test_transaction.rs b/lib/tests/test_transaction.rs index 97d63faf7..6c050686d 100644 --- a/lib/tests/test_transaction.rs +++ b/lib/tests/test_transaction.rs @@ -428,3 +428,83 @@ fn test_remove_head_ancestor_git_ref(use_git: bool) { assert!(!heads.contains(commit2.id())); assert!(heads.contains(commit1.id())); } + +#[test_case(false ; "local store")] +// #[test_case(true ; "git store")] +fn test_add_public_head(use_git: bool) { + // Test that Transaction::add_public_head() adds the head, and that it's still + // there after commit. + let settings = testutils::user_settings(); + let (_temp_dir, mut repo) = testutils::init_repo(&settings, use_git); + + let mut tx = repo.start_transaction("test"); + let commit1 = testutils::create_random_commit(&settings, &repo).write_to_transaction(&mut tx); + tx.commit(); + Arc::get_mut(&mut repo).unwrap().reload(); + + let mut tx = repo.start_transaction("test"); + let public_heads: HashSet<_> = tx.as_repo().view().public_heads().cloned().collect(); + assert!(!public_heads.contains(commit1.id())); + tx.add_public_head(&commit1); + let public_heads: HashSet<_> = tx.as_repo().view().public_heads().cloned().collect(); + assert!(public_heads.contains(commit1.id())); + tx.commit(); + Arc::get_mut(&mut repo).unwrap().reload(); + let public_heads: HashSet<_> = repo.view().public_heads().cloned().collect(); + assert!(public_heads.contains(commit1.id())); +} + +#[test_case(false ; "local store")] +// #[test_case(true ; "git store")] +fn test_add_public_head_ancestor(use_git: bool) { + // Test that Transaction::add_public_head() does not add a public head if it's + // an ancestor of an existing public head. + let settings = testutils::user_settings(); + let (_temp_dir, mut repo) = testutils::init_repo(&settings, use_git); + + let mut tx = repo.start_transaction("test"); + let commit1 = testutils::create_random_commit(&settings, &repo).write_to_transaction(&mut tx); + let commit2 = testutils::create_random_commit(&settings, &repo) + .set_parents(vec![commit1.id().clone()]) + .write_to_transaction(&mut tx); + tx.add_public_head(&commit2); + tx.commit(); + Arc::get_mut(&mut repo).unwrap().reload(); + + let mut tx = repo.start_transaction("test"); + let public_heads: HashSet<_> = tx.as_repo().view().public_heads().cloned().collect(); + assert!(!public_heads.contains(commit1.id())); + tx.add_public_head(&commit1); + let public_heads: HashSet<_> = tx.as_repo().view().public_heads().cloned().collect(); + assert!(!public_heads.contains(commit1.id())); + tx.commit(); + Arc::get_mut(&mut repo).unwrap().reload(); + let public_heads: HashSet<_> = repo.view().public_heads().cloned().collect(); + assert!(!public_heads.contains(commit1.id())); +} + +#[test_case(false ; "local store")] +// #[test_case(true ; "git store")] +fn test_remove_public_head(use_git: bool) { + // Test that Transaction::remove_public_head() removes the head, and that it's + // still removed after commit. + let settings = testutils::user_settings(); + let (_temp_dir, mut repo) = testutils::init_repo(&settings, use_git); + + let mut tx = repo.start_transaction("test"); + let commit1 = testutils::create_random_commit(&settings, &repo).write_to_transaction(&mut tx); + tx.add_public_head(&commit1); + tx.commit(); + Arc::get_mut(&mut repo).unwrap().reload(); + + let mut tx = repo.start_transaction("test"); + let public_heads: HashSet<_> = tx.as_repo().view().public_heads().cloned().collect(); + assert!(public_heads.contains(commit1.id())); + tx.remove_public_head(&commit1); + let public_heads: HashSet<_> = tx.as_repo().view().public_heads().cloned().collect(); + assert!(!public_heads.contains(commit1.id())); + tx.commit(); + Arc::get_mut(&mut repo).unwrap().reload(); + let public_heads: HashSet<_> = repo.view().public_heads().cloned().collect(); + assert!(!public_heads.contains(commit1.id())); +}