Another pass at removing ProjectConflict

As this causes quite a lot of changes, there is this commit to capture them.
This commit is contained in:
Sebastian Thiel 2024-05-30 20:53:17 +02:00
parent 1dc52a44c0
commit 61a53c6346
No known key found for this signature in database
GPG Key ID: 9CB5EE7895E8268B
11 changed files with 105 additions and 425 deletions

View File

@ -104,6 +104,7 @@ fn persist_tempfile(
/// Reads and parses the state file.
///
/// If the file does not exist, it will be created.
// TODO(ST): make this anyhow.
pub(crate) fn read_toml_file_or_default<T: DeserializeOwned + Default>(
path: &Path,
) -> Result<T, crate::reader::Error> {

View File

@ -87,6 +87,22 @@ impl Repository {
conflicts::is_resolving(self)
}
pub fn assure_resolved(&self) -> Result<()> {
if self.is_resolving() {
Err(anyhow!("project has active conflicts")).context(Code::ProjectConflict)
} else {
Ok(())
}
}
pub fn assure_unconflicted(&self) -> Result<()> {
if conflicts::is_conflicting(self, None)? {
Err(anyhow!("project has active conflicts")).context(Code::ProjectConflict)
} else {
Ok(())
}
}
pub fn path(&self) -> &path::Path {
path::Path::new(&self.project.path)
}

View File

@ -5,7 +5,7 @@ use git2::Index;
use serde::Serialize;
use super::{
branch, errors,
branch,
integration::{
get_workspace_head, update_gitbutler_integration, GITBUTLER_INTEGRATION_REFERENCE,
},
@ -333,13 +333,7 @@ pub fn update_base_branch(
project_repository: &project_repository::Repository,
user: Option<&users::User>,
) -> anyhow::Result<Vec<branch::Branch>> {
if project_repository.is_resolving() {
anyhow::bail!(errors::UpdateBaseBranchError::Conflict(
errors::ProjectConflict {
project_id: project_repository.project().id,
},
));
}
project_repository.assure_resolved()?;
// look up the target and see if there is a new oid
let target = default_target(&project_repository.project().gb_dir())?;

View File

@ -455,7 +455,7 @@ impl ControllerInner {
user,
run_hooks,
)
.map_err(Error::from_err);
.map_err(Into::into);
let _ = snapshot_tree.and_then(|snapshot_tree| {
project_repository.project().snapshot_commit_creation(
snapshot_tree,
@ -975,7 +975,7 @@ impl ControllerInner {
.project()
.create_snapshot(SnapshotDetails::new(OperationKind::MoveCommit));
super::move_commit(project_repository, target_branch_id, commit_oid, user)
.map_err(Error::from_err)
.map_err(Into::into)
})
}
}

View File

@ -9,8 +9,6 @@ use crate::{
// Generic error enum for use in the virtual branches module.
#[derive(Debug, thiserror::Error)]
pub enum VirtualBranchError {
#[error("project")]
Conflict(ProjectConflict),
#[error("branch not found")]
BranchNotFound(BranchNotFound),
#[error("default target not set")]
@ -64,8 +62,6 @@ pub enum ResetBranchError {
#[derive(Debug, thiserror::Error)]
pub enum ApplyBranchError {
#[error("project")]
Conflict(ProjectConflict),
#[error("branch not found")]
BranchNotFound(BranchNotFound),
#[error("branch {0} is in a conflicting state")]
BranchConflicts(BranchId),
@ -77,48 +73,6 @@ pub enum ApplyBranchError {
Other(#[from] anyhow::Error),
}
#[derive(Debug, thiserror::Error)]
pub enum UnapplyOwnershipError {
#[error("default target not set")]
DefaultTargetNotSet(DefaultTargetNotSet),
#[error("project is in conflict state")]
Conflict(ProjectConflict),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
impl ErrorWithContext for UnapplyOwnershipError {
fn context(&self) -> Option<Context> {
Some(match self {
UnapplyOwnershipError::DefaultTargetNotSet(error) => error.to_context(),
UnapplyOwnershipError::Conflict(error) => error.to_context(),
UnapplyOwnershipError::Other(error) => {
return error.custom_context_or_root_cause().into()
}
})
}
}
#[derive(Debug, thiserror::Error)]
pub enum UnapplyBranchError {
#[error("default target not set")]
DefaultTargetNotSet(DefaultTargetNotSet),
#[error("branch not found")]
BranchNotFound(BranchNotFound),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
impl ErrorWithContext for UnapplyBranchError {
fn context(&self) -> Option<Context> {
Some(match self {
UnapplyBranchError::DefaultTargetNotSet(ctx) => ctx.to_context(),
UnapplyBranchError::BranchNotFound(ctx) => ctx.to_context(),
UnapplyBranchError::Other(error) => return error.custom_context_or_root_cause().into(),
})
}
}
#[derive(Debug, thiserror::Error)]
pub enum ListVirtualBranchesError {
#[error("project")]
@ -153,22 +107,6 @@ impl ErrorWithContext for CreateVirtualBranchError {
}
}
#[derive(Debug, thiserror::Error)]
pub enum CommitError {
#[error("branch not found")]
BranchNotFound(BranchNotFound),
#[error("default target not set")]
DefaultTargetNotSet(DefaultTargetNotSet),
#[error("will not commit conflicted files")]
Conflicted(ProjectConflict),
#[error("commit hook rejected: {0}")]
CommitHookRejected(String),
#[error("commit-msg hook rejected: {0}")]
CommitMsgHookRejected(String),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
#[derive(Debug, thiserror::Error)]
pub enum PushError {
#[error("default target not set")]
@ -244,8 +182,6 @@ pub enum CherryPickError {
CommitNotFound(git::Oid),
#[error("can not cherry pick not applied branch")]
NotApplied,
#[error("project is in conflict state")]
Conflict(ProjectConflict),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
@ -260,8 +196,6 @@ pub enum SquashError {
CommitNotFound(git::Oid),
#[error("branch not found")]
BranchNotFound(BranchNotFound),
#[error("project is in conflict state")]
Conflict(ProjectConflict),
#[error("can not squash root commit")]
CantSquashRootCommit,
#[error(transparent)]
@ -300,46 +234,6 @@ pub enum UpdateCommitMessageError {
CommitNotFound(git::Oid),
#[error("branch not found")]
BranchNotFound(BranchNotFound),
#[error("project is in conflict state")]
Conflict(ProjectConflict),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
#[derive(Debug, thiserror::Error)]
pub enum UpdateBaseBranchError {
#[error("project is in conflicting state")]
Conflict(ProjectConflict),
#[error("no default target set")]
DefaultTargetNotSet(DefaultTargetNotSet),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
impl ErrorWithContext for UpdateBaseBranchError {
fn context(&self) -> Option<Context> {
Some(match self {
UpdateBaseBranchError::Conflict(ctx) => ctx.to_context(),
UpdateBaseBranchError::DefaultTargetNotSet(ctx) => ctx.to_context(),
UpdateBaseBranchError::Other(error) => {
return error.custom_context_or_root_cause().into()
}
})
}
}
#[derive(Debug, thiserror::Error)]
pub enum MoveCommitError {
#[error("source branch contains hunks locked to the target commit")]
SourceLocked,
#[error("project is in conflicted state")]
Conflicted(ProjectConflict),
#[error("default target not set")]
DefaultTargetNotSet(DefaultTargetNotSet),
#[error("branch not found")]
BranchNotFound(BranchNotFound),
#[error("commit {0} not found")]
CommitNotFound(git::Oid),
#[error(transparent)]
Other(#[from] anyhow::Error),
}
@ -358,21 +252,6 @@ pub enum CreateVirtualBranchFromBranchError {
Other(#[from] anyhow::Error),
}
#[derive(Debug)]
pub struct ProjectConflict {
pub project_id: ProjectId,
}
impl ProjectConflict {
fn to_context(&self) -> error::Context {
error::Context::new(format!(
"project {} is in a conflicted state",
self.project_id
))
.with_code(Code::ProjectConflict)
}
}
#[derive(Debug)]
pub struct DefaultTargetNotSet {
pub project_id: ProjectId,

View File

@ -52,7 +52,7 @@ impl VirtualBranchesHandle {
let virtual_branches = self.read_file();
virtual_branches?
.default_target
.ok_or(anyhow!("No default target"))
.ok_or(anyhow!("there is no default target"))
}
/// Sets the target for the given virtual branch.
@ -88,13 +88,16 @@ impl VirtualBranchesHandle {
/// Gets the state of the given virtual branch.
///
/// Errors if the file cannot be read or written.
pub fn get_branch(&self, id: BranchId) -> Result<Branch, crate::reader::Error> {
pub fn get_branch(&self, id: BranchId) -> anyhow::Result<Branch> {
self.try_branch(id)?
.ok_or_else(|| anyhow!("branch with ID {id} not found"))
}
/// Gets the state of the given virtual branch returning `Some(branch)` or `None`
/// if that branch doesn't exist.
pub fn try_branch(&self, id: BranchId) -> anyhow::Result<Option<Branch>> {
let virtual_branches = self.read_file()?;
virtual_branches
.branches
.get(&id)
.cloned()
.ok_or(crate::reader::Error::NotFound)
Ok(virtual_branches.branches.get(&id).cloned())
}
/// Lists all virtual branches.

View File

@ -38,7 +38,7 @@ use crate::{
Refname, RemoteRefname,
},
project_repository::{self, conflicts, LogUntil},
reader, users,
users,
};
use crate::{error::Error, git::diff::GitHunk};
@ -221,28 +221,13 @@ pub fn apply_branch(
branch_id: BranchId,
user: Option<&users::User>,
) -> Result<String, errors::ApplyBranchError> {
if project_repository.is_resolving() {
return Err(errors::ApplyBranchError::Conflict(
errors::ProjectConflict {
project_id: project_repository.project().id,
},
));
}
project_repository.assure_resolved()?;
let repo = &project_repository.git_repository;
let vb_state = project_repository.project().virtual_branches();
let default_target = vb_state.get_default_target()?;
let mut branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => Err(errors::ApplyBranchError::BranchNotFound(
errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id,
},
)),
Err(error) => Err(errors::ApplyBranchError::Other(error.into())),
}?;
let mut branch = vb_state.get_branch(branch_id)?;
if branch.applied {
return Ok(branch.name);
@ -281,7 +266,7 @@ pub fn apply_branch(
if merge_index.has_conflicts() {
// currently we can only deal with the merge problem branch
for mut branch in
super::get_status_by_branch(project_repository, Some(&target_commit.id().into()))?
get_status_by_branch(project_repository, Some(&target_commit.id().into()))?
.0
.into_iter()
.map(|(branch, _)| branch)
@ -472,14 +457,8 @@ pub fn apply_branch(
pub fn unapply_ownership(
project_repository: &project_repository::Repository,
ownership: &BranchOwnershipClaims,
) -> Result<(), errors::UnapplyOwnershipError> {
if conflicts::is_resolving(project_repository) {
return Err(errors::UnapplyOwnershipError::Conflict(
errors::ProjectConflict {
project_id: project_repository.project().id,
},
));
}
) -> Result<()> {
project_repository.assure_resolved()?;
let vb_state = project_repository.project().virtual_branches();
let default_target = vb_state.get_default_target()?;
@ -491,8 +470,7 @@ pub fn unapply_ownership(
.filter(|b| b.applied)
.collect::<Vec<_>>();
let integration_commit_id =
super::integration::get_workspace_head(&vb_state, project_repository)?;
let integration_commit_id = get_workspace_head(&vb_state, project_repository)?;
let (applied_statuses, _) = get_applied_status(
project_repository,
@ -536,9 +514,7 @@ pub fn unapply_ownership(
if let Some(reversed_hunk) = diff::reverse_hunk(h.1) {
diff.entry(h.0).or_insert_with(Vec::new).push(reversed_hunk);
} else {
return Err(errors::UnapplyOwnershipError::Other(anyhow::anyhow!(
"failed to reverse hunk"
)));
bail!("failed to reverse hunk")
}
}
@ -582,14 +558,8 @@ pub fn unapply_ownership(
pub fn reset_files(
project_repository: &project_repository::Repository,
files: &Vec<String>,
) -> Result<(), errors::UnapplyOwnershipError> {
if conflicts::is_resolving(project_repository) {
return Err(errors::UnapplyOwnershipError::Conflict(
errors::ProjectConflict {
project_id: project_repository.project().id,
},
));
}
) -> Result<()> {
project_repository.assure_resolved()?;
// for each tree, we need to checkout the entry from the index at that path
// or if it doesn't exist, remove the file from the working directory
@ -617,21 +587,10 @@ pub fn reset_files(
pub fn unapply_branch(
project_repository: &project_repository::Repository,
branch_id: BranchId,
) -> Result<Option<branch::Branch>, errors::UnapplyBranchError> {
) -> Result<Option<branch::Branch>> {
let vb_state = project_repository.project().virtual_branches();
let mut target_branch = vb_state
.get_branch(branch_id)
.map_err(|error| match error {
reader::Error::NotFound => {
errors::UnapplyBranchError::BranchNotFound(errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id,
})
}
error => errors::UnapplyBranchError::Other(error.into()),
})?;
let mut target_branch = vb_state.get_branch(branch_id)?;
if !target_branch.applied {
return Ok(Some(target_branch));
}
@ -1166,14 +1125,14 @@ pub fn integrate_upstream_commits(
project_repository: &project_repository::Repository,
branch_id: BranchId,
user: Option<&users::User>,
) -> Result<(), anyhow::Error> {
) -> Result<()> {
conflicts::is_conflicting(project_repository, None)?;
let repo = &project_repository.git_repository;
let project = project_repository.project();
let vb_state = project.virtual_branches();
let mut branch = vb_state.get_branch(branch_id).map_err(Error::from_err)?;
let mut branch = vb_state.get_branch(branch_id)?;
let default_target = vb_state.get_default_target()?;
let upstream_branch = branch.upstream.as_ref().context("upstream not found")?;
@ -1364,19 +1323,9 @@ pub fn integrate_with_merge(
pub fn update_branch(
project_repository: &project_repository::Repository,
branch_update: branch::BranchUpdateRequest,
) -> Result<branch::Branch, errors::UpdateBranchError> {
) -> Result<branch::Branch> {
let vb_state = project_repository.project().virtual_branches();
let mut branch = vb_state
.get_branch(branch_update.id)
.map_err(|error| match error {
reader::Error::NotFound => {
errors::UpdateBranchError::BranchNotFound(errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id: branch_update.id,
})
}
_ => errors::UpdateBranchError::Other(error.into()),
})?;
let mut branch = vb_state.get_branch(branch_update.id)?;
_ = project_repository
.project()
.snapshot_branch_update(&branch, &branch_update);
@ -1456,12 +1405,9 @@ pub fn delete_branch(
branch_id: BranchId,
) -> Result<(), Error> {
let vb_state = project_repository.project().virtual_branches();
let branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => return Ok(()),
Err(error) => Err(error),
}
.context("failed to read branch")?;
let Some(branch) = vb_state.try_branch(branch_id)? else {
return Ok(());
};
_ = project_repository
.project()
.snapshot_branch_deletion(branch.name.clone());
@ -2003,17 +1949,7 @@ pub fn reset_branch(
let default_target = vb_state.get_default_target()?;
let mut branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => Err(errors::ResetBranchError::BranchNotFound(
errors::BranchNotFound {
branch_id,
project_id: project_repository.project().id,
},
)),
Err(error) => Err(errors::ResetBranchError::Other(error.into())),
}?;
let mut branch = vb_state.get_branch(branch_id)?;
if branch.head == target_commit_oid {
// nothing to do
return Ok(());
@ -2029,7 +1965,7 @@ pub fn reset_branch(
));
}
// Compute the old workspace before resetting so we can can figure out
// Compute the old workspace before resetting, so we can figure out
// what hunks were released by this reset, and assign them to this branch.
let old_head = get_workspace_head(&vb_state, project_repository)?;
@ -2285,7 +2221,7 @@ pub fn commit(
ownership: Option<&branch::BranchOwnershipClaims>,
user: Option<&users::User>,
run_hooks: bool,
) -> Result<git::Oid, errors::CommitError> {
) -> Result<git::Oid> {
let mut message_buffer = message.to_owned();
let vb_state = project_repository.project().virtual_branches();
@ -2296,7 +2232,7 @@ pub fn commit(
.context("failed to run hook")?;
if let HookResult::RunNotSuccessful { stdout, .. } = hook_result {
return Err(errors::CommitError::CommitMsgHookRejected(stdout));
bail!("commit-msg hook rejected: {}", stdout.trim());
}
let hook_result = project_repository
@ -2305,14 +2241,13 @@ pub fn commit(
.context("failed to run hook")?;
if let HookResult::RunNotSuccessful { stdout, .. } = hook_result {
return Err(errors::CommitError::CommitHookRejected(stdout));
bail!("commit hook rejected: {}", stdout.trim());
}
}
let message = &message_buffer;
let integration_commit_id =
super::integration::get_workspace_head(&vb_state, project_repository)?;
let integration_commit_id = get_workspace_head(&vb_state, project_repository)?;
// get the files to commit
let (statuses, _) = get_status_by_branch(project_repository, Some(&integration_commit_id))
.context("failed to get status by branch")?;
@ -2320,20 +2255,11 @@ pub fn commit(
let (ref mut branch, files) = statuses
.into_iter()
.find(|(branch, _)| branch.id == branch_id)
.ok_or_else(|| {
errors::CommitError::BranchNotFound(errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id,
})
})?;
.with_context(|| format!("branch {branch_id} not found"))?;
update_conflict_markers(project_repository, &files)?;
if conflicts::is_conflicting(project_repository, None)? {
return Err(errors::CommitError::Conflicted(errors::ProjectConflict {
project_id: project_repository.project().id,
}));
}
project_repository.assure_unconflicted()?;
let tree_oid = if let Some(ownership) = ownership {
let files = files.into_iter().filter_map(|(filepath, hunks)| {
@ -2422,16 +2348,7 @@ pub fn push(
) -> Result<(), errors::PushError> {
let vb_state = project_repository.project().virtual_branches();
let mut vbranch = vb_state
.get_branch(branch_id)
.map_err(|error| match error {
reader::Error::NotFound => errors::PushError::BranchNotFound(errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id,
}),
error => errors::PushError::Other(error.into()),
})?;
let mut vbranch = vb_state.get_branch(branch_id)?;
let remote_branch = if let Some(upstream_branch) = &vbranch.upstream {
upstream_branch.clone()
} else {
@ -2610,17 +2527,7 @@ pub fn is_virtual_branch_mergeable(
branch_id: BranchId,
) -> Result<bool, errors::IsVirtualBranchMergeable> {
let vb_state = project_repository.project().virtual_branches();
let branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => Err(errors::IsVirtualBranchMergeable::BranchNotFound(
errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id,
},
)),
Err(error) => Err(errors::IsVirtualBranchMergeable::Other(error.into())),
}?;
let branch = vb_state.get_branch(branch_id)?;
if branch.applied {
return Ok(true);
}
@ -2685,12 +2592,9 @@ pub fn move_commit_file(
) -> Result<git::Oid, errors::VirtualBranchError> {
let vb_state = project_repository.project().virtual_branches();
let mut target_branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => return Ok(to_commit_oid), // this is wrong
Err(error) => Err(error),
}
.context("failed to read branch")?;
let Some(mut target_branch) = vb_state.try_branch(branch_id)? else {
return Ok(to_commit_oid); // this is wrong
};
let default_target = vb_state.get_default_target()?;
@ -2940,14 +2844,7 @@ pub fn amend(
commit_oid: git::Oid,
target_ownership: &BranchOwnershipClaims,
) -> Result<git::Oid, errors::VirtualBranchError> {
if conflicts::is_conflicting(project_repository, None)? {
return Err(errors::VirtualBranchError::Conflict(
errors::ProjectConflict {
project_id: project_repository.project().id,
},
));
}
project_repository.assure_resolved()?;
let vb_state = project_repository.project().virtual_branches();
let all_branches = vb_state
@ -3124,17 +3021,7 @@ pub fn reorder_commit(
let default_target = vb_state.get_default_target()?;
let mut branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => Err(errors::VirtualBranchError::BranchNotFound(
errors::BranchNotFound {
branch_id,
project_id: project_repository.project().id,
},
)),
Err(error) => Err(errors::VirtualBranchError::Other(error.into())),
}?;
let mut branch = vb_state.get_branch(branch_id)?;
// find the commit to offset from
let commit = project_repository
.git_repository
@ -3223,17 +3110,7 @@ pub fn insert_blank_commit(
) -> Result<(), errors::VirtualBranchError> {
let vb_state = project_repository.project().virtual_branches();
let mut branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => Err(errors::VirtualBranchError::BranchNotFound(
errors::BranchNotFound {
branch_id,
project_id: project_repository.project().id,
},
)),
Err(error) => Err(errors::VirtualBranchError::Other(error.into())),
}?;
let mut branch = vb_state.get_branch(branch_id)?;
// find the commit to offset from
let mut commit = project_repository
.git_repository
@ -3290,17 +3167,7 @@ pub fn undo_commit(
) -> Result<(), errors::VirtualBranchError> {
let vb_state = project_repository.project().virtual_branches();
let mut branch = match vb_state.get_branch(branch_id) {
Ok(branch) => Ok(branch),
Err(reader::Error::NotFound) => Err(errors::VirtualBranchError::BranchNotFound(
errors::BranchNotFound {
branch_id,
project_id: project_repository.project().id,
},
)),
Err(error) => Err(errors::VirtualBranchError::Other(error.into())),
}?;
let mut branch = vb_state.get_branch(branch_id)?;
let commit = project_repository
.git_repository
.find_commit(commit_oid)
@ -3444,11 +3311,7 @@ pub fn cherry_pick(
branch_id: BranchId,
target_commit_oid: git::Oid,
) -> Result<Option<git::Oid>, errors::CherryPickError> {
if conflicts::is_conflicting(project_repository, None)? {
return Err(errors::CherryPickError::Conflict(errors::ProjectConflict {
project_id: project_repository.project().id,
}));
}
project_repository.assure_unconflicted()?;
let vb_state = project_repository.project().virtual_branches();
@ -3627,28 +3490,11 @@ pub fn squash(
branch_id: BranchId,
commit_oid: git::Oid,
) -> Result<(), errors::SquashError> {
if conflicts::is_conflicting(project_repository, None)? {
return Err(errors::SquashError::Conflict(errors::ProjectConflict {
project_id: project_repository.project().id,
}));
}
project_repository.assure_resolved()?;
let vb_state = project_repository.project().virtual_branches();
let mut branch = vb_state.get_branch(branch_id)?;
let default_target = vb_state.get_default_target()?;
let mut branch = vb_state
.get_branch(branch_id)
.map_err(|error| match error {
reader::Error::NotFound => {
errors::SquashError::BranchNotFound(errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id,
})
}
error => errors::SquashError::Other(error.into()),
})?;
let branch_commit_oids = project_repository.l(
branch.head,
project_repository::LogUntil::Commit(default_target.sha),
@ -3753,30 +3599,12 @@ pub fn update_commit_message(
if message.is_empty() {
return Err(errors::UpdateCommitMessageError::EmptyMessage);
}
if conflicts::is_conflicting(project_repository, None)? {
return Err(errors::UpdateCommitMessageError::Conflict(
errors::ProjectConflict {
project_id: project_repository.project().id,
},
));
}
project_repository.assure_unconflicted()?;
let vb_state = project_repository.project().virtual_branches();
let default_target = vb_state.get_default_target()?;
let mut branch = vb_state
.get_branch(branch_id)
.map_err(|error| match error {
reader::Error::NotFound => {
errors::UpdateCommitMessageError::BranchNotFound(errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id,
})
}
error => errors::UpdateCommitMessageError::Other(error.into()),
})?;
let mut branch = vb_state.get_branch(branch_id)?;
let branch_commit_oids = project_repository.l(
branch.head,
project_repository::LogUntil::Commit(default_target.sha),
@ -3859,17 +3687,10 @@ pub fn update_commit_message(
pub fn move_commit(
project_repository: &project_repository::Repository,
target_branch_id: BranchId,
commit_oid: git::Oid,
commit_id: git::Oid,
user: Option<&users::User>,
) -> Result<(), errors::MoveCommitError> {
if project_repository.is_resolving() {
return Err(errors::MoveCommitError::Conflicted(
errors::ProjectConflict {
project_id: project_repository.project().id,
},
));
}
) -> Result<()> {
project_repository.assure_resolved()?;
let vb_state = project_repository.project().virtual_branches();
let applied_branches = vb_state
@ -3880,12 +3701,7 @@ pub fn move_commit(
.collect::<Vec<_>>();
if !applied_branches.iter().any(|b| b.id == target_branch_id) {
return Err(errors::MoveCommitError::BranchNotFound(
errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id: target_branch_id,
},
));
bail!("branch {target_branch_id} is not among applied branches")
}
let default_target = vb_state.get_default_target()?;
@ -3902,14 +3718,14 @@ pub fn move_commit(
let (ref mut source_branch, source_status) = applied_statuses
.iter_mut()
.find(|(b, _)| b.head == commit_oid)
.ok_or_else(|| errors::MoveCommitError::CommitNotFound(commit_oid))?;
.find(|(b, _)| b.head == commit_id)
.ok_or_else(|| anyhow!("commit {commit_id} to be moved could not be found"))?;
let source_branch_non_comitted_files = source_status;
let source_branch_head = project_repository
.git_repository
.find_commit(commit_oid)
.find_commit(commit_id)
.context("failed to find commit")?;
let source_branch_head_parent = source_branch_head
.parent(0)
@ -3945,7 +3761,7 @@ pub fn move_commit(
});
if is_source_locked {
return Err(errors::MoveCommitError::SourceLocked);
bail!("the source branch contains hunks locked to the target commit")
}
// move files ownerships from source branch to the destination branch
@ -3970,18 +3786,7 @@ pub fn move_commit(
// move the commit to destination branch target branch
{
let mut destination_branch =
vb_state
.get_branch(target_branch_id)
.map_err(|error| match error {
reader::Error::NotFound => {
errors::MoveCommitError::BranchNotFound(errors::BranchNotFound {
project_id: project_repository.project().id,
branch_id: target_branch_id,
})
}
error => errors::MoveCommitError::Other(error.into()),
})?;
let mut destination_branch = vb_state.get_branch(target_branch_id)?;
for ownership in ownerships_to_transfer {
destination_branch.ownership.put(ownership);

View File

@ -1,6 +1,7 @@
use std::path::PathBuf;
use std::{fs, path, str::FromStr};
use gitbutler_core::error::Code;
use gitbutler_core::{
git,
projects::{self, Project, ProjectId},
@ -169,7 +170,7 @@ async fn resolve_conflict_flow() {
.await
.unwrap_err()
.downcast_ref(),
Some(errors::CommitError::Conflicted(_))
Some(Code::ProjectConflict)
));
}

View File

@ -2,7 +2,7 @@ use std::str::FromStr;
use gitbutler_core::{
git,
virtual_branches::{branch, errors, BranchId},
virtual_branches::{branch, BranchId},
};
use crate::suite::virtual_branches::Test;
@ -238,14 +238,14 @@ async fn locked_hunks_on_source_branch() {
.await
.unwrap();
assert!(matches!(
assert_eq!(
controller
.move_commit(*project_id, target_branch_id, commit_oid)
.await
.unwrap_err()
.downcast_ref(),
Some(errors::MoveCommitError::SourceLocked)
));
.to_string(),
"the source branch contains hunks locked to the target commit"
);
}
#[tokio::test]
@ -279,18 +279,19 @@ async fn no_commit() {
.await
.unwrap();
assert!(matches!(
let commit_id_hex = "a99c95cca7a60f1a2180c2f86fb18af97333c192";
assert_eq!(
controller
.move_commit(
*project_id,
target_branch_id,
git::Oid::from_str("a99c95cca7a60f1a2180c2f86fb18af97333c192").unwrap()
git::Oid::from_str(commit_id_hex).unwrap()
)
.await
.unwrap_err()
.downcast_ref(),
Some(errors::MoveCommitError::CommitNotFound(_))
));
.to_string(),
format!("commit {commit_id_hex} to be moved could not be found")
);
}
#[tokio::test]
@ -319,12 +320,13 @@ async fn no_branch() {
.await
.unwrap();
assert!(matches!(
let id = BranchId::generate();
assert_eq!(
controller
.move_commit(*project_id, BranchId::generate(), commit_oid)
.move_commit(*project_id, id, commit_oid)
.await
.unwrap_err()
.downcast_ref(),
Some(errors::MoveCommitError::BranchNotFound(_))
));
.to_string(),
format!("branch {id} is not among applied branches")
);
}

View File

@ -40,7 +40,6 @@ mod error {
}
mod go_back_to_integration {
use gitbutler_core::error::Code;
use pretty_assertions::assert_eq;
use super::*;

View File

@ -19,9 +19,7 @@ use gitbutler_core::{
virtual_branches::{
self, apply_branch,
branch::{BranchCreateRequest, BranchOwnershipClaims},
commit, create_virtual_branch,
errors::CommitError,
integrate_upstream_commits,
commit, create_virtual_branch, integrate_upstream_commits,
integration::verify_branch,
is_remote_branch_mergeable, is_virtual_branch_mergeable, list_remote_branches,
unapply_ownership, update_branch,
@ -2144,15 +2142,8 @@ fn pre_commit_hook_rejection() -> Result<()> {
true,
);
let error = res.unwrap_err();
assert!(matches!(error, CommitError::CommitHookRejected(_)));
let CommitError::CommitHookRejected(output) = error else {
unreachable!()
};
assert_eq!(&output, "rejected\n");
let err = res.unwrap_err();
assert_eq!(err.to_string(), "commit hook rejected: rejected");
Ok(())
}
@ -2256,15 +2247,8 @@ fn commit_msg_hook_rejection() -> Result<()> {
true,
);
let error = res.unwrap_err();
assert!(matches!(error, CommitError::CommitMsgHookRejected(_)));
let CommitError::CommitMsgHookRejected(output) = error else {
unreachable!()
};
assert_eq!(&output, "rejected\n");
let err = res.unwrap_err();
assert_eq!(err.to_string(), "commit-msg hook rejected: rejected");
Ok(())
}
@ -2276,8 +2260,6 @@ where
tree.walk(git2::TreeWalkMode::PreOrder, |root, entry| {
match callback(root, &entry.clone()) {
TreeWalkResult::Continue => git2::TreeWalkResult::Ok,
// TreeWalkResult::Skip => git2::TreeWalkResult::Skip,
// TreeWalkResult::Stop => git2::TreeWalkResult::Abort,
}
})
.map_err(Into::into)
@ -2285,6 +2267,4 @@ where
enum TreeWalkResult {
Continue,
// Skip,
// Stop,
}