diff --git a/crates/but-core/src/repo_ext.rs b/crates/but-core/src/repo_ext.rs index b8d8cac097e..4a641d8a975 100644 --- a/crates/but-core/src/repo_ext.rs +++ b/crates/but-core/src/repo_ext.rs @@ -139,6 +139,13 @@ pub trait RepositoryExt: Sized { /// to favor ours, both when dealing with content merges and with tree merges. fn merge_options_force_ours(&self) -> anyhow::Result; + /// Tree merge options that enforce undecidable file/content conflicts to be + /// forcefully resolved to favor theirs. + /// + /// `gix` does not currently expose a tree-level `Theirs` mode, so tree + /// conflicts keep the default tree behavior. + fn merge_options_force_theirs(&self) -> anyhow::Result; + /// Return options suitable for merging so that the merge stops immediately after the first conflict. /// It also returns the conflict kind to use when checking for unresolved conflicts. fn merge_options_fail_fast( @@ -449,6 +456,12 @@ impl RepositoryExt for gix::Repository { .with_file_favor(Some(gix::merge::tree::FileFavor::Ours))) } + fn merge_options_force_theirs(&self) -> anyhow::Result { + Ok(self + .tree_merge_options()? + .with_file_favor(Some(gix::merge::tree::FileFavor::Theirs))) + } + fn merge_options_fail_fast( &self, ) -> anyhow::Result<(gix::merge::tree::Options, TreatAsUnresolved)> { diff --git a/crates/but-rebase/src/graph_rebase/cherry_pick.rs b/crates/but-rebase/src/graph_rebase/cherry_pick.rs index fac0b612925..ab413349c52 100644 --- a/crates/but-rebase/src/graph_rebase/cherry_pick.rs +++ b/crates/but-rebase/src/graph_rebase/cherry_pick.rs @@ -145,8 +145,52 @@ pub fn cherry_pick( } } +/// Merge `theirs_tree` into `ours_tree` using `base_tree` as merge base and +/// write the result as a commit that preserves `target` metadata. +pub(crate) fn merge_trees_into_target_commit( + repo: &gix::Repository, + parents: &[gix::ObjectId], + target: gix::ObjectId, + base_tree: gix::ObjectId, + ours_tree: gix::ObjectId, + theirs_tree: gix::ObjectId, + sign_if_configured: bool, +) -> Result { + let target = but_core::Commit::from_id(target.attach(repo))?; + let mut outcome = repo.merge_trees( + base_tree, + ours_tree, + theirs_tree, + repo.default_merge_labels(), + repo.merge_options_force_ours()?, + )?; + let tree_id = outcome.tree.write()?; + + let conflict_kind = gix::merge::tree::TreatAsUnresolved::forced_resolution(); + if outcome.has_unresolved_conflicts(conflict_kind) { + let conflicted_commit = commit_from_conflicted_tree( + parents, + target, + tree_id, + outcome, + conflict_kind, + base_tree, + ours_tree, + theirs_tree, + sign_if_configured, + )?; + Ok(CherryPickOutcome::ConflictedCommit( + conflicted_commit.detach(), + )) + } else { + Ok(CherryPickOutcome::Commit( + commit_from_unconflicted_tree(parents, target, tree_id, sign_if_configured)?.detach(), + )) + } +} + #[derive(Debug, Clone)] -enum MergeOutcome { +pub(crate) enum MergeOutcome { Success(gix::ObjectId), NoCommit, Conflict { @@ -156,7 +200,7 @@ enum MergeOutcome { } impl MergeOutcome { - fn object_id(&self) -> Option { + pub(crate) fn object_id(&self) -> Option { match self { Self::Success(oid) => Some(*oid), _ => None, @@ -164,6 +208,13 @@ impl MergeOutcome { } } +pub(crate) fn auto_resolution_tree_from_merging_commits( + repo: &gix::Repository, + commits: &[gix::ObjectId], +) -> Result { + tree_from_merging_commits(repo, commits, TreeKind::AutoResolution) +} + fn find_base_tree(target: &but_core::Commit) -> Result { if target.is_conflicted() { Ok(MergeOutcome::Success( diff --git a/crates/but-rebase/src/graph_rebase/materialize.rs b/crates/but-rebase/src/graph_rebase/materialize.rs index e4242b56b51..ff5e07b63a1 100644 --- a/crates/but-rebase/src/graph_rebase/materialize.rs +++ b/crates/but-rebase/src/graph_rebase/materialize.rs @@ -28,6 +28,9 @@ impl<'ws, 'graph, M: RefMetadata> SuccessfulRebase<'ws, 'graph, M> { let new_head = match step { Step::None => bail!("Checkout selector is pointing to none"), + Step::PickDivergent(_) => { + bail!("Checkout selector is pointing to an unresolved PickDivergent") + } Step::Pick(Pick { id, .. }) => id, Step::Reference { .. } => { let parents = collect_ordered_parents(&self.graph, selector.id); diff --git a/crates/but-rebase/src/graph_rebase/mod.rs b/crates/but-rebase/src/graph_rebase/mod.rs index b0cbde30d41..b8c3bed4257 100644 --- a/crates/but-rebase/src/graph_rebase/mod.rs +++ b/crates/but-rebase/src/graph_rebase/mod.rs @@ -84,11 +84,52 @@ impl Pick { } } +/// Represents a divergent-change resolution step. +/// +/// Each remote family member gets one `PickDivergent` step. Steps with the +/// same `family_id` share `local_commits` and `ancestor` and are processed +/// in normal graph traversal order. After resolution, each step is lowered +/// into an ordinary `Pick` in the output graph. +#[derive(Debug, Clone, PartialEq)] +pub struct PickDivergent { + /// The ordered local commits (parentmost → childmost) that form the + /// local side of the divergence. Identical across all steps in a family. + pub local_commits: Vec, + /// The optional divergence ancestor (common base before local/remote + /// diverged). Identical across all steps in a family. + pub ancestor: Option, + /// The remote commit that occupies this position in the family. + pub remote_commit: gix::ObjectId, + /// Opaque identifier that groups related remote commits into a single + /// family. All steps sharing this value are processed together. + pub family_id: [u8; 20], +} + +impl PickDivergent { + /// Creates a `PickDivergent` with the given parameters. + pub fn new( + local_commits: Vec, + ancestor: Option, + remote_commit: gix::ObjectId, + family_id: [u8; 20], + ) -> Self { + Self { + local_commits, + ancestor, + remote_commit, + family_id, + } + } +} + /// Describes what action the engine should take #[derive(Debug, Clone, PartialEq)] pub enum Step { /// Cherry picks the given commit into the new location in the graph Pick(Pick), + /// Resolves one position of a divergent local/remote change. See + /// [`PickDivergent`] for details. + PickDivergent(PickDivergent), /// Represents applying a reference to the commit found at it's first parent Reference { /// The refname @@ -111,6 +152,21 @@ impl Step { pub fn new_untracked_pick(id: gix::ObjectId) -> Self { Self::Pick(Pick::new_untracked_pick(id)) } + + /// Creates a `PickDivergent` step. + pub fn new_pick_divergent( + local_commits: Vec, + ancestor: Option, + remote_commit: gix::ObjectId, + family_id: [u8; 20], + ) -> Self { + Self::PickDivergent(PickDivergent::new( + local_commits, + ancestor, + remote_commit, + family_id, + )) + } } /// Used to represent a connection between a given commit. diff --git a/crates/but-rebase/src/graph_rebase/rebase.rs b/crates/but-rebase/src/graph_rebase/rebase.rs index 962ae43a0b0..72794fc639b 100644 --- a/crates/but-rebase/src/graph_rebase/rebase.rs +++ b/crates/but-rebase/src/graph_rebase/rebase.rs @@ -18,6 +18,59 @@ use crate::graph_rebase::{ cherry_pick::{CherryPickOutcome, cherry_pick}, util::collect_ordered_parents, }; +mod pick_divergent; + +fn resolved_cherry_pick_id( + target: gix::ObjectId, + outcome: CherryPickOutcome, +) -> Result { + match outcome { + CherryPickOutcome::Commit(new_id) + | CherryPickOutcome::ConflictedCommit(new_id) + | CherryPickOutcome::Identity(new_id) => Ok(new_id), + CherryPickOutcome::FailedToMergeBases { + base_merge_failed, + bases, + onto_merge_failed, + ontos, + } => { + bail!(format_base_merge_error( + target, + base_merge_failed, + bases, + onto_merge_failed, + ontos + )); + } + } +} + +fn output_pick_id( + output_graph: &StepGraph, + graph_mapping: &HashMap, + graph_parent: StepGraphIndex, +) -> Result { + let Some(new_idx) = graph_mapping.get(&graph_parent) else { + bail!("A matching parent can't be found in the output graph"); + }; + + match output_graph[*new_idx] { + Step::Pick(Pick { id, .. }) => Ok(id), + _ => bail!("A parent in the output graph is not a pick"), + } +} + +fn collect_output_pick_parents( + graph: &StepGraph, + output_graph: &StepGraph, + graph_mapping: &HashMap, + step_idx: StepGraphIndex, +) -> Result> { + collect_ordered_parents(graph, step_idx) + .into_iter() + .map(|graph_parent| output_pick_id(output_graph, graph_mapping, graph_parent)) + .collect() +} impl<'ws, 'graph, M: RefMetadata> Editor<'ws, 'graph, M> { /// Perform the rebase @@ -44,7 +97,7 @@ impl<'ws, 'graph, M: RefMetadata> Editor<'ws, 'graph, M> { // The step graph with updated commit oids let mut output_graph = StepGraph::new(); let mut unchanged_references = vec![]; - + let mut divergent_state = pick_divergent::State::new(&self.graph, &steps_to_pick); let mut history = self.history; for step_idx in steps_to_pick { @@ -52,22 +105,14 @@ impl<'ws, 'graph, M: RefMetadata> Editor<'ws, 'graph, M> { let step = self.graph[step_idx].clone(); let new_idx = match step { Step::Pick(pick) => { - let graph_parents = collect_ordered_parents(&self.graph, step_idx); let ontos = match pick.preserved_parents.clone() { Some(ontos) => ontos, - None => graph_parents - .iter() - .map(|idx| { - let Some(new_idx) = graph_mapping.get(idx) else { - bail!("A matching parent can't be found in the output graph"); - }; - - match output_graph[*new_idx] { - Step::Pick(Pick { id, .. }) => Ok(id), - _ => bail!("A parent in the output graph is not a pick"), - } - }) - .collect::>>()?, + None => collect_output_pick_parents( + &self.graph, + &output_graph, + &graph_mapping, + step_idx, + )?, }; let outcome = @@ -82,50 +127,27 @@ impl<'ws, 'graph, M: RefMetadata> Editor<'ws, 'graph, M> { ); } - match outcome { - CherryPickOutcome::Commit(new_id) - | CherryPickOutcome::ConflictedCommit(new_id) - | CherryPickOutcome::Identity(new_id) => { - let mut new_pick = pick.clone(); - new_pick.id = new_id; - let new_idx = output_graph.add_node(Step::Pick(new_pick)); - graph_mapping.insert(step_idx, new_idx); - if !pick.exclude_from_tracking { - history.update_mapping(pick.id, new_id); - } - - new_idx - } - CherryPickOutcome::FailedToMergeBases { - base_merge_failed, - bases, - onto_merge_failed, - ontos, - } => { - // Exit early - the rebase failed because it encountered a commit it couldn't pick - bail!(format_base_merge_error( - pick.id, - base_merge_failed, - bases, - onto_merge_failed, - ontos - )); - } + let new_id = resolved_cherry_pick_id(pick.id, outcome)?; + let mut new_pick = pick.clone(); + new_pick.id = new_id; + let new_idx = output_graph.add_node(Step::Pick(new_pick)); + graph_mapping.insert(step_idx, new_idx); + if !pick.exclude_from_tracking { + history.update_mapping(pick.id, new_id); } + + new_idx } Step::Reference { refname } => { - let graph_parents = collect_ordered_parents(&self.graph, step_idx); - let first_parent_idx = graph_parents - .first() - .context("References should have at least one parent")?; - let Some(new_idx) = graph_mapping.get(first_parent_idx) else { - bail!("A matching parent can't be found in the output graph"); - }; - - let to_reference = match output_graph[*new_idx] { - Step::Pick(Pick { id, .. }) => id, - _ => bail!("A parent in the output graph is not a pick"), - }; + let to_reference = collect_output_pick_parents( + &self.graph, + &output_graph, + &graph_mapping, + step_idx, + )? + .into_iter() + .next() + .context("References should have at least one parent")?; let reference = self.repo.try_find_reference(&refname)?; @@ -167,6 +189,19 @@ impl<'ws, 'graph, M: RefMetadata> Editor<'ws, 'graph, M> { output_graph.add_node(Step::Reference { refname }) } + Step::PickDivergent(ref pick_divergent) => { + let ontos = collect_output_pick_parents( + &self.graph, + &output_graph, + &graph_mapping, + step_idx, + )?; + let resolved_id = + divergent_state.rebase_step(&self.repo, &ontos, pick_divergent)?; + let new_idx = output_graph.add_node(Step::Pick(Pick::new_pick(resolved_id))); + history.update_mapping(pick_divergent.remote_commit, resolved_id); + new_idx + } Step::None => output_graph.add_node(Step::None), }; @@ -256,7 +291,7 @@ pub(crate) fn all_parents_are_references(graph: &StepGraph, target: StepGraphInd match graph[candidate.target()] { // We encountered a `pick` step before a `reference` step so we can // stop the search and return false early. - Step::Pick(_) => return false, + Step::Pick(_) | Step::PickDivergent(_) => return false, // We can stop searching down this leg since we found a reference. Step::Reference { .. } => continue, // Skip over `None`s and consider their parents. @@ -577,6 +612,70 @@ mod test { } } + mod pick_divergent_ordering { + use std::str::FromStr; + + use anyhow::Result; + + use crate::graph_rebase::{ + Edge, Step, StepGraph, rebase::order_steps_picking, testing::render_ascii_graph, + }; + + /// Given a graph where two divergent remote commits (R1, R2) from the + /// same family sit on top of a normal `Pick` base: + /// + /// ```text + /// Before (graph edges, child -> parent): + /// R2 (cc00000, divergent) -> R1 (bb00000, divergent) -> Base (aa00000, pick) + /// + /// Expected ordering (parentmost first): + /// [Base, R1, R2] + /// ``` + /// + /// The ordering algorithm must emit the base pick first, then walk + /// the divergent family from parentmost (R1) to childmost (R2). + #[test] + fn family_of_two_orders_correctly() -> Result<()> { + let base_id = gix::ObjectId::from_str("aa00000000000000000000000000000000000000")?; + let r1_id = gix::ObjectId::from_str("bb00000000000000000000000000000000000000")?; + let r2_id = gix::ObjectId::from_str("cc00000000000000000000000000000000000000")?; + let l1_id = gix::ObjectId::from_str("dd00000000000000000000000000000000000000")?; + let family_id = [0xFFu8; 20]; + + let mut graph = StepGraph::new(); + let base = graph.add_node(Step::new_pick(base_id)); + let r1 = graph.add_node(Step::new_pick_divergent( + vec![l1_id], + None, + r1_id, + family_id, + )); + let r2 = graph.add_node(Step::new_pick_divergent( + vec![l1_id], + None, + r2_id, + family_id, + )); + + // r2 is the head (childmost), r1 is its parent, base is r1's parent + graph.add_edge(r2, r1, Edge { order: 0 }); + graph.add_edge(r1, base, Edge { order: 0 }); + + insta::assert_snapshot!(render_ascii_graph(&graph, |_| None), @" + ● cc00000 [divergent] + ● bb00000 [divergent] + ● aa00000 + ╵ + "); + + let ordered = order_steps_picking(&graph, &[r2]); + // base first, then r1 (parentmost family member), then r2 (childmost) + assert_eq!(&ordered, &[base, r1, r2]); + + Ok(()) + } + } + mod all_parents_are_references { use std::str::FromStr; diff --git a/crates/but-rebase/src/graph_rebase/rebase/pick_divergent.rs b/crates/but-rebase/src/graph_rebase/rebase/pick_divergent.rs new file mode 100644 index 00000000000..c10aadd7c87 --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/rebase/pick_divergent.rs @@ -0,0 +1,793 @@ +use std::collections::{HashMap, HashSet, VecDeque}; + +use anyhow::{Context, Result, bail}; +use bstr::{BString, ByteSlice as _}; +use but_core::RepositoryExt as _; +use but_core::commit::TreeKind; +use but_core::{HunkHeader, TreeChange, TreeStatus, UnifiedPatch}; +use gix::prelude::ObjectIdExt as _; + +use crate::graph_rebase::{ + PickDivergent, Step, StepGraph, StepGraphIndex, + cherry_pick::{ + CherryPickOutcome, MergeOutcome, auto_resolution_tree_from_merging_commits, cherry_pick, + merge_trees_into_target_commit, + }, +}; + +use super::resolved_cherry_pick_id; + +pub(super) struct State { + families: HashMap<[u8; 20], DivergentFamilyState>, + family_sizes: HashMap<[u8; 20], usize>, +} + +impl State { + pub(super) fn new(graph: &StepGraph, steps_to_pick: &VecDeque) -> Self { + let mut family_sizes = HashMap::new(); + for step_idx in steps_to_pick { + if let Step::PickDivergent(pick_divergent) = &graph[*step_idx] { + *family_sizes.entry(pick_divergent.family_id).or_insert(0) += 1; + } + } + + Self { + families: HashMap::new(), + family_sizes, + } + } + + pub(super) fn rebase_step( + &mut self, + repo: &gix::Repository, + ontos: &[gix::ObjectId], + pick_divergent: &PickDivergent, + ) -> Result { + let family_state = self + .families + .entry(pick_divergent.family_id) + .or_insert_with(DivergentFamilyState::new); + family_state.advance_member(); + + // Attempt the remote materialization before enforcing the current + // single-output-parent restriction so merge-parent failures still + // surface through the existing graph cherry-pick error path. + let remote_id = resolved_cherry_pick_id( + pick_divergent.remote_commit, + // TODO(CTO): This should tie into signing settings better. This may + // not be used in the final tree either which would be an extra + // signing that is not required. + cherry_pick(repo, pick_divergent.remote_commit, ontos, true)?, + )?; + + let onto_tree = rebase_base_tree(repo, ontos)?; + let local_tree = + family_state.current_local_tree(repo, onto_tree, &pick_divergent.local_commits)?; + + let merge_base_tree = match pick_divergent.ancestor { + Some(ancestor) => rebase_divergence_ancestor_tree(repo, onto_tree, ancestor)?, + None => onto_tree, + }; + let remote_tree = commit_tree(repo, remote_id)?; + let is_final_family_member = family_state.is_final_member( + *self + .family_sizes + .get(&pick_divergent.family_id) + .expect("family sizes are precomputed for all PickDivergent steps"), + ); + + let partitioned_pool = if is_final_family_member { + PartitionedHunkPool { + matching_tree: local_tree, + remainder_tree: onto_tree, + overlaps_remote: local_tree != onto_tree, + } + } else { + partition_local_hunk_pool(repo, onto_tree, remote_tree, local_tree)? + }; + + let resolved_id = if !partitioned_pool.overlaps_remote + || remote_tree == partitioned_pool.matching_tree + { + remote_id + } else { + match merge_trees_into_target_commit( + repo, + ontos, + pick_divergent.remote_commit, + merge_base_tree, + remote_tree, + partitioned_pool.matching_tree, + true, + )? { + CherryPickOutcome::Commit(new_id) + | CherryPickOutcome::ConflictedCommit(new_id) + | CherryPickOutcome::Identity(new_id) => new_id, + CherryPickOutcome::FailedToMergeBases { .. } => { + bail!( + "BUG: merge_trees_into_target_commit unexpectedly failed for PickDivergent" + ); + } + } + }; + + if !is_final_family_member { + family_state.store_remainder(onto_tree, partitioned_pool.remainder_tree); + } + + Ok(resolved_id) + } +} + +/// Mutable state shared across all [`PickDivergent`] steps that belong to the +/// same family (identified by `family_id`) during a single rebase execution. +/// +/// The struct is created when the first family member is encountered and +/// carried forward as subsequent members are processed in graph traversal +/// order. Future passes will populate the optional fields to drive the +/// divergent-change combining algorithm. +#[derive(Debug)] +enum DivergentHunkPool { + Uninitialized, + Tree { + base_tree: gix::ObjectId, + tree: gix::ObjectId, + }, + Empty, +} + +#[derive(Debug)] +struct DivergentFamilyState { + /// How many family members have been processed so far. + members_processed: usize, + /// The carry-forward hunk-pool tree together with the tree it + /// is currently based on. + hunk_pool: DivergentHunkPool, +} + +impl DivergentFamilyState { + fn new() -> Self { + Self { + members_processed: 0, + hunk_pool: DivergentHunkPool::Uninitialized, + } + } + + fn advance_member(&mut self) { + self.members_processed += 1; + } + + fn is_final_member(&self, family_size: usize) -> bool { + self.members_processed == family_size + } + + fn current_local_tree( + &mut self, + repo: &gix::Repository, + onto_tree: gix::ObjectId, + local_commits: &[gix::ObjectId], + ) -> Result { + match self.hunk_pool { + DivergentHunkPool::Tree { base_tree, tree } => { + let tree = rebase_hunk_pool_tree(repo, onto_tree, base_tree, tree)?; + self.hunk_pool = DivergentHunkPool::Tree { + base_tree: onto_tree, + tree, + }; + Ok(tree) + } + DivergentHunkPool::Empty => Ok(onto_tree), + DivergentHunkPool::Uninitialized => { + let local_tree = flatten_local_sequence_once(repo, onto_tree, local_commits)?; + self.hunk_pool = DivergentHunkPool::Tree { + base_tree: onto_tree, + tree: local_tree, + }; + Ok(local_tree) + } + } + } + + fn store_remainder(&mut self, onto_tree: gix::ObjectId, remainder_tree: gix::ObjectId) { + self.hunk_pool = if remainder_tree == onto_tree { + DivergentHunkPool::Empty + } else { + DivergentHunkPool::Tree { + base_tree: onto_tree, + tree: remainder_tree, + } + }; + } +} + +fn flatten_local_sequence_once( + repo: &gix::Repository, + onto_tree: gix::ObjectId, + local_commits: &[gix::ObjectId], +) -> Result { + let mut local_cursor_tree = onto_tree; + for local_commit in local_commits { + local_cursor_tree = + replay_local_commit_preferring_local_result(repo, local_cursor_tree, *local_commit)?; + } + + Ok(local_cursor_tree) +} + +fn replay_local_commit_preferring_local_result( + repo: &gix::Repository, + onto_tree: gix::ObjectId, + local_commit: gix::ObjectId, +) -> Result { + let local_commit = but_core::Commit::from_id(local_commit.attach(repo))?; + + if local_commit.parents.len() > 1 { + bail!("Cannot yet flatten merge commits into the divergent hunk pool"); + } + + let (base_tree, local_tree) = find_local_replay_trees(&local_commit)?; + let mut replay = repo.merge_trees( + base_tree, + onto_tree.attach(repo), + local_tree, + repo.default_merge_labels(), + repo.merge_options_force_theirs()?, + )?; + Ok(replay.tree.write()?.detach()) +} + +fn find_local_replay_trees<'repo>( + local_commit: &but_core::Commit<'repo>, +) -> Result<(gix::Id<'repo>, gix::Id<'repo>)> { + let repo = local_commit.id.repo; + let base_tree = if local_commit.is_conflicted() { + find_commit_tree(local_commit, TreeKind::Base)? + } else { + let base_commit_id = local_commit.parents.first().context("no parent")?; + let base_commit = but_core::Commit::from_id(base_commit_id.attach(repo))?; + find_commit_tree(&base_commit, TreeKind::AutoResolution)? + }; + + Ok((base_tree, find_commit_tree(local_commit, TreeKind::Theirs)?)) +} + +fn find_commit_tree<'repo>( + commit: &but_core::Commit<'repo>, + side: TreeKind, +) -> Result> { + Ok(if commit.is_conflicted() { + let tree = commit.id.repo.find_tree(commit.tree)?; + tree.find_entry(side.as_tree_entry_name()) + .context("Failed to get conflicted side of commit")? + .id() + } else { + commit.tree_id_or_auto_resolution()? + }) +} + +fn commit_tree(repo: &gix::Repository, commit: gix::ObjectId) -> Result { + Ok(but_core::Commit::from_id(commit.attach(repo))? + .tree_id_or_auto_resolution()? + .detach()) +} + +#[derive(Debug)] +struct PartitionedHunkPool { + matching_tree: gix::ObjectId, + remainder_tree: gix::ObjectId, + overlaps_remote: bool, +} + +fn partition_local_hunk_pool( + repo: &gix::Repository, + base: gix::ObjectId, + remote: gix::ObjectId, + local: gix::ObjectId, +) -> Result { + let remote_changes = tree_changes_with_rewrites(repo, base, remote)?; + let local_changes = tree_changes_with_rewrites(repo, base, local)?; + let forced_full_overlap_additions = + split_rename_addition_overlap_paths(&remote_changes, &local_changes); + let mut matching_tree = repo.edit_tree(base)?; + let mut remainder_tree = repo.edit_tree(base)?; + let mut overlaps_remote = false; + + for local_change in local_changes { + let overlap = if forced_full_overlap_additions.contains(&local_change.path) { + LocalChangeOverlap::Full + } else { + classify_local_change_overlap(repo, &remote_changes, &local_change)? + }; + match overlap { + LocalChangeOverlap::None => { + apply_whole_tree_change(&mut remainder_tree, &local_change)?; + } + LocalChangeOverlap::Full => { + overlaps_remote = true; + apply_whole_tree_change(&mut matching_tree, &local_change)?; + } + LocalChangeOverlap::Partial { + matching_hunks, + remainder_hunks, + } => { + overlaps_remote = true; + apply_selected_hunks(repo, &mut matching_tree, &local_change, &matching_hunks)?; + apply_selected_hunks(repo, &mut remainder_tree, &local_change, &remainder_hunks)?; + } + } + } + + Ok(PartitionedHunkPool { + matching_tree: matching_tree.write()?.detach(), + remainder_tree: remainder_tree.write()?.detach(), + overlaps_remote, + }) +} + +fn tree_changes_with_rewrites( + repo: &gix::Repository, + base: gix::ObjectId, + tree: gix::ObjectId, +) -> Result> { + let base_tree = repo.find_tree(base)?; + let tree = repo.find_tree(tree)?; + let options = gix::diff::Options::default().with_rewrites(Some(gix::diff::Rewrites::default())); + let mut changes: Vec<_> = repo + .diff_tree_to_tree(Some(&base_tree), Some(&tree), Some(options))? + .into_iter() + .filter(|change| !change.entry_mode().is_tree()) + .map(Into::into) + .collect(); + changes.sort_by(|a: &TreeChange, b: &TreeChange| a.path.cmp(&b.path)); + Ok(changes) +} + +fn split_rename_addition_overlap_paths( + remote_changes: &[TreeChange], + local_changes: &[TreeChange], +) -> HashSet { + let Some(remote_deleted_path) = only_matching_path(remote_changes, deletion_path) else { + return HashSet::new(); + }; + let Some(remote_added_path) = only_matching_path(remote_changes, addition_path) else { + return HashSet::new(); + }; + let Some(local_deleted_path) = only_matching_path(local_changes, deletion_path) else { + return HashSet::new(); + }; + let Some(local_added_path) = only_matching_path(local_changes, addition_path) else { + return HashSet::new(); + }; + + if remote_deleted_path == local_deleted_path && remote_added_path != local_added_path { + HashSet::from([local_added_path.to_owned()]) + } else { + HashSet::new() + } +} + +fn only_matching_path<'a, F>(changes: &'a [TreeChange], matcher: F) -> Option<&'a bstr::BStr> +where + F: Fn(&'a TreeChange) -> Option<&'a bstr::BStr>, +{ + let mut matches = changes.iter().filter_map(matcher); + let first = matches.next()?; + matches.next().is_none().then_some(first) +} + +fn deletion_path(change: &TreeChange) -> Option<&bstr::BStr> { + matches!(change.status, TreeStatus::Deletion { .. }).then(|| change.path.as_bstr()) +} + +fn addition_path(change: &TreeChange) -> Option<&bstr::BStr> { + matches!(change.status, TreeStatus::Addition { .. }).then(|| change.path.as_bstr()) +} + +enum LocalChangeOverlap { + None, + Full, + Partial { + matching_hunks: Vec, + remainder_hunks: Vec, + }, +} + +fn classify_local_change_overlap( + repo: &gix::Repository, + remote_changes: &[TreeChange], + local_change: &TreeChange, +) -> Result { + let overlapping_remote_changes: Vec<_> = remote_changes + .iter() + .filter(|remote_change| changes_touch_same_path(remote_change, local_change)) + .collect(); + + if overlapping_remote_changes.is_empty() { + return Ok(LocalChangeOverlap::None); + } + + if change_requires_full_file_overlap(local_change) + || overlapping_remote_changes + .iter() + .any(|remote_change| change_requires_full_file_overlap(remote_change)) + { + return Ok(LocalChangeOverlap::Full); + } + + let Some(UnifiedPatch::Patch { + hunks: local_hunks, .. + }) = local_change.unified_patch(repo, 0)? + else { + return Ok(LocalChangeOverlap::Full); + }; + + let mut remote_hunks = Vec::::new(); + for remote_change in overlapping_remote_changes { + let Some(UnifiedPatch::Patch { hunks, .. }) = remote_change.unified_patch(repo, 0)? else { + return Ok(LocalChangeOverlap::Full); + }; + remote_hunks.extend(hunks.into_iter().map(HunkHeader::from)); + } + + let (matching_hunks, remainder_hunks): (Vec<_>, Vec<_>) = local_hunks + .into_iter() + .map(HunkHeader::from) + .partition(|local_hunk| { + remote_hunks.iter().any(|remote_hunk| { + remote_hunk.old_range().intersects(local_hunk.old_range()) + || remote_hunk.new_range().intersects(local_hunk.new_range()) + }) + }); + + if matching_hunks.is_empty() { + Ok(LocalChangeOverlap::None) + } else { + Ok(LocalChangeOverlap::Partial { + matching_hunks, + remainder_hunks, + }) + } +} + +fn change_requires_full_file_overlap(change: &TreeChange) -> bool { + change.previous_path().is_some() + || match &change.status { + TreeStatus::Addition { .. } + | TreeStatus::Deletion { .. } + | TreeStatus::Rename { .. } => true, + TreeStatus::Modification { flags, .. } => { + flags.is_some_and(|flag| flag.is_typechange()) + } + } +} + +fn changes_touch_same_path(a: &TreeChange, b: &TreeChange) -> bool { + let a_path = a.path.as_bstr(); + let b_path = b.path.as_bstr(); + let a_previous = a.previous_path(); + let b_previous = b.previous_path(); + + a_path == b_path + || Some(a_path) == b_previous + || a_previous == Some(b_path) + || a_previous + .zip(b_previous) + .is_some_and(|(a_prev, b_prev)| a_prev == b_prev) +} + +fn apply_whole_tree_change( + builder: &mut gix::object::tree::Editor<'_>, + change: &TreeChange, +) -> Result<()> { + if let Some(previous_path) = change.previous_path() { + builder.remove(previous_path)?; + } + + match change.status.state() { + Some(state) => { + builder.upsert(change.path.as_bstr(), state.kind, state.id)?; + } + None => { + builder.remove(change.path.as_bstr())?; + } + } + + Ok(()) +} + +fn apply_selected_hunks( + repo: &gix::Repository, + builder: &mut gix::object::tree::Editor<'_>, + change: &TreeChange, + selected_hunks: &[HunkHeader], +) -> Result<()> { + if selected_hunks.is_empty() { + return Ok(()); + } + + let Some(current_state) = change.status.state() else { + bail!( + "BUG: hunk-based application requires a current state for path {}", + change.path + ); + }; + let Some((previous_state, _previous_path)) = change.status.previous_state_and_path() else { + bail!( + "BUG: hunk-based application requires a previous state for path {}", + change.path + ); + }; + + let previous_blob = repo.find_blob(previous_state.id)?; + let current_blob = repo.find_blob(current_state.id)?; + let blob = but_core::apply_hunks( + previous_blob.data.as_bstr(), + current_blob.data.as_bstr(), + selected_hunks, + )?; + let blob_id = repo.write_blob(blob.as_slice())?; + builder.upsert(change.path.as_bstr(), current_state.kind, blob_id)?; + Ok(()) +} + +fn rebase_base_tree(repo: &gix::Repository, ontos: &[gix::ObjectId]) -> Result { + match ontos { + [onto] => commit_tree(repo, *onto), + [] => bail!("PickDivergent does not yet support root commits without output parents"), + _ => match auto_resolution_tree_from_merging_commits(repo, ontos)? { + MergeOutcome::Success(tree) => Ok(tree), + MergeOutcome::NoCommit => Ok(gix::ObjectId::empty_tree(repo.object_hash())), + MergeOutcome::Conflict { .. } => bail!( + "BUG: PickDivergent output parents should already have been merged successfully" + ), + }, + } +} + +fn rebase_hunk_pool_tree( + repo: &gix::Repository, + onto_tree: gix::ObjectId, + base_tree: gix::ObjectId, + tree: gix::ObjectId, +) -> Result { + if onto_tree == base_tree { + return Ok(tree); + } + + let mut rebased = repo.merge_trees( + base_tree.attach(repo), + onto_tree.attach(repo), + tree.attach(repo), + repo.default_merge_labels(), + repo.merge_options_force_ours()?, + )?; + Ok(rebased.tree.write()?.detach()) +} + +fn rebase_divergence_ancestor_tree( + repo: &gix::Repository, + onto_tree: gix::ObjectId, + ancestor: gix::ObjectId, +) -> Result { + let ancestor_commit = but_core::Commit::from_id(ancestor.attach(repo))?; + if ancestor_commit.parents.len() > 1 { + bail!("Cannot yet cherry-pick merge-commits - use rebasing for that"); + } + + let base_tree = if ancestor_commit.is_conflicted() { + find_commit_tree(&ancestor_commit, TreeKind::Base)? + } else if let Some(parent) = ancestor_commit.parents.first() { + let parent_commit = but_core::Commit::from_id(parent.attach(repo))?; + find_commit_tree(&parent_commit, TreeKind::AutoResolution)? + } else { + gix::ObjectId::empty_tree(repo.object_hash()).attach(repo) + }; + let theirs_tree = find_commit_tree(&ancestor_commit, TreeKind::Theirs)?; + let mut rebased = repo.merge_trees( + base_tree, + onto_tree.attach(repo), + theirs_tree, + repo.default_merge_labels(), + repo.merge_options_force_ours()?, + )?; + Ok(rebased.tree.write()?.detach()) +} + +#[cfg(test)] +mod test { + mod split_rename_addition_overlap_paths { + use bstr::BString; + use but_core::{ChangeState, TreeChange, TreeStatus}; + use gix::{hash::Kind, object::tree::EntryKind}; + + use crate::graph_rebase::rebase::pick_divergent::split_rename_addition_overlap_paths; + + fn state() -> ChangeState { + ChangeState { + id: gix::ObjectId::null(Kind::Sha1), + kind: EntryKind::Blob, + } + } + + fn addition(path: &str) -> TreeChange { + TreeChange { + path: path.into(), + status: TreeStatus::Addition { + state: state(), + is_untracked: false, + }, + } + } + + fn deletion(path: &str) -> TreeChange { + TreeChange { + path: path.into(), + status: TreeStatus::Deletion { + previous_state: state(), + }, + } + } + + fn modification(path: &str) -> TreeChange { + TreeChange { + path: path.into(), + status: TreeStatus::Modification { + previous_state: state(), + state: state(), + flags: None, + }, + } + } + + #[test] + fn ignores_unrelated_changes_while_detecting_split_rename() { + let remote_changes = vec![ + deletion("old.txt"), + addition("remote.txt"), + modification("remote-only.txt"), + ]; + let local_changes = vec![ + deletion("old.txt"), + addition("local.txt"), + modification("local-only.txt"), + ]; + + assert_eq!( + split_rename_addition_overlap_paths(&remote_changes, &local_changes), + [BString::from("local.txt")].into_iter().collect() + ); + } + + #[test] + fn bails_out_when_split_rename_shape_is_ambiguous() { + let remote_changes = vec![deletion("old.txt"), addition("remote.txt")]; + let local_changes = vec![ + deletion("old.txt"), + addition("local.txt"), + addition("another-local.txt"), + ]; + + assert!( + split_rename_addition_overlap_paths(&remote_changes, &local_changes).is_empty() + ); + } + } + + mod divergent_hunk_pool_partitioning { + use anyhow::Result; + use gix::prelude::ObjectIdExt as _; + + use crate::graph_rebase::rebase::pick_divergent::partition_local_hunk_pool; + + fn tree_id(repo: &gix::Repository, commit_id: gix::ObjectId) -> Result { + Ok(but_core::Commit::from_id(commit_id.attach(repo))? + .tree_id_or_auto_resolution()? + .detach()) + } + + #[test] + fn earlier_member_claims_ambiguous_overlap_before_later_member() -> Result<()> { + let (repo, _tmpdir) = + but_testsupport::writable_scenario("pick-divergent-hunk-pool-partitioning"); + + let base_id = repo.rev_parse_single("base")?.detach(); + let local_id = repo.rev_parse_single("local")?.detach(); + let remote_one_id = repo.rev_parse_single("remote-one")?.detach(); + let carried_local_id = repo.rev_parse_single("carried-local")?.detach(); + let remote_two_id = repo.rev_parse_single("remote-two")?.detach(); + + let base_tree = tree_id(&repo, base_id)?; + let local_tree = tree_id(&repo, local_id)?; + let remote_one_tree = tree_id(&repo, remote_one_id)?; + let carried_local_tree = tree_id(&repo, carried_local_id)?; + let remote_two_tree = tree_id(&repo, remote_two_id)?; + + let first_member = + partition_local_hunk_pool(&repo, base_tree, remote_one_tree, local_tree)?; + let second_member_if_unclaimed = partition_local_hunk_pool( + &repo, + remote_one_tree, + remote_two_tree, + carried_local_tree, + )?; + + assert!( + first_member.overlaps_remote, + "expected the first remote member to overlap the local insertion" + ); + assert!( + second_member_if_unclaimed.overlaps_remote, + "expected the same local insertion to also overlap the later remote member if it remained unclaimed" + ); + assert_eq!( + first_member.matching_tree, local_tree, + "expected the earlier remote member to claim the entire ambiguous local hunk" + ); + assert_eq!( + first_member.remainder_tree, base_tree, + "expected no remainder after the earlier member claims the ambiguous local hunk" + ); + + Ok(()) + } + } + + mod divergent_family_state { + use crate::graph_rebase::rebase::pick_divergent::{ + DivergentFamilyState, DivergentHunkPool, + }; + + #[test] + fn new_state_starts_at_zero_members() { + let state = DivergentFamilyState::new(); + assert_eq!(state.members_processed, 0); + assert!(matches!(state.hunk_pool, DivergentHunkPool::Uninitialized)); + } + + #[test] + fn members_processed_increments() { + let mut state = DivergentFamilyState::new(); + state.advance_member(); + assert_eq!(state.members_processed, 1); + state.advance_member(); + assert_eq!(state.members_processed, 2); + } + + #[test] + fn separate_families_get_separate_state() { + use std::collections::HashMap; + + let family_a = [0xAAu8; 20]; + let family_b = [0xBBu8; 20]; + + let mut families: HashMap<[u8; 20], DivergentFamilyState> = HashMap::new(); + + families + .entry(family_a) + .or_insert_with(DivergentFamilyState::new) + .advance_member(); + families + .entry(family_b) + .or_insert_with(DivergentFamilyState::new) + .advance_member(); + families + .entry(family_a) + .or_insert_with(DivergentFamilyState::new) + .advance_member(); + + assert_eq!(families[&family_a].members_processed, 2); + assert_eq!(families[&family_b].members_processed, 1); + } + + #[test] + fn final_member_detection_tracks_processed_members() { + let mut state = DivergentFamilyState::new(); + + state.advance_member(); + assert!(!state.is_final_member(2)); + + state.advance_member(); + assert!(state.is_final_member(2)); + } + } +} diff --git a/crates/but-rebase/src/graph_rebase/testing.rs b/crates/but-rebase/src/graph_rebase/testing.rs index 2b3a23e7f4b..22a74d38d8d 100644 --- a/crates/but-rebase/src/graph_rebase/testing.rs +++ b/crates/but-rebase/src/graph_rebase/testing.rs @@ -59,6 +59,9 @@ impl TestingDot for StepGraph { &|_, (_, step)| { match step { Step::Pick(Pick { id, .. }) => format!("label=\"pick: {id}\""), + Step::PickDivergent(pick_div) => { + format!("label=\"pick_divergent: {}\"", pick_div.remote_commit) + } Step::Reference { refname } => { format!("label=\"reference: {}\"", refname.as_bstr()) } @@ -100,7 +103,7 @@ trait ToSymbol { impl ToSymbol for Step { fn to_symbol(&self) -> char { match self { - Self::Pick(_) => chars::STEP_PICK, + Self::Pick(_) | Self::PickDivergent(_) => chars::STEP_PICK, Self::Reference { .. } => chars::STEP_REFERENCE, Self::None => chars::STEP_NONE, } @@ -211,6 +214,11 @@ fn format_step(step: &Step, title: Option) -> String { None => sha, } } + Step::PickDivergent(pick_div) => { + let mut sha = pick_div.remote_commit.to_string(); + sha.truncate(7); + format!("{sha} [divergent]") + } Step::Reference { refname } => refname.as_bstr().to_string(), Step::None => "no-op".to_string(), } diff --git a/crates/but-rebase/src/graph_rebase/util.rs b/crates/but-rebase/src/graph_rebase/util.rs index 26483835616..5eab785a09e 100644 --- a/crates/but-rebase/src/graph_rebase/util.rs +++ b/crates/but-rebase/src/graph_rebase/util.rs @@ -4,7 +4,7 @@ use std::collections::HashSet; use petgraph::visit::EdgeRef as _; -use crate::graph_rebase::{Pick, Step, StepGraph, StepGraphIndex}; +use crate::graph_rebase::{Pick, PickDivergent, Step, StepGraph, StepGraphIndex}; /// Find the parents of a given node that are commit - in correct parent /// ordering. @@ -28,7 +28,10 @@ pub(crate) fn collect_ordered_parents( let mut parents = vec![]; while let Some(candidate) = potential_parent_edges.pop() { - if let Step::Pick(Pick { .. }) = graph[candidate.target()] { + if matches!( + graph[candidate.target()], + Step::Pick(Pick { .. }) | Step::PickDivergent(PickDivergent { .. }) + ) { parents.push(candidate.target()); // Don't pursue the children continue; diff --git a/crates/but-rebase/tests/fixtures/scenario/pick-divergent-hunk-pool-partitioning.sh b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-hunk-pool-partitioning.sh new file mode 100755 index 00000000000..e3ea5840a2d --- /dev/null +++ b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-hunk-pool-partitioning.sh @@ -0,0 +1,71 @@ +#!/bin/bash + +set -eu -o pipefail + +git init + +cat <<'EOF' >file.txt +1 +2 +3 +4 +5 +EOF +git add file.txt +git commit -m "divergence-base" +git update-ref refs/heads/base "$(git rev-parse HEAD)" + +git branch local +git checkout local +cat <<'EOF' >file.txt +1 +2 +LOCAL +3 +4 +5 +EOF +git add file.txt +git commit -m "local-insert" +git update-ref refs/heads/local "$(git rev-parse HEAD)" + +git checkout main +cat <<'EOF' >file.txt +1 +2 +R1 +3 +4 +5 +EOF +git add file.txt +git commit -m "remote-1" +git update-ref refs/heads/remote-one "$(git rev-parse HEAD)" + +git checkout -b carried +cat <<'EOF' >file.txt +1 +2 +R1 +LOCAL +3 +4 +5 +EOF +git add file.txt +git commit -m "carried-local" +git update-ref refs/heads/carried-local "$(git rev-parse HEAD)" + +git checkout main +cat <<'EOF' >file.txt +1 +2 +R1 +R2 +3 +4 +5 +EOF +git add file.txt +git commit -m "remote-2" +git update-ref refs/heads/remote-two "$(git rev-parse HEAD)" diff --git a/crates/but-rebase/tests/fixtures/scenario/pick-divergent-multi-member-carry-forward.sh b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-multi-member-carry-forward.sh new file mode 100644 index 00000000000..1619f8fc14a --- /dev/null +++ b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-multi-member-carry-forward.sh @@ -0,0 +1,89 @@ +#!/bin/bash + +set -eu -o pipefail + +git init + +cat <<'EOF' >file.txt +1 +2 +3 +4 +5 +6 +7 +8 +EOF +git add file.txt +git commit -m "divergence-base" +git update-ref refs/heads/ancestor "$(git rev-parse HEAD)" + +git branch local +git checkout local +cat <<'EOF' >file.txt +1 +2 +local-top +3 +4 +5 +6 +7 +8 +EOF +git add file.txt +git commit -m "local-top" +git update-ref refs/heads/local-top "$(git rev-parse HEAD)" + +cat <<'EOF' >file.txt +1 +2 +local-top +3 +4 +5 +6 +7 +8 +local-bottom +EOF +git add file.txt +git commit -m "local-bottom" +git update-ref refs/heads/local-bottom "$(git rev-parse HEAD)" + +git checkout main +cat <<'EOF' >file.txt +1 +2 +remote-1 +3 +4 +5 +6 +7 +8 +EOF +git add file.txt +git commit -m "remote-1" +git update-ref refs/heads/remote-one "$(git rev-parse HEAD)" + +cat <<'EOF' >file.txt +1 +2 +remote-1 +3 +4 +remote-2 +5 +6 +7 +8 +EOF +git add file.txt +git commit -m "remote-2" +git update-ref refs/heads/remote-two "$(git rev-parse HEAD)" + +echo "remote-extra" >extra.txt +git add extra.txt +git commit -m "remote-3" +git update-ref refs/heads/remote-three "$(git rev-parse HEAD)" diff --git a/crates/but-rebase/tests/fixtures/scenario/pick-divergent-multi-member-happy-path.sh b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-multi-member-happy-path.sh new file mode 100755 index 00000000000..fc31de84f6c --- /dev/null +++ b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-multi-member-happy-path.sh @@ -0,0 +1,68 @@ +#!/bin/bash + +set -eu -o pipefail + +git init + +cat <<'EOF' >file.txt +1 +2 +3 +4 +5 +6 +7 +8 +EOF +git add file.txt +git commit -m "divergence-base" +git update-ref refs/heads/ancestor "$(git rev-parse HEAD)" + +git branch local +git checkout local +cat <<'EOF' >file.txt +1 +2 +3 +4 +5 +6 +7 +8 +local-end +EOF +git add file.txt +git commit -m "local-end" +git update-ref refs/heads/local-end "$(git rev-parse HEAD)" + +git checkout main +cat <<'EOF' >file.txt +1 +remote-1 +2 +3 +4 +5 +6 +7 +8 +EOF +git add file.txt +git commit -m "remote-1" +git update-ref refs/heads/remote-one "$(git rev-parse HEAD)" + +cat <<'EOF' >file.txt +1 +remote-1 +2 +3 +remote-2 +4 +5 +6 +7 +8 +EOF +git add file.txt +git commit -m "remote-2" +git update-ref refs/heads/remote-two "$(git rev-parse HEAD)" diff --git a/crates/but-rebase/tests/fixtures/scenario/pick-divergent-no-ancestor-fallback.sh b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-no-ancestor-fallback.sh new file mode 100644 index 00000000000..9736551eea2 --- /dev/null +++ b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-no-ancestor-fallback.sh @@ -0,0 +1,35 @@ +#!/bin/bash + +set -eu -o pipefail + +git init + +cat <<'EOF' >divergent.txt +shared-1 +shared-2 +shared-3 +EOF +git add divergent.txt +git commit -m "shared-base" +git update-ref refs/heads/onto "$(git rev-parse HEAD)" + +git branch local +git checkout local +cat <<'EOF' >divergent.txt +shared-1 +shared-2 +local-3 +EOF +git add divergent.txt +git commit -m "local-change-line-three" +git update-ref refs/heads/local "$(git rev-parse HEAD)" + +git checkout main +cat <<'EOF' >divergent.txt +shared-1 +remote-2 +shared-3 +EOF +git add divergent.txt +git commit -m "remote-change-line-two" +git update-ref refs/heads/remote "$(git rev-parse HEAD)" diff --git a/crates/but-rebase/tests/fixtures/scenario/pick-divergent-single-member-conflicted-rewrite.sh b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-single-member-conflicted-rewrite.sh new file mode 100644 index 00000000000..93ee1cdf430 --- /dev/null +++ b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-single-member-conflicted-rewrite.sh @@ -0,0 +1,35 @@ +#!/bin/bash + +set -eu -o pipefail + +git init + +cat <<'EOF' >divergent.txt +shared-1 +shared-2 +shared-3 +EOF +git add divergent.txt +git commit -m "divergence-base" +git update-ref refs/heads/ancestor "$(git rev-parse HEAD)" + +git branch local +git checkout local +cat <<'EOF' >divergent.txt +shared-1 +local-2 +shared-3 +EOF +git add divergent.txt +git commit -m "local-change-middle-line" +git update-ref refs/heads/local "$(git rev-parse HEAD)" + +git checkout main +cat <<'EOF' >divergent.txt +shared-1 +remote-2 +shared-3 +EOF +git add divergent.txt +git commit -m "remote-change-middle-line" +git update-ref refs/heads/remote "$(git rev-parse HEAD)" diff --git a/crates/but-rebase/tests/fixtures/scenario/pick-divergent-single-member-happy-path.sh b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-single-member-happy-path.sh new file mode 100644 index 00000000000..8343ff0c264 --- /dev/null +++ b/crates/but-rebase/tests/fixtures/scenario/pick-divergent-single-member-happy-path.sh @@ -0,0 +1,42 @@ +#!/bin/bash + +set -eu -o pipefail + +git init + +cat <<'EOF' >divergent.txt +1 +2 +3 +4 +EOF +git add divergent.txt +git commit -m "divergence-base" +git update-ref refs/heads/ancestor "$(git rev-parse HEAD)" + +git branch local +git checkout local +cat <<'EOF' >divergent.txt +1 +local-2 +3 +4 +EOF +git add divergent.txt +git commit -m "local-change-line-two" +git update-ref refs/heads/local "$(git rev-parse HEAD)" + +git checkout main +cat <<'EOF' >divergent.txt +1 +2 +3 +remote-4 +EOF +git add divergent.txt +git commit -m "remote-change-line-four" +git update-ref refs/heads/remote "$(git rev-parse HEAD)" + +echo "tip" >tip.txt +git add tip.txt +git commit -m "tip-after-remote" diff --git a/crates/but-rebase/tests/rebase/graph_rebase/mod.rs b/crates/but-rebase/tests/rebase/graph_rebase/mod.rs index d091694e6a9..b7dc81d9fa5 100644 --- a/crates/but-rebase/tests/rebase/graph_rebase/mod.rs +++ b/crates/but-rebase/tests/rebase/graph_rebase/mod.rs @@ -14,6 +14,7 @@ mod insert_segment; mod materialize; mod multiple_operations; mod parents_must_be_references_restriction; +mod pick_divergent; mod rebase_identities; mod replace; mod signing_preferences; diff --git a/crates/but-rebase/tests/rebase/graph_rebase/pick_divergent.rs b/crates/but-rebase/tests/rebase/graph_rebase/pick_divergent.rs new file mode 100644 index 00000000000..0766f5a620d --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/pick_divergent.rs @@ -0,0 +1,514 @@ +//! Tests for `PickDivergent` execution in the graph rebase engine. + +use anyhow::Result; +use but_graph::Graph; +use but_rebase::graph_rebase::{Editor, Step}; +use but_testsupport::{visualize_commit_graph_all, visualize_tree}; +use gix::prelude::ObjectIdExt as _; + +use crate::utils::{fixture_writable, standard_options}; + +/// Single divergent member where local and remote touch different lines. +/// +/// ```text +/// divergent.txt at ancestor: +/// 1 +/// 2 +/// 3 +/// 4 +/// +/// local (rewrites line 2): remote (rewrites line 4): +/// 1 1 +/// local-2 2 +/// 3 3 +/// 4 remote-4 +/// +/// Graph before: +/// * local-change-line-two (local) +/// | * tip-after-remote (HEAD -> main) +/// | * remote-change-line-four (remote) +/// |/ +/// * divergence-base (ancestor) +/// +/// Expected after PickDivergent(remote, local=[local], ancestor=ancestor): +/// - Remote commit is kept in place, graph shape unchanged. +/// - Output tree merges both rewrites: +/// divergent.txt = "1\nlocal-2\n3\nremote-4\n" +/// - tip-after-remote is rebased on top. +/// ``` +#[test] +fn single_member_happy_path_merges_local_change_into_remote_shape() -> Result<()> { + let (repo, _tmpdir, mut meta) = fixture_writable("pick-divergent-single-member-happy-path")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 530051c (local) local-change-line-two + | * 5d75912 (HEAD -> main) tip-after-remote + | * c133905 (remote) remote-change-line-four + |/ + * 070b254 (ancestor) divergence-base + "); + + let ancestor = repo.rev_parse_single("ancestor")?.detach(); + let local = repo.rev_parse_single("local")?.detach(); + let remote = repo.rev_parse_single("remote")?.detach(); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + let mut ws = graph.into_workspace()?; + let mut editor = Editor::create(&mut ws, &mut *meta, &repo)?; + + let remote_selector = editor.select_commit(remote)?; + editor.replace( + remote_selector, + Step::new_pick_divergent(vec![local], Some(ancestor), remote, [0x11; 20]), + )?; + + let outcome = editor.rebase()?; + let _outcome = outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 530051c (local) local-change-line-two + | * 74d3ff6 (HEAD -> main) tip-after-remote + | * 916c82c (remote) remote-change-line-four + |/ + * 070b254 (ancestor) divergence-base + "); + + let remote_out = repo.rev_parse_single("remote")?; + insta::assert_snapshot!(visualize_tree(remote_out).to_string(), @r#" + 4b1e8a9 + └── divergent.txt:100644:4b1ac72 "1\nlocal-2\n3\nremote-4\n" + "#); + + let head = repo.rev_parse_single("HEAD")?; + insta::assert_snapshot!(visualize_tree(head).to_string(), @r#" + 54ce68c + ├── divergent.txt:100644:4b1ac72 "1\nlocal-2\n3\nremote-4\n" + └── tip.txt:100644:b218ebd "tip\n" + "#); + + Ok(()) +} + +/// Two-member remote family with one local commit, all touching different +/// regions of the file. No conflicts expected. +/// +/// ```text +/// file.txt at ancestor: +/// 1 +/// 2 +/// 3 +/// 4 +/// 5 +/// 6 +/// 7 +/// 8 +/// +/// local-end (appends after line 8): +/// 1 +/// 2 +/// 3 +/// 4 +/// 5 +/// 6 +/// 7 +/// 8 +/// local-end +/// +/// remote-1 (inserts after line 1): +/// 1 +/// remote-1 +/// 2 +/// 3 ... +/// +/// remote-2 (inserts after line 3, i.e. between original 3 and 4): +/// 1 +/// remote-1 +/// 2 +/// 3 +/// remote-2 +/// 4 ... +/// +/// Graph before: +/// * local-end (local-end, local) +/// | * remote-2 (HEAD -> main, remote-two) +/// | * remote-1 (remote-one) +/// |/ +/// * divergence-base (ancestor) +/// +/// Expected after PickDivergent family (both remotes, locals=[local-end]): +/// - Graph shape preserved: two remote commits in sequence. +/// - Neither remote overlaps local-end, so no conflicts. +/// - local-end carries forward to remote-2 (final member) and is merged. +/// - Per-commit output trees: +/// remote-1: file.txt = "1\nremote-1\n2\n3\n4\n5\n6\n7\n8\n" +/// remote-2: file.txt = "1\nremote-1\n2\n3\nremote-2\n4\n5\n6\n7\n8\nlocal-end\n" +/// ``` +#[test] +fn multi_member_happy_path_no_conflicts() -> Result<()> { + let (repo, _tmpdir, mut meta) = fixture_writable("pick-divergent-multi-member-happy-path")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * b67c6e2 (local-end, local) local-end + | * 36f1b48 (HEAD -> main, remote-two) remote-2 + | * 58679a8 (remote-one) remote-1 + |/ + * 62402e9 (ancestor) divergence-base + "); + + let ancestor = repo.rev_parse_single("ancestor")?.detach(); + let local_end = repo.rev_parse_single("local-end")?.detach(); + let remote_one = repo.rev_parse_single("remote-one")?.detach(); + let remote_two = repo.rev_parse_single("remote-two")?.detach(); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + let mut ws = graph.into_workspace()?; + let mut editor = Editor::create(&mut ws, &mut *meta, &repo)?; + + let family_id = [0x55; 20]; + editor.replace( + editor.select_commit(remote_one)?, + Step::new_pick_divergent(vec![local_end], Some(ancestor), remote_one, family_id), + )?; + editor.replace( + editor.select_commit(remote_two)?, + Step::new_pick_divergent(vec![local_end], Some(ancestor), remote_two, family_id), + )?; + + let outcome = editor.rebase()?; + let _outcome = outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * b67c6e2 (local-end, local) local-end + | * 6f82cbf (HEAD -> main, remote-two) remote-2 + | * 58679a8 (remote-one) remote-1 + |/ + * 62402e9 (ancestor) divergence-base + "); + + let remote_one_out = repo.rev_parse_single("remote-one")?; + insta::assert_snapshot!(visualize_tree(remote_one_out).to_string(), @r#" + 2bb7f47 + └── file.txt:100644:79132d8 "1\nremote-1\n2\n3\n4\n5\n6\n7\n8\n" + "#); + + let remote_two_out = repo.rev_parse_single("remote-two")?; + insta::assert_snapshot!(visualize_tree(remote_two_out).to_string(), @r#" + 75911e9 + └── file.txt:100644:c4f6b31 "1\nremote-1\n2\n3\nremote-2\n4\n5\n6\n7\n8\nlocal-end\n" + "#); + + Ok(()) +} + +/// Three-member remote family with two local commits. Earlier members claim +/// overlapping hunks first; the remainder carries forward to the final member. +/// +/// ```text +/// file.txt at ancestor: +/// 1 +/// 2 +/// 3 +/// 4 +/// 5 +/// 6 +/// 7 +/// 8 +/// +/// local-top (inserts after line 2): local-bottom (appends after line 8): +/// 1 1 +/// 2 2 +/// local-top local-top +/// 3 3 +/// 4 4 +/// 5 5 +/// 6 6 +/// 7 7 +/// 8 8 +/// local-bottom +/// +/// remote-1 (inserts after line 2): +/// 1 +/// 2 +/// remote-1 +/// 3 ... +/// +/// remote-2 (inserts after line 4): +/// 1 +/// 2 +/// remote-1 +/// 3 +/// 4 +/// remote-2 +/// 5 ... +/// +/// remote-3: adds extra.txt = "remote-extra\n" (no file.txt change) +/// +/// Graph before: +/// * local-bottom (local-bottom, local) +/// * local-top (local-top) +/// | * remote-3 (HEAD -> main, remote-three) +/// | * remote-2 (remote-two) +/// | * remote-1 (remote-one) +/// |/ +/// * divergence-base (ancestor) +/// +/// Expected after PickDivergent family (all 3 remotes, locals=[local-top, local-bottom]): +/// - Graph shape preserved: three remote commits in sequence. +/// - remote-1 overlaps local-top (both insert at line 2) → conflicted commit. +/// - remote-2 has no remaining local overlap → clean commit. +/// - local-bottom carries forward to remote-3 (final member) → merged in. +/// - Per-commit output trees: +/// remote-1: conflicted (remote-1 vs local-top at line 2) +/// remote-2: file.txt = "1\n2\nremote-1\n3\n4\nremote-2\n5\n6\n7\n8\n" +/// remote-3: file.txt = "1\n2\nremote-1\n3\n4\nremote-2\n5\n6\n7\n8\nlocal-bottom\n" +/// extra.txt = "remote-extra\n" +/// ``` +#[test] +fn multi_member_carry_forward_preserves_remote_family_shape() -> Result<()> { + let (repo, _tmpdir, mut meta) = fixture_writable("pick-divergent-multi-member-carry-forward")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 39e1d31 (local-bottom, local) local-bottom + * aa816ab (local-top) local-top + | * a36fb98 (HEAD -> main, remote-three) remote-3 + | * 98c0547 (remote-two) remote-2 + | * 10ab6f8 (remote-one) remote-1 + |/ + * 62402e9 (ancestor) divergence-base + "); + + let ancestor = repo.rev_parse_single("ancestor")?.detach(); + let local_top = repo.rev_parse_single("local-top")?.detach(); + let local_bottom = repo.rev_parse_single("local-bottom")?.detach(); + let remote_one = repo.rev_parse_single("remote-one")?.detach(); + let remote_two = repo.rev_parse_single("remote-two")?.detach(); + let remote_three = repo.rev_parse_single("remote-three")?.detach(); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + let mut ws = graph.into_workspace()?; + let mut editor = Editor::create(&mut ws, &mut *meta, &repo)?; + + let family_id = [0x22; 20]; + editor.replace( + editor.select_commit(remote_one)?, + Step::new_pick_divergent( + vec![local_top, local_bottom], + Some(ancestor), + remote_one, + family_id, + ), + )?; + editor.replace( + editor.select_commit(remote_two)?, + Step::new_pick_divergent( + vec![local_top, local_bottom], + Some(ancestor), + remote_two, + family_id, + ), + )?; + editor.replace( + editor.select_commit(remote_three)?, + Step::new_pick_divergent( + vec![local_top, local_bottom], + Some(ancestor), + remote_three, + family_id, + ), + )?; + + let outcome = editor.rebase()?; + let _outcome = outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 39e1d31 (local-bottom, local) local-bottom + * aa816ab (local-top) local-top + | * d52b0f1 (HEAD -> main, remote-three) remote-3 + | * 5b5bc58 (remote-two) remote-2 + | * 027d16b (remote-one) remote-1 + |/ + * 62402e9 (ancestor) divergence-base + "); + + let remote_one_out = repo.rev_parse_single("remote-one")?; + insta::assert_snapshot!(visualize_tree(remote_one_out).to_string(), @r#" + 5e960c0 + ├── .auto-resolution:34a3ba0 + │ └── file.txt:100644:0616b2f "1\n2\nremote-1\n3\n4\n5\n6\n7\n8\n" + ├── .conflict-base-0:83d31c1 + │ └── file.txt:100644:535d2b0 "1\n2\n3\n4\n5\n6\n7\n8\n" + ├── .conflict-files:100644:2ea32f0 "ancestorEntries = [\"file.txt\"]\nourEntries = [\"file.txt\"]\ntheirEntries = [\"file.txt\"]\n" + ├── .conflict-side-0:34a3ba0 + │ └── file.txt:100644:0616b2f "1\n2\nremote-1\n3\n4\n5\n6\n7\n8\n" + ├── .conflict-side-1:3d2fb31 + │ └── file.txt:100644:81b972a "1\n2\nlocal-top\n3\n4\n5\n6\n7\n8\n" + ├── CONFLICT-README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this." + └── file.txt:100644:0616b2f "1\n2\nremote-1\n3\n4\n5\n6\n7\n8\n" + "#); + + let remote_two_out = repo.rev_parse_single("remote-two")?; + insta::assert_snapshot!(visualize_tree(remote_two_out).to_string(), @r#" + 871cb84 + └── file.txt:100644:e2b4d17 "1\n2\nremote-1\n3\n4\nremote-2\n5\n6\n7\n8\n" + "#); + + let remote_three_out = repo.rev_parse_single("remote-three")?; + insta::assert_snapshot!(visualize_tree(remote_three_out).to_string(), @r#" + 1d9458c + ├── extra.txt:100644:ff56302 "remote-extra\n" + └── file.txt:100644:6c65f68 "1\n2\nremote-1\n3\n4\nremote-2\n5\n6\n7\n8\nlocal-bottom\n" + "#); + + Ok(()) +} + +/// When `ancestor` is `None`, the output parent is used as the merge base +/// instead. The merge should still combine non-overlapping local and remote +/// changes correctly. +/// +/// ```text +/// divergent.txt at shared-base (onto): +/// shared-1 +/// shared-2 +/// shared-3 +/// +/// local (rewrites line 3): remote (rewrites line 2): +/// shared-1 shared-1 +/// shared-2 remote-2 +/// local-3 shared-3 +/// +/// Graph before: +/// * local-change-line-three (local) +/// | * remote-change-line-two (HEAD -> main, remote) +/// |/ +/// * shared-base (onto) +/// +/// Expected after PickDivergent(remote, local=[local], ancestor=None): +/// - No explicit ancestor, so the output parent (onto) is the merge base. +/// - Output tree merges both rewrites: +/// divergent.txt = "shared-1\nremote-2\nlocal-3\n" +/// ``` +#[test] +fn no_ancestor_falls_back_to_output_parent_as_merge_base() -> Result<()> { + let (repo, _tmpdir, mut meta) = fixture_writable("pick-divergent-no-ancestor-fallback")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 6442b34 (local) local-change-line-three + | * 44b08a5 (HEAD -> main, remote) remote-change-line-two + |/ + * 642085f (onto) shared-base + "); + + let local = repo.rev_parse_single("local")?.detach(); + let remote = repo.rev_parse_single("remote")?.detach(); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + let mut ws = graph.into_workspace()?; + let mut editor = Editor::create(&mut ws, &mut *meta, &repo)?; + + editor.replace( + editor.select_commit(remote)?, + Step::new_pick_divergent(vec![local], None, remote, [0x33; 20]), + )?; + + let outcome = editor.rebase()?; + let _outcome = outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 6442b34 (local) local-change-line-three + | * 35304df (HEAD -> main, remote) remote-change-line-two + |/ + * 642085f (onto) shared-base + "); + + let head = repo.rev_parse_single("HEAD")?; + insta::assert_snapshot!(visualize_tree(head).to_string(), @r#" + 79bf5b2 + └── divergent.txt:100644:4a5c370 "shared-1\nremote-2\nlocal-3\n" + "#); + + Ok(()) +} + +/// When local and remote both rewrite the same line to different values, +/// the result should be a conflicted commit preserving the remote position. +/// +/// ```text +/// divergent.txt at ancestor: +/// shared-1 +/// shared-2 +/// shared-3 +/// +/// local (rewrites line 2): remote (rewrites line 2): +/// shared-1 shared-1 +/// local-2 remote-2 +/// shared-3 shared-3 +/// +/// Graph before: +/// * local-change-middle-line (local) +/// | * remote-change-middle-line (HEAD -> main, remote) +/// |/ +/// * divergence-base (ancestor) +/// +/// Expected after PickDivergent(remote, local=[local], ancestor=ancestor): +/// - Remote commit kept in place, graph shape unchanged. +/// - Output commit is conflicted (both sides rewrite line 2). +/// - conflict-side-0 (remote): "shared-1\nremote-2\nshared-3\n" +/// - conflict-side-1 (local): "shared-1\nlocal-2\nshared-3\n" +/// - auto-resolution favors remote: "shared-1\nremote-2\nshared-3\n" +/// ``` +#[test] +fn single_member_conflicted_rewrite_materializes_conflicted_remote_commit() -> Result<()> { + let (repo, _tmpdir, mut meta) = + fixture_writable("pick-divergent-single-member-conflicted-rewrite")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 5c7ceac (local) local-change-middle-line + | * 1c1a8ed (HEAD -> main, remote) remote-change-middle-line + |/ + * cb3180d (ancestor) divergence-base + "); + + let ancestor = repo.rev_parse_single("ancestor")?.detach(); + let local = repo.rev_parse_single("local")?.detach(); + let remote = repo.rev_parse_single("remote")?.detach(); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + let mut ws = graph.into_workspace()?; + let mut editor = Editor::create(&mut ws, &mut *meta, &repo)?; + + editor.replace( + editor.select_commit(remote)?, + Step::new_pick_divergent(vec![local], Some(ancestor), remote, [0x44; 20]), + )?; + + let outcome = editor.rebase()?; + let _outcome = outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @" + * 5c7ceac (local) local-change-middle-line + | * 650eec9 (HEAD -> main, remote) remote-change-middle-line + |/ + * cb3180d (ancestor) divergence-base + "); + + let head = repo.rev_parse_single("HEAD")?; + assert!(but_core::Commit::from_id(head.detach().attach(&repo))?.is_conflicted()); + + insta::assert_snapshot!(visualize_tree(head).to_string(), @r#" + d1bd82d + ├── .auto-resolution:1cfe446 + │ └── divergent.txt:100644:18807f6 "shared-1\nremote-2\nshared-3\n" + ├── .conflict-base-0:0372008 + │ └── divergent.txt:100644:306f78e "shared-1\nshared-2\nshared-3\n" + ├── .conflict-files:100644:60e6e3f "ancestorEntries = [\"divergent.txt\"]\nourEntries = [\"divergent.txt\"]\ntheirEntries = [\"divergent.txt\"]\n" + ├── .conflict-side-0:1cfe446 + │ └── divergent.txt:100644:18807f6 "shared-1\nremote-2\nshared-3\n" + ├── .conflict-side-1:6088436 + │ └── divergent.txt:100644:bd896ff "shared-1\nlocal-2\nshared-3\n" + ├── CONFLICT-README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this." + └── divergent.txt:100644:18807f6 "shared-1\nremote-2\nshared-3\n" + "#); + + Ok(()) +} diff --git a/crates/but-workspace/src/commit/commit_create.rs b/crates/but-workspace/src/commit/commit_create.rs index 22eb1aa114d..7dbbc052948 100644 --- a/crates/but-workspace/src/commit/commit_create.rs +++ b/crates/but-workspace/src/commit/commit_create.rs @@ -4,7 +4,8 @@ pub(crate) mod function { use anyhow::Result; use but_core::{DiffSpec, RefMetadata}; use but_rebase::graph_rebase::{ - Editor, LookupStep, Pick, Selector, Step, SuccessfulRebase, ToSelector, mutate::InsertSide, + Editor, LookupStep, Pick, PickDivergent, Selector, Step, SuccessfulRebase, ToSelector, + mutate::InsertSide, }; use crate::commit_engine::{Destination, create_commit}; @@ -97,8 +98,20 @@ pub(crate) mod function { side: InsertSide, ) -> Result> { Ok(match (target_step, side) { - (Step::Pick(Pick { id, .. }), InsertSide::Above) => Some(id), - (Step::Pick(Pick { id, .. }), InsertSide::Below) => { + ( + Step::Pick(Pick { id, .. }) + | Step::PickDivergent(PickDivergent { + remote_commit: id, .. + }), + InsertSide::Above, + ) => Some(id), + ( + Step::Pick(Pick { id, .. }) + | Step::PickDivergent(PickDivergent { + remote_commit: id, .. + }), + InsertSide::Below, + ) => { let commit = editor.find_commit(id)?; commit.parents.first().copied() }