From 6703810c6edef5a1b35abc69e3ea6d28ccad8654 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Fri, 4 Nov 2022 22:34:24 -0700 Subject: [PATCH] backend: remove `Commit::is_open` field from data model --- lib/src/backend.rs | 2 -- lib/src/commit.rs | 4 ---- lib/src/commit_builder.rs | 7 ------ lib/src/git_backend.rs | 6 ----- lib/src/local_backend.rs | 2 -- lib/src/protos/store.proto | 2 +- lib/tests/test_commit_builder.rs | 2 -- lib/tests/test_init.rs | 1 - lib/tests/test_mut_repo.rs | 40 ++++++++++---------------------- lib/tests/test_rewrite.rs | 9 ++----- src/commands.rs | 4 +--- 11 files changed, 16 insertions(+), 63 deletions(-) diff --git a/lib/src/backend.rs b/lib/src/backend.rs index 5fc7e9d01..08cd46fbe 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -266,7 +266,6 @@ pub struct Commit { pub description: String, pub author: Signature, pub committer: Signature, - pub is_open: bool, } #[derive(Debug, PartialEq, Eq, Clone)] @@ -393,7 +392,6 @@ pub fn make_root_commit(empty_tree_id: TreeId) -> Commit { description: String::new(), author: signature.clone(), committer: signature, - is_open: false, } } diff --git a/lib/src/commit.rs b/lib/src/commit.rs index 052b4247e..d6bdb5b33 100644 --- a/lib/src/commit.rs +++ b/lib/src/commit.rs @@ -117,10 +117,6 @@ impl Commit { &self.data } - pub fn is_open(&self) -> bool { - self.data.is_open - } - pub fn is_empty(&self) -> bool { let parents = self.parents(); // TODO: Perhaps the root commit should also be considered empty. diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs index 147c0684b..b51b1415f 100644 --- a/lib/src/commit_builder.rs +++ b/lib/src/commit_builder.rs @@ -46,7 +46,6 @@ impl CommitBuilder { description: String::new(), author: signature.clone(), committer: signature, - is_open: false, }; CommitBuilder { commit, @@ -86,7 +85,6 @@ impl CommitBuilder { description: String::new(), author: signature.clone(), committer: signature, - is_open: true, }; CommitBuilder { commit, @@ -125,11 +123,6 @@ impl CommitBuilder { self } - pub fn set_open(mut self, is_open: bool) -> Self { - self.commit.is_open = is_open; - self - } - pub fn set_author(mut self, author: Signature) -> Self { self.commit.author = author; self diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index ba0ba7918..8f0c697c3 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -125,7 +125,6 @@ fn signature_to_git(signature: &Signature) -> git2::Signature { fn serialize_extras(commit: &Commit) -> Vec { let mut proto = crate::protos::store::Commit::new(); - proto.is_open = commit.is_open; proto.change_id = commit.change_id.to_bytes(); for predecessor in &commit.predecessors { proto.predecessors.push(predecessor.to_bytes()); @@ -136,7 +135,6 @@ fn serialize_extras(commit: &Commit) -> Vec { fn deserialize_extras(commit: &mut Commit, bytes: &[u8]) { let mut cursor = Cursor::new(bytes); let proto: crate::protos::store::Commit = Message::parse_from_reader(&mut cursor).unwrap(); - commit.is_open = proto.is_open; commit.change_id = ChangeId::new(proto.change_id); for predecessor in &proto.predecessors { commit.predecessors.push(CommitId::from_bytes(predecessor)); @@ -390,7 +388,6 @@ impl Backend for GitBackend { description, author, committer, - is_open: false, }; let table = self.extra_metadata_store.get_head().map_err(|err| { @@ -591,7 +588,6 @@ mod tests { assert_eq!(commit.parents, vec![CommitId::from_bytes(&[0; 20])]); assert_eq!(commit.predecessors, vec![]); assert_eq!(commit.root_tree.as_bytes(), root_tree_id.as_bytes()); - assert!(!commit.is_open); assert_eq!(commit.description, "git commit message"); assert_eq!(commit.author.name, "git author"); assert_eq!(commit.author.email, "git.author@example.com"); @@ -668,7 +664,6 @@ mod tests { description: "initial".to_string(), author: signature.clone(), committer: signature, - is_open: false, }; let commit_id = store.write_commit(&commit).unwrap(); let git_refs = store @@ -704,7 +699,6 @@ mod tests { description: "initial".to_string(), author: signature.clone(), committer: signature, - is_open: false, }; let commit_id1 = store.write_commit(&commit1).unwrap(); let mut commit2 = commit1; diff --git a/lib/src/local_backend.rs b/lib/src/local_backend.rs index 53e55fe98..73b3159a9 100644 --- a/lib/src/local_backend.rs +++ b/lib/src/local_backend.rs @@ -267,7 +267,6 @@ pub fn commit_to_proto(commit: &Commit) -> crate::protos::store::Commit { proto.description = commit.description.clone(); proto.author = MessageField::some(signature_to_proto(&commit.author)); proto.committer = MessageField::some(signature_to_proto(&commit.committer)); - proto.is_open = commit.is_open; proto } @@ -289,7 +288,6 @@ fn commit_from_proto(proto: &crate::protos::store::Commit) -> Commit { description: proto.description.clone(), author: signature_from_proto(&proto.author), committer: signature_from_proto(&proto.committer), - is_open: proto.is_open, } } diff --git a/lib/src/protos/store.proto b/lib/src/protos/store.proto index 7f119c989..299ea197f 100644 --- a/lib/src/protos/store.proto +++ b/lib/src/protos/store.proto @@ -58,7 +58,7 @@ message Commit { Signature author = 6; Signature committer = 7; - bool is_open = 8; + bool is_open = 8 [deprecated = true]; bool is_pruned = 9 [deprecated = true]; } diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index 7d8b38634..082f581b1 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -50,7 +50,6 @@ fn test_initial(use_git: bool) { assert_eq!(commit.parents(), vec![store.root_commit()]); assert_eq!(commit.predecessors(), vec![]); - assert!(!commit.is_open()); assert_eq!(commit.description(), ""); assert_eq!(commit.author().name, settings.user_name()); assert_eq!(commit.author().email, settings.user_email()); @@ -123,7 +122,6 @@ fn test_rewrite(use_git: bool) { rewritten_commit.predecessors(), vec![initial_commit.clone()] ); - assert!(!rewritten_commit.is_open()); assert_eq!(rewritten_commit.author().name, settings.user_name()); assert_eq!(rewritten_commit.author().email, settings.user_email()); assert_eq!( diff --git a/lib/tests/test_init.rs b/lib/tests/test_init.rs index 7a6dfbe4d..82809b522 100644 --- a/lib/tests/test_init.rs +++ b/lib/tests/test_init.rs @@ -125,7 +125,6 @@ fn test_init_checkout(use_git: bool) { ); assert_eq!(wc_commit.predecessors(), vec![]); assert_eq!(wc_commit.description(), ""); - assert!(wc_commit.is_open()); assert_eq!(wc_commit.author().name, settings.user_name()); assert_eq!(wc_commit.author().email, settings.user_email()); assert_eq!(wc_commit.committer().name, settings.user_name()); diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index bbd7a8d8c..b7de03ffb 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -22,15 +22,13 @@ use test_case::test_case; #[test_case(false ; "local backend")] #[test_case(true ; "git backend")] fn test_edit(use_git: bool) { - // Test that MutableRepo::check_out() uses the requested commit if it's open + // Test that MutableRepo::edit() uses the requested commit (not a new child) let settings = testutils::user_settings(); let test_repo = TestRepo::init(use_git); let repo = &test_repo.repo; let mut tx = repo.start_transaction("test"); - let wc_commit = testutils::create_random_commit(&settings, repo) - .set_open(true) - .write_to_repo(tx.mut_repo()); + let wc_commit = testutils::create_random_commit(&settings, repo).write_to_repo(tx.mut_repo()); let repo = tx.commit(); let mut tx = repo.start_transaction("test"); @@ -42,17 +40,15 @@ fn test_edit(use_git: bool) { #[test_case(false ; "local backend")] #[test_case(true ; "git backend")] -fn test_checkout_closed(use_git: bool) { - // Test that MutableRepo::check_out() creates a child if the requested commit is - // closed +fn test_checkout(use_git: bool) { + // Test that MutableRepo::check_out() creates a child let settings = testutils::user_settings(); let test_repo = TestRepo::init(use_git); let repo = &test_repo.repo; let mut tx = repo.start_transaction("test"); - let requested_checkout = testutils::create_random_commit(&settings, repo) - .set_open(false) - .write_to_repo(tx.mut_repo()); + let requested_checkout = + testutils::create_random_commit(&settings, repo).write_to_repo(tx.mut_repo()); let repo = tx.commit(); let mut tx = repo.start_transaction("test"); @@ -81,18 +77,14 @@ fn test_checkout_previous_not_empty(use_git: bool) { let mut tx = repo.start_transaction("test"); let mut_repo = tx.mut_repo(); - let old_checkout = testutils::create_random_commit(&settings, repo) - .set_open(true) - .write_to_repo(mut_repo); + let old_checkout = testutils::create_random_commit(&settings, repo).write_to_repo(mut_repo); let ws_id = WorkspaceId::default(); mut_repo.edit(ws_id.clone(), &old_checkout).unwrap(); let repo = tx.commit(); let mut tx = repo.start_transaction("test"); let mut_repo = tx.mut_repo(); - let new_checkout = testutils::create_random_commit(&settings, &repo) - .set_open(true) - .write_to_repo(mut_repo); + let new_checkout = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo); mut_repo.edit(ws_id, &new_checkout).unwrap(); mut_repo.rebase_descendants(&settings).unwrap(); assert!(mut_repo.view().heads().contains(old_checkout.id())); @@ -121,9 +113,7 @@ fn test_checkout_previous_empty(use_git: bool) { let mut tx = repo.start_transaction("test"); let mut_repo = tx.mut_repo(); - let new_wc_commit = testutils::create_random_commit(&settings, &repo) - .set_open(true) - .write_to_repo(mut_repo); + let new_wc_commit = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo); mut_repo.edit(ws_id, &new_wc_commit).unwrap(); mut_repo.rebase_descendants(&settings).unwrap(); assert!(!mut_repo.view().heads().contains(old_checkout.id())); @@ -153,9 +143,7 @@ fn test_checkout_previous_empty_with_description(use_git: bool) { let mut tx = repo.start_transaction("test"); let mut_repo = tx.mut_repo(); - let new_checkout = testutils::create_random_commit(&settings, &repo) - .set_open(true) - .write_to_repo(mut_repo); + let new_checkout = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo); mut_repo.edit(ws_id, &new_checkout).unwrap(); mut_repo.rebase_descendants(&settings).unwrap(); assert!(mut_repo.view().heads().contains(old_checkout.id())); @@ -190,9 +178,7 @@ fn test_checkout_previous_empty_non_head(use_git: bool) { let mut tx = repo.start_transaction("test"); let mut_repo = tx.mut_repo(); - let new_checkout = testutils::create_random_commit(&settings, &repo) - .set_open(true) - .write_to_repo(mut_repo); + let new_checkout = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo); mut_repo.edit(ws_id, &new_checkout).unwrap(); mut_repo.rebase_descendants(&settings).unwrap(); assert_eq!( @@ -211,9 +197,7 @@ fn test_edit_initial(use_git: bool) { let repo = &test_repo.repo; let mut tx = repo.start_transaction("test"); - let checkout = testutils::create_random_commit(&settings, repo) - .set_open(true) - .write_to_repo(tx.mut_repo()); + let checkout = testutils::create_random_commit(&settings, repo).write_to_repo(tx.mut_repo()); let repo = tx.commit(); let mut tx = repo.start_transaction("test"); diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index 8bd8df14f..7e66a5530 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -1275,7 +1275,6 @@ fn test_rebase_descendants_update_checkout(use_git: bool) { let commit_a = create_random_commit(&settings, repo).write_to_repo(tx.mut_repo()); let commit_b = create_random_commit(&settings, repo) .set_parents(vec![commit_a.id().clone()]) - .set_open(true) .write_to_repo(tx.mut_repo()); let ws1_id = WorkspaceId::new("ws1".to_string()); let ws2_id = WorkspaceId::new("ws2".to_string()); @@ -1312,7 +1311,7 @@ fn test_rebase_descendants_update_checkout_abandoned(use_git: bool) { let test_repo = TestRepo::init(use_git); let repo = &test_repo.repo; - // Checked-out, open commit B was abandoned. A child of A + // Checked-out commit B was abandoned. A child of A // should become checked out. // // B @@ -1322,7 +1321,6 @@ fn test_rebase_descendants_update_checkout_abandoned(use_git: bool) { let commit_a = create_random_commit(&settings, repo).write_to_repo(tx.mut_repo()); let commit_b = create_random_commit(&settings, repo) .set_parents(vec![commit_a.id().clone()]) - .set_open(true) .write_to_repo(tx.mut_repo()); let ws1_id = WorkspaceId::new("ws1".to_string()); let ws2_id = WorkspaceId::new("ws2".to_string()); @@ -1353,7 +1351,6 @@ fn test_rebase_descendants_update_checkout_abandoned(use_git: bool) { .store() .get_commit(repo.view().get_wc_commit_id(&ws1_id).unwrap()) .unwrap(); - assert!(checkout.is_open()); assert_eq!(checkout.parent_ids(), vec![commit_a.id().clone()]); assert_eq!(repo.view().get_wc_commit_id(&ws3_id), Some(commit_a.id())); } @@ -1365,7 +1362,7 @@ fn test_rebase_descendants_update_checkout_abandoned_merge(use_git: bool) { let test_repo = TestRepo::init(use_git); let repo = &test_repo.repo; - // Checked-out, open merge commit D was abandoned. A parent commit should become + // Checked-out merge commit D was abandoned. A parent commit should become // checked out. // // D @@ -1383,7 +1380,6 @@ fn test_rebase_descendants_update_checkout_abandoned_merge(use_git: bool) { .write_to_repo(tx.mut_repo()); let commit_d = create_random_commit(&settings, repo) .set_parents(vec![commit_b.id().clone(), commit_c.id().clone()]) - .set_open(true) .write_to_repo(tx.mut_repo()); let workspace_id = WorkspaceId::default(); tx.mut_repo() @@ -1398,6 +1394,5 @@ fn test_rebase_descendants_update_checkout_abandoned_merge(use_git: bool) { let new_checkout_id = repo.view().get_wc_commit_id(&workspace_id).unwrap(); let checkout = repo.store().get_commit(new_checkout_id).unwrap(); - assert!(checkout.is_open()); assert_eq!(checkout.parent_ids(), vec![commit_b.id().clone()]); } diff --git a/src/commands.rs b/src/commands.rs index c779298e0..9af972221 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -2374,8 +2374,7 @@ fn cmd_commit(ui: &mut Ui, command: &CommandHelper, args: &CommitArgs) -> Result .ok_or_else(|| UserError("This command requires a working copy".to_string()))?; let commit = workspace_command.repo().store().get_commit(commit_id)?; - let mut commit_builder = - CommitBuilder::for_rewrite_from(ui.settings(), &commit).set_open(false); + let mut commit_builder = CommitBuilder::for_rewrite_from(ui.settings(), &commit); let description = if let Some(message) = &args.message { message.to_string() } else { @@ -2505,7 +2504,6 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C let new_commit = CommitBuilder::for_new_commit(ui.settings(), parent_ids, merged_tree.id().clone()) .set_description(args.message.clone()) - .set_open(true) .write_to_repo(tx.mut_repo()); let workspace_id = workspace_command.workspace_id(); tx.mut_repo().edit(workspace_id, &new_commit).unwrap();