Skip to content

Commit f4c590e

Browse files
committed
refactor(rewrite): create RebaseCommand::Replace
We already have a mechanism for commit replacement for merge commits specifically, to handle cases where we couldn't compute the merge from scratch (and where the result of the merge should be the same anyways). This commit generalizes the mechanism to work for all kinds of commits, so that we can replace arbitrary commits with different trees without reapplying patches. I think this also fixes a bug in merge commits where the parent order was not preserved when conducting a new merge.
1 parent aee8870 commit f4c590e

File tree

12 files changed

+650
-261
lines changed

12 files changed

+650
-261
lines changed

git-branchless-lib/src/core/check_out.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ pub struct CheckOutCommitOptions {
4343
/// Additional arguments to pass to `git checkout`.
4444
pub additional_args: Vec<OsString>,
4545

46+
/// Use `git reset` rather than `git checkout`; that is, leave the index and
47+
/// working copy unchanged, and just adjust the `HEAD` pointer.
48+
pub reset: bool,
49+
4650
/// Whether or not to render the smartlog after the checkout has completed.
4751
pub render_smartlog: bool,
4852
}
@@ -51,6 +55,7 @@ impl Default for CheckOutCommitOptions {
5155
fn default() -> Self {
5256
Self {
5357
additional_args: Default::default(),
58+
reset: false,
5459
render_smartlog: true,
5560
}
5661
}
@@ -104,6 +109,7 @@ pub fn check_out_commit(
104109
) -> eyre::Result<ExitCode> {
105110
let CheckOutCommitOptions {
106111
additional_args,
112+
reset,
107113
render_smartlog,
108114
} = options;
109115

@@ -126,16 +132,25 @@ pub fn check_out_commit(
126132
} else {
127133
target
128134
};
129-
let args = {
135+
136+
if *reset {
137+
if let Some(target) = &target {
138+
let exit_code = git_run_info.run(effects, Some(event_tx_id), &["reset", target])?;
139+
if !exit_code.is_success() {
140+
return Ok(exit_code);
141+
}
142+
}
143+
}
144+
145+
let checkout_args = {
130146
let mut args = vec![OsStr::new("checkout")];
131147
if let Some(target) = &target {
132148
args.push(OsStr::new(target.as_str()));
133149
}
134150
args.extend(additional_args.iter().map(OsStr::new));
135151
args
136152
};
137-
let exit_code = git_run_info.run(effects, Some(event_tx_id), args.as_slice())?;
138-
153+
let exit_code = git_run_info.run(effects, Some(event_tx_id), checkout_args.as_slice())?;
139154
if !exit_code.is_success() {
140155
writeln!(
141156
effects.get_output_stream(),

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

Lines changed: 52 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -422,20 +422,13 @@ mod in_memory {
422422
.iter()
423423
.find_map(|command| match command {
424424
RebaseCommand::Merge {
425-
replacement_commit_oid: None,
426425
commit_oid,
427426
commits_to_merge: _,
428427
} => Some(commit_oid),
429-
RebaseCommand::Merge {
430-
// If we know exactly what tree to use, we don't
431-
// actually have to carry out a merge in memory.
432-
replacement_commit_oid: Some(_),
433-
commit_oid: _,
434-
commits_to_merge: _,
435-
}
436-
| RebaseCommand::CreateLabel { .. }
428+
RebaseCommand::CreateLabel { .. }
437429
| RebaseCommand::Reset { .. }
438430
| RebaseCommand::Pick { .. }
431+
| RebaseCommand::Replace { .. }
439432
| RebaseCommand::Break
440433
| RebaseCommand::RegisterExtraPostRewriteHook
441434
| RebaseCommand::DetectEmptyCommit { .. }
@@ -488,6 +481,7 @@ mod in_memory {
488481
| RebaseCommand::DetectEmptyCommit { .. } => false,
489482
RebaseCommand::Pick { .. }
490483
| RebaseCommand::Merge { .. }
484+
| RebaseCommand::Replace { .. }
491485
| RebaseCommand::SkipUpstreamAppliedCommit { .. } => true,
492486
})
493487
.count();
@@ -629,7 +623,6 @@ mod in_memory {
629623
}
630624

631625
RebaseCommand::Merge {
632-
replacement_commit_oid: None,
633626
commit_oid,
634627
commits_to_merge: _,
635628
} => {
@@ -642,73 +635,78 @@ mod in_memory {
642635
});
643636
}
644637

645-
RebaseCommand::Merge {
646-
replacement_commit_oid: Some(replacement_commit_oid),
638+
RebaseCommand::Replace {
647639
commit_oid,
648-
commits_to_merge,
640+
replacement_commit_oid,
641+
parents,
649642
} => {
650-
let current_commit = repo
651-
.find_commit_or_fail(current_oid)
643+
let original_commit = repo
644+
.find_commit_or_fail(*commit_oid)
652645
.wrap_err("Finding current commit")?;
653-
let commit_to_apply = repo
654-
.find_commit_or_fail(*replacement_commit_oid)
655-
.wrap_err("Finding commit to apply")?;
656-
i += 1;
657-
658-
let commit_description = effects
646+
let original_commit_description = effects
659647
.get_glyphs()
660-
.render(commit_to_apply.friendly_describe(effects.get_glyphs())?)?;
648+
.render(original_commit.friendly_describe(effects.get_glyphs())?)?;
649+
650+
i += 1;
661651
let commit_num = format!("[{}/{}]", i, num_picks);
662652
progress.notify_progress(i, num_picks);
663-
664653
progress.notify_status(
665654
OperationIcon::InProgress,
666-
format!("Applying merge commit: {}", commit_description),
655+
format!("Replacing commit: {}", original_commit_description),
667656
);
668-
let commit = repo.find_commit_or_fail(*replacement_commit_oid)?;
669-
let commit_tree = commit.get_tree()?;
670-
let commit_message = commit_to_apply.get_message_raw()?;
671-
let commit_message = commit_message.to_str().with_context(|| {
672-
eyre::eyre!(
673-
"Could not decode commit message for commit: {:?}",
674-
replacement_commit_oid,
675-
)
676-
})?;
677657

658+
let replacement_commit = repo.find_commit_or_fail(*replacement_commit_oid)?;
659+
let replacement_tree = replacement_commit.get_tree()?;
660+
let replacement_message = replacement_commit.get_message_raw()?;
661+
let replacement_commit_message =
662+
replacement_message.to_str().with_context(|| {
663+
eyre::eyre!(
664+
"Could not decode commit message for replacement commit: {:?}",
665+
replacement_commit
666+
)
667+
})?;
668+
669+
let replacement_commit_description = effects
670+
.get_glyphs()
671+
.render(replacement_commit.friendly_describe(effects.get_glyphs())?)?;
678672
progress.notify_status(
679673
OperationIcon::InProgress,
680-
format!("Committing to repository: {}", commit_description),
674+
format!(
675+
"Committing to repository: {}",
676+
replacement_commit_description
677+
),
681678
);
682679
let committer_signature = if *preserve_timestamps {
683-
commit_to_apply.get_committer()
680+
replacement_commit.get_committer()
684681
} else {
685-
commit_to_apply.get_committer().update_timestamp(*now)?
682+
replacement_commit.get_committer().update_timestamp(*now)?
686683
};
687684
let parents = {
688-
let mut result = vec![current_commit];
689-
for parent in commits_to_merge {
690-
let parent = match parent {
691-
OidOrLabel::Oid(oid) => repo.find_commit_or_fail(*oid)?,
685+
let mut result = Vec::new();
686+
for parent in parents {
687+
let parent_oid = match parent {
688+
OidOrLabel::Oid(oid) => *oid,
692689
OidOrLabel::Label(label) => {
693690
let oid = labels.get(label).ok_or_else(|| {
694691
eyre::eyre!(
695692
"Label {label} could not be resolved to a commit"
696693
)
697694
})?;
698-
repo.find_commit_or_fail(*oid)?
695+
*oid
699696
}
700697
};
701-
result.push(parent);
698+
let parent_commit = repo.find_commit_or_fail(parent_oid)?;
699+
result.push(parent_commit);
702700
}
703701
result
704702
};
705703
let rebased_commit_oid = repo
706704
.create_commit(
707705
None,
708-
&commit_to_apply.get_author(),
706+
&replacement_commit.get_author(),
709707
&committer_signature,
710-
commit_message,
711-
&commit_tree,
708+
replacement_commit_message,
709+
&replacement_tree,
712710
parents.iter().collect(),
713711
)
714712
.wrap_err("Applying rebased commit")?;
@@ -1187,7 +1185,7 @@ pub fn execute_rebase_plan(
11871185
"Attempting rebase in-memory..."
11881186
)?;
11891187

1190-
match rebase_in_memory(effects, repo, rebase_plan, options)? {
1188+
let merge_conflict = match rebase_in_memory(effects, repo, rebase_plan, options)? {
11911189
RebaseInMemoryResult::Succeeded {
11921190
rewritten_oids,
11931191
new_head_oid,
@@ -1229,6 +1227,7 @@ pub fn execute_rebase_plan(
12291227
repo.friendly_describe_commit_from_oid(effects.get_glyphs(), commit_oid)?
12301228
)?,
12311229
)?;
1230+
None
12321231
}
12331232

12341233
RebaseInMemoryResult::MergeConflict(merge_conflict) => {
@@ -1251,8 +1250,9 @@ pub fn execute_rebase_plan(
12511250
repo.friendly_describe_commit_from_oid(effects.get_glyphs(), commit_oid)?
12521251
)?,
12531252
)?;
1253+
Some(merge_conflict)
12541254
}
1255-
}
1255+
};
12561256

12571257
// The rebase has failed at this point, decide whether or not to try
12581258
// again with an on-disk rebase.
@@ -1261,8 +1261,11 @@ pub fn execute_rebase_plan(
12611261
effects.get_output_stream(),
12621262
"Aborting since an in-memory rebase was requested."
12631263
)?;
1264-
return Ok(ExecuteRebasePlanResult::Failed {
1265-
exit_code: ExitCode(1),
1264+
return Ok(match merge_conflict {
1265+
Some(merge_conflict) => ExecuteRebasePlanResult::DeclinedToMerge { merge_conflict },
1266+
None => ExecuteRebasePlanResult::Failed {
1267+
exit_code: ExitCode(1),
1268+
},
12661269
});
12671270
} else {
12681271
writeln!(effects.get_output_stream(), "Trying again on-disk...")?;

0 commit comments

Comments
 (0)