evolution: rewrite orphan resolution as iterator

This commit is contained in:
Martin von Zweigbergk 2021-05-14 06:36:01 -07:00
parent 79b5b8d681
commit a71c56e5e1
3 changed files with 145 additions and 138 deletions

View file

@ -18,6 +18,7 @@ use std::sync::Arc;
use crate::commit::Commit;
use crate::commit_builder::CommitBuilder;
use crate::dag_walk::{bfs, closest_common_node, leaves};
use crate::index::IndexPosition;
use crate::repo::{MutableRepo, ReadonlyRepo, RepoRef};
use crate::repo_path::DirRepoPath;
use crate::rewrite::{merge_commit_trees, rebase_commit};
@ -372,11 +373,6 @@ pub struct ReadonlyEvolution {
state: State,
}
pub trait EvolveListener {
fn orphan_evolved(&mut self, mut_repo: &mut MutableRepo, orphan: &Commit, new_commit: &Commit);
fn orphan_target_ambiguous(&mut self, mut_repo: &mut MutableRepo, orphan: &Commit);
}
impl ReadonlyEvolution {
pub fn new(repo: &ReadonlyRepo) -> ReadonlyEvolution {
ReadonlyEvolution {
@ -497,45 +493,62 @@ pub enum DivergenceResolution {
},
}
pub fn evolve(
user_settings: &UserSettings,
mut_repo: &mut MutableRepo,
listener: &mut dyn EvolveListener,
) {
let orphans_topo_order: Vec<_> = mut_repo
.index()
.topo_order(mut_repo.evolution().state.orphan_commits.iter())
.iter()
.map(|entry| entry.position())
.collect();
pub struct OrphanResolver<'settings> {
user_settings: &'settings UserSettings,
remaining_orphans: Vec<IndexPosition>,
}
let store = mut_repo.store().clone();
for orphan_pos in orphans_topo_order {
let orphan_entry = mut_repo.index().entry_by_pos(orphan_pos);
let mut new_parents = vec![];
let mut ambiguous_new_parents = false;
let evolution = mut_repo.evolution();
for old_parent in orphan_entry.parents() {
let new_parent_candidates =
evolution.new_parent(mut_repo.as_repo_ref(), &old_parent.commit_id());
if new_parent_candidates.len() > 1 {
ambiguous_new_parents = true;
break;
}
new_parents.push(
store
.get_commit(new_parent_candidates.iter().next().unwrap())
.unwrap(),
);
}
let orphan = store.get_commit(&orphan_entry.commit_id()).unwrap();
if ambiguous_new_parents {
listener.orphan_target_ambiguous(mut_repo, &orphan);
} else {
let new_commit = rebase_commit(user_settings, mut_repo, &orphan, &new_parents);
listener.orphan_evolved(mut_repo, &orphan, &new_commit);
impl<'settings> OrphanResolver<'settings> {
pub fn new(user_settings: &'settings UserSettings, mut_repo: &MutableRepo) -> Self {
let mut orphans_topo_order: Vec<_> = mut_repo
.index()
.topo_order(mut_repo.evolution().state.orphan_commits.iter())
.iter()
.map(|entry| entry.position())
.collect();
// Reverse so we can pop then efficiently later
orphans_topo_order.reverse();
OrphanResolver {
user_settings,
remaining_orphans: orphans_topo_order,
}
}
pub fn resolve_next(&mut self, mut_repo: &mut MutableRepo) -> Option<OrphanResolution> {
self.remaining_orphans.pop().map(|orphan_pos| {
let store = mut_repo.store();
let orphan_entry = mut_repo.index().entry_by_pos(orphan_pos);
let mut new_parents = vec![];
let mut ambiguous_new_parents = false;
let evolution = mut_repo.evolution();
for old_parent in orphan_entry.parents() {
let new_parent_candidates =
evolution.new_parent(mut_repo.as_repo_ref(), &old_parent.commit_id());
if new_parent_candidates.len() > 1 {
ambiguous_new_parents = true;
break;
}
new_parents.push(
store
.get_commit(new_parent_candidates.iter().next().unwrap())
.unwrap(),
);
}
let orphan = store.get_commit(&orphan_entry.commit_id()).unwrap();
if ambiguous_new_parents {
OrphanResolution::AmbiguousTarget { orphan }
} else {
let new_commit = rebase_commit(self.user_settings, mut_repo, &orphan, &new_parents);
OrphanResolution::Resolved { orphan, new_commit }
}
})
}
}
#[derive(PartialEq, Eq, Clone, Hash, Debug)]
pub enum OrphanResolution {
Resolved { orphan: Commit, new_commit: Commit },
AmbiguousTarget { orphan: Commit },
}
fn evolve_divergent_change(

View file

@ -16,8 +16,10 @@
use jujube_lib::commit::Commit;
use jujube_lib::commit_builder::CommitBuilder;
use jujube_lib::evolution::{evolve, DivergenceResolution, DivergenceResolver, EvolveListener};
use jujube_lib::repo::{MutableRepo, ReadonlyRepo};
use jujube_lib::evolution::{
DivergenceResolution, DivergenceResolver, OrphanResolution, OrphanResolver,
};
use jujube_lib::repo::ReadonlyRepo;
use jujube_lib::repo_path::FileRepoPath;
use jujube_lib::settings::UserSettings;
use jujube_lib::testutils;
@ -506,35 +508,6 @@ fn test_new_parent_split_forked_pruned(use_git: bool) {
tx.discard();
}
struct RecordingEvolveListener {
evolved_orphans: Vec<(Commit, Commit)>,
}
impl Default for RecordingEvolveListener {
fn default() -> Self {
RecordingEvolveListener {
evolved_orphans: Default::default(),
}
}
}
impl EvolveListener for RecordingEvolveListener {
fn orphan_evolved(
&mut self,
_mut_repo: &mut MutableRepo,
orphan: &Commit,
new_commit: &Commit,
) {
self.evolved_orphans
.push((orphan.clone(), new_commit.clone()));
}
fn orphan_target_ambiguous(&mut self, _mut_repo: &mut MutableRepo, _orphan: &Commit) {
// TODO: Record this too and add tests
panic!("unexpected call to orphan_target_ambiguous");
}
}
#[test_case(false ; "local store")]
// #[test_case(true ; "git store")]
fn test_evolve_orphan(use_git: bool) {
@ -552,16 +525,29 @@ fn test_evolve_orphan(use_git: bool) {
.set_description("rewritten".to_string())
.write_to_repo(mut_repo);
let mut listener = RecordingEvolveListener::default();
evolve(&settings, mut_repo, &mut listener);
assert_eq!(listener.evolved_orphans.len(), 2);
assert_eq!(&listener.evolved_orphans[0].0, &child);
assert_eq!(&listener.evolved_orphans[0].1.parents(), &vec![rewritten]);
assert_eq!(&listener.evolved_orphans[1].0, &grandchild);
assert_eq!(
&listener.evolved_orphans[1].1.parents(),
&vec![listener.evolved_orphans[0].1.clone()]
);
let mut resolver = OrphanResolver::new(&settings, mut_repo);
let resolution1 = resolver.resolve_next(mut_repo);
assert_matches!(resolution1, Some(OrphanResolution::Resolved { .. }));
let resolution2 = resolver.resolve_next(mut_repo);
assert_matches!(resolution2, Some(OrphanResolution::Resolved { .. }));
assert_eq!(resolver.resolve_next(mut_repo), None);
if let Some(OrphanResolution::Resolved {
orphan: orphan1,
new_commit: new_commit1,
}) = resolution1
{
assert_eq!(orphan1, child);
assert_eq!(new_commit1.parents(), vec![rewritten]);
if let Some(OrphanResolution::Resolved {
orphan: orphan2,
new_commit: new_commit2,
}) = resolution2
{
assert_eq!(orphan2, grandchild);
assert_eq!(new_commit2.parents(), vec![new_commit1.clone()]);
}
}
tx.discard();
}
@ -586,10 +572,17 @@ fn test_evolve_pruned_orphan(use_git: bool) {
.set_description("rewritten".to_string())
.write_to_repo(mut_repo);
let mut listener = RecordingEvolveListener::default();
evolve(&settings, mut_repo, &mut listener);
assert_eq!(listener.evolved_orphans.len(), 1);
assert_eq!(listener.evolved_orphans[0].0.id(), child.id());
let mut resolver = OrphanResolver::new(&settings, mut_repo);
let resolution = resolver.resolve_next(mut_repo);
assert_matches!(resolution, Some(OrphanResolution::Resolved { .. }));
assert_eq!(resolver.resolve_next(mut_repo), None);
if let Some(OrphanResolution::Resolved {
orphan,
new_commit: _,
}) = resolution
{
assert_eq!(orphan, child);
}
tx.discard();
}
@ -612,21 +605,39 @@ fn test_evolve_multiple_orphans(use_git: bool) {
.set_description("rewritten".to_string())
.write_to_repo(mut_repo);
let mut listener = RecordingEvolveListener::default();
evolve(&settings, mut_repo, &mut listener);
assert_eq!(listener.evolved_orphans.len(), 3);
assert_eq!(&listener.evolved_orphans[0].0, &child);
assert_eq!(&listener.evolved_orphans[0].1.parents(), &vec![rewritten]);
assert_eq!(&listener.evolved_orphans[1].0, &grandchild);
assert_eq!(
&listener.evolved_orphans[1].1.parents(),
&vec![listener.evolved_orphans[0].1.clone()]
);
assert_eq!(&listener.evolved_orphans[2].0, &grandchild2);
assert_eq!(
&listener.evolved_orphans[2].1.parents(),
&vec![listener.evolved_orphans[0].1.clone()]
);
let mut resolver = OrphanResolver::new(&settings, mut_repo);
let resolution1 = resolver.resolve_next(mut_repo);
assert_matches!(resolution1, Some(OrphanResolution::Resolved { .. }));
let resolution2 = resolver.resolve_next(mut_repo);
assert_matches!(resolution2, Some(OrphanResolution::Resolved { .. }));
let resolution3 = resolver.resolve_next(mut_repo);
assert_matches!(resolution3, Some(OrphanResolution::Resolved { .. }));
assert_eq!(resolver.resolve_next(mut_repo), None);
if let Some(OrphanResolution::Resolved {
orphan: orphan1,
new_commit: new_commit1,
}) = resolution1
{
assert_eq!(orphan1, child);
assert_eq!(new_commit1.parents(), vec![rewritten]);
if let Some(OrphanResolution::Resolved {
orphan: orphan2,
new_commit: new_commit2,
}) = resolution2
{
assert_eq!(orphan2, grandchild);
assert_eq!(new_commit2.parents(), vec![new_commit1.clone()]);
if let Some(OrphanResolution::Resolved {
orphan: orphan3,
new_commit: new_commit3,
}) = resolution3
{
assert_eq!(orphan3, grandchild2);
assert_eq!(new_commit3.parents(), vec![new_commit1.clone()]);
}
}
}
tx.discard();
}

View file

@ -31,7 +31,9 @@ use criterion::Criterion;
use jujube_lib::commit::Commit;
use jujube_lib::commit_builder::CommitBuilder;
use jujube_lib::dag_walk::topo_order_reverse;
use jujube_lib::evolution::{evolve, DivergenceResolution, DivergenceResolver, EvolveListener};
use jujube_lib::evolution::{
DivergenceResolution, DivergenceResolver, OrphanResolution, OrphanResolver,
};
use jujube_lib::files::DiffLine;
use jujube_lib::git::GitFetchError;
use jujube_lib::index::HexPrefix;
@ -1801,44 +1803,25 @@ fn cmd_evolve<'s>(
}
}
struct Listener<'a, 's> {
ui: &'a mut Ui<'s>,
}
// TODO: Handle errors that happen in this listener. Should the listener take a
// type parameter for the error type? Should the evolve operation be aborted
// if the listener returns an error? Maybe rewrite it as an iterator?
impl<'a, 's> EvolveListener for Listener<'a, 's> {
fn orphan_evolved(
&mut self,
mut_repo: &mut MutableRepo,
orphan: &Commit,
new_commit: &Commit,
) {
self.ui.write("Resolving orphan: ").unwrap();
self.ui
.write_commit_summary(mut_repo.as_repo_ref(), &orphan)
.unwrap();
self.ui.write("\n").unwrap();
self.ui.write("Resolved as: ").unwrap();
self.ui
.write_commit_summary(mut_repo.as_repo_ref(), &new_commit)
.unwrap();
self.ui.write("\n").unwrap();
}
fn orphan_target_ambiguous(&mut self, mut_repo: &mut MutableRepo, orphan: &Commit) {
self.ui
.write("Skipping orphan with ambiguous new parents: ")
.unwrap();
self.ui
.write_commit_summary(mut_repo.as_repo_ref(), &orphan)
.unwrap();
self.ui.write("\n").unwrap();
let mut orphan_resolver = OrphanResolver::new(&user_settings, mut_repo);
while let Some(resolution) = orphan_resolver.resolve_next(mut_repo) {
match resolution {
OrphanResolution::Resolved { orphan, new_commit } => {
ui.write("Resolving orphan: ")?;
ui.write_commit_summary(mut_repo.as_repo_ref(), &orphan)?;
ui.write("\n")?;
ui.write("Resolved as: ")?;
ui.write_commit_summary(mut_repo.as_repo_ref(), &new_commit)?;
ui.write("\n")?;
}
OrphanResolution::AmbiguousTarget { orphan } => {
ui.write("Skipping orphan with ambiguous new parents: ")?;
ui.write_commit_summary(mut_repo.as_repo_ref(), &orphan)?;
ui.write("\n")?;
}
}
}
let mut listener = Listener { ui };
evolve(&user_settings, tx.mut_repo(), &mut listener);
repo_command.finish_transaction(ui, tx)?;
Ok(())