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

cleanup: use ReadonlyRepo returned from Transaction::commit()

Browse source 
I thought I had looked for this case and cleaned up all the places
when I made `Transaction::commit()` return a new `ReadonlyRepo`. I
must have forgotten to do that, because there we tons of places to
clean up left.
This commit is contained in:
Martin von Zweigbergk 2021-05-19 13:39:03 -07:00
parent ecc2a6b968
commit 6809a88d42
5 changed files with 47 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
lib/tests/test_git.rs
View file

@ -104,7 +104,7 @@ fn test_import_refs() {
#[test]
fn test_import_refs_reimport() {
let settings = testutils::user_settings();
let (_temp_dir, mut repo) = testutils::init_repo(&settings, true);
let (_temp_dir, repo) = testutils::init_repo(&settings, true);
let git_repo = repo.store().git_repo().unwrap();
let commit1 = empty_git_commit(&git_repo, "refs/heads/main", &[]);
@ -119,14 +119,13 @@ fn test_import_refs_reimport() {
let heads_before = repo.view().heads().clone();
let mut tx = repo.start_transaction("test");
jujutsu_lib::git::import_refs(tx.mut_repo(), &git_repo).unwrap_or_default();
tx.commit();
let repo = tx.commit();
// Delete feature1 and rewrite feature2
delete_git_ref(&git_repo, "refs/heads/feature1");
delete_git_ref(&git_repo, "refs/heads/feature2");
let commit5 = empty_git_commit(&git_repo, "refs/heads/feature2", &[&commit2]);
repo = repo.reload().unwrap();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
jujutsu_lib::git::import_refs(mut_repo, &git_repo).unwrap_or_default();
@ -165,7 +164,7 @@ fn delete_git_ref(git_repo: &git2::Repository, name: &str) {
#[test]
fn test_import_refs_merge() {
let settings = testutils::user_settings();
let (_temp_dir, mut repo) = testutils::init_repo(&settings, true);
let (_temp_dir, repo) = testutils::init_repo(&settings, true);
let git_repo = repo.store().git_repo().unwrap();
// Set up the following refs and update them as follows:
@ -202,8 +201,7 @@ fn test_import_refs_merge() {
git_ref(&git_repo, "refs/heads/remove-forward", commit1.id());
let mut tx = repo.start_transaction("initial import");
jujutsu_lib::git::import_refs(tx.mut_repo(), &git_repo).unwrap_or_default();
tx.commit();
repo = repo.reload().unwrap();
let repo = tx.commit();
// One of the concurrent operations:
git_ref(&git_repo, "refs/heads/sideways-unchanged", commit4.id());
@ -230,7 +228,7 @@ fn test_import_refs_merge() {
tx2.commit();
// Reload the repo, causing the operations to be merged.
repo = repo.reload().unwrap();
let repo = repo.reload().unwrap();
let view = repo.view();
let git_refs = view.git_refs();

21
lib/tests/test_index.rs
View file

@ -66,7 +66,7 @@ fn test_index_commits_empty_repo(use_git: bool) {
#[test_case(true ; "git store")]
fn test_index_commits_standard_cases(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, mut repo) = testutils::init_repo(&settings, use_git);
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
// o H
// o | G
@ -94,8 +94,7 @@ fn test_index_commits_standard_cases(use_git: bool) {
let commit_f = graph_builder.commit_with_parents(&[&commit_b, &commit_e]);
let commit_g = graph_builder.commit_with_parents(&[&commit_f]);
let commit_h = graph_builder.commit_with_parents(&[&commit_e]);
tx.commit();
repo = repo.reload().unwrap();
let repo = tx.commit();
let index = repo.index();
// There should be the root commit and the working copy commit, plus
@ -137,7 +136,7 @@ fn test_index_commits_standard_cases(use_git: bool) {
#[test_case(true ; "git store")]
fn test_index_commits_criss_cross(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, mut repo) = testutils::init_repo(&settings, use_git);
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
let num_generations = 50;
@ -156,8 +155,7 @@ fn test_index_commits_criss_cross(use_git: bool) {
left_commits.push(new_left);
right_commits.push(new_right);
}
tx.commit();
repo = repo.reload().unwrap();
let repo = tx.commit();
let index = repo.index();
// There should the root commit and the working copy commit, plus 2 for each
@ -238,7 +236,7 @@ fn test_index_commits_criss_cross(use_git: bool) {
fn test_index_commits_previous_operations(use_git: bool) {
// Test that commits visible only in previous operations are indexed.
let settings = testutils::user_settings();
let (_temp_dir, mut repo) = testutils::init_repo(&settings, use_git);
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
// Remove commit B and C in one operation and make sure they're still
// visible in the index after that operation.
@ -254,13 +252,11 @@ fn test_index_commits_previous_operations(use_git: bool) {
let commit_a = graph_builder.initial_commit();
let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
let commit_c = graph_builder.commit_with_parents(&[&commit_b]);
tx.commit();
repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
tx.mut_repo().remove_head(&commit_c);
tx.commit();
repo = repo.reload().unwrap();
let repo = tx.commit();
// Delete index from disk
let index_operations_dir = repo
@ -418,8 +414,7 @@ fn create_n_commits(
for _ in 0..num_commits {
create_random_commit(settings, repo).write_to_repo(tx.mut_repo());
}
tx.commit();
repo.reload().unwrap()
tx.commit()
}
fn commits_by_level(repo: &ReadonlyRepo) -> Vec<u32> {

7
lib/tests/test_load_repo.rs
View file

@ -45,12 +45,11 @@ fn test_load_from_subdir(use_git: bool) {
#[test_case(true ; "git store")]
fn test_load_at_operation(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, mut repo) = testutils::init_repo(&settings, use_git);
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
let mut tx = repo.start_transaction("add commit");
let commit = testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
let op = tx.commit().operation().clone();
repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("remove commit");
tx.mut_repo().remove_head(&commit);
@ -65,6 +64,6 @@ fn test_load_at_operation(use_git: bool) {
// If we load the repo at the previous operation, we should see the commit since
// it has not been removed yet
let loader = RepoLoader::init(&settings, repo.working_copy_path().clone()).unwrap();
let old_repo = loader.load_at(&op).unwrap();
let old_repo = loader.load_at(&repo.operation()).unwrap();
assert!(old_repo.view().heads().contains(commit.id()));
}

77
lib/tests/test_mut_repo.rs
View file

@ -36,14 +36,12 @@ fn test_checkout_open(use_git: bool) {
let requested_checkout = testutils::create_random_commit(&settings, &repo)
.set_open(true)
.write_to_repo(tx.mut_repo());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout);
assert_eq!(actual_checkout.id(), requested_checkout.id());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert_eq!(repo.view().checkout(), actual_checkout.id());
}
@ -59,16 +57,14 @@ fn test_checkout_closed(use_git: bool) {
let requested_checkout = testutils::create_random_commit(&settings, &repo)
.set_open(false)
.write_to_repo(tx.mut_repo());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout);
assert_eq!(actual_checkout.tree().id(), requested_checkout.tree().id());
assert_eq!(actual_checkout.parents().len(), 1);
assert_eq!(actual_checkout.parents()[0].id(), requested_checkout.id());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert_eq!(repo.view().checkout(), actual_checkout.id());
}
@ -93,8 +89,7 @@ fn test_checkout_open_with_conflict(use_git: bool) {
let requested_checkout = CommitBuilder::for_new_commit(&settings, store, tree_id)
.set_open(true)
.write_to_repo(tx.mut_repo());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout);
@ -108,8 +103,7 @@ fn test_checkout_open_with_conflict(use_git: bool) {
}
assert_eq!(actual_checkout.parents().len(), 1);
assert_eq!(actual_checkout.parents()[0].id(), requested_checkout.id());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert_eq!(repo.view().checkout(), actual_checkout.id());
}
@ -134,8 +128,7 @@ fn test_checkout_closed_with_conflict(use_git: bool) {
let requested_checkout = CommitBuilder::for_new_commit(&settings, store, tree_id)
.set_open(false)
.write_to_repo(tx.mut_repo());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout);
@ -149,8 +142,7 @@ fn test_checkout_closed_with_conflict(use_git: bool) {
}
assert_eq!(actual_checkout.parents().len(), 1);
assert_eq!(actual_checkout.parents()[0].id(), requested_checkout.id());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert_eq!(repo.view().checkout(), actual_checkout.id());
}
@ -197,8 +189,7 @@ fn test_checkout_previous_not_empty(use_git: bool) {
.set_open(true)
.write_to_repo(mut_repo);
mut_repo.check_out(&settings, &old_checkout);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
@ -228,8 +219,7 @@ fn test_checkout_previous_empty(use_git: bool) {
)
.write_to_repo(mut_repo);
mut_repo.check_out(&settings, &old_checkout);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
@ -261,8 +251,7 @@ fn test_checkout_previous_empty_and_obsolete(use_git: bool) {
let successor = CommitBuilder::for_rewrite_from(&settings, repo.store(), &old_checkout)
.write_to_repo(mut_repo);
mut_repo.check_out(&settings, &old_checkout);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
@ -291,8 +280,7 @@ fn test_checkout_previous_empty_and_pruned(use_git: bool) {
.set_pruned(true)
.write_to_repo(mut_repo);
mut_repo.check_out(&settings, &old_checkout);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
@ -332,8 +320,7 @@ fn test_add_head_success(use_git: bool) {
mut_repo.add_head(&new_commit);
assert!(mut_repo.view().heads().contains(new_commit.id()));
assert!(mut_repo.index().has_id(new_commit.id()));
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert!(repo.view().heads().contains(new_commit.id()));
assert!(repo.index().has_id(new_commit.id()));
let index_stats = repo.index().stats();
@ -355,8 +342,7 @@ fn test_add_head_ancestor(use_git: bool) {
let commit1 = graph_builder.initial_commit();
let commit2 = graph_builder.commit_with_parents(&[&commit1]);
let _commit3 = graph_builder.commit_with_parents(&[&commit2]);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let index_stats = repo.index().stats();
assert_eq!(index_stats.num_heads, 2);
@ -383,8 +369,7 @@ fn test_add_head_not_immediate_child(use_git: bool) {
let mut tx = repo.start_transaction("test");
let initial = testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
// Create some commit outside of the repo by using a temporary transaction. Then
// add one of them as a head.
@ -437,8 +422,7 @@ fn test_remove_head(use_git: bool) {
let commit1 = graph_builder.initial_commit();
let commit2 = graph_builder.commit_with_parents(&[&commit1]);
let commit3 = graph_builder.commit_with_parents(&[&commit2]);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
@ -451,9 +435,7 @@ fn test_remove_head(use_git: bool) {
assert!(mut_repo.index().has_id(commit1.id()));
assert!(mut_repo.index().has_id(commit2.id()));
assert!(mut_repo.index().has_id(commit3.id()));
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let heads = repo.view().heads().clone();
assert!(!heads.contains(commit3.id()));
assert!(!heads.contains(commit2.id()));
@ -478,8 +460,7 @@ fn test_remove_head_ancestor_git_ref(use_git: bool) {
let commit3 = graph_builder.commit_with_parents(&[&commit2]);
tx.mut_repo()
.insert_git_ref("refs/heads/main".to_string(), commit1.id().clone());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
@ -490,9 +471,7 @@ fn test_remove_head_ancestor_git_ref(use_git: bool) {
assert!(!heads.contains(commit3.id()));
assert!(!heads.contains(commit2.id()));
assert!(heads.contains(commit1.id()));
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let heads = repo.view().heads().clone();
assert!(!heads.contains(commit3.id()));
assert!(!heads.contains(commit2.id()));
@ -509,16 +488,14 @@ fn test_add_public_head(use_git: bool) {
let mut tx = repo.start_transaction("test");
let commit1 = testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
assert!(!mut_repo.view().public_heads().contains(commit1.id()));
mut_repo.add_public_head(&commit1);
assert!(mut_repo.view().public_heads().contains(commit1.id()));
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert!(repo.view().public_heads().contains(commit1.id()));
}
@ -535,16 +512,14 @@ fn test_add_public_head_ancestor(use_git: bool) {
let commit1 = graph_builder.initial_commit();
let commit2 = graph_builder.commit_with_parents(&[&commit1]);
tx.mut_repo().add_public_head(&commit2);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
assert!(!mut_repo.view().public_heads().contains(commit1.id()));
mut_repo.add_public_head(&commit1);
assert!(!mut_repo.view().public_heads().contains(commit1.id()));
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert!(!repo.view().public_heads().contains(commit1.id()));
}
@ -560,15 +535,13 @@ fn test_remove_public_head(use_git: bool) {
let mut_repo = tx.mut_repo();
let commit1 = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo);
mut_repo.add_public_head(&commit1);
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
assert!(mut_repo.view().public_heads().contains(commit1.id()));
mut_repo.remove_public_head(&commit1);
assert!(!mut_repo.view().public_heads().contains(commit1.id()));
tx.commit();
let repo = repo.reload().unwrap();
let repo = tx.commit();
assert!(!repo.view().public_heads().contains(commit1.id()));
}

13
src/commands.rs
View file

@ -771,21 +771,20 @@ fn cmd_init(
fs::create_dir(&wc_path).unwrap();
}
let repo;
if let Some(git_store_str) = sub_matches.value_of("git-store") {
let repo = if let Some(git_store_str) = sub_matches.value_of("git-store") {
let git_store_path = ui.cwd().join(git_store_str);
repo = ReadonlyRepo::init_external_git(ui.settings(), wc_path, git_store_path);
let repo = ReadonlyRepo::init_external_git(ui.settings(), wc_path, git_store_path);
let git_repo = repo.store().git_repo().unwrap();
let mut tx = repo.start_transaction("import git refs");
git::import_refs(tx.mut_repo(), &git_repo).unwrap();
// TODO: Check out a recent commit. Maybe one with the highest generation
// number.
tx.commit();
tx.commit()
} else if sub_matches.is_present("git") {
repo = ReadonlyRepo::init_internal_git(ui.settings(), wc_path);
ReadonlyRepo::init_internal_git(ui.settings(), wc_path)
} else {
repo = ReadonlyRepo::init_local(ui.settings(), wc_path);
}
ReadonlyRepo::init_local(ui.settings(), wc_path)
};
writeln!(
ui,
"Initialized repo in \"{}\"",