repo: don't materialize conflicts on checkout

Since the working copy can now handle conflicts, we don't need to
materialize conflicts when checking out a commit.

Before this patch, we used to create a new commit on top whenever we
checked out a commit with conflicts. That new commit was intended just
for resolving the conflicts. The typical workflow was the resolve the
conflicts and then amend. To use the same workflow after this patch,
one needs to explicitly create a new commit on top with `jj new` after
checking out a commit with conflict.
This commit is contained in:
Martin von Zweigbergk 2021-11-03 22:54:59 -07:00 committed by Martin von Zweigbergk
parent ea82340654
commit e901a4e66b
2 changed files with 5 additions and 131 deletions

View file

@ -38,7 +38,7 @@ use crate::store::Store;
use crate::transaction::Transaction;
use crate::view::{RefName, View};
use crate::working_copy::WorkingCopy;
use crate::{backend, conflicts, op_store};
use crate::{backend, op_store};
#[derive(Debug, Error, PartialEq, Eq)]
pub enum RepoError {
@ -601,34 +601,20 @@ impl MutableRepo {
assert!(current_checkout.is_open(), "current checkout is closed");
if current_checkout.is_empty() {
// Abandon the checkout we're leaving if it's empty.
// TODO: Also abandon it if the only changes are conflicts that got
// materialized.
self.record_abandoned_commit(current_checkout_id);
}
let store = self.store();
// Create a new tree with any conflicts resolved.
let mut tree_builder = store.tree_builder(commit.tree().id().clone());
for (path, conflict_id) in commit.tree().conflicts() {
let conflict = store.read_conflict(&conflict_id).unwrap();
let materialized_value =
conflicts::conflict_to_materialized_value(store, &path, &conflict);
tree_builder.set(path, materialized_value);
}
let tree_id = tree_builder.write_tree();
let open_commit;
if !commit.is_open() || &tree_id != commit.tree().id() {
// If the commit is closed, or if it had conflicts, create a new open commit on
// top
if !commit.is_open() {
// If the commit is closed, create a new open commit on top
open_commit = CommitBuilder::for_open_commit(
settings,
self.store(),
commit.id().clone(),
tree_id,
commit.tree().id().clone(),
)
.write_to_repo(self);
} else {
// Otherwise the commit was open and didn't have any conflicts, so just use
// that commit as is.
// Otherwise the commit was open, so just use that commit as is.
open_commit = commit.clone();
}
let id = open_commit.id().clone();

View file

@ -12,12 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
use std::sync::Arc;
use jujutsu_lib::backend::{Conflict, ConflictId, ConflictPart, TreeValue};
use jujutsu_lib::commit_builder::CommitBuilder;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::store::Store;
use jujutsu_lib::testutils;
use jujutsu_lib::testutils::{assert_rebased, CommitGraphBuilder};
use test_case::test_case;
@ -68,113 +63,6 @@ fn test_checkout_closed(use_git: bool) {
assert_eq!(repo.view().checkout(), actual_checkout.id());
}
#[test_case(false ; "local backend")]
// #[test_case(true ; "git backend")]
fn test_checkout_open_with_conflict(use_git: bool) {
// Test that MutableRepo::check_out() creates a child if the requested
// commit is open and has conflicts
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
let store = repo.store();
let file_path = RepoPath::from_internal_string("file");
let conflict_id = write_conflict(store, &file_path);
let mut tree_builder = repo
.store()
.tree_builder(repo.store().empty_tree_id().clone());
tree_builder.set(file_path.clone(), TreeValue::Conflict(conflict_id));
let tree_id = tree_builder.write_tree();
let mut tx = repo.start_transaction("test");
let requested_checkout = CommitBuilder::for_new_commit(&settings, store, tree_id)
.set_open(true)
.write_to_repo(tx.mut_repo());
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout);
let file_value = actual_checkout.tree().path_value(&file_path);
match file_value {
Some(TreeValue::Normal {
id: _,
executable: false,
}) => {}
_ => panic!("unexpected tree value: {:?}", file_value),
}
assert_eq!(actual_checkout.parents().len(), 1);
assert_eq!(actual_checkout.parents()[0].id(), requested_checkout.id());
let repo = tx.commit();
assert_eq!(repo.view().checkout(), actual_checkout.id());
}
#[test_case(false ; "local backend")]
// #[test_case(true ; "git backend")]
fn test_checkout_closed_with_conflict(use_git: bool) {
// Test that MutableRepo::check_out() creates a child if the requested commit is
// closed and has conflicts
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
let store = repo.store();
let file_path = RepoPath::from_internal_string("file");
let conflict_id = write_conflict(store, &file_path);
let mut tree_builder = repo
.store()
.tree_builder(repo.store().empty_tree_id().clone());
tree_builder.set(file_path.clone(), TreeValue::Conflict(conflict_id));
let tree_id = tree_builder.write_tree();
let mut tx = repo.start_transaction("test");
let requested_checkout = CommitBuilder::for_new_commit(&settings, store, tree_id)
.set_open(false)
.write_to_repo(tx.mut_repo());
let repo = tx.commit();
let mut tx = repo.start_transaction("test");
let actual_checkout = tx.mut_repo().check_out(&settings, &requested_checkout);
let file_value = actual_checkout.tree().path_value(&file_path);
match file_value {
Some(TreeValue::Normal {
id: _,
executable: false,
}) => {}
_ => panic!("unexpected tree value: {:?}", file_value),
}
assert_eq!(actual_checkout.parents().len(), 1);
assert_eq!(actual_checkout.parents()[0].id(), requested_checkout.id());
let repo = tx.commit();
assert_eq!(repo.view().checkout(), actual_checkout.id());
}
fn write_conflict(store: &Arc<Store>, file_path: &RepoPath) -> ConflictId {
let file_id1 = testutils::write_file(store, file_path, "a\n");
let file_id2 = testutils::write_file(store, file_path, "b\n");
let file_id3 = testutils::write_file(store, file_path, "c\n");
let conflict = Conflict {
removes: vec![ConflictPart {
value: TreeValue::Normal {
id: file_id1,
executable: false,
},
}],
adds: vec![
ConflictPart {
value: TreeValue::Normal {
id: file_id2,
executable: false,
},
},
ConflictPart {
value: TreeValue::Normal {
id: file_id3,
executable: false,
},
},
],
};
store.write_conflict(&conflict).unwrap()
}
#[test_case(false ; "local backend")]
// #[test_case(true ; "git backend")]
fn test_checkout_previous_not_empty(use_git: bool) {