diff --git a/Cargo.lock b/Cargo.lock index b08fcbb81f..08e9051f86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1247,10 +1247,13 @@ dependencies = [ "but-core", "but-error", "but-gerrit", + "but-graph", + "but-meta", "but-oxidize", "but-testsupport", "gix", "insta", + "petgraph", "serde", "tempfile", "toml 0.9.8", diff --git a/Cargo.toml b/Cargo.toml index 45ea0a50ef..aa9aea4783 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -65,7 +65,8 @@ gitbutler-workspace = { path = "crates/gitbutler-workspace" } bstr = "1.12.1" # Add the `tracing` or `tracing-detail` features to see more of gitoxide in the logs. Useful to see which programs it invokes. -gix = { version = "0.75.0", git = "https://github.com/GitoxideLabs/gitoxide", branch = "main", default-features = false, features = [] } +gix = { version = "0.75.0", git = "https://github.com/GitoxideLabs/gitoxide", branch = "main", default-features = false, features = [ +] } ts-rs = { version = "11.1.0", features = ["serde-compat", "no-serde-warnings"] } gix-testtools = "0.16.1" insta = "1.44.3" @@ -111,6 +112,11 @@ bitflags = "2.9.4" notify = "8.2.0" snapbox = "0.6.23" url = "2.5.7" +petgraph = { version = "0.8.3", default-features = false, features = [ + "stable_graph", + "std", +] } + schemars = { version = "1.0.4", default-features = false, features = [ "std", "derive", @@ -119,9 +125,17 @@ async-openai = { version = "0.31.1", default-features = false, features = [ "rustls", "chat-completion", ] } -open = "5.3.3" -regex = { version = "1.11.3", default-features = false, features = ["std", "unicode" ] } -clap = { version = "4.5.53", default-features = false, features = ["derive", "std", "help", "unstable-markdown"]} +open = "5.3.2" +regex = { version = "1.11.3", default-features = false, features = [ + "std", + "unicode", +] } +clap = { version = "4.5.51", default-features = false, features = [ + "derive", + "std", + "help", + "unstable-markdown", +] } tracing-forest = { version = "0.3.0" } sysinfo = { version = "0.37.2", default-features = false } shell-words = "1.1.0" diff --git a/crates/but-claude/src/bridge.rs b/crates/but-claude/src/bridge.rs index d0d5ad005b..9857f38467 100644 --- a/crates/but-claude/src/bridge.rs +++ b/crates/but-claude/src/bridge.rs @@ -914,6 +914,8 @@ impl Default for Claudes { pub enum ClaudeCheckResult { /// Claude Code is available and returned a version Available { version: String }, + /// Claude Code _could_ be found, but failed to execute + ExecutionFailed { stdout: String, stderr: String }, /// Claude Code is not available or failed to execute NotAvailable, } @@ -1046,12 +1048,15 @@ pub async fn check_claude_available(claude_executable: &str) -> ClaudeCheckResul } match command.output().await { - Ok(output) if output.status.success() => match String::from_utf8(output.stdout) { - Ok(version) => ClaudeCheckResult::Available { - version: version.trim().to_string(), - }, - Err(_) => ClaudeCheckResult::NotAvailable, - }, + Ok(output) if output.status.success() => { + let version = str::from_utf8(&output.stdout).unwrap_or("").trim().into(); + ClaudeCheckResult::Available { version } + } + Ok(output) if !output.status.success() => { + let stdout = String::from_utf8(output.stdout).unwrap_or_default(); + let stderr = String::from_utf8(output.stderr).unwrap_or_default(); + ClaudeCheckResult::ExecutionFailed { stdout, stderr } + } _ => ClaudeCheckResult::NotAvailable, } } diff --git a/crates/but-graph/Cargo.toml b/crates/but-graph/Cargo.toml index 7b6ff9e488..d7d95bc15f 100644 --- a/crates/but-graph/Cargo.toml +++ b/crates/but-graph/Cargo.toml @@ -16,7 +16,7 @@ but-core.workspace = true gix = { workspace = true, features = ["revision"] } bstr.workspace = true -petgraph = { version = "0.8.3", default-features = false, features = ["stable_graph", "std"] } +petgraph.workspace = true anyhow.workspace = true bitflags.workspace = true tracing.workspace = true @@ -30,4 +30,3 @@ gitbutler-reference.workspace = true gix-testtools.workspace = true insta = "1.44.3" - diff --git a/crates/but-rebase/Cargo.toml b/crates/but-rebase/Cargo.toml index cf7e4eacdf..0fd657c003 100644 --- a/crates/but-rebase/Cargo.toml +++ b/crates/but-rebase/Cargo.toml @@ -14,16 +14,19 @@ but-core.workspace = true but-gerrit.workspace = true but-error.workspace = true but-oxidize.workspace = true +but-graph.workspace = true -gix = { workspace = true, features = ["revision", "merge"]} +gix = { workspace = true, features = ["revision", "merge"] } anyhow.workspace = true tracing.workspace = true bstr.workspace = true tempfile.workspace = true serde.workspace = true toml.workspace = true +petgraph.workspace = true [dev-dependencies] but-testsupport.workspace = true insta = "1.44.3" but-core = { workspace = true, features = ["testing"] } +but-meta.workspace = true diff --git a/crates/but-rebase/src/cherry_pick.rs b/crates/but-rebase/src/cherry_pick.rs index 8560ae58e8..440c247692 100644 --- a/crates/but-rebase/src/cherry_pick.rs +++ b/crates/but-rebase/src/cherry_pick.rs @@ -321,13 +321,13 @@ pub(crate) mod function { #[derive(Default, Debug, Clone, Serialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct ConflictEntries { - ancestor_entries: Vec, - our_entries: Vec, - their_entries: Vec, + pub(crate) ancestor_entries: Vec, + pub(crate) our_entries: Vec, + pub(crate) their_entries: Vec, } impl ConflictEntries { - fn has_entries(&self) -> bool { + pub(crate) fn has_entries(&self) -> bool { !self.ancestor_entries.is_empty() || !self.our_entries.is_empty() || !self.their_entries.is_empty() @@ -345,7 +345,7 @@ pub(crate) mod function { } /// Return the `conflicted` header field value. - fn conflicted_header_field(&self) -> Option { + pub(crate) fn conflicted_header_field(&self) -> Option { let entries = self.total_entries(); Some(if entries > 0 { entries as u64 } else { 1 }) } diff --git a/crates/but-rebase/src/graph_rebase/cherry_pick.rs b/crates/but-rebase/src/graph_rebase/cherry_pick.rs new file mode 100644 index 0000000000..37179a7b5b --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/cherry_pick.rs @@ -0,0 +1,356 @@ +//! Cherry picking generalised for N to M cases. + +use std::path::PathBuf; + +use crate::{cherry_pick::function::ConflictEntries, commit::DateMode}; +use anyhow::{Context as _, Result, bail}; +use bstr::BString; +use but_core::commit::{HEADERS_CONFLICTED_FIELD, HeadersV2, TreeKind}; +use but_oxidize::GixRepositoryExt as _; +use gix::{objs::tree::EntryKind, prelude::ObjectIdExt as _}; + +/// Describes the outcome of cherrypick. +#[derive(Debug, Clone, Copy)] +pub enum CherryPickOutcome { + /// Successfully cherry picked cleanly. + Commit(gix::ObjectId), + /// Successfully cherry picked and created a conflicted commit. + ConflictedCommit(gix::ObjectId), + /// No cherry pick was required since all the parents remained the same. + Identity(gix::ObjectId), + /// Represents the cases where either the source or the target commits failed to + /// merge cleanly. + FailedToMergeBases, +} + +/// Cherry pick, but supports supports cherry-picking merge commits. +/// +/// When cherry-picking a commit onto two or more commits, we first find the +/// merge of the two "onto" commits, and then cherry-pick onto that. +/// +/// When cherry-picking a commit with two or more parents, we first find the +/// merge of the parents, and use that as the "base". +/// +/// EG, if I've got commit X, with the parents A and B, and I want to cherry +/// pick onto C and D. +/// - I first find the merge of A and B, which I'll call N. +/// - Then, I find the merge of C and D, which I'll call M. +/// - Then, I do the three way merge where N is the "base", M is "ours", and X +/// is "theirs". +/// +/// Except in the case where X is conflicted. In that case we then make use of +/// X's "base" sub-tree as the base. +pub fn cherry_pick( + repo: &gix::Repository, + target: gix::ObjectId, + ontos: &[gix::ObjectId], +) -> Result { + let target = but_core::Commit::from_id(target.attach(repo))?; + if ontos == target.parents.as_slice() { + // We don't need to rebase + return Ok(CherryPickOutcome::Identity(target.id.detach())); + } + + let base_t = find_base_tree(&target)?; + // We always want the "theirs-ist" side of the target if it's conflicted. + let target_t = find_real_tree(&target, TreeKind::Theirs)?; + // We want to cherry-pick onto the merge result. + let onto_t = tree_from_merging_commits(repo, ontos, TreeKind::AutoResolution)?; + + match (base_t, onto_t) { + (MergeOutcome::NoCommit, MergeOutcome::NoCommit) => { + // We shouldn't actually ever hit this because it should be handled + // by the ontos & parents comparison. + Ok(CherryPickOutcome::Identity(target.id.detach())) + } + (MergeOutcome::Conflict, _) | (_, MergeOutcome::Conflict) => { + // TODO(cto): We can handle the specific case where (the base is + // _not_ conflicted & the ontos _is_ conflicted & the target == base + // & ontos.len() == 2) by writing out the ontos conflict as a + // conflicted merge commit, without expanding our conflict + // representation. + Ok(CherryPickOutcome::FailedToMergeBases) + } + ( + MergeOutcome::Success(_) | MergeOutcome::NoCommit, + MergeOutcome::Success(_) | MergeOutcome::NoCommit, + ) => { + let empty_tree = gix::ObjectId::empty_tree(gix::hash::Kind::Sha1); + let base_t = base_t.object_id().unwrap_or(empty_tree); + let onto_t = onto_t.object_id().unwrap_or(empty_tree); + + let mut outcome = repo.merge_trees( + base_t, + onto_t, + target_t, + 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( + ontos, + target, + tree_id, + outcome, + conflict_kind, + base_t, + onto_t, + target_t.detach(), + )?; + Ok(CherryPickOutcome::ConflictedCommit( + conflicted_commit.detach(), + )) + } else { + Ok(CherryPickOutcome::Commit( + commit_from_unconflicted_tree(ontos, target, tree_id)?.detach(), + )) + } + } + } +} + +#[derive(Debug, Clone, Copy)] +enum MergeOutcome { + Success(gix::ObjectId), + NoCommit, + Conflict, +} + +impl MergeOutcome { + fn object_id(&self) -> Option { + match self { + Self::Success(oid) => Some(*oid), + _ => None, + } + } +} + +fn find_base_tree(target: &but_core::Commit) -> Result { + if target.is_conflicted() { + Ok(MergeOutcome::Success( + find_real_tree(target, TreeKind::Base)?.detach(), + )) + } else { + tree_from_merging_commits(target.id.repo, &target.parents, TreeKind::AutoResolution) + } +} + +/// Merge together many commits, making use of the preferenced tree. +fn tree_from_merging_commits( + repo: &gix::Repository, + commits: &[gix::ObjectId], + preference: TreeKind, +) -> Result { + let mut to_merge = commits.to_vec(); + let Some(sum) = to_merge.pop() else { + // Handle the case where no commits are given. + return Ok(MergeOutcome::NoCommit); + }; + let mut sum = find_real_tree(&but_core::Commit::from_id(sum.attach(repo))?, preference)?; + + let base_tree = match repo.merge_base_octopus(commits.to_owned()) { + Ok(oid) => { + let commit = but_core::Commit::from_id(oid)?; + find_real_tree(&commit, TreeKind::AutoResolution)?.detach() + } + // It's very possible we'll see scenarios where there are two parents + // that have no common ancestor. We should handle that well by using the + // empty tree as the base. + Err(gix::repository::merge_base_octopus::Error::MergeBaseOctopus( + gix::repository::merge_base_octopus_with_graph::Error::NoMergeBase, + )) => gix::ObjectId::empty_tree(gix::hash::Kind::Sha1), + Err(e) => bail!(e), + }; + + while let Some(commit) = to_merge.pop() { + let commit = but_core::Commit::from_id(commit.attach(repo))?; + let tree = find_real_tree(&commit, preference)?; + let (options, conflicts) = repo.merge_options_fail_fast()?; + + let mut output = + repo.merge_trees(base_tree, sum, tree, repo.default_merge_labels(), options)?; + + if output.has_unresolved_conflicts(conflicts) { + return Ok(MergeOutcome::Conflict); + } + + sum = output.tree.write()?; + } + + Ok(MergeOutcome::Success(sum.detach())) +} + +fn find_real_tree<'repo>( + commit: &but_core::Commit<'repo>, + side: TreeKind, +) -> anyhow::Result> { + Ok(if commit.is_conflicted() { + let tree = commit.id.repo.find_tree(commit.tree)?; + let conflicted_side = tree + .find_entry(side.as_tree_entry_name()) + .context("Failed to get conflicted side of commit")?; + conflicted_side.id() + } else { + commit.tree_id_or_auto_resolution()? + }) +} + +fn commit_from_unconflicted_tree<'repo>( + parents: &[gix::ObjectId], + to_rebase: but_core::Commit<'repo>, + resolved_tree_id: gix::Id<'repo>, +) -> anyhow::Result> { + let repo = to_rebase.id.repo; + + let headers = to_rebase.headers(); + let to_rebase_is_conflicted = headers.as_ref().is_some_and(|hdr| hdr.is_conflicted()); + let mut new_commit = to_rebase.inner; + new_commit.tree = resolved_tree_id.detach(); + + // Ensure the commit isn't thinking it's conflicted. + if to_rebase_is_conflicted { + if let Some(pos) = new_commit + .extra_headers() + .find_pos(HEADERS_CONFLICTED_FIELD) + { + new_commit.extra_headers.remove(pos); + } + } else if headers.is_none() { + new_commit + .extra_headers + .extend(Vec::<(BString, BString)>::from(&HeadersV2::default())); + } + new_commit.parents = parents.into(); + + Ok(crate::commit::create(repo, new_commit, DateMode::CommitterUpdateAuthorKeep)?.attach(repo)) +} + +#[allow(clippy::too_many_arguments)] +fn commit_from_conflicted_tree<'repo>( + parents: &[gix::ObjectId], + mut to_rebase: but_core::Commit<'repo>, + resolved_tree_id: gix::Id<'repo>, + cherry_pick: gix::merge::tree::Outcome<'_>, + treat_as_unresolved: gix::merge::tree::TreatAsUnresolved, + base_tree_id: gix::ObjectId, + ours_tree_id: gix::ObjectId, + theirs_tree_id: gix::ObjectId, +) -> anyhow::Result> { + let repo = resolved_tree_id.repo; + // in case someone checks this out with vanilla Git, we should warn why it looks like this + let readme_content = + b"You have checked out a GitButler Conflicted commit. You probably didn't mean to do this."; + let readme_blob = repo.write_blob(readme_content)?; + + let conflicted_files = + extract_conflicted_files(resolved_tree_id, cherry_pick, treat_as_unresolved)?; + + // convert files into a string and save as a blob + let conflicted_files_string = toml::to_string(&conflicted_files)?; + let conflicted_files_blob = repo.write_blob(conflicted_files_string.as_bytes())?; + + let mut tree = repo.empty_tree().edit()?; + + tree.upsert( + TreeKind::Ours.as_tree_entry_name(), + EntryKind::Tree, + ours_tree_id, + )?; + tree.upsert( + TreeKind::Theirs.as_tree_entry_name(), + EntryKind::Tree, + theirs_tree_id, + )?; + tree.upsert( + TreeKind::Base.as_tree_entry_name(), + EntryKind::Tree, + base_tree_id, + )?; + tree.upsert( + TreeKind::AutoResolution.as_tree_entry_name(), + EntryKind::Tree, + resolved_tree_id, + )?; + tree.upsert(".conflict-files", EntryKind::Blob, conflicted_files_blob)?; + tree.upsert("README.txt", EntryKind::Blob, readme_blob)?; + + let mut headers = to_rebase.headers().unwrap_or_default(); + headers.conflicted = conflicted_files.conflicted_header_field(); + to_rebase.tree = tree.write().context("failed to write tree")?.detach(); + to_rebase.parents = parents.into(); + + to_rebase.set_headers(&headers); + Ok( + crate::commit::create(repo, to_rebase.inner, DateMode::CommitterUpdateAuthorKeep)? + .attach(repo), + ) +} + +fn extract_conflicted_files( + merged_tree_id: gix::Id<'_>, + merge_result: gix::merge::tree::Outcome<'_>, + treat_as_unresolved: gix::merge::tree::TreatAsUnresolved, +) -> anyhow::Result { + use gix::index::entry::Stage; + let repo = merged_tree_id.repo; + let mut index = repo.index_from_tree(&merged_tree_id)?; + merge_result.index_changed_after_applying_conflicts( + &mut index, + treat_as_unresolved, + gix::merge::tree::apply_index_entries::RemovalMode::Mark, + ); + let (mut ancestor_entries, mut our_entries, mut their_entries) = + (Vec::new(), Vec::new(), Vec::new()); + for entry in index.entries() { + let stage = entry.stage(); + let storage = match stage { + Stage::Unconflicted => { + continue; + } + Stage::Base => &mut ancestor_entries, + Stage::Ours => &mut our_entries, + Stage::Theirs => &mut their_entries, + }; + + let path = entry.path(&index); + storage.push(gix::path::from_bstr(path).into_owned()); + } + let mut out = ConflictEntries { + ancestor_entries, + our_entries, + their_entries, + }; + + // Since we typically auto-resolve with 'ours', it maybe that conflicting entries don't have an + // unconflicting counterpart anymore, so they are not applied (which is also what Git does). + // So to have something to show for - we *must* produce a conflict, extract paths manually. + // TODO(ST): instead of doing this, don't pre-record the paths. Instead redo the merge without + // merge-strategy so that the index entries can be used instead. + if !out.has_entries() { + fn push_unique(v: &mut Vec, change: &gix::diff::tree_with_rewrites::Change) { + let path = gix::path::from_bstr(change.location()).into_owned(); + if !v.contains(&path) { + v.push(path); + } + } + for conflict in merge_result + .conflicts + .iter() + .filter(|c| c.is_unresolved(treat_as_unresolved)) + { + let (ours, theirs) = conflict.changes_in_resolution(); + push_unique(&mut out.our_entries, ours); + push_unique(&mut out.their_entries, theirs); + } + } + assert_eq!( + out.has_entries(), + merge_result.has_unresolved_conflicts(treat_as_unresolved), + "Must have entries to indicate conflicting files, or bad things will happen later: {:#?}", + merge_result.conflicts + ); + Ok(out) +} diff --git a/crates/but-rebase/src/graph_rebase/commit.rs b/crates/but-rebase/src/graph_rebase/commit.rs new file mode 100644 index 0000000000..2af466a1fd --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/commit.rs @@ -0,0 +1,25 @@ +//! Provides some slightly higher level tools to help with manipulating commits, in preparation for use in the editor. + +use anyhow::Result; +use gix::prelude::ObjectIdExt; + +use crate::{ + commit::{DateMode, create}, + graph_rebase::Editor, +}; + +impl Editor { + /// Finds a commit from inside the editor's in memory repository. + pub fn find_commit(&self, id: gix::ObjectId) -> Result> { + but_core::Commit::from_id(id.attach(&self.repo)) + } + + /// Writes a commit with correct signing to the in memory repository. + pub fn write_commit( + &self, + commit: but_core::Commit<'_>, + date_mode: DateMode, + ) -> Result { + create(&self.repo, commit.inner, date_mode) + } +} diff --git a/crates/but-rebase/src/graph_rebase/creation.rs b/crates/but-rebase/src/graph_rebase/creation.rs new file mode 100644 index 0000000000..9ec4cecef2 --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/creation.rs @@ -0,0 +1,152 @@ +use std::collections::BTreeMap; + +use anyhow::{Result, bail}; +use but_graph::{Commit, CommitFlags, Graph, Segment}; +use petgraph::Direction; + +use crate::graph_rebase::{Checkouts, Edge, Editor, Step, StepGraph, StepGraphIndex}; + +/// Provides an extension for creating an Editor out of the segment graph +pub trait GraphExt { + /// Creates an editor. + fn to_editor(&self, repo: &gix::Repository) -> Result; +} + +impl GraphExt for Graph { + /// Creates an editor out of the segment graph. + fn to_editor(&self, repo: &gix::Repository) -> Result { + // TODO(CTO): Look into traversing "in workspace" segments that are not reachable from the entrypoint + // TODO(CTO): Look into stopping at the common base + let entrypoint = self.lookup_entrypoint()?; + + // Commits in this list are ordered such that iterating in reverse will + // have any relevant parent commits already inserted in the graph. + let mut commits: Vec = Vec::new(); + // References are ordered from child-most to parent-most + let mut references: BTreeMap> = BTreeMap::new(); + + self.visit_all_segments_including_start_until( + entrypoint.segment_index, + Direction::Outgoing, + |segment| { + // Make a note to create a reference for named segments + if let Some(refname) = segment.ref_name() + && let Some(commit) = find_nearest_commit(self, segment) + { + references + .entry(commit.id) + .and_modify(|rs| rs.push(refname.to_owned())) + .or_insert_with(|| vec![refname.to_owned()]); + } + + // Make a note to create a references that sit on commits + for commit in &segment.commits { + if !commit.refs.is_empty() { + commit.flags.contains(CommitFlags::InWorkspace); + let refs = commit + .refs + .iter() + .map(|r| r.ref_name.clone()) + .collect::>(); + if let Some(entry) = references.get_mut(&commit.id) { + entry.extend(refs); + } else { + references.insert(commit.id, refs); + } + } + } + + commits.extend(segment.commits.clone()); + + false + }, + ); + + // When adding child-nodes, this lookup tells us where to find the + // relevant "parent" to point to. + let mut steps_for_commits: BTreeMap = BTreeMap::new(); + let mut graph = StepGraph::new(); + + while let Some(c) = commits.pop() { + let parent_steps = c + .parent_ids + .iter() + .enumerate() + .filter_map(|(o, step)| Some((o, steps_for_commits.get(step)?))) + .collect::>(); + + let has_no_parents = c.parent_ids.is_empty(); + let has_missing_parent_steps = parent_steps.len() != c.parent_ids.len(); + let has_no_parent_steps = parent_steps.is_empty(); + + // If we are missing _some_ but not all parents, something has gone wrong. + if has_missing_parent_steps && !has_no_parent_steps { + bail!( + "Rebase creation has failed. Expected {} parent steps, found {}", + c.parent_ids.len(), + parent_steps.len() + ); + }; + + // If the commit has parents in the commit graph, but none of + // them are in the graph, this means but-graph did a partial + // traversal and we want to preserve the commit as it is. + let preserved_parents = if !has_no_parents && has_no_parent_steps { + Some(c.parent_ids) + } else { + None + }; + + let mut ni = graph.add_node(Step::Pick { + id: c.id, + preserved_parents, + }); + + for (order, parent_ni) in parent_steps { + graph.add_edge(ni, *parent_ni, Edge { order }); + } + + if let Some(refs) = references.get_mut(&c.id) { + // We insert in reverse to preserve the child-most to + // parent-most ordering that the frontend sees in the step graph + for r in refs.iter().rev() { + let ref_ni = graph.add_node(Step::Reference { refname: r.clone() }); + graph.add_edge(ref_ni, ni, Edge { order: 0 }); + ni = ref_ni; + } + } + + steps_for_commits.insert(c.id, ni); + } + + Ok(Editor { + graph, + initial_references: references.values().flatten().cloned().collect(), + // TODO(CTO): We need to eventually list all worktrees that we own + // here so we can `safe_checkout` them too. + checkouts: vec![Checkouts::Head], + repo: repo.clone().with_object_memory(), + }) + } +} + +/// Find the commit that is nearest to the top of the segment via a first parent +/// traversal. +fn find_nearest_commit<'graph>( + graph: &'graph Graph, + segment: &'graph Segment, +) -> Option<&'graph Commit> { + let mut target = Some(segment); + while let Some(s) = target { + if let Some(c) = s.commits.first() { + return Some(c); + } + + target = graph + .segments_below_in_order(s.id) + .next() + .map(|s| &graph[s.1]); + } + + None +} diff --git a/crates/but-rebase/src/graph_rebase/materialize.rs b/crates/but-rebase/src/graph_rebase/materialize.rs new file mode 100644 index 0000000000..a07bbb6345 --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/materialize.rs @@ -0,0 +1,45 @@ +//! Functions for materializing a rebase +use crate::graph_rebase::{Checkouts, rebase::SuccessfulRebase}; +use anyhow::Result; +use but_core::{ + ObjectStorageExt as _, + worktree::{ + checkout::{Options, UncommitedWorktreeChanges}, + safe_checkout_from_head, + }, +}; + +impl SuccessfulRebase { + /// Materializes a history rewrite + pub fn materialize(mut self) -> Result<()> { + let repo = self.repo.clone(); + if let Some(memory) = self.repo.objects.take_object_memory() { + memory.persist(self.repo)?; + } + + for checkout in self.checkouts { + match checkout { + Checkouts::Head => { + let head_oid = repo.head_commit()?.id; + if let Some(new_head) = self.commit_mapping.get(&head_oid) { + // If the head has changed (which means it's in the + // commit mapping), perform a safe checkout. + safe_checkout_from_head( + *new_head, + &repo, + Options { + uncommitted_changes: + UncommitedWorktreeChanges::KeepAndAbortOnConflict, + skip_head_update: true, + }, + )?; + } + } + } + } + + repo.edit_references(self.ref_edits.clone())?; + + Ok(()) + } +} diff --git a/crates/but-rebase/src/graph_rebase/mod.rs b/crates/but-rebase/src/graph_rebase/mod.rs new file mode 100644 index 0000000000..bffeb9c461 --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/mod.rs @@ -0,0 +1,92 @@ +#![deny(missing_docs)] +//! One graph based engine to rule them all, +//! one vector based to find them, +//! one mess of git2 code to bring them all, +//! and in the darknes bind them. + +mod creation; +pub mod rebase; +pub use creation::GraphExt; +pub mod cherry_pick; +pub mod commit; +pub mod materialize; +pub mod mutate; + +/// Utilities for testing +pub mod testing; + +/// Describes what action the engine should take +#[derive(Debug, Clone)] +pub enum Step { + /// Cherry picks the given commit into the new location in the graph + Pick { + /// The ID of the commit getting picked + id: gix::ObjectId, + /// If we are dealing with a sub-graph with an incomplete history, we + /// need to represent the bottom most commits in a way that we preserve + /// their parents. + /// + /// If this is Some, the commit WILL NOT be picked onto the parents the + /// graph implies but instead on to the parents listed here. + /// + /// This is intended to be a private API + preserved_parents: Option>, + }, + /// Represents applying a reference to the commit found at it's first parent + Reference { + /// The refname + refname: gix::refs::FullName, + }, + /// Used as a placeholder after removing a pick or reference + None, +} + +impl Step { + /// Creates a pick with the expected defaults + pub fn new_pick(id: gix::ObjectId) -> Self { + Self::Pick { + id, + preserved_parents: None, + } + } +} + +/// Used to represent a connection between a given commit. +#[derive(Debug, Clone)] +pub(crate) struct Edge { + /// Represents in which order the `parent` fields should be written out + /// + /// A child commit should have edges that all have unique orders. In order + /// to achive that we can employ the following semantics. + order: usize, +} + +type StepGraphIndex = petgraph::stable_graph::NodeIndex; +type StepGraph = petgraph::stable_graph::StableDiGraph; + +/// Points to a step in the rebase editor. +#[derive(Debug, Clone)] +pub struct Selector { + id: StepGraphIndex, +} + +/// Represents places where `safe_checkout` should be called from +#[derive(Debug, Clone)] +pub(crate) enum Checkouts { + /// The HEAD of the `repo` the editor was created for. + Head, +} + +/// Used to manipulate a set of picks. +#[derive(Debug, Clone)] +pub struct Editor { + /// The internal graph of steps + graph: StepGraph, + /// Initial references. This is used to track any references that might need + /// deleted. + initial_references: Vec, + /// Worktrees that we might need to perform `safe_checkout` on. + checkouts: Vec, + /// The in-memory repository that the rebase engine works with. + repo: gix::Repository, +} diff --git a/crates/but-rebase/src/graph_rebase/mutate.rs b/crates/but-rebase/src/graph_rebase/mutate.rs new file mode 100644 index 0000000000..174ead27d7 --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/mutate.rs @@ -0,0 +1,115 @@ +//! Operations for mutating the editor + +use anyhow::{Result, anyhow}; +use petgraph::{Direction, visit::EdgeRef}; + +use crate::graph_rebase::{Edge, Editor, Selector, Step}; + +/// Describes where relative to the selector a step should be inserted +#[derive(Debug, Clone, Copy)] +pub enum InsertSide { + /// When inserting above, any nodes that point to the selector will now + /// point to the inserted node instead. + Above, + /// When inserting below, any nodes that the selector points to will now be + /// pointed to by the inserted node instead. + Below, +} + +/// Operations for mutating the commit graph +impl Editor { + /// Get a selector to a particular commit in the graph + pub fn select_commit(&self, target: gix::ObjectId) -> Result { + match self.try_select_commit(target) { + Some(selector) => Ok(selector), + None => Err(anyhow!("Failed to find commit {target} in rebase editor")), + } + } + + /// Get a selector to a particular reference in the graph + pub fn select_reference(&self, target: &gix::refs::FullNameRef) -> Result { + match self.try_select_reference(target) { + Some(selector) => Ok(selector), + None => Err(anyhow!( + "Failed to find reference {target} in rebase editor" + )), + } + } + + /// Get a selector to a particular commit in the graph + pub fn try_select_commit(&self, target: gix::ObjectId) -> Option { + for node_idx in self.graph.node_indices() { + if let Step::Pick { id, .. } = self.graph[node_idx] + && id == target + { + return Some(Selector { id: node_idx }); + } + } + + None + } + + /// Get a selector to a particular reference in the graph + pub fn try_select_reference(&self, target: &gix::refs::FullNameRef) -> Option { + for node_idx in self.graph.node_indices() { + if let Step::Reference { refname } = &self.graph[node_idx] + && target == refname.as_ref() + { + return Some(Selector { id: node_idx }); + } + } + + None + } + + /// Replaces the node that the function was pointing to. + /// + /// Returns the replaced step. + pub fn replace(&mut self, target: &Selector, step: Step) -> Step { + let old = self.graph[target.id].clone(); + self.graph[target.id] = step; + old + } + + /// Inserts a new node relative to a selector + /// + /// + /// When inserting above, any nodes that point to the selector will now + /// point to the inserted node instead. When inserting below, any nodes + /// that the selector points to will now be pointed to by the inserted node + /// instead. + pub fn insert(&mut self, target: &Selector, step: Step, side: InsertSide) { + match side { + InsertSide::Above => { + let edges = self + .graph + .edges_directed(target.id, Direction::Incoming) + .map(|e| (e.id(), e.weight().to_owned(), e.source())) + .collect::>(); + + let new_idx = self.graph.add_node(step); + self.graph.add_edge(new_idx, target.id, Edge { order: 0 }); + + for (edge_id, edge_weight, edge_source) in edges { + self.graph.remove_edge(edge_id); + self.graph.add_edge(edge_source, new_idx, edge_weight); + } + } + InsertSide::Below => { + let edges = self + .graph + .edges_directed(target.id, Direction::Outgoing) + .map(|e| (e.id(), e.weight().to_owned(), e.target())) + .collect::>(); + + let new_idx = self.graph.add_node(step); + self.graph.add_edge(target.id, new_idx, Edge { order: 0 }); + + for (edge_id, edge_weight, edge_target) in edges { + self.graph.remove_edge(edge_id); + self.graph.add_edge(new_idx, edge_target, edge_weight); + } + } + } + } +} diff --git a/crates/but-rebase/src/graph_rebase/rebase.rs b/crates/but-rebase/src/graph_rebase/rebase.rs new file mode 100644 index 0000000000..8bed56215c --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/rebase.rs @@ -0,0 +1,661 @@ +//! Perform the actual rebase operations + +use std::collections::{HashMap, HashSet, VecDeque}; + +use crate::graph_rebase::{ + Checkouts, Editor, Step, StepGraph, StepGraphIndex, + cherry_pick::{CherryPickOutcome, cherry_pick}, +}; +use anyhow::{Context, Result, bail}; +use gix::refs::{ + Target, + transaction::{Change, LogChange, PreviousValue, RefEdit}, +}; +use petgraph::{Direction, visit::EdgeRef}; + +/// Represents a successful rebase, and any valid, but potentially conflicting scenarios it had. +#[allow(unused)] +#[derive(Debug, Clone)] +pub struct SuccessfulRebase { + pub(crate) repo: gix::Repository, + /// A mapping of any commits that were rewritten as part of the rebase + pub(crate) commit_mapping: HashMap, + /// A mapping between the original step graph and the new one + pub(crate) graph_mapping: HashMap, + /// Any reference edits that need to be committed as a result of the history + /// rewrite + pub(crate) ref_edits: Vec, + /// The new step graph + pub(crate) graph: StepGraph, + /// To checkout + pub(crate) checkouts: Vec, +} + +impl Editor { + /// Perform the rebase + pub fn rebase(self) -> Result { + // First we want to get a list of nodes that can be reached by + // traversing downwards from the heads that we care about. + // Usually there would be just one "head" which is an index to access + // the reference step for `gitbutler/workspace`, but there could be + // multiple. + + let mut ref_edits = vec![]; + let steps_to_pick = order_steps_picking( + &self.graph, + &self + .graph + .externals(Direction::Incoming) + .collect::>(), + ); + + // A 1 to 1 mapping between the incoming graph and the output graph + let mut graph_mapping: HashMap = HashMap::new(); + // The step graph with updated commit oids + let mut output_graph = StepGraph::new(); + let mut commit_mapping = HashMap::new(); + let mut unchanged_references = vec![]; + + for step_idx in steps_to_pick { + // Do the frikkin rebase man! + let step = self.graph[step_idx].clone(); + let new_idx = match step { + Step::Pick { + id, + preserved_parents, + } => { + let graph_parents = collect_ordered_parents(&self.graph, step_idx); + let ontos = match 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 { id, .. } => Ok(id), + _ => bail!("A parent in the output graph is not a pick"), + } + }) + .collect::>>()?, + }; + + let outcome = cherry_pick(&self.repo, id, &ontos)?; + + match outcome { + CherryPickOutcome::Commit(new_id) + | CherryPickOutcome::ConflictedCommit(new_id) + | CherryPickOutcome::Identity(new_id) => { + let new_idx = output_graph.add_node(Step::Pick { + id: new_id, + preserved_parents, + }); + graph_mapping.insert(step_idx, new_idx); + if id != new_id { + commit_mapping.insert(id, new_id); + } + + new_idx + } + CherryPickOutcome::FailedToMergeBases => { + // Exit early - the rebase failed because it encountered a commit it couldn't pick + // TODO(CTO): Detect if this was the merge commit itself & signal that seperatly + bail!("Failed to merge bases for commit {id}"); + } + } + } + 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 { id, .. } => id, + _ => bail!("A parent in the output graph is not a pick"), + }; + + let reference = self.repo.try_find_reference(&refname)?; + + if let Some(reference) = reference { + let target = reference.target(); + match target { + gix::refs::TargetRef::Object(id) => { + if id == to_reference { + unchanged_references.push(refname.clone()); + } else { + ref_edits.push(RefEdit { + name: refname.clone(), + change: Change::Update { + log: LogChange::default(), + expected: PreviousValue::MustExistAndMatch( + target.into(), + ), + new: Target::Object(to_reference), + }, + deref: false, + }); + } + } + gix::refs::TargetRef::Symbolic(name) => { + bail!("Attempted to update the symbolic reference {}", name); + } + } + } else { + ref_edits.push(RefEdit { + name: refname.clone(), + change: Change::Update { + log: LogChange::default(), + expected: PreviousValue::MustNotExist, + new: Target::Object(to_reference), + }, + deref: false, + }); + }; + + output_graph.add_node(Step::Reference { refname }) + } + Step::None => output_graph.add_node(Step::None), + }; + + graph_mapping.insert(step_idx, new_idx); + + let mut edges = self + .graph + .edges_directed(step_idx, petgraph::Direction::Outgoing) + .collect::>(); + edges.sort_by_key(|e| e.weight().order); + edges.reverse(); + + for e in edges { + let Some(new_parent) = graph_mapping.get(&e.target()) else { + bail!("Failed to find corresponding parent"); + }; + + output_graph.add_edge(new_idx, *new_parent, e.weight().clone()); + } + } + + // Find deleted references + for reference in self.initial_references.iter() { + if !ref_edits + .iter() + .any(|e| e.name.as_ref() == reference.as_ref()) + && !unchanged_references + .iter() + .any(|e| e.as_ref() == reference.as_ref()) + { + ref_edits.push(RefEdit { + name: reference.clone(), + change: Change::Delete { + log: gix::refs::transaction::RefLog::AndReference, + expected: PreviousValue::MustExist, + }, + deref: false, + }); + } + } + + Ok(SuccessfulRebase { + repo: self.repo, + ref_edits, + commit_mapping, + graph_mapping, + graph: output_graph, + checkouts: self.checkouts.to_owned(), + }) + } +} + +/// Find the parents of a given node that are commit - in correct parent +/// ordering. +/// +/// We do this via a pruned depth first search. +fn collect_ordered_parents(graph: &StepGraph, target: StepGraphIndex) -> Vec { + let mut potential_parent_edges = graph + .edges_directed(target, petgraph::Direction::Outgoing) + .collect::>(); + potential_parent_edges.sort_by_key(|e| e.weight().order); + potential_parent_edges.reverse(); + + let mut seen = potential_parent_edges + .iter() + .map(|e| e.target()) + .collect::>(); + + let mut parents = vec![]; + + while let Some(candidate) = potential_parent_edges.pop() { + if let Step::Pick { .. } = graph[candidate.target()] { + parents.push(candidate.target()); + // Don't pursue the children + continue; + }; + + let mut outgoings = graph + .edges_directed(candidate.target(), petgraph::Direction::Outgoing) + .collect::>(); + outgoings.sort_by_key(|e| e.weight().order); + outgoings.reverse(); + + for edge in outgoings { + if seen.insert(edge.target()) { + potential_parent_edges.push(edge); + } + } + } + + parents +} + +/// Creates a list of step indicies ordered in the dependency order. +/// +/// We do this by first doing a breadth-first traversal down from the heads +/// (which would usually be the `gitbutler/workspace` reference step) in order +/// to determine which steps are reachable, and what the bottom most steps are. +/// +/// Then, we do a second traversal up from those bottom most +/// steps. +/// +/// This second traversal ensures that all the parents of any given node have +/// been seen, before traversing it. +fn order_steps_picking(graph: &StepGraph, heads: &[StepGraphIndex]) -> VecDeque { + let mut heads = heads.to_vec(); + let mut seen = heads.iter().cloned().collect::>(); + // Reachable nodes with no outgoing nodes. + let mut bases = VecDeque::new(); + + while let Some(head) = heads.pop() { + let edges = graph.edges_directed(head, petgraph::Direction::Outgoing); + + if edges.clone().count() == 0 { + bases.push_back(head); + continue; + } + + for edge in edges { + let t = edge.target(); + if seen.insert(t) { + heads.push(t); + } + } + } + + // Now we want to create a vector that contains all the steps in + // dependency order. + let mut ordered = bases.clone(); + let mut retraversed = bases.iter().cloned().collect::>(); + + while let Some(base) = bases.pop_front() { + for edge in graph.edges_directed(base, petgraph::Direction::Incoming) { + // We only want to queue nodes for traversing that have had all of their parents traversed. + let s = edge.source(); + let mut outgoing_edges = graph.edges_directed(s, petgraph::Direction::Outgoing); + let all_parents_seen = outgoing_edges.clone().count() == 0 + || outgoing_edges.all(|e| retraversed.contains(&e.target())); + if all_parents_seen && seen.contains(&s) && retraversed.insert(s) { + bases.push_back(s); + ordered.push_back(s); + }; + } + } + + ordered +} + +#[cfg(test)] +mod test { + mod collect_ordered_parents { + use std::str::FromStr as _; + + use anyhow::Result; + + use crate::graph_rebase::{Edge, Step, StepGraph, rebase::collect_ordered_parents}; + + #[test] + fn basic_scenario() -> Result<()> { + let mut graph = StepGraph::new(); + let a_id = gix::ObjectId::from_str("1000000000000000000000000000000000000000")?; + let a = graph.add_node(Step::Pick { + id: a_id, + preserved_parents: None, + }); + // First parent + let b_id = gix::ObjectId::from_str("1000000000000000000000000000000000000000")?; + let b = graph.add_node(Step::Pick { + id: b_id, + preserved_parents: None, + }); + // Second parent - is a reference + let c = graph.add_node(Step::Reference { + refname: "refs/heads/foobar".try_into()?, + }); + // Second parent's first child + let d_id = gix::ObjectId::from_str("3000000000000000000000000000000000000000")?; + let d = graph.add_node(Step::Pick { + id: d_id, + preserved_parents: None, + }); + // Second parent's second child + let e_id = gix::ObjectId::from_str("4000000000000000000000000000000000000000")?; + let e = graph.add_node(Step::Pick { + id: e_id, + preserved_parents: None, + }); + // Third parent + let f_id = gix::ObjectId::from_str("5000000000000000000000000000000000000000")?; + let f = graph.add_node(Step::Pick { + id: f_id, + preserved_parents: None, + }); + + // A's parents + graph.add_edge(a, b, Edge { order: 0 }); + graph.add_edge(a, c, Edge { order: 1 }); + graph.add_edge(a, f, Edge { order: 2 }); + + // C's parents + graph.add_edge(c, d, Edge { order: 0 }); + graph.add_edge(c, e, Edge { order: 1 }); + + let parents = collect_ordered_parents(&graph, a); + assert_eq!(&parents, &[b, d, e, f]); + + Ok(()) + } + + #[test] + fn insertion_order_is_irrelevant() -> Result<()> { + let mut graph = StepGraph::new(); + let a_id = gix::ObjectId::from_str("1000000000000000000000000000000000000000")?; + let a = graph.add_node(Step::Pick { + id: a_id, + preserved_parents: None, + }); + // First parent + let b_id = gix::ObjectId::from_str("1000000000000000000000000000000000000000")?; + let b = graph.add_node(Step::Pick { + id: b_id, + preserved_parents: None, + }); + // Second parent - is a reference + let c = graph.add_node(Step::Reference { + refname: "refs/heads/foobar".try_into()?, + }); + // Second parent's second child + let d_id = gix::ObjectId::from_str("3000000000000000000000000000000000000000")?; + let d = graph.add_node(Step::Pick { + id: d_id, + preserved_parents: None, + }); + // Second parent's first child + let e_id = gix::ObjectId::from_str("4000000000000000000000000000000000000000")?; + let e = graph.add_node(Step::Pick { + id: e_id, + preserved_parents: None, + }); + // Third parent + let f_id = gix::ObjectId::from_str("5000000000000000000000000000000000000000")?; + let f = graph.add_node(Step::Pick { + id: f_id, + preserved_parents: None, + }); + + // A's parents + graph.add_edge(a, f, Edge { order: 2 }); + graph.add_edge(a, c, Edge { order: 1 }); + graph.add_edge(a, b, Edge { order: 0 }); + + // C's parents + graph.add_edge(c, d, Edge { order: 1 }); + graph.add_edge(c, e, Edge { order: 0 }); + + let parents = collect_ordered_parents(&graph, a); + assert_eq!(&parents, &[b, e, d, f]); + + Ok(()) + } + } + + mod order_steps_picking { + use anyhow::Result; + use std::str::FromStr; + + use crate::graph_rebase::{ + Edge, Step, StepGraph, rebase::order_steps_picking, testing::TestingDot as _, + }; + + #[test] + fn basic_scenario() -> Result<()> { + let mut graph = StepGraph::new(); + let a = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("1000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let b = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("2000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let c = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("3000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + + graph.add_edge(a, b, Edge { order: 0 }); + graph.add_edge(b, c, Edge { order: 0 }); + + insta::assert_snapshot!(graph.steps_dot(), @r#" + digraph { + 0 [ label="pick: 1000000000000000000000000000000000000000"] + 1 [ label="pick: 2000000000000000000000000000000000000000"] + 2 [ label="pick: 3000000000000000000000000000000000000000"] + 0 -> 1 [ label="order: 0"] + 1 -> 2 [ label="order: 0"] + } + "#); + + let ordered_from_a = order_steps_picking(&graph, &[a]); + assert_eq!(&ordered_from_a, &[c, b, a]); + let ordered_from_b = order_steps_picking(&graph, &[b]); + assert_eq!(&ordered_from_b, &[c, b]); + let ordered_from_c = order_steps_picking(&graph, &[c]); + assert_eq!(&ordered_from_c, &[c]); + + Ok(()) + } + + #[test] + fn complex_scenario() -> Result<()> { + let mut graph = StepGraph::new(); + let a = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("1000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let b = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("2000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let c = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("3000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let d = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("4000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let e = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("5000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let f = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("6000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let g = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("7000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let h = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("8000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let i = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("8000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let j = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("8000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + + graph.add_edge(a, b, Edge { order: 0 }); + graph.add_edge(b, c, Edge { order: 0 }); + graph.add_edge(c, d, Edge { order: 0 }); + graph.add_edge(d, e, Edge { order: 0 }); + + graph.add_edge(f, g, Edge { order: 0 }); + graph.add_edge(g, c, Edge { order: 0 }); + + graph.add_edge(h, d, Edge { order: 0 }); + + graph.add_edge(i, j, Edge { order: 0 }); + + insta::assert_snapshot!(graph.steps_dot(), @r#" + digraph { + 0 [ label="pick: 1000000000000000000000000000000000000000"] + 1 [ label="pick: 2000000000000000000000000000000000000000"] + 2 [ label="pick: 3000000000000000000000000000000000000000"] + 3 [ label="pick: 4000000000000000000000000000000000000000"] + 4 [ label="pick: 5000000000000000000000000000000000000000"] + 5 [ label="pick: 6000000000000000000000000000000000000000"] + 6 [ label="pick: 7000000000000000000000000000000000000000"] + 7 [ label="pick: 8000000000000000000000000000000000000000"] + 8 [ label="pick: 8000000000000000000000000000000000000000"] + 9 [ label="pick: 8000000000000000000000000000000000000000"] + 0 -> 1 [ label="order: 0"] + 1 -> 2 [ label="order: 0"] + 2 -> 3 [ label="order: 0"] + 3 -> 4 [ label="order: 0"] + 5 -> 6 [ label="order: 0"] + 6 -> 2 [ label="order: 0"] + 7 -> 3 [ label="order: 0"] + 8 -> 9 [ label="order: 0"] + } + "#); + + let ordered_from_a = order_steps_picking(&graph, &[f, h]); + assert_eq!(&ordered_from_a, &[e, d, h, c, g, f]); + + Ok(()) + } + + #[test] + fn merge_scenario() -> Result<()> { + let mut graph = StepGraph::new(); + let a = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("1000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let b = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("2000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let c = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("3000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let d = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("4000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let e = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("5000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + + graph.add_edge(a, b, Edge { order: 0 }); + graph.add_edge(b, c, Edge { order: 0 }); + + graph.add_edge(a, d, Edge { order: 1 }); + graph.add_edge(d, e, Edge { order: 0 }); + graph.add_edge(e, b, Edge { order: 0 }); + + insta::assert_snapshot!(graph.steps_dot(), @r#" + digraph { + 0 [ label="pick: 1000000000000000000000000000000000000000"] + 1 [ label="pick: 2000000000000000000000000000000000000000"] + 2 [ label="pick: 3000000000000000000000000000000000000000"] + 3 [ label="pick: 4000000000000000000000000000000000000000"] + 4 [ label="pick: 5000000000000000000000000000000000000000"] + 0 -> 1 [ label="order: 0"] + 1 -> 2 [ label="order: 0"] + 0 -> 3 [ label="order: 1"] + 3 -> 4 [ label="order: 0"] + 4 -> 1 [ label="order: 0"] + } + "#); + + let ordered_from_a = order_steps_picking(&graph, &[a]); + assert_eq!(&ordered_from_a, &[c, b, e, d, a]); + + Ok(()) + } + + #[test] + fn merge_flipped_scenario() -> Result<()> { + let mut graph = StepGraph::new(); + let a = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("1000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let b = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("2000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let c = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("3000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let d = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("4000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + let e = graph.add_node(Step::Pick { + id: gix::ObjectId::from_str("5000000000000000000000000000000000000000")?, + preserved_parents: None, + }); + + graph.add_edge(a, d, Edge { order: 0 }); + graph.add_edge(d, e, Edge { order: 0 }); + graph.add_edge(e, b, Edge { order: 0 }); + graph.add_edge(b, c, Edge { order: 0 }); + + graph.add_edge(a, b, Edge { order: 1 }); + + insta::assert_snapshot!(graph.steps_dot(), @r#" + digraph { + 0 [ label="pick: 1000000000000000000000000000000000000000"] + 1 [ label="pick: 2000000000000000000000000000000000000000"] + 2 [ label="pick: 3000000000000000000000000000000000000000"] + 3 [ label="pick: 4000000000000000000000000000000000000000"] + 4 [ label="pick: 5000000000000000000000000000000000000000"] + 0 -> 3 [ label="order: 0"] + 3 -> 4 [ label="order: 0"] + 4 -> 1 [ label="order: 0"] + 1 -> 2 [ label="order: 0"] + 0 -> 1 [ label="order: 1"] + } + "#); + + let ordered_from_a = order_steps_picking(&graph, &[a]); + assert_eq!(&ordered_from_a, &[c, b, e, d, a]); + + Ok(()) + } + } +} diff --git a/crates/but-rebase/src/graph_rebase/testing.rs b/crates/but-rebase/src/graph_rebase/testing.rs new file mode 100644 index 0000000000..42208c3e48 --- /dev/null +++ b/crates/but-rebase/src/graph_rebase/testing.rs @@ -0,0 +1,46 @@ +#![deny(missing_docs)] +//! Testing utilities + +use petgraph::dot::{Config, Dot}; + +use crate::graph_rebase::{Editor, Step, StepGraph, rebase::SuccessfulRebase}; + +/// An extension trait that adds debugging output for graphs +pub trait TestingDot { + /// Creates a dot graph with labels + fn steps_dot(&self) -> String; +} + +impl TestingDot for Editor { + fn steps_dot(&self) -> String { + self.graph.steps_dot() + } +} + +impl TestingDot for SuccessfulRebase { + fn steps_dot(&self) -> String { + self.graph.steps_dot() + } +} + +impl TestingDot for StepGraph { + fn steps_dot(&self) -> String { + format!( + "{:?}", + Dot::with_attr_getters( + &self, + &[Config::EdgeNoLabel, Config::NodeNoLabel], + &|_, v| format!("label=\"order: {}\"", v.weight().order), + &|_, (_, step)| { + match step { + Step::Pick { id, .. } => format!("label=\"pick: {}\"", id), + Step::Reference { refname } => { + format!("label=\"reference: {}\"", refname.as_bstr()) + } + Step::None => "label=\"none\"".into(), + } + }, + ) + ) + } +} diff --git a/crates/but-rebase/src/lib.rs b/crates/but-rebase/src/lib.rs index 10cbb9d89b..b2ee52040e 100644 --- a/crates/but-rebase/src/lib.rs +++ b/crates/but-rebase/src/lib.rs @@ -10,6 +10,9 @@ use tracing::instrument; use crate::commit::DateMode; +/// Tools for manipulating commits +pub mod graph_rebase; + /// Types for use with cherry-picking pub mod cherry_pick; pub use cherry_pick::function::cherry_pick_one; diff --git a/crates/but-rebase/tests/fixtures/rebase.sh b/crates/but-rebase/tests/fixtures/rebase.sh index 9a0d1b041c..b88e8b4487 100755 --- a/crates/but-rebase/tests/fixtures/rebase.sh +++ b/crates/but-rebase/tests/fixtures/rebase.sh @@ -10,6 +10,17 @@ git init four-commits echo "c" >c && git add . && git commit -m "c" ) +git init many-references +(cd many-references + echo "base" >base && git add . && git commit -m "base" + echo "a" >a && git add . && git commit -m "a" + git branch X + git branch Y + git branch Z + echo "b" >b && git add . && git commit -m "b" + echo "c" >c && git add . && git commit -m "c" +) + git init three-branches-merged (cd three-branches-merged seq 50 60 >file && git add . && git commit -m "base" && git tag base @@ -47,3 +58,58 @@ git init merge-in-the-middle git checkout with-inner-merge && git merge --no-ff B echo seq 10 >'added-after-with-inner-merge' && git add . && git commit -m "on top of inner merge" ) + +git init cherry-pick-scenario +(cd cherry-pick-scenario + echo "a" > base-f && git add . && git commit -m "base" && git branch base + + git checkout -b single-clean-parent + echo "clean" > clean-f && git add . && git commit -m "single-clean-parent" + + git checkout -b single-clean-commit + echo "clean-commit" > clean-commit-f && git add . && git commit -m "single-clean-commit" + + git checkout -b single-target + git reset --hard base + echo "target" > target-f && git add . && git commit -m "single-target" + + git checkout -b single-conflicting-commit + git reset --hard single-clean-parent + echo "conflict" > target-f && git add . && git commit -m "single-conflicting-commit" + + git checkout -b second-target + git reset --hard base + echo "target 2" > target-2-f && git add . && git commit -m "second-target" + + git checkout -b second-conflicting-target + git reset --hard base + echo "target 2" > target-f && git add . && git commit -m "second-conflicting-target" + + git checkout -b second-clean-parent + git reset --hard base + echo "clean 2" > clean-2-f && git add . && git commit -m "second-clean-parent" + + git checkout -b merge-clean-commit + git merge single-clean-parent + echo "clean-commit" > clean-commit-f && git add . && git commit -m "merge-clean-commit" --amend + + git checkout -b merge-conflicting-commit + rm clean-commit-f && echo "conflict" > target-f && git add . && git commit -m "merge-conflicting-commit" --amend + + git checkout -b second-conflicting-parent + git reset --hard base + echo "conflict" > clean-f && git add . && git commit -m "second-conflicting-parent" + + git checkout -b merge-clean-commit-conflicting-parents + set +e + git merge single-clean-parent --no-edit + set -e + echo "resolved" > clean-f + git add . + git commit -m "foo" + echo "clean-commit" > clean-commit-f && git add . && git commit -m "merge-clean-commit-conflicting-parents" --amend + + git checkout -b base-conflicting + git reset --hard base + echo "conflict" > target-f && git add . && git commit -m "base-conflicting" --amend +) \ No newline at end of file diff --git a/crates/but-rebase/tests/rebase/error_handling.rs b/crates/but-rebase/tests/rebase/error_handling.rs index f71776cd57..46a6f17e20 100644 --- a/crates/but-rebase/tests/rebase/error_handling.rs +++ b/crates/but-rebase/tests/rebase/error_handling.rs @@ -11,7 +11,7 @@ fn non_existing_commit() -> gix::ObjectId { #[test] fn base_non_existing() -> anyhow::Result<()> { - let repo = fixture("four-commits")?; + let (repo, _) = fixture("four-commits")?; let result = Rebase::new(&repo, non_existing_commit(), None); assert_eq!( result.unwrap_err().to_string(), diff --git a/crates/but-rebase/tests/rebase/graph_rebase/cherry_pick.rs b/crates/but-rebase/tests/rebase/graph_rebase/cherry_pick.rs new file mode 100644 index 0000000000..8370537b4b --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/cherry_pick.rs @@ -0,0 +1,616 @@ +use anyhow::{Result, bail}; +use but_rebase::graph_rebase::cherry_pick::{CherryPickOutcome, cherry_pick}; +use but_testsupport::visualize_tree; +use gix::prelude::ObjectIdExt; + +use crate::{graph_rebase::set_var, utils::fixture_writable}; + +fn get_parents(id: &gix::Id) -> Result> { + Ok(id + .object()? + .peel_to_commit()? + .parent_ids() + .map(|i| i.detach()) + .collect()) +} + +#[test] +fn basic_cherry_pick_clean() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("single-clean-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto])?; + + insta::assert_debug_snapshot!(result, @r" + Commit( + Sha1(d455a27686ec5773502fb699b9d44ef2d3acc7aa), + ) + "); + + let CherryPickOutcome::Commit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 96a9057 + ├── base-f:100644:7898192 "a\n" + ├── clean-commit-f:100644:20a3acd "clean-commit\n" + └── target-f:100644:eb5a316 "target\n" + "#); + + Ok(()) +} +// Basic cherry pick - conflicting +#[test] +fn basic_cherry_pick_cp_conflicts() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("single-conflicting-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto])?; + + insta::assert_debug_snapshot!(result, @r" + ConflictedCommit( + Sha1(e016a9c4dc3335a33bfe340bdb227df31d1c1766), + ) + "); + + let CherryPickOutcome::ConflictedCommit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 3417b4c + ├── .auto-resolution:aa3d213 + │ ├── base-f:100644:7898192 "a\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-base-0:45eb973 + │ ├── base-f:100644:7898192 "a\n" + │ └── clean-f:100644:8312630 "clean\n" + ├── .conflict-files:100644:68fb397 "ancestorEntries = []\nourEntries = [\"target-f\"]\ntheirEntries = [\"target-f\"]\n" + ├── .conflict-side-0:aa3d213 + │ ├── base-f:100644:7898192 "a\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-side-1:c1a7ba6 + │ ├── base-f:100644:7898192 "a\n" + │ ├── clean-f:100644:8312630 "clean\n" + │ └── target-f:100644:9b1719f "conflict\n" + └── README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this." + "#); + + Ok(()) +} +// Basic cherry pick - identity +#[test] +fn basic_cherry_pick_identity() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("single-conflicting-commit")?; + let parents = get_parents(&target)?; + let result = cherry_pick(&repo, target.detach(), &parents)?; + + insta::assert_debug_snapshot!(result, @r" + Identity( + Sha1(b23d933c3781f649b740445e5337362d74b9103e), + ) + "); + + Ok(()) +} +// single parent to multiple parents - clean... this one is SFW +#[test] +fn single_parent_to_multiple_parents_clean() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("single-clean-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @r" + Commit( + Sha1(2cfca95cebf548c07da49b0dd7695957756a8a40), + ) + "); + + let CherryPickOutcome::Commit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto, onto2]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 2d609c4 + ├── base-f:100644:7898192 "a\n" + ├── clean-commit-f:100644:20a3acd "clean-commit\n" + ├── target-2-f:100644:caac8f9 "target 2\n" + └── target-f:100644:eb5a316 "target\n" + "#); + + Ok(()) +} + +// single parent to multiple parents - cp conflicts +#[test] +fn single_parent_to_multiple_parents_cp_conflicts() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("single-conflicting-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @r" + ConflictedCommit( + Sha1(21c6f513d6642236f0ce5406a2172fd7ccbd6e18), + ) + "); + + let CherryPickOutcome::ConflictedCommit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto, onto2]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 75fdd2c + ├── .auto-resolution:744efa9 + │ ├── base-f:100644:7898192 "a\n" + │ ├── target-2-f:100644:caac8f9 "target 2\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-base-0:45eb973 + │ ├── base-f:100644:7898192 "a\n" + │ └── clean-f:100644:8312630 "clean\n" + ├── .conflict-files:100644:68fb397 "ancestorEntries = []\nourEntries = [\"target-f\"]\ntheirEntries = [\"target-f\"]\n" + ├── .conflict-side-0:744efa9 + │ ├── base-f:100644:7898192 "a\n" + │ ├── target-2-f:100644:caac8f9 "target 2\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-side-1:c1a7ba6 + │ ├── base-f:100644:7898192 "a\n" + │ ├── clean-f:100644:8312630 "clean\n" + │ └── target-f:100644:9b1719f "conflict\n" + └── README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this." + "#); + + Ok(()) +} + +// single parent to multiple parents - parents conflict +#[test] +fn single_parent_to_multiple_parents_parents_conflict() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("single-clean-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-conflicting-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @"FailedToMergeBases"); + + Ok(()) +} + +// multiple parent to single parent - clean +#[test] +fn multiple_parents_to_single_parent_clean() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("merge-clean-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto])?; + + insta::assert_debug_snapshot!(result, @r" + Commit( + Sha1(239ca76f9cca49b56e8bce507e776cd8e8b5994a), + ) + "); + + let CherryPickOutcome::Commit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 96a9057 + ├── base-f:100644:7898192 "a\n" + ├── clean-commit-f:100644:20a3acd "clean-commit\n" + └── target-f:100644:eb5a316 "target\n" + "#); + + Ok(()) +} + +// multiple parent to single parent - cp conflicts +#[test] +fn multiple_parents_to_single_parent_cp_conflicts() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("merge-conflicting-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto])?; + + insta::assert_debug_snapshot!(result, @r" + ConflictedCommit( + Sha1(d89a10354a95cf16dfb39f339c56eb8f0e206f3d), + ) + "); + + let CherryPickOutcome::ConflictedCommit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + fde5970 + ├── .auto-resolution:aa3d213 + │ ├── base-f:100644:7898192 "a\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-base-0:4acd705 + │ ├── base-f:100644:7898192 "a\n" + │ ├── clean-2-f:100644:13e9394 "clean 2\n" + │ └── clean-f:100644:8312630 "clean\n" + ├── .conflict-files:100644:68fb397 "ancestorEntries = []\nourEntries = [\"target-f\"]\ntheirEntries = [\"target-f\"]\n" + ├── .conflict-side-0:aa3d213 + │ ├── base-f:100644:7898192 "a\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-side-1:09af0e9 + │ ├── base-f:100644:7898192 "a\n" + │ ├── clean-2-f:100644:13e9394 "clean 2\n" + │ ├── clean-f:100644:8312630 "clean\n" + │ └── target-f:100644:9b1719f "conflict\n" + └── README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this." + "#); + + Ok(()) +} + +// multiple parent to single parent - parents conflict +#[test] +fn multiple_parents_to_single_parent_parents_conflict() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo + .rev_parse_single("merge-clean-commit-conflicting-parents")? + .detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto])?; + + insta::assert_debug_snapshot!(result, @"FailedToMergeBases"); + + Ok(()) +} + +// multiple parents to multiple parents - clean +#[test] +fn multiple_parents_to_multiple_parents_clean() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("merge-clean-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @r" + Commit( + Sha1(7175c289fa60949d9e4f62b9dde4bbc9091fd81a), + ) + "); + + let CherryPickOutcome::Commit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto, onto2]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 2d609c4 + ├── base-f:100644:7898192 "a\n" + ├── clean-commit-f:100644:20a3acd "clean-commit\n" + ├── target-2-f:100644:caac8f9 "target 2\n" + └── target-f:100644:eb5a316 "target\n" + "#); + + Ok(()) +} + +// multiple parents to multiple parents - cp conflicts +#[test] +fn multiple_parents_to_multiple_parents_cp_conflicts() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("merge-conflicting-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @r" + ConflictedCommit( + Sha1(622983759b4d4e58b1bd8278f2be80f39b79d276), + ) + "); + + let CherryPickOutcome::ConflictedCommit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto, onto2]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + acdd833 + ├── .auto-resolution:744efa9 + │ ├── base-f:100644:7898192 "a\n" + │ ├── target-2-f:100644:caac8f9 "target 2\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-base-0:4acd705 + │ ├── base-f:100644:7898192 "a\n" + │ ├── clean-2-f:100644:13e9394 "clean 2\n" + │ └── clean-f:100644:8312630 "clean\n" + ├── .conflict-files:100644:68fb397 "ancestorEntries = []\nourEntries = [\"target-f\"]\ntheirEntries = [\"target-f\"]\n" + ├── .conflict-side-0:744efa9 + │ ├── base-f:100644:7898192 "a\n" + │ ├── target-2-f:100644:caac8f9 "target 2\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-side-1:09af0e9 + │ ├── base-f:100644:7898192 "a\n" + │ ├── clean-2-f:100644:13e9394 "clean 2\n" + │ ├── clean-f:100644:8312630 "clean\n" + │ └── target-f:100644:9b1719f "conflict\n" + └── README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this." + "#); + + Ok(()) +} + +// multiple parents to multiple parents - parents conflict +#[test] +fn multiple_parents_to_multiple_parents_base_parents_conflict() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo + .rev_parse_single("merge-clean-commit-conflicting-parents")? + .detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @"FailedToMergeBases"); + + Ok(()) +} + +#[test] +fn multiple_parents_to_multiple_parents_target_parents_conflict() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("merge-clean-commit")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-conflicting-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @"FailedToMergeBases"); + + Ok(()) +} + +// multiple parents to multiple parents - identity +#[test] +fn multiple_parents_to_multiple_parents_identity() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("merge-clean-commit")?; + let parents = get_parents(&target)?; + + let result = cherry_pick(&repo, target.detach(), &parents)?; + + insta::assert_debug_snapshot!(result, @r" + Identity( + Sha1(bec85a3ab113b86032660cac3d09afb4d342e135), + ) + "); + + Ok(()) +} + +// no parents cherry pick - is identity +#[test] +fn no_parents_identity() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("base")?; + + let result = cherry_pick(&repo, target.detach(), &[])?; + + insta::assert_debug_snapshot!(result, @r" + Identity( + Sha1(7a749663ddce268238da073e025f30a281120ef5), + ) + "); + + Ok(()) +} + +// single parent to no parents - clean +#[test] +fn single_parent_to_no_parents_clean() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("single-clean-commit")?.detach(); + + let result = cherry_pick(&repo, target, &[])?; + + insta::assert_debug_snapshot!(result, @r" + Commit( + Sha1(cc5199a198b9762a9c90249bff790d3ca0f77dea), + ) + "); + + let CherryPickOutcome::Commit(id) = result else { + bail!("impossible"); + }; + + assert!(&get_parents(&id.attach(&repo))?.is_empty()); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 3b64efb + └── clean-commit-f:100644:20a3acd "clean-commit\n" + "#); + + Ok(()) +} + +// no parents to single parent - clean +#[test] +fn no_parents_to_single_parent_clean() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("base")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto])?; + + insta::assert_debug_snapshot!(result, @r" + Commit( + Sha1(705a1d43c7949ab6f7bcd2fd4dad0f19c4aa5a1a), + ) + "); + + let CherryPickOutcome::Commit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + aa3d213 + ├── base-f:100644:7898192 "a\n" + └── target-f:100644:eb5a316 "target\n" + "#); + + Ok(()) +} + +// no parents to single parent - cp conflicts +#[test] +fn no_parents_to_single_parent_cp_conflicts() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("base-conflicting")?.detach(); + let onto = repo.rev_parse_single("single-target")?.detach(); + + let result = cherry_pick(&repo, target, &[onto])?; + + insta::assert_debug_snapshot!(result, @r" + ConflictedCommit( + Sha1(4a81c311fd985bb9f19c9e9be25ff9ed7af3018c), + ) + "); + + let CherryPickOutcome::ConflictedCommit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto]); + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + d92cbc8 + ├── .auto-resolution:aa3d213 + │ ├── base-f:100644:7898192 "a\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-base-0:4b825dc + ├── .conflict-files:100644:68fb397 "ancestorEntries = []\nourEntries = [\"target-f\"]\ntheirEntries = [\"target-f\"]\n" + ├── .conflict-side-0:aa3d213 + │ ├── base-f:100644:7898192 "a\n" + │ └── target-f:100644:eb5a316 "target\n" + ├── .conflict-side-1:144e5f5 + │ ├── base-f:100644:7898192 "a\n" + │ └── target-f:100644:9b1719f "conflict\n" + └── README.txt:100644:2af04b7 "You have checked out a GitButler Conflicted commit. You probably didn\'t mean to do this." + "#); + + Ok(()) +} + +#[test] +fn cherry_pick_back_to_original_parents_unconflicts() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, _meta) = fixture_writable("cherry-pick-scenario")?; + + let target = repo.rev_parse_single("merge-conflicting-commit")?; + let parents = get_parents(&target)?; + let onto = repo.rev_parse_single("single-target")?.detach(); + let onto2 = repo.rev_parse_single("second-target")?.detach(); + + let result = cherry_pick(&repo, target.detach(), &[onto, onto2])?; + + insta::assert_debug_snapshot!(result, @r" + ConflictedCommit( + Sha1(622983759b4d4e58b1bd8278f2be80f39b79d276), + ) + "); + + let CherryPickOutcome::ConflictedCommit(id) = result else { + bail!("impossible"); + }; + + assert_eq!(&get_parents(&id.attach(&repo))?, &[onto, onto2]); + + let result = cherry_pick(&repo, id, &parents)?; + + insta::assert_debug_snapshot!(result, @r" + Commit( + Sha1(9233ce21eeb24186978323222f61317661124337), + ) + "); + + let CherryPickOutcome::Commit(id) = result else { + bail!("impossible"); + }; + + insta::assert_snapshot!(visualize_tree(id.attach(&repo)), @r#" + 09af0e9 + ├── base-f:100644:7898192 "a\n" + ├── clean-2-f:100644:13e9394 "clean 2\n" + ├── clean-f:100644:8312630 "clean\n" + └── target-f:100644:9b1719f "conflict\n" + "#); + + Ok(()) +} diff --git a/crates/but-rebase/tests/rebase/graph_rebase/editor_creation.rs b/crates/but-rebase/tests/rebase/graph_rebase/editor_creation.rs new file mode 100644 index 0000000000..97a436e031 --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/editor_creation.rs @@ -0,0 +1,187 @@ +use anyhow::Result; +use but_graph::Graph; +use but_rebase::graph_rebase::{GraphExt, testing::TestingDot as _}; +use but_testsupport::{graph_tree, visualize_commit_graph_all}; + +use crate::utils::{fixture, standard_options}; + +#[test] +fn four_commits() -> Result<()> { + let (repo, meta) = fixture("four-commits")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * 120e3a9 (HEAD -> main) c + * a96434e b + * d591dfe a + * 35b8235 base + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let editor = graph.to_editor(&repo)?; + + insta::assert_snapshot!(editor.steps_dot(), @r#" + digraph { + 0 [ label="pick: 35b8235197020a417e9405ab5d4db6f204e8d84b"] + 1 [ label="pick: d591dfed1777b8f00f5b7b6f427537eeb5878178"] + 2 [ label="pick: a96434e2505c2ea0896cf4f58fec0778e074d3da"] + 3 [ label="pick: 120e3a90b753a492cef9a552ae3b9ba1f1391362"] + 4 [ label="reference: refs/heads/main"] + 1 -> 0 [ label="order: 0"] + 2 -> 1 [ label="order: 0"] + 3 -> 2 [ label="order: 0"] + 4 -> 3 [ label="order: 0"] + } + "#); + + Ok(()) +} + +#[test] +fn merge_in_the_middle() -> Result<()> { + let (repo, meta) = fixture("merge-in-the-middle")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * e8ee978 (HEAD -> with-inner-merge) on top of inner merge + * 2fc288c Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base, main) base + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let editor = graph.to_editor(&repo)?; + + insta::assert_snapshot!(editor.steps_dot(), @r#" + digraph { + 0 [ label="pick: 8f0d33828e5c859c95fb9e9fc063374fdd482536"] + 1 [ label="reference: refs/tags/base"] + 2 [ label="reference: refs/heads/main"] + 3 [ label="pick: add59d26b2ffd7468fcb44c2db48111dd8f481e5"] + 4 [ label="reference: refs/heads/A"] + 5 [ label="pick: 984fd1c6d3975901147b1f02aae6ef0a16e5904e"] + 6 [ label="reference: refs/heads/B"] + 7 [ label="pick: 2fc288c36c8bb710c78203f78ea9883724ce142b"] + 8 [ label="pick: e8ee978dac10e6a85006543ef08be07c5824b4f7"] + 9 [ label="reference: refs/heads/with-inner-merge"] + 1 -> 0 [ label="order: 0"] + 2 -> 1 [ label="order: 0"] + 3 -> 2 [ label="order: 0"] + 4 -> 3 [ label="order: 0"] + 5 -> 2 [ label="order: 0"] + 6 -> 5 [ label="order: 0"] + 7 -> 4 [ label="order: 0"] + 7 -> 6 [ label="order: 1"] + 8 -> 7 [ label="order: 0"] + 9 -> 8 [ label="order: 0"] + } + "#); + + Ok(()) +} + +#[test] +fn three_branches_merged() -> Result<()> { + let (repo, meta) = fixture("three-branches-merged")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + *-. 1348870 (HEAD -> main) Merge branches 'A', 'B' and 'C' + |\ \ + | | * 930563a (C) C: add another 10 lines to new file + | | * 68a2fc3 C: add 10 lines to new file + | | * 984fd1c C: new file with 10 lines + | * | a748762 (B) B: another 10 lines at the bottom + | * | 62e05ba B: 10 lines at the bottom + | |/ + * / add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base) base + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let editor = graph.to_editor(&repo)?; + + insta::assert_snapshot!(editor.steps_dot(), @r#" + digraph { + 0 [ label="pick: 8f0d33828e5c859c95fb9e9fc063374fdd482536"] + 1 [ label="reference: refs/tags/base"] + 2 [ label="pick: add59d26b2ffd7468fcb44c2db48111dd8f481e5"] + 3 [ label="reference: refs/heads/A"] + 4 [ label="pick: 62e05ba0716487f5e494a72952e296eca8c9f276"] + 5 [ label="pick: a7487625f079bedf4d20e48f052312c010117b38"] + 6 [ label="reference: refs/heads/B"] + 7 [ label="pick: 984fd1c6d3975901147b1f02aae6ef0a16e5904e"] + 8 [ label="pick: 68a2fc349e13a186e6d65871a31bad244d25e6f4"] + 9 [ label="pick: 930563a048351f05b14cc7b9c0a48640e5a306b0"] + 10 [ label="reference: refs/heads/C"] + 11 [ label="pick: 134887021e06909021776c023a608f8ef179e859"] + 12 [ label="reference: refs/heads/main"] + 1 -> 0 [ label="order: 0"] + 2 -> 1 [ label="order: 0"] + 3 -> 2 [ label="order: 0"] + 4 -> 1 [ label="order: 0"] + 5 -> 4 [ label="order: 0"] + 6 -> 5 [ label="order: 0"] + 7 -> 1 [ label="order: 0"] + 8 -> 7 [ label="order: 0"] + 9 -> 8 [ label="order: 0"] + 10 -> 9 [ label="order: 0"] + 11 -> 3 [ label="order: 0"] + 11 -> 6 [ label="order: 1"] + 11 -> 10 [ label="order: 2"] + 12 -> 11 [ label="order: 0"] + } + "#); + + Ok(()) +} + +#[test] +fn many_references() -> Result<()> { + let (repo, meta) = fixture("many-references")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * 120e3a9 (HEAD -> main) c + * a96434e b + * d591dfe (Z, Y, X) a + * 35b8235 base + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + insta::assert_snapshot!(graph_tree(&graph), @r" + └── 👉►:0[0]:main[🌳] + ├── ·120e3a9 (⌂|1) + ├── ·a96434e (⌂|1) + ├── ·d591dfe (⌂|1) ►X, ►Y, ►Z + └── ·35b8235 (⌂|1) + "); + + let editor = graph.to_editor(&repo)?; + + insta::assert_snapshot!(editor.steps_dot(), @r#" + digraph { + 0 [ label="pick: 35b8235197020a417e9405ab5d4db6f204e8d84b"] + 1 [ label="pick: d591dfed1777b8f00f5b7b6f427537eeb5878178"] + 2 [ label="reference: refs/heads/Z"] + 3 [ label="reference: refs/heads/Y"] + 4 [ label="reference: refs/heads/X"] + 5 [ label="pick: a96434e2505c2ea0896cf4f58fec0778e074d3da"] + 6 [ label="pick: 120e3a90b753a492cef9a552ae3b9ba1f1391362"] + 7 [ label="reference: refs/heads/main"] + 1 -> 0 [ label="order: 0"] + 2 -> 1 [ label="order: 0"] + 3 -> 2 [ label="order: 0"] + 4 -> 3 [ label="order: 0"] + 5 -> 4 [ label="order: 0"] + 6 -> 5 [ label="order: 0"] + 7 -> 6 [ label="order: 0"] + } + "#); + + Ok(()) +} diff --git a/crates/but-rebase/tests/rebase/graph_rebase/insert.rs b/crates/but-rebase/tests/rebase/graph_rebase/insert.rs new file mode 100644 index 0000000000..bb0e27365d --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/insert.rs @@ -0,0 +1,142 @@ +//! These tests exercise the insert operation. +use anyhow::{Context, Result}; +use but_graph::Graph; +use but_rebase::graph_rebase::{GraphExt, Step, mutate::InsertSide}; +use but_testsupport::{git_status, visualize_commit_graph_all}; + +use crate::{ + graph_rebase::set_var, + utils::{fixture_writable, standard_options}, +}; + +/// Inserting below a merge commit should inherit all of it's parents +#[test] +fn insert_below_merge_commit() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, meta) = fixture_writable("merge-in-the-middle")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * e8ee978 (HEAD -> with-inner-merge) on top of inner merge + * 2fc288c Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base, main) base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let mut editor = graph.to_editor(&repo)?; + + let merge_id = repo.rev_parse_single("HEAD~")?; + + // Create a commit that we can stick below the merge commit + let mut merge_obj = but_core::Commit::from_id(merge_id)?; + merge_obj.message = "Commit below the merge commit".into(); + merge_obj.parents = vec![].into(); + let new_commit = repo.write_object(merge_obj.inner)?.detach(); + + // select the merge commit + let selector = editor + .select_commit(merge_id.detach()) + .context("Failed to find commit a in editor graph")?; + // replace it with the new one + editor.insert( + &selector, + Step::Pick { + id: new_commit, + preserved_parents: None, + }, + InsertSide::Below, + ); + + let outcome = editor.rebase()?; + outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * bd07069 (HEAD -> with-inner-merge) on top of inner merge + * 3b80a45 Merge branch 'B' into with-inner-merge + * 181b10a Commit below the merge commit + |\ + | * 984fd1c (B) C: new file with 10 lines + * | add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base, main) base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + Ok(()) +} + +/// Inserting above a commit should inherit it's parents +#[test] +fn insert_above_commit_with_two_children() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, meta) = fixture_writable("merge-in-the-middle")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * e8ee978 (HEAD -> with-inner-merge) on top of inner merge + * 2fc288c Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base, main) base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let mut editor = graph.to_editor(&repo)?; + + let base_id = repo.rev_parse_single("base")?; + + // Create a commit that we can stick below the merge commit + let mut base_obj = but_core::Commit::from_id(base_id)?; + base_obj.message = "Commit above base commit".into(); + base_obj.parents = vec![].into(); + let new_commit = repo.write_object(base_obj.inner)?.detach(); + + // select the merge commit + let selector = editor + .select_commit(base_id.detach()) + .context("Failed to find commit a in editor graph")?; + // replace it with the new one + editor.insert( + &selector, + Step::Pick { + id: new_commit, + preserved_parents: None, + }, + InsertSide::Above, + ); + + let outcome = editor.rebase()?; + outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * 64487d8 (HEAD -> with-inner-merge) on top of inner merge + * 04fa958 Merge branch 'B' into with-inner-merge + |\ + | * dd12e5b (B) C: new file with 10 lines + * | f2c305e (A) A: 10 lines on top + |/ + * 6c1b9f3 (tag: base, main) Commit above base commit + * 8f0d338 base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + Ok(()) +} + +#[test] +#[ignore] +fn inserts_violating_fp_protection_should_cause_rebase_failure() -> Result<()> { + panic!("Branch protection hasn't been implemented yet"); +} diff --git a/crates/but-rebase/tests/rebase/graph_rebase/mod.rs b/crates/but-rebase/tests/rebase/graph_rebase/mod.rs new file mode 100644 index 0000000000..7122767811 --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/mod.rs @@ -0,0 +1,12 @@ +mod cherry_pick; +mod editor_creation; +mod insert; +mod multiple_operations; +mod rebase_identities; +mod replace; + +pub fn set_var(key: &str, value: &str) { + unsafe { + std::env::set_var(key, value); + } +} diff --git a/crates/but-rebase/tests/rebase/graph_rebase/multiple_operations.rs b/crates/but-rebase/tests/rebase/graph_rebase/multiple_operations.rs new file mode 100644 index 0000000000..2ebb7f4ff0 --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/multiple_operations.rs @@ -0,0 +1,8 @@ +//! These tests exercise the replace operation. +use anyhow::Result; + +#[test] +#[ignore = "non-legacy reordering is not implemented, and should be considered a requirement for any operations that want to work in SBM"] +fn reorder_two_references() -> Result<()> { + panic!("Reference orders are not updated"); +} diff --git a/crates/but-rebase/tests/rebase/graph_rebase/rebase_identities.rs b/crates/but-rebase/tests/rebase/graph_rebase/rebase_identities.rs new file mode 100644 index 0000000000..0832277588 --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/rebase_identities.rs @@ -0,0 +1,120 @@ +/// These tests demonstrate that if none of the steps are changed, the same +/// graphs are returned. +use anyhow::Result; +use but_graph::Graph; +use but_rebase::graph_rebase::GraphExt; +use but_testsupport::{graph_workspace, visualize_commit_graph_all}; + +use crate::utils::{fixture_writable, standard_options}; + +#[test] +fn four_commits() -> Result<()> { + let (repo, _tmpdir, meta) = fixture_writable("four-commits")?; + + let before = visualize_commit_graph_all(&repo)?; + insta::assert_snapshot!(before, @r" + * 120e3a9 (HEAD -> main) c + * a96434e b + * d591dfe a + * 35b8235 base + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let editor = graph.to_editor(&repo)?; + let outcome = editor.rebase()?; + outcome.materialize()?; + + assert_eq!(visualize_commit_graph_all(&repo)?, before); + + Ok(()) +} + +#[test] +fn four_commits_with_short_traversal() -> Result<()> { + let (repo, _tmpdir, meta) = fixture_writable("four-commits")?; + + let before = visualize_commit_graph_all(&repo)?; + insta::assert_snapshot!(before, @r" + * 120e3a9 (HEAD -> main) c + * a96434e b + * d591dfe a + * 35b8235 base + "); + + let options = standard_options().with_hard_limit(4); + let graph = Graph::from_head(&repo, &*meta, options)?.validated()?; + let workspace = graph.to_workspace()?; + + insta::assert_snapshot!(graph_workspace(&workspace), @r" + ⌂:0:main[🌳] <> ✓! + └── ≡:0:main[🌳] + └── :0:main[🌳] + ├── ·120e3a9 + └── ❌·a96434e + "); + + let editor = graph.to_editor(&repo)?; + let outcome = editor.rebase()?; + outcome.materialize()?; + + assert_eq!(visualize_commit_graph_all(&repo)?, before); + + Ok(()) +} + +#[test] +fn merge_in_the_middle() -> Result<()> { + let (repo, _tmpdir, meta) = fixture_writable("merge-in-the-middle")?; + + let before = visualize_commit_graph_all(&repo)?; + insta::assert_snapshot!(before, @r" + * e8ee978 (HEAD -> with-inner-merge) on top of inner merge + * 2fc288c Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base, main) base + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let editor = graph.to_editor(&repo)?; + let outcome = editor.rebase()?; + outcome.materialize()?; + + assert_eq!(visualize_commit_graph_all(&repo)?, before); + + Ok(()) +} + +#[test] +fn three_branches_merged() -> Result<()> { + let (repo, _tmpdir, meta) = fixture_writable("three-branches-merged")?; + + let before = visualize_commit_graph_all(&repo)?; + insta::assert_snapshot!(before, @r" + *-. 1348870 (HEAD -> main) Merge branches 'A', 'B' and 'C' + |\ \ + | | * 930563a (C) C: add another 10 lines to new file + | | * 68a2fc3 C: add 10 lines to new file + | | * 984fd1c C: new file with 10 lines + | * | a748762 (B) B: another 10 lines at the bottom + | * | 62e05ba B: 10 lines at the bottom + | |/ + * / add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base) base + "); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let editor = graph.to_editor(&repo)?; + let outcome = editor.rebase()?; + outcome.materialize()?; + + assert_eq!(visualize_commit_graph_all(&repo)?, before); + + Ok(()) +} diff --git a/crates/but-rebase/tests/rebase/graph_rebase/replace.rs b/crates/but-rebase/tests/rebase/graph_rebase/replace.rs new file mode 100644 index 0000000000..76c47c0fbe --- /dev/null +++ b/crates/but-rebase/tests/rebase/graph_rebase/replace.rs @@ -0,0 +1,178 @@ +//! These tests exercise the replace operation. +use anyhow::{Context, Result}; +use but_graph::Graph; +use but_rebase::graph_rebase::{GraphExt, Step}; +use but_testsupport::{git_status, visualize_commit_graph_all, visualize_tree}; + +use crate::{ + graph_rebase::set_var, + utils::{fixture_writable, standard_options}, +}; + +#[test] +fn reword_a_commit() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, meta) = fixture_writable("merge-in-the-middle")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * e8ee978 (HEAD -> with-inner-merge) on top of inner merge + * 2fc288c Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base, main) base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + let head_tree = repo.head_tree()?.id; + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let mut editor = graph.to_editor(&repo)?; + + // get the original a + let a = repo.rev_parse_single("A")?.detach(); + + // reword commit a + let a_obj = repo.find_commit(a)?; + let mut a_obj = a_obj.decode()?; + a_obj.message = "A: a second coming".into(); + let a_new = repo.write_object(a_obj)?.detach(); + + // select the original a out of the graph + let a_selector = editor + .select_commit(a) + .context("Failed to find commit a in editor graph")?; + // replace it with the new one + editor.replace( + &a_selector, + Step::Pick { + id: a_new, + preserved_parents: None, + }, + ); + + let outcome = editor.rebase()?; + outcome.materialize()?; + + assert_eq!(head_tree, repo.head_tree()?.id); + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * 4894b95 (HEAD -> with-inner-merge) on top of inner merge + * af38519 Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | 6de6b92 (A) A: a second coming + |/ + * 8f0d338 (tag: base, main) base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + Ok(()) +} + +#[test] +fn amend_a_commit() -> Result<()> { + set_var("GITBUTLER_CHANGE_ID", "1"); + let (repo, _tmpdir, meta) = fixture_writable("merge-in-the-middle")?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * e8ee978 (HEAD -> with-inner-merge) on top of inner merge + * 2fc288c Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | add59d2 (A) A: 10 lines on top + |/ + * 8f0d338 (tag: base, main) base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + let head_tree = repo.head_tree()?.id(); + insta::assert_snapshot!(visualize_tree(head_tree), @r#" + f766d1f + ├── added-after-with-inner-merge:100644:861be1b "seq 10\n" + ├── file:100644:d78dd4f "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n50\n51\n52\n53\n54\n55\n56\n57\n58\n59\n60\n" + └── new-file:100644:f00c965 "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" + "#); + + let graph = Graph::from_head(&repo, &*meta, standard_options())?.validated()?; + + let mut editor = graph.to_editor(&repo)?; + + // get the original a + let a = repo.rev_parse_single("A")?; + insta::assert_snapshot!(visualize_tree(a), @r#" + 0cc630c + └── file:100644:d78dd4f "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n50\n51\n52\n53\n54\n55\n56\n57\n58\n59\n60\n" + "#); + + // reword commit a + let mut a_obj = but_core::Commit::from_id(a)?; + + let mut builder = repo.edit_tree(a_obj.tree)?; + let new_blob = repo.write_blob("I'm a new file :D\n")?; + builder.upsert("new-file.txt", gix::objs::tree::EntryKind::Blob, new_blob)?; + let tree = builder.write()?; + + a_obj.tree = tree.detach(); + a_obj.message = "A: a second coming".into(); + let a_new = repo.write_object(a_obj.inner)?.detach(); + + // select the original a out of the graph + let a_selector = editor + .select_commit(a.detach()) + .context("Failed to find commit a in editor graph")?; + // replace it with the new one + editor.replace( + &a_selector, + Step::Pick { + id: a_new, + preserved_parents: None, + }, + ); + + let outcome = editor.rebase()?; + outcome.materialize()?; + + insta::assert_snapshot!(visualize_commit_graph_all(&repo)?, @r" + * 4a8642c (HEAD -> with-inner-merge) on top of inner merge + * fa3d6b8 Merge branch 'B' into with-inner-merge + |\ + | * 984fd1c (B) C: new file with 10 lines + * | f1905a8 (A) A: a second coming + |/ + * 8f0d338 (tag: base, main) base + "); + insta::assert_snapshot!(git_status(&repo)?, @r" + "); + + // A should include our extra blob + let a = repo.rev_parse_single("A")?; + insta::assert_snapshot!(visualize_tree(a), @r#" + 0c482d4 + ├── file:100644:d78dd4f "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n50\n51\n52\n53\n54\n55\n56\n57\n58\n59\n60\n" + └── new-file.txt:100644:715faaf "I\'m a new file :D\n" + "#); + + // New head tree should also include our extra blob + let new_head_tree = repo.head_tree()?.id(); + insta::assert_snapshot!(visualize_tree(new_head_tree), @r#" + 89042ca + ├── added-after-with-inner-merge:100644:861be1b "seq 10\n" + ├── file:100644:d78dd4f "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n50\n51\n52\n53\n54\n55\n56\n57\n58\n59\n60\n" + ├── new-file:100644:f00c965 "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" + └── new-file.txt:100644:715faaf "I\'m a new file :D\n" + "#); + + Ok(()) +} + +#[test] +#[ignore] +fn replaces_violating_fp_protection_should_cause_rebase_failure() -> Result<()> { + panic!("Branch protection hasn't been implemented yet"); +} diff --git a/crates/but-rebase/tests/rebase/main.rs b/crates/but-rebase/tests/rebase/main.rs index 77bc5f3ecc..962ed1587e 100644 --- a/crates/but-rebase/tests/rebase/main.rs +++ b/crates/but-rebase/tests/rebase/main.rs @@ -9,6 +9,7 @@ use crate::utils::{ }; mod error_handling; +mod graph_rebase; mod commit { mod store_author_globally_if_unset { @@ -18,7 +19,7 @@ mod commit { #[test] fn fail_if_nothing_can_be_written() -> anyhow::Result<()> { - let mut repo = fixture("four-commits")?; + let (mut repo, _) = fixture("four-commits")?; { let mut config = repo.config_snapshot_mut(); config.set_raw_value(&"user.name", "name")?; @@ -40,7 +41,7 @@ mod commit { #[test] fn keep_comments_and_customizations() -> anyhow::Result<()> { - let (repo, _tmp) = fixture_writable("four-commits")?; + let (repo, _tmp, _meta) = fixture_writable("four-commits")?; let local_config_path = repo.path().join("config"); std::fs::write( &local_config_path, @@ -150,7 +151,7 @@ fn single_stack_journey() -> Result<()> { #[test] fn amended_commit() -> Result<()> { assure_stable_env(); - let (repo, _tmp) = fixture_writable("three-branches-merged")?; + let (repo, _tmp, _meta) = fixture_writable("three-branches-merged")?; insta::assert_snapshot!(visualize_commit_graph(&repo, "@")?, @r" *-. 1348870 (HEAD -> main) Merge branches 'A', 'B' and 'C' |\ \ @@ -224,7 +225,7 @@ fn amended_commit() -> Result<()> { #[test] fn reorder_merge_in_reverse() -> Result<()> { assure_stable_env(); - let (repo, _tmp) = fixture_writable("merge-in-the-middle")?; + let (repo, _tmp, _meta) = fixture_writable("merge-in-the-middle")?; insta::assert_snapshot!(visualize_commit_graph(&repo, "with-inner-merge")?, @r" * e8ee978 (HEAD -> with-inner-merge) on top of inner merge * 2fc288c Merge branch 'B' into with-inner-merge @@ -303,7 +304,7 @@ fn reorder_merge_in_reverse() -> Result<()> { #[test] fn reorder_with_conflict_and_remerge_and_pick_from_conflicts() -> Result<()> { assure_stable_env(); - let (repo, _tmp) = fixture_writable("three-branches-merged")?; + let (repo, _tmp, _meta) = fixture_writable("three-branches-merged")?; insta::assert_snapshot!(visualize_commit_graph(&repo, "@")?, @r" *-. 1348870 (HEAD -> main) Merge branches 'A', 'B' and 'C' |\ \ @@ -506,7 +507,7 @@ fn reorder_with_conflict_and_remerge_and_pick_from_conflicts() -> Result<()> { fn reversible_conflicts() -> anyhow::Result<()> { assure_stable_env(); // If conflicts are created one way, putting them back the other way auto-resolves them. - let (repo, _tmp) = fixture_writable("three-branches-merged")?; + let (repo, _tmp, _meta) = fixture_writable("three-branches-merged")?; let mut builder = Rebase::new(&repo, repo.rev_parse_single("base")?.detach(), None)?; // Re-order commits with conflict, and trigger a re-merge. @@ -684,28 +685,52 @@ fn pick_the_first_commit_with_no_parents_for_squashing() -> Result<()> { pub mod utils { use anyhow::Result; + use but_meta::VirtualBranchesTomlMetadata; use but_rebase::RebaseOutput; use but_testsupport::gix_testtools; use gix::{ObjectId, prelude::ObjectIdExt}; /// Returns a fixture that may not be written to, objects will never touch disk either. - pub fn fixture(fixture_name: &str) -> Result { + pub fn fixture( + fixture_name: &str, + ) -> anyhow::Result<( + gix::Repository, + std::mem::ManuallyDrop, + )> { let root = gix_testtools::scripted_fixture_read_only("rebase.sh") .map_err(anyhow::Error::from_boxed)?; let worktree_root = root.join(fixture_name); let repo = - gix::open_opts(worktree_root, gix::open::Options::isolated())?.with_object_memory(); - Ok(repo) + gix::open_opts(&worktree_root, gix::open::Options::isolated())?.with_object_memory(); + + let meta = VirtualBranchesTomlMetadata::from_path( + repo.path() + .join(".git") + .join("should-never-be-written.toml"), + )?; + Ok((repo, std::mem::ManuallyDrop::new(meta))) } /// Returns a fixture that may be written to. - pub fn fixture_writable(fixture_name: &str) -> Result<(gix::Repository, tempfile::TempDir)> { + pub fn fixture_writable( + fixture_name: &str, + ) -> Result<( + gix::Repository, + tempfile::TempDir, + std::mem::ManuallyDrop, + )> { // TODO: remove the need for this, impl everything in `gitoxide`, allowing this to be in-memory entirely. let tmp = gix_testtools::scripted_fixture_writable("rebase.sh") .map_err(anyhow::Error::from_boxed)?; let worktree_root = tmp.path().join(fixture_name); let repo = but_testsupport::open_repo(&worktree_root)?; - Ok((repo, tmp)) + + let meta = VirtualBranchesTomlMetadata::from_path( + repo.path() + .join(".git") + .join("should-never-be-written.toml"), + )?; + Ok((repo, tmp, std::mem::ManuallyDrop::new(meta))) } #[derive(Debug)] @@ -722,7 +747,7 @@ pub mod utils { /// The commits in the fixture repo, starting from the oldest pub fn four_commits() -> Result<(gix::Repository, Commits)> { - let repo = fixture("four-commits")?; + let (repo, _) = fixture("four-commits")?; let commits: Vec<_> = repo .head_id()? .ancestors() @@ -743,7 +768,7 @@ pub mod utils { } pub fn four_commits_writable() -> Result<(gix::Repository, Commits, tempfile::TempDir)> { - let (repo, tmp) = fixture_writable("four-commits")?; + let (repo, tmp, _meta) = fixture_writable("four-commits")?; let commits: Vec<_> = repo .head_id()? .ancestors() @@ -790,4 +815,15 @@ pub mod utils { }) .collect() } + + pub fn standard_options() -> but_graph::init::Options { + but_graph::init::Options { + collect_tags: true, + commits_limit_hint: None, + commits_limit_recharge_location: vec![], + hard_limit: None, + extra_target_commit_id: None, + dangerously_skip_postprocessing_for_debugging: false, + } + } } diff --git a/crates/gitbutler-repo-actions/src/repository.rs b/crates/gitbutler-repo-actions/src/repository.rs index 3be2240340..ca5f66a8af 100644 --- a/crates/gitbutler-repo-actions/src/repository.rs +++ b/crates/gitbutler-repo-actions/src/repository.rs @@ -219,8 +219,8 @@ impl RepoActionsExt for Context { .unwrap() { Ok(result) => Ok(result), Err(err) => match err { - gitbutler_git::Error::ForcePushProtection(_) => { - Err(anyhow!("The force push was blocked because the remote branch contains commits that would be overwritten") + gitbutler_git::Error::ForcePushProtection(e) => { + Err(anyhow!("The force push was blocked because the remote branch contains commits that would be overwritten.\n\n{e}") .context(Code::GitForcePushProtection)) }, gitbutler_git::Error::GerritNoNewChanges(_) => {