diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index 5759f6a38..425e5e2b4 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -29,7 +29,6 @@ pub struct Transaction { description: String, start_time: Timestamp, tags: HashMap, - closed: bool, } impl Transaction { @@ -41,7 +40,6 @@ impl Transaction { description: description.to_owned(), start_time: Timestamp::now(), tags: Default::default(), - closed: false, } } @@ -94,21 +92,8 @@ impl Transaction { .index_store() .associate_file_with_operation(&index, operation.id()) .unwrap(); - self.closed = true; UnpublishedOperation::new(base_repo.loader(), operation, view, index) } - - pub fn discard(mut self) { - self.closed = true; - } -} - -impl Drop for Transaction { - fn drop(&mut self) { - if !std::thread::panicking() { - debug_assert!(self.closed, "Transaction was dropped without being closed."); - } - } } struct NewRepoData { diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index d25694adf..ac82db510 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -171,7 +171,6 @@ fn test_commit_builder_descendants(use_git: bool) { .write_to_repo(tx.mut_repo()); let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); assert!(rebaser.rebase_next().is_none()); - tx.discard(); // Test with for_open_commit() let mut tx = repo.start_transaction("test"); @@ -184,7 +183,6 @@ fn test_commit_builder_descendants(use_git: bool) { .write_to_repo(tx.mut_repo()); let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); assert!(rebaser.rebase_next().is_none()); - tx.discard(); // Test with for_rewrite_from() let mut tx = repo.start_transaction("test"); @@ -193,7 +191,6 @@ fn test_commit_builder_descendants(use_git: bool) { let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); assert_rebased(rebaser.rebase_next(), &commit3, &[&commit4]); assert!(rebaser.rebase_next().is_none()); - tx.discard(); // Test with for_rewrite_from() but new change id let mut tx = repo.start_transaction("test"); @@ -202,5 +199,4 @@ fn test_commit_builder_descendants(use_git: bool) { .write_to_repo(tx.mut_repo()); let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); assert!(rebaser.rebase_next().is_none()); - tx.discard(); } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 6ee2be60f..1de0cbbdd 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -352,8 +352,6 @@ fn test_fetch_prune_deleted_ref() { .unwrap(); git::fetch(tx.mut_repo(), &clone_git_repo, "origin").unwrap(); assert!(tx.mut_repo().get_branch("main").is_none()); - - tx.discard(); } #[test] @@ -382,7 +380,6 @@ fn test_fetch_no_default_branch() { let default_branch = git::fetch(tx.mut_repo(), &clone_git_repo, "origin").unwrap(); // There is no default branch assert_eq!(default_branch, None); - tx.discard(); } #[test] @@ -398,7 +395,6 @@ fn test_fetch_no_such_remote() { let mut tx = jj_repo.start_transaction("test"); let result = git::fetch(tx.mut_repo(), &git_repo, "invalid-remote"); assert!(matches!(result, Err(GitFetchError::NoSuchRemote(_)))); - tx.discard(); } struct PushTestSetup { diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index ca628277f..bbd980939 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -396,7 +396,6 @@ fn test_index_commits_incremental_already_indexed(use_git: bool) { let mut_repo = tx.mut_repo(); mut_repo.add_head(&commit_a); assert_eq!(mut_repo.index().num_commits(), 2 + 1); - tx.discard(); } #[must_use] diff --git a/lib/tests/test_init.rs b/lib/tests/test_init.rs index c566a28f6..a86b94800 100644 --- a/lib/tests/test_init.rs +++ b/lib/tests/test_init.rs @@ -30,7 +30,6 @@ fn test_init_local() { // Just test that we can write a commit to the store let mut tx = repo.start_transaction("test"); testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo()); - tx.discard(); } #[test] @@ -46,7 +45,6 @@ fn test_init_internal_git() { // Just test that we ca write a commit to the store let mut tx = repo.start_transaction("test"); testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo()); - tx.discard(); } #[test] @@ -66,7 +64,6 @@ fn test_init_external_git() { // Just test that we can write a commit to the store let mut tx = repo.start_transaction("test"); testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo()); - tx.discard(); } #[test_case(false ; "local backend")] diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index b8520bac0..4d30bb9f4 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -90,7 +90,6 @@ fn test_checkout_previous_not_empty(use_git: bool) { mut_repo.check_out(&settings, &new_checkout); mut_repo.create_descendant_rebaser(&settings).rebase_all(); assert!(mut_repo.view().heads().contains(old_checkout.id())); - tx.discard(); } #[test_case(false ; "local backend")] @@ -122,7 +121,6 @@ fn test_checkout_previous_empty(use_git: bool) { mut_repo.check_out(&settings, &new_checkout); mut_repo.create_descendant_rebaser(&settings).rebase_all(); assert!(!mut_repo.view().heads().contains(old_checkout.id())); - tx.discard(); } #[test_case(false ; "local backend")] @@ -138,7 +136,7 @@ fn test_add_head_success(use_git: bool) { // add that as a head. let mut tx = repo.start_transaction("test"); let new_commit = testutils::create_random_commit(&settings, repo).write_to_repo(tx.mut_repo()); - tx.discard(); + drop(tx); let index_stats = repo.index().stats(); assert_eq!(index_stats.num_heads, 1); @@ -188,7 +186,6 @@ fn test_add_head_ancestor(use_git: bool) { assert_eq!(index_stats.num_heads, 2); assert_eq!(index_stats.num_commits, 5); assert_eq!(index_stats.max_generation_number, 3); - tx.discard(); } #[test_case(false ; "local backend")] @@ -214,7 +211,7 @@ fn test_add_head_not_immediate_child(use_git: bool) { let child = testutils::create_random_commit(&settings, &repo) .set_parents(vec![rewritten.id().clone()]) .write_to_repo(tx.mut_repo()); - tx.discard(); + drop(tx); let index_stats = repo.index().stats(); assert_eq!(index_stats.num_heads, 2); @@ -233,7 +230,6 @@ fn test_add_head_not_immediate_child(use_git: bool) { assert_eq!(index_stats.num_heads, 3); assert_eq!(index_stats.num_commits, 5); assert_eq!(index_stats.max_generation_number, 2); - tx.discard(); } #[test_case(false ; "local backend")] @@ -385,7 +381,6 @@ fn test_rebase_descendants_simple(use_git: bool) { .create_descendant_rebaser(&settings) .rebase_next() .is_none()); - tx.discard(); } #[test_case(false ; "local backend")] @@ -420,5 +415,4 @@ fn test_rebase_descendants_conflicting_rewrite(use_git: bool) { .create_descendant_rebaser(&settings) .rebase_next() .is_none()); - tx.discard(); } diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index b96e236ef..0d3ad381a 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -258,8 +258,6 @@ fn test_resolve_symbol_checkout(use_git: bool) { resolve_symbol(mut_repo.as_repo_ref(), "@"), Ok(vec![commit2.id().clone()]) ); - - tx.discard(); } #[test] @@ -382,8 +380,6 @@ fn test_resolve_symbol_git_refs() { resolve_symbol(mut_repo.as_repo_ref(), "refs/heads/conflicted"), Ok(vec![commit1.id().clone(), commit3.id().clone()]) ); - - tx.discard(); } fn resolve_commit_ids(repo: RepoRef, revset_str: &str) -> Vec { @@ -421,8 +417,6 @@ fn test_evaluate_expression_root_and_checkout(use_git: bool) { resolve_commit_ids(mut_repo.as_repo_ref(), "@"), vec![commit1.id().clone()] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -485,8 +479,6 @@ fn test_evaluate_expression_heads_of(use_git: bool) { resolve_commit_ids(mut_repo.as_repo_ref(), "heads(all())"), resolve_commit_ids(mut_repo.as_repo_ref(), "heads()") ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -548,8 +540,6 @@ fn test_evaluate_expression_parents(use_git: bool) { ), vec![commit3.id().clone(), commit2.id().clone()] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -605,8 +595,6 @@ fn test_evaluate_expression_children(use_git: bool) { ), vec![commit5.id().clone()] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -643,8 +631,6 @@ fn test_evaluate_expression_ancestors(use_git: bool) { root_commit.id().clone(), ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -708,8 +694,6 @@ fn test_evaluate_expression_range(use_git: bool) { ), vec![commit3.id().clone()] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -780,8 +764,6 @@ fn test_evaluate_expression_dag_range(use_git: bool) { commit2.id().clone(), ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -833,8 +815,6 @@ fn test_evaluate_expression_descendants(use_git: bool) { commit2.id().clone(), ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -876,8 +856,6 @@ fn test_evaluate_expression_all(use_git: bool) { root_commit_id, ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -898,8 +876,6 @@ fn test_evaluate_expression_heads(use_git: bool) { resolve_commit_ids(mut_repo.as_repo_ref(), "heads()"), vec![commit2.id().clone(), checkout_id] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -933,8 +909,6 @@ fn test_evaluate_expression_public_heads(use_git: bool) { resolve_commit_ids(mut_repo.as_repo_ref(), "public_heads()"), vec![commit2.id().clone(), commit1.id().clone()] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1004,8 +978,6 @@ fn test_evaluate_expression_git_refs(use_git: bool) { commit2.id().clone() ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1075,8 +1047,6 @@ fn test_evaluate_expression_branches(use_git: bool) { commit2.id().clone() ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1157,8 +1127,6 @@ fn test_evaluate_expression_remote_branches(use_git: bool) { commit2.id().clone() ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1190,8 +1158,6 @@ fn test_evaluate_expression_merges(use_git: bool) { ), vec![commit5.id().clone()] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1235,8 +1201,6 @@ fn test_evaluate_expression_description(use_git: bool) { resolve_commit_ids(mut_repo.as_repo_ref(), "description(\"commit 2\",heads())"), vec![] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1310,8 +1274,6 @@ fn test_evaluate_expression_union(use_git: bool) { commit3.id().clone() ] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1352,8 +1314,6 @@ fn test_evaluate_expression_intersection(use_git: bool) { ), vec![] ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -1428,6 +1388,4 @@ fn test_evaluate_expression_difference(use_git: bool) { ), vec![commit4.id().clone()] ); - - tx.discard(); } diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index b8d37f3c2..2f4ebe3d8 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -68,8 +68,6 @@ fn test_rebase_descendants_sideways(use_git: bool) { new_commit_e.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -123,8 +121,6 @@ fn test_rebase_descendants_forward(use_git: bool) { new_commit_e.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -163,8 +159,6 @@ fn test_rebase_descendants_backward(use_git: bool) { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), new_commit_d.id().clone()} ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -215,8 +209,6 @@ fn test_rebase_descendants_internal_merge(use_git: bool) { *tx.mut_repo().view().heads(), hashset! { repo.view().checkout().clone(), new_commit_e.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -262,8 +254,6 @@ fn test_rebase_descendants_external_merge(use_git: bool) { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), new_commit_e.id().clone()} ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -311,8 +301,6 @@ fn test_rebase_descendants_abandon(use_git: bool) { new_commit_f.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -349,8 +337,6 @@ fn test_rebase_descendants_abandon_no_descendants(use_git: bool) { commit_a.id().clone(), } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -390,8 +376,6 @@ fn test_rebase_descendants_abandon_and_replace(use_git: bool) { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), new_commit_d.id().clone()} ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -430,8 +414,6 @@ fn test_rebase_descendants_abandon_degenerate_merge(use_git: bool) { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), new_commit_d.id().clone()} ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -478,8 +460,6 @@ fn test_rebase_descendants_abandon_widen_merge(use_git: bool) { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), new_commit_f.id().clone()} ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -528,8 +508,6 @@ fn test_rebase_descendants_multiple_sideways(use_git: bool) { new_commit_e.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -576,8 +554,6 @@ fn test_rebase_descendants_multiple_swap(use_git: bool) { new_commit_e.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -618,8 +594,6 @@ fn test_rebase_descendants_multiple_no_descendants(use_git: bool) { commit_c.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -677,8 +651,6 @@ fn test_rebase_descendants_multiple_forward_and_backward(use_git: bool) { new_commit_h.id().clone() } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -748,8 +720,6 @@ fn test_rebase_descendants_divergent_rewrite(use_git: bool) { new_commit_g.id().clone(), } ); - - tx.discard(); } #[test_case(false ; "local backend")] @@ -816,8 +786,6 @@ fn test_rebase_descendants_contents(use_git: bool) { new_commit_c.tree().path_value(&path2), commit_b.tree().path_value(&path2) ); - - tx.discard(); } #[test] @@ -855,8 +823,6 @@ fn test_rebase_descendants_basic_branch_update() { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), commit_b2.id().clone()} ); - - tx.discard(); } #[test] @@ -914,8 +880,6 @@ fn test_rebase_descendants_basic_branch_update_with_non_local_branch() { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), commit_b2.id().clone()} ); - - tx.discard(); } #[test] @@ -952,8 +916,6 @@ fn test_rebase_descendants_update_branch_after_abandon() { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), commit_a.id().clone()} ); - - tx.discard(); } #[test] @@ -1013,8 +975,6 @@ fn test_rebase_descendants_update_branches_after_divergent_rewrite() { commit_b4.id().clone(), } ); - - tx.discard(); } #[test] @@ -1084,8 +1044,6 @@ fn test_rebase_descendants_rewrite_updates_branch_conflict() { commit_c.id().clone(), } ); - - tx.discard(); } #[test] @@ -1131,8 +1089,6 @@ fn test_rebase_descendants_rewrite_resolves_branch_conflict() { *tx.mut_repo().view().heads(), hashset! {repo.view().checkout().clone(), commit_b2.id().clone()} ); - - tx.discard(); } // TODO: Add a test for the following case, which can't happen with our current diff --git a/src/commands.rs b/src/commands.rs index 68c29d30d..2f6709de2 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -451,7 +451,6 @@ impl WorkspaceCommandHelper { fn finish_transaction(&mut self, ui: &mut Ui, mut tx: Transaction) -> Result<(), CommandError> { let mut_repo = tx.mut_repo(); if !mut_repo.has_changes() { - tx.discard(); writeln!(ui, "Nothing changed.")?; return Ok(()); } @@ -1406,8 +1405,6 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &ArgMatches) -> Result<( // number. if tx.mut_repo().has_changes() { workspace_command.finish_transaction(ui, tx)?; - } else { - tx.discard(); } } else if args.is_present("git") { Workspace::init_internal_git(ui.settings(), wc_path.clone())?;