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

operation: propagate OpStoreError

Browse source 
This is a rote propagation of the error value to hopefully improve the panic in https://github.com/martinvonz/jj/issues/1907.
This commit is contained in:
Waleed Khan 2023-07-25 09:40:12 -05:00
parent 4acc5870e5
commit 70d3c64b1e
9 changed files with 73 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
lib/src/default_index_store.rs
View file

@ -40,12 +40,20 @@ use crate::file_util::persist_content_addressed_temp_file;
use crate::index::{
HexPrefix, Index, IndexStore, IndexWriteError, MutableIndex, PrefixResolution, ReadonlyIndex,
};
use crate::op_store::OperationId;
use crate::op_store::{OpStoreError, OperationId};
use crate::operation::Operation;
use crate::revset::{ResolvedExpression, Revset, RevsetEvaluationError};
use crate::store::Store;
use crate::{backend, dag_walk, default_revset_engine};
#[derive(Debug, Error)]
pub enum DefaultIndexStoreError {
#[error(transparent)]
Io(#[from] io::Error),
#[error(transparent)]
OpStore(#[from] OpStoreError),
}
#[derive(Debug)]
pub struct DefaultIndexStore {
dir: PathBuf,
@ -99,8 +107,8 @@ impl DefaultIndexStore {
&self,
store: &Arc<Store>,
operation: &Operation,
) -> io::Result<Arc<ReadonlyIndexImpl>> {
let view = operation.view();
) -> Result<Arc<ReadonlyIndexImpl>, DefaultIndexStoreError> {
let view = operation.view()?;
let operations_dir = self.dir.join("operations");
let commit_id_length = store.commit_id_length();
let change_id_length = store.change_id_length();
@ -116,7 +124,7 @@ impl DefaultIndexStore {
parent_op_id = Some(op.id().clone())
}
} else {
for head in op.view().heads() {
for head in op.view()?.heads() {
new_heads.insert(head.clone());
}
}

8
lib/src/op_heads_store.rs
View file

@ -22,15 +22,17 @@ use itertools::Itertools;
use thiserror::Error;
use crate::dag_walk;
use crate::op_store::{OpStore, OperationId};
use crate::op_store::{OpStore, OpStoreError, OperationId};
use crate::operation::Operation;
#[derive(Debug, Error, PartialEq, Eq)]
#[derive(Debug, Error)]
pub enum OpHeadResolutionError<E> {
#[error("Operation log has no heads")]
NoHeads,
#[error(transparent)]
OpStore(#[from] OpStoreError),
#[error("Op resolution error: {0}")]
Err(#[from] E),
Err(#[source] E),
}
pub trait OpHeadsStoreLock<'a> {

9
lib/src/operation.rs
View file

@ -21,7 +21,7 @@ use std::hash::{Hash, Hasher};
use std::sync::Arc;
use crate::backend::CommitId;
use crate::op_store::{OpStore, OperationId, ViewId};
use crate::op_store::{OpStore, OpStoreResult, OperationId, ViewId};
use crate::{dag_walk, op_store};
#[derive(Clone)]
@ -93,9 +93,10 @@ impl Operation {
parents
}
pub fn view(&self) -> View {
let data = self.op_store.read_view(&self.data.view_id).unwrap();
View::new(self.op_store.clone(), self.data.view_id.clone(), data)
pub fn view(&self) -> OpStoreResult<View> {
let data = self.op_store.read_view(&self.data.view_id)?;
let view = View::new(self.op_store.clone(), self.data.view_id.clone(), data);
Ok(view)
}
pub fn store_operation(&self) -> &op_store::Operation {

30
lib/src/repo.rs
View file

@ -41,7 +41,7 @@ use crate::git_backend::GitBackend;
use crate::index::{HexPrefix, Index, IndexStore, MutableIndex, PrefixResolution, ReadonlyIndex};
use crate::local_backend::LocalBackend;
use crate::op_heads_store::{self, OpHeadResolutionError, OpHeadsStore};
use crate::op_store::{BranchTarget, OpStore, OperationId, RefTarget, WorkspaceId};
use crate::op_store::{BranchTarget, OpStore, OpStoreError, OperationId, RefTarget, WorkspaceId};
use crate::operation::Operation;
use crate::refs::merge_ref_targets;
use crate::revset::{self, ChangeIdIndex, Revset, RevsetExpression};
@ -298,11 +298,11 @@ impl ReadonlyRepo {
pub fn reload_at_head(
&self,
user_settings: &UserSettings,
) -> Result<Arc<ReadonlyRepo>, OpHeadResolutionError<TreeMergeError>> {
) -> Result<Arc<ReadonlyRepo>, OpHeadResolutionError<RepoLoaderError>> {
self.loader().load_at_head(user_settings)
}
pub fn reload_at(&self, operation: &Operation) -> Arc<ReadonlyRepo> {
pub fn reload_at(&self, operation: &Operation) -> Result<Arc<ReadonlyRepo>, RepoLoaderError> {
self.loader().load_at(operation)
}
}
@ -558,6 +558,14 @@ fn read_store_type_compat(
.map_err(|source| StoreLoadError::ReadError { store, source })
}
#[derive(Debug, Error)]
pub enum RepoLoaderError {
#[error(transparent)]
TreeMerge(#[from] TreeMergeError),
#[error(transparent)]
OpStore(#[from] OpStoreError),
}
#[derive(Clone)]
pub struct RepoLoader {
repo_path: PathBuf,
@ -617,19 +625,19 @@ impl RepoLoader {
pub fn load_at_head(
&self,
user_settings: &UserSettings,
) -> Result<Arc<ReadonlyRepo>, OpHeadResolutionError<TreeMergeError>> {
) -> Result<Arc<ReadonlyRepo>, OpHeadResolutionError<RepoLoaderError>> {
let op = op_heads_store::resolve_op_heads(
self.op_heads_store.as_ref(),
&self.op_store,
|op_heads| self._resolve_op_heads(op_heads, user_settings),
)?;
let view = View::new(op.view().take_store_view());
let view = View::new(op.view()?.take_store_view());
Ok(self._finish_load(op, view))
}
pub fn load_at(&self, op: &Operation) -> Arc<ReadonlyRepo> {
let view = View::new(op.view().take_store_view());
self._finish_load(op.clone(), view)
pub fn load_at(&self, op: &Operation) -> Result<Arc<ReadonlyRepo>, RepoLoaderError> {
let view = View::new(op.view()?.take_store_view());
Ok(self._finish_load(op.clone(), view))
}
pub fn create_from(
@ -658,11 +666,11 @@ impl RepoLoader {
&self,
op_heads: Vec<Operation>,
user_settings: &UserSettings,
) -> Result<Operation, TreeMergeError> {
let base_repo = self.load_at(&op_heads[0]);
) -> Result<Operation, RepoLoaderError> {
let base_repo = self.load_at(&op_heads[0])?;
let mut tx = base_repo.start_transaction(user_settings, "resolve concurrent operations");
for other_op_head in op_heads.into_iter().skip(1) {
tx.merge_operation(other_op_head);
tx.merge_operation(other_op_head)?;
tx.mut_repo().rebase_descendants(user_settings)?;
}
let merged_repo = tx.write().leave_unpublished();

9
lib/src/transaction.rs
View file

@ -22,7 +22,7 @@ use crate::index::ReadonlyIndex;
use crate::op_store;
use crate::op_store::OperationMetadata;
use crate::operation::Operation;
use crate::repo::{MutableRepo, ReadonlyRepo, Repo, RepoLoader};
use crate::repo::{MutableRepo, ReadonlyRepo, Repo, RepoLoader, RepoLoaderError};
use crate::settings::UserSettings;
use crate::view::View;
@ -70,7 +70,7 @@ impl Transaction {
&mut self.mut_repo
}
pub fn merge_operation(&mut self, other_op: Operation) {
pub fn merge_operation(&mut self, other_op: Operation) -> Result<(), RepoLoaderError> {
let ancestor_op = closest_common_node(
self.parent_ops.clone(),
vec![other_op.clone()],
@ -79,11 +79,12 @@ impl Transaction {
)
.unwrap();
let repo_loader = self.base_repo().loader();
let base_repo = repo_loader.load_at(&ancestor_op);
let other_repo = repo_loader.load_at(&other_op);
let base_repo = repo_loader.load_at(&ancestor_op)?;
let other_repo = repo_loader.load_at(&other_op)?;
self.parent_ops.push(other_op);
let merged_repo = self.mut_repo();
merged_repo.merge(&base_repo, &other_repo);
Ok(())
}
/// Writes the transaction to the operation store and publishes it.

2
lib/tests/test_load_repo.rs
View file

@ -40,6 +40,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.repo_path(), &StoreFactories::default()).unwrap();
let old_repo = loader.load_at(repo.operation());
let old_repo = loader.load_at(repo.operation()).unwrap();
assert!(old_repo.view().heads().contains(commit.id()));
}

28
src/cli_util.rs
View file

@ -43,7 +43,7 @@ use jj_lib::op_store::{OpStore, OpStoreError, OperationId, RefTarget, WorkspaceI
use jj_lib::operation::Operation;
use jj_lib::repo::{
CheckOutCommitError, EditCommitError, MutableRepo, ReadonlyRepo, Repo, RepoLoader,
RewriteRootCommit, StoreFactories, StoreLoadError,
RepoLoaderError, RewriteRootCommit, StoreFactories, StoreLoadError,
};
use jj_lib::repo_path::{FsPathParseError, RepoPath};
use jj_lib::revset::{
@ -196,6 +196,7 @@ impl From<OpHeadResolutionError<CommandError>> for CommandError {
OpHeadResolutionError::NoHeads => CommandError::InternalError(
"Corrupt repository: there are no operations".to_string(),
),
OpHeadResolutionError::OpStore(err) => err.into(),
OpHeadResolutionError::Err(e) => e,
}
}
@ -213,6 +214,18 @@ impl From<TreeMergeError> for CommandError {
}
}
impl From<OpStoreError> for CommandError {
fn from(err: OpStoreError) -> Self {
CommandError::InternalError(format!("Failed to load an operation: {err}"))
}
}
impl From<RepoLoaderError> for CommandError {
fn from(err: RepoLoaderError) -> Self {
CommandError::InternalError(format!("Failed to load the repo: {err}"))
}
}
impl From<ResetError> for CommandError {
fn from(_: ResetError) -> Self {
CommandError::InternalError("Failed to reset the working copy".to_string())
@ -514,7 +527,7 @@ impl CommandHelper {
) -> Result<WorkspaceCommandHelper, CommandError> {
let workspace = self.load_workspace()?;
let op_head = self.resolve_operation(ui, workspace.repo_loader())?;
let repo = workspace.repo_loader().load_at(&op_head);
let repo = workspace.repo_loader().load_at(&op_head)?;
let mut workspace_command = self.for_loaded_repo(ui, workspace, repo)?;
if snapshot {
workspace_command.snapshot(ui)?;
@ -554,7 +567,7 @@ impl CommandHelper {
ui,
"Concurrent modification detected, resolving automatically.",
)?;
let base_repo = repo_loader.load_at(&op_heads[0]);
let base_repo = repo_loader.load_at(&op_heads[0])?;
// TODO: It may be helpful to print each operation we're merging here
let mut tx = start_repo_transaction(
&base_repo,
@ -563,7 +576,7 @@ impl CommandHelper {
"resolve concurrent operations",
);
for other_op_head in op_heads.into_iter().skip(1) {
tx.merge_operation(other_op_head);
tx.merge_operation(other_op_head)?;
let num_rebased = tx.mut_repo().rebase_descendants(&self.settings)?;
if num_rebased > 0 {
writeln!(
@ -606,7 +619,7 @@ impl CommandHelper {
.read_operation(op_id)
.map_err(|e| CommandError::InternalError(format!("Failed to read operation: {e}")))?;
let operation = Operation::new(op_store.clone(), op_id.clone(), op_data);
let repo = workspace.repo_loader().load_at(&operation);
let repo = workspace.repo_loader().load_at(&operation)?;
self.for_loaded_repo(ui, workspace, repo)
}
}
@ -1107,7 +1120,7 @@ impl WorkspaceCommandHelper {
let (repo, wc_commit) = match check_stale_working_copy(&locked_wc, &wc_commit, &repo) {
Ok(None) => (repo, wc_commit),
Ok(Some(wc_operation)) => {
let repo = repo.reload_at(&wc_operation);
let repo = repo.reload_at(&wc_operation)?;
let wc_commit = if let Some(wc_commit) = get_wc_commit(&repo)? {
wc_commit
} else {
@ -1614,7 +1627,8 @@ fn resolve_op_for_load(
)))
})
};
let operation = resolve_single_op(op_store, op_heads_store, get_current_op, op_str)?;
let operation = resolve_single_op(op_store, op_heads_store, get_current_op, op_str)
.map_err(OpHeadResolutionError::Err)?;
Ok(operation)
}

4
src/commands/debug.rs
View file

@ -148,7 +148,7 @@ pub fn cmd_debug(
repo.index_store().as_any().downcast_ref();
if let Some(default_index_store) = default_index_store {
default_index_store.reinit();
let repo = repo.reload_at(repo.operation());
let repo = repo.reload_at(repo.operation())?;
let index_impl: &ReadonlyIndexWrapper = repo
.readonly_index()
.as_any()
@ -177,7 +177,7 @@ pub fn cmd_debug(
writeln!(ui, "{:#?}", op.store_operation())?;
}
if operation_args.display != DebugOperationDisplay::Operation {
writeln!(ui, "{:#?}", op.view().store_view())?;
writeln!(ui, "{:#?}", op.view()?.store_view())?;
}
}
DebugCommands::Watchman(watchman_subcommand) => {

6
src/commands/operation.rs
View file

@ -260,8 +260,8 @@ pub fn cmd_op_undo(
let mut tx =
workspace_command.start_transaction(&format!("undo operation {}", bad_op.id().hex()));
let repo_loader = tx.base_repo().loader();
let bad_repo = repo_loader.load_at(&bad_op);
let parent_repo = repo_loader.load_at(&parent_ops[0]);
let bad_repo = repo_loader.load_at(&bad_op)?;
let parent_repo = repo_loader.load_at(&parent_ops[0])?;
tx.mut_repo().merge(&bad_repo, &parent_repo);
let new_view = view_with_desired_portions_restored(
tx.repo().view().store_view(),
@ -285,7 +285,7 @@ fn cmd_op_restore(
let mut tx = workspace_command
.start_transaction(&format!("restore to operation {}", target_op.id().hex()));
let new_view = view_with_desired_portions_restored(
target_op.view().store_view(),
target_op.view()?.store_view(),
tx.base_repo().view().store_view(),
&process_what_arg(&args.what, repo_is_colocated),
);