Skip to content

Commit 7ab4cdd

Browse files
claytonrcarterclaytonrcarter
committed
feat(rebase): Support squashing commits with RebaseCommand::Pick 
1 parent bd56152 commit 7ab4cdd

File tree

3 files changed

+330
-117
lines changed

3 files changed

+330
-117
lines changed

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

Lines changed: 119 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +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-
CherryPickFastOptions, CreateCommitFastError, GitRunInfo, MaybeZeroOid, NonZeroOid, Repo,
438+
AmendFastOptions, CherryPickFastOptions, CreateCommitFastError, GitRunInfo, MaybeZeroOid,
439+
NonZeroOid, Repo,
439440
};
440441
use crate::util::EyreExitOr;
441442

@@ -546,14 +547,25 @@ mod in_memory {
546547
} => {
547548
current_oid = match labels.get(label_name) {
548549
Some(oid) => *oid,
549-
None => eyre::bail!("BUG: no associated OID for label: {}", label_name),
550+
None => eyre::bail!("BUG: no associated OID for label: {label_name}"),
550551
};
551552
}
552553

553554
RebaseCommand::Reset {
554555
target: OidOrLabel::Oid(commit_oid),
555556
} => {
556-
current_oid = *commit_oid;
557+
current_oid = match rewritten_oids.get(commit_oid) {
558+
Some(MaybeZeroOid::NonZero(rewritten_oid)) => {
559+
// HEAD has been rewritten.
560+
*rewritten_oid
561+
}
562+
Some(MaybeZeroOid::Zero) | None => {
563+
// Either HEAD was not rewritten, or it was but its
564+
// associated commit was skipped. Either way, just
565+
// use the current OID.
566+
*commit_oid
567+
}
568+
};
557569
}
558570

559571
RebaseCommand::Pick {
@@ -563,107 +575,144 @@ mod in_memory {
563575
let current_commit = repo
564576
.find_commit_or_fail(current_oid)
565577
.wrap_err("Finding current commit")?;
566-
let commit_to_apply_oid = match commits_to_apply_oids.as_slice() {
567-
[commit_to_apply_oid] => commit_to_apply_oid,
568-
[..] => unimplemented!("Picking multiple commits"),
569-
};
570-
let commit_to_apply = repo
571-
.find_commit_or_fail(*commit_to_apply_oid)
578+
579+
let original_commit = repo
580+
.find_commit_or_fail(*original_commit_oid)
572581
.wrap_err("Finding commit to apply")?;
573582
i += 1;
574583

575-
let commit_description = effects
576-
.get_glyphs()
577-
.render(commit_to_apply.friendly_describe(effects.get_glyphs())?)?;
578584
let commit_num = format!("[{i}/{num_picks}]");
579585
progress.notify_progress(i, num_picks);
580586

581-
if commit_to_apply.get_parent_count() > 1 {
582-
warn!(
583-
?commit_to_apply_oid,
584-
"BUG: Merge commit should have been detected during planning phase"
585-
);
586-
return Ok(RebaseInMemoryResult::MergeFailed(
587-
FailedMergeInfo::CannotRebaseMergeInMemory {
588-
commit_oid: *commit_to_apply_oid,
589-
},
590-
));
591-
};
592-
593-
progress.notify_status(
594-
OperationIcon::InProgress,
595-
format!("Applying patch for commit: {commit_description}"),
596-
);
597-
let commit_tree = match repo.cherry_pick_fast(
598-
&commit_to_apply,
599-
&current_commit,
600-
&CherryPickFastOptions {
601-
reuse_parent_tree_if_possible: true,
602-
},
603-
) {
604-
Ok(rebased_commit) => rebased_commit,
605-
Err(CreateCommitFastError::MergeConflict { conflicting_paths }) => {
606-
return Ok(RebaseInMemoryResult::MergeFailed(
607-
FailedMergeInfo::Conflict {
608-
commit_oid: *commit_to_apply_oid,
609-
conflicting_paths,
610-
},
611-
))
612-
}
613-
Err(other) => eyre::bail!(other),
614-
};
615-
616-
let commit_message = commit_to_apply.get_message_raw();
587+
let commit_message = original_commit.get_message_raw();
617588
let commit_message = commit_message.to_str().with_context(|| {
618589
eyre::eyre!(
619590
"Could not decode commit message for commit: {:?}",
620-
commit_to_apply_oid
591+
original_commit_oid
621592
)
622593
})?;
623594

624-
progress.notify_status(
625-
OperationIcon::InProgress,
626-
format!("Committing to repository: {commit_description}"),
627-
);
595+
let commit_author = original_commit.get_author();
628596
let committer_signature = if *preserve_timestamps {
629-
commit_to_apply.get_committer()
597+
original_commit.get_committer()
630598
} else {
631-
commit_to_apply.get_committer().update_timestamp(*now)?
599+
original_commit.get_committer().update_timestamp(*now)?
632600
};
633-
let rebased_commit_oid = repo
634-
.create_commit(
635-
None,
636-
&commit_to_apply.get_author(),
637-
&committer_signature,
638-
commit_message,
639-
&commit_tree,
640-
vec![&current_commit],
641-
)
642-
.wrap_err("Applying rebased commit")?;
601+
let mut rebased_commit_oid = None;
602+
let mut rebased_commit = None;
603+
604+
for commit_oid in commits_to_apply_oids.iter() {
605+
let commit_to_apply = repo
606+
.find_commit_or_fail(*commit_oid)
607+
.wrap_err("Finding commit to apply")?;
608+
let commit_description = effects
609+
.get_glyphs()
610+
.render(commit_to_apply.friendly_describe(effects.get_glyphs())?)?;
611+
612+
if commit_to_apply.get_parent_count() > 1 {
613+
warn!(
614+
?commit_oid,
615+
"BUG: Merge commit should have been detected during planning phase"
616+
);
617+
return Ok(RebaseInMemoryResult::MergeFailed(
618+
FailedMergeInfo::CannotRebaseMergeInMemory {
619+
commit_oid: *commit_oid,
620+
},
621+
));
622+
};
623+
624+
progress.notify_status(
625+
OperationIcon::InProgress,
626+
format!("Applying patch for commit: {commit_description}"),
627+
);
628+
629+
// Create a commit and then repeatedly amend & re-create it
630+
// FIXME what #perf gains can be had by working directly on a tree?
631+
// Is it even possible to repeatedly amend a tree and then commit
632+
// it once at the end?
633+
634+
let maybe_tree = if rebased_commit.is_none() {
635+
repo.cherry_pick_fast(
636+
&commit_to_apply,
637+
&current_commit,
638+
&CherryPickFastOptions {
639+
reuse_parent_tree_if_possible: true,
640+
},
641+
)
642+
} else {
643+
repo.amend_fast(
644+
&rebased_commit.expect("rebased commit should not be None"),
645+
&AmendFastOptions::FromCommit {
646+
commit: commit_to_apply,
647+
},
648+
)
649+
};
650+
let commit_tree = match maybe_tree {
651+
Ok(tree) => tree,
652+
Err(CreateCommitFastError::MergeConflict { conflicting_paths }) => {
653+
return Ok(RebaseInMemoryResult::MergeFailed(
654+
FailedMergeInfo::Conflict {
655+
commit_oid: *commit_oid,
656+
conflicting_paths,
657+
},
658+
))
659+
}
660+
Err(other) => eyre::bail!(other),
661+
};
643662

644-
let rebased_commit = repo
645-
.find_commit_or_fail(rebased_commit_oid)
646-
.wrap_err("Looking up just-rebased commit")?;
663+
// this is the description of each fixup commit
664+
// FIXME should we instead be using the description of the base commit?
665+
// or use a different message altogether when squashing multiple commits?
666+
progress.notify_status(
667+
OperationIcon::InProgress,
668+
format!("Committing to repository: {commit_description}"),
669+
);
670+
rebased_commit_oid = Some(
671+
repo.create_commit(
672+
None,
673+
&commit_author,
674+
&committer_signature,
675+
commit_message,
676+
&commit_tree,
677+
vec![&current_commit],
678+
)
679+
.wrap_err("Applying rebased commit")?,
680+
);
681+
682+
rebased_commit = repo.find_commit(rebased_commit_oid.unwrap())?;
683+
}
684+
685+
let rebased_commit_oid =
686+
rebased_commit_oid.expect("rebased oid should not be None");
647687
let commit_description =
648688
effects
649689
.get_glyphs()
650690
.render(repo.friendly_describe_commit_from_oid(
651691
effects.get_glyphs(),
652692
rebased_commit_oid,
653693
)?)?;
654-
if rebased_commit.is_empty() {
694+
695+
if rebased_commit
696+
.expect("rebased commit should not be None")
697+
.is_empty()
698+
{
655699
rewritten_oids.insert(*original_commit_oid, MaybeZeroOid::Zero);
656700
maybe_set_skipped_head_new_oid(*original_commit_oid, current_oid);
657701

658702
writeln!(
659703
effects.get_output_stream(),
660-
"[{i}/{num_picks}] Skipped now-empty commit: {commit_description}"
704+
"{commit_num} Skipped now-empty commit: {commit_description}",
661705
)?;
662706
} else {
663707
rewritten_oids.insert(
664708
*original_commit_oid,
665709
MaybeZeroOid::NonZero(rebased_commit_oid),
666710
);
711+
for commit_oid in commits_to_apply_oids {
712+
rewritten_oids
713+
.insert(*commit_oid, MaybeZeroOid::NonZero(rebased_commit_oid));
714+
}
715+
667716
current_oid = rebased_commit_oid;
668717

669718
writeln!(

0 commit comments

Comments
 (0)