Skip to content

Commit 3423be3

Browse files
temp(repo): Correctly report merge conflicts while amending
Is this needed? Can we just use CherryPickFastError?
1 parent ff752b3 commit 3423be3

File tree

3 files changed

+71
-8
lines changed

3 files changed

+71
-8
lines changed

git-branchless-lib/src/core/rewrite/execute.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,8 @@ mod in_memory {
435435
use crate::core::rewrite::move_branches;
436436
use crate::core::rewrite::plan::{OidOrLabel, RebaseCommand, RebasePlan};
437437
use crate::git::{
438-
AmendFastOptions, CherryPickFastError, CherryPickFastOptions, GitRunInfo, MaybeZeroOid,
439-
NonZeroOid, Repo,
438+
AmendFastError, AmendFastOptions, CherryPickFastError, CherryPickFastOptions, GitRunInfo,
439+
MaybeZeroOid, NonZeroOid, Repo,
440440
};
441441
use crate::util::EyreExitOr;
442442

@@ -651,12 +651,23 @@ mod in_memory {
651651
Err(other) => eyre::bail!(other),
652652
}
653653
} else {
654-
repo.amend_fast(
654+
match repo.amend_fast(
655655
&rebased_commit.expect("rebased commit should not be None"),
656656
&AmendFastOptions::FromCommit {
657657
commit: commit_to_apply,
658658
},
659-
)?
659+
) {
660+
Ok(amended_tree) => amended_tree,
661+
Err(AmendFastError::MergeConflict { conflicting_paths }) => {
662+
return Ok(RebaseInMemoryResult::MergeFailed(
663+
FailedMergeInfo::Conflict {
664+
commit_oid: *commit_oid,
665+
conflicting_paths,
666+
},
667+
))
668+
}
669+
Err(other) => eyre::bail!(other),
670+
}
660671
};
661672

662673
// FIXME should this be the description of this fixup commit or the original commit?

git-branchless-lib/src/git/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub use reference::{
2222
Branch, BranchType, CategorizedReferenceName, Reference, ReferenceName, ReferenceTarget,
2323
};
2424
pub use repo::{
25-
message_prettify, AmendFastOptions, CherryPickFastError, CherryPickFastOptions,
25+
message_prettify, AmendFastError, AmendFastOptions, CherryPickFastError, CherryPickFastOptions,
2626
Error as RepoError, GitVersion, PatchId, Repo, ResolvedReferenceInfo, Result as RepoResult,
2727
Time,
2828
};

git-branchless-lib/src/git/repo.rs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,37 @@ pub enum AmendFastOptions<'repo> {
444444
},
445445
}
446446

447+
/// An error was raised when attempting the `Repo::amend_fast` operation.
448+
#[allow(missing_docs)]
449+
#[derive(Debug, Error)]
450+
pub enum AmendFastError {
451+
/// A merge conflict occurred, so the amend could not continue.
452+
#[error("merge conflict in {} paths", conflicting_paths.len())]
453+
MergeConflict {
454+
/// The paths that were in conflict.
455+
conflicting_paths: HashSet<PathBuf>,
456+
},
457+
458+
#[error("could not get paths touched by commit {commit}")]
459+
GetPatch { commit: NonZeroOid },
460+
461+
#[error("could not get conflicts generated by cherry-pick of {commit} onto {onto}: {source}")]
462+
GetConflicts {
463+
source: git2::Error,
464+
commit: NonZeroOid,
465+
onto: NonZeroOid,
466+
},
467+
468+
#[error(transparent)]
469+
HydrateTree(tree::Error),
470+
471+
#[error(transparent)]
472+
Repo(#[from] Error),
473+
474+
#[error(transparent)]
475+
Git(git2::Error),
476+
}
477+
447478
impl<'repo> AmendFastOptions<'repo> {
448479
/// Returns whether there are any paths to be amended.
449480
pub fn is_empty(&self) -> bool {
@@ -740,6 +771,18 @@ impl Repo {
740771
/// If the commit has more than one parent, returns `None`.
741772
#[instrument]
742773
pub fn get_patch_for_commit(&self, effects: &Effects, commit: &Commit) -> Result<Option<Diff>> {
774+
self.get_patch_for_commit_inner(effects, commit, 3)
775+
}
776+
777+
/// Get a patch for a commit with a specific amount of context.
778+
///
779+
/// If the commit has more than one parent, returns `None`.
780+
fn get_patch_for_commit_inner(
781+
&self,
782+
effects: &Effects,
783+
commit: &Commit,
784+
num_context_lines: usize,
785+
) -> Result<Option<Diff>> {
743786
let changed_paths = match self.get_paths_touched_by_commit(commit)? {
744787
None => return Ok(None),
745788
Some(changed_paths) => changed_paths,
@@ -760,7 +803,12 @@ impl Repo {
760803
None => None,
761804
};
762805
let current_tree = dehydrated_commit.get_tree()?;
763-
let diff = self.get_diff_between_trees(effects, parent_tree.as_ref(), &current_tree, 3)?;
806+
let diff = self.get_diff_between_trees(
807+
effects,
808+
parent_tree.as_ref(),
809+
&current_tree,
810+
num_context_lines,
811+
)?;
764812
Ok(Some(diff))
765813
}
766814

@@ -1425,10 +1473,14 @@ impl Repo {
14251473
/// See `Repo::cherry_pick_fast` for motivation for performing the operation
14261474
/// in-memory.
14271475
#[instrument]
1428-
pub fn amend_fast(&self, parent_commit: &Commit, opts: &AmendFastOptions) -> Result<Tree> {
1476+
pub fn amend_fast(
1477+
&self,
1478+
parent_commit: &Commit,
1479+
opts: &AmendFastOptions,
1480+
) -> std::result::Result<Tree, AmendFastError> {
14291481
let parent_commit_pathbufs = self
14301482
.get_paths_touched_by_commit(parent_commit)?
1431-
.ok_or_else(|| Error::GetPatch {
1483+
.ok_or_else(|| AmendFastError::GetPatch {
14321484
commit: parent_commit.get_oid(),
14331485
})?
14341486
.into_iter()

0 commit comments

Comments
 (0)