mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2025-01-18 18:27:38 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

working_copy: replace commit ID by tree ID for checking for changes

Browse source 
What matters for the working copy is the tree ID. We should be able to
remove the commit ID. This patch gets us close.
This commit is contained in:
Martin von Zweigbergk 2022-02-12 16:21:44 -08:00
parent 21d277b7dc
commit e098c01935
3 changed files with 41 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
lib/src/working_copy.rs
View file

@ -864,14 +864,17 @@ impl WorkingCopy {
// Re-read from disk after taking the lock
self.load_proto();
// TODO: It's expensive to reload the whole tree. We should first check if it
// has changed.
self.tree_state.replace(None);
let old_operation_id = self.operation_id();
let old_commit_id = self.current_commit_id();
let old_tree_id = self.current_tree_id();
LockedWorkingCopy {
wc: self,
lock,
old_operation_id,
old_commit_id,
old_tree_id,
closed: false,
}
}
@ -879,15 +882,15 @@ impl WorkingCopy {
pub fn check_out(
&mut self,
operation_id: OperationId,
old_commit_id: Option<&CommitId>,
old_tree_id: Option<&TreeId>,
new_commit: Commit,
) -> Result<CheckoutStats, CheckoutError> {
let mut locked_wc = self.start_mutation();
// Check if the current checkout has changed on disk compared to what the caller
// expected. It's safe to check out another commit regardless, but it's
// probably not what the caller wanted, so we let them know.
if let Some(old_commit_id) = old_commit_id {
if *old_commit_id != locked_wc.old_commit_id {
if let Some(old_tree_id) = old_tree_id {
if *old_tree_id != locked_wc.old_tree_id {
locked_wc.discard();
return Err(CheckoutError::ConcurrentCheckout);
}
@ -906,7 +909,7 @@ pub struct LockedWorkingCopy<'a> {
#[allow(dead_code)]
lock: FileLock,
old_operation_id: OperationId,
old_commit_id: CommitId,
old_tree_id: TreeId,
closed: bool,
}
@ -916,9 +919,9 @@ impl LockedWorkingCopy<'_> {
&self.old_operation_id
}
/// The commit at the time the lock was taken
pub fn old_commit_id(&self) -> &CommitId {
&self.old_commit_id
/// The tree at the time the lock was taken
pub fn old_tree_id(&self) -> &TreeId {
&self.old_tree_id
}
pub fn write_tree(&mut self) -> TreeId {

6
lib/tests/test_working_copy_concurrent.rs
View file

@ -47,7 +47,7 @@ fn test_concurrent_checkout(use_git: bool) {
// Check out commit1
let wc1 = test_workspace1.workspace.working_copy_mut();
let commit_id1 = commit1.id().clone();
let tree_id1 = commit1.tree_id().clone();
// The operation ID is not correct, but that doesn't matter for this test
wc1.check_out(repo1.op_id().clone(), None, commit1).unwrap();
@ -56,12 +56,12 @@ fn test_concurrent_checkout(use_git: bool) {
let mut workspace2 = Workspace::load(&settings, workspace1_root.clone()).unwrap();
workspace2
.working_copy_mut()
.check_out(repo1.op_id().clone(), Some(&commit_id1), commit2.clone())
.check_out(repo1.op_id().clone(), Some(&tree_id1), commit2.clone())
.unwrap();
// Checking out another commit (via the first repo instance) should now fail.
assert_eq!(
wc1.check_out(repo1.op_id().clone(), Some(&commit_id1), commit3),
wc1.check_out(repo1.op_id().clone(), Some(&tree_id1), commit3),
Err(CheckoutError::ConcurrentCheckout)
);

49
src/commands.rs
View file

@ -33,7 +33,7 @@ use clap::{crate_version, App, Arg, ArgMatches};
use criterion::Criterion;
use git2::{Oid, Repository};
use itertools::Itertools;
use jujutsu_lib::backend::{BackendError, CommitId, Timestamp, TreeValue};
use jujutsu_lib::backend::{BackendError, CommitId, Timestamp, TreeId, TreeValue};
use jujutsu_lib::commit::Commit;
use jujutsu_lib::commit_builder::CommitBuilder;
use jujutsu_lib::dag_walk::topo_order_reverse;
@ -501,8 +501,8 @@ impl WorkspaceCommandHelper {
// in the index and view, and so we don't cause unnecessary
// divergence.
let checkout_commit = repo.store().get_commit(&checkout_id).unwrap();
let wc_commit_id = locked_wc.old_commit_id().clone();
if *checkout_commit.id() != wc_commit_id {
let wc_tree_id = locked_wc.old_tree_id().clone();
if *checkout_commit.tree_id() != wc_tree_id {
let wc_operation_data = self
.repo
.op_store()
@ -619,6 +619,7 @@ impl WorkspaceCommandHelper {
fn finish_transaction(&mut self, ui: &mut Ui, mut tx: Transaction) -> Result<(), CommandError> {
let mut_repo = tx.mut_repo();
let store = mut_repo.store().clone();
if !mut_repo.has_changes() {
writeln!(ui, "Nothing changed.")?;
return Ok(());
@ -629,11 +630,11 @@ impl WorkspaceCommandHelper {
writeln!(ui, "Rebased {} descendant commits", num_rebased)?;
}
}
let maybe_old_commit_id = tx
let maybe_old_tree_id = tx
.base_repo()
.view()
.get_checkout(&self.workspace_id())
.cloned();
.map(|commit_id| store.get_commit(commit_id).unwrap().tree_id().clone());
self.repo = tx.commit();
if self.may_update_working_copy {
let stats = update_working_copy(
@ -641,7 +642,7 @@ impl WorkspaceCommandHelper {
&self.repo,
&self.workspace_id(),
self.workspace.working_copy_mut(),
maybe_old_commit_id.as_ref(),
maybe_old_tree_id.as_ref(),
)?;
if let Some(stats) = stats {
if stats.added_files > 0 || stats.updated_files > 0 || stats.removed_files > 0 {
@ -783,7 +784,7 @@ fn update_working_copy(
repo: &Arc<ReadonlyRepo>,
workspace_id: &WorkspaceId,
wc: &mut WorkingCopy,
old_commit_id: Option<&CommitId>,
old_tree_id: Option<&TreeId>,
) -> Result<Option<CheckoutStats>, CommandError> {
let new_commit_id = match repo.view().get_checkout(workspace_id) {
Some(new_commit_id) => new_commit_id,
@ -792,25 +793,27 @@ fn update_working_copy(
return Ok(None);
}
};
if Some(new_commit_id) == old_commit_id {
return Ok(None);
}
// TODO: CheckoutError::ConcurrentCheckout should probably just result in a
// warning for most commands (but be an error for the checkout command)
let new_commit = repo.store().get_commit(new_commit_id).unwrap();
let stats = wc
.check_out(repo.op_id().clone(), old_commit_id, new_commit.clone())
.map_err(|err| {
CommandError::InternalError(format!(
"Failed to check out commit {}: {}",
new_commit.id().hex(),
err
))
})?;
let stats = if Some(new_commit.tree_id()) != old_tree_id {
// TODO: CheckoutError::ConcurrentCheckout should probably just result in a
// warning for most commands (but be an error for the checkout command)
let stats = wc
.check_out(repo.op_id().clone(), old_tree_id, new_commit.clone())
.map_err(|err| {
CommandError::InternalError(format!(
"Failed to check out commit {}: {}",
new_commit.id().hex(),
err
))
})?;
Some(stats)
} else {
None
};
ui.write("Working copy now at: ")?;
ui.write_commit_summary(repo.as_repo_ref(), workspace_id, &new_commit)?;
ui.write("\n")?;
Ok(Some(stats))
Ok(stats)
}
fn get_app<'help>() -> App<'help> {
@ -1765,7 +1768,7 @@ fn cmd_untrack(
let mut tx = workspace_command.start_transaction("untrack paths");
let mut locked_working_copy = workspace_command.working_copy_mut().start_mutation();
if current_checkout.id() != locked_working_copy.old_commit_id() {
if current_checkout.tree_id() != locked_working_copy.old_tree_id() {
return Err(CommandError::UserError(
"Concurrent working copy operation. Try again.".to_string(),
));