refactor(graph): Delinearize by default

This will help us avoid biasing our algorithms based on implementation,
like we could do with #6.

This involved an audit of different graph ops to see if this would have
an impact on them.

Author: epage

src/bin/git-stack/stack.rs

@@ -330,8 +330,6 @@ fn plan_rebase(state: &State, stack: &StackState) -> eyre::Result<git_stack::git
 
     git_stack::graph::rebase_branches(&mut root, stack.onto.id)?;
 
-    git_stack::graph::delinearize(&mut root);
-
     let script = git_stack::graph::to_script(&root);
 
     Ok(script)
@@ -379,7 +377,6 @@ fn show(state: &State, colored_stdout: bool) -> eyre::Result<()> {
 
     git_stack::graph::protect_branches(&mut root, &state.repo, &state.protected_branches)?;
     git_stack::graph::pushable(&mut root)?;
-    git_stack::graph::delinearize(&mut root);
     if state.show_stacked {
         git_stack::graph::linearize_by_size(&mut root);
     }

src/graph/node.rs

@@ -62,7 +62,8 @@ impl Node {
 
         if merge_base_id != self_id {
             let mut prefix = Node::populate(repo, merge_base_id, self_id, possible_branches)?;
-            prefix.push(self);
+            let pushed = prefix.push(&self);
+            assert!(pushed);
             self = prefix;
             self_id = merge_base_id;
         }
@@ -125,31 +126,28 @@ impl Node {
             .map(|commit| Node::new(commit, branches))
             .collect();
         stack.reverse();
+        crate::graph::ops::delinearize_stack(&mut stack);
         if !stack.is_empty() {
             root.stacks.push(stack);
         }
 
         Ok(root)
     }
 
-    fn push(&mut self, other: Self) {
-        let other_oid = other.local_commit.id;
-        if self.local_commit.id == other_oid {
-            self.merge(other);
-        } else if self.stacks.len() == 1 {
-            let stack = &mut self.stacks[0];
-            for node in stack.iter_mut() {
-                if node.local_commit.id == other_oid {
-                    node.merge(other);
-                    if let Some(ext) = stack.last_mut().unwrap().stacks.pop() {
-                        stack.extend(ext);
+    #[must_use]
+    fn push(&mut self, other: &Self) -> bool {
+        if self.local_commit.id == other.local_commit.id {
+            self.merge(other.clone());
+            true
+        } else {
+            for stack in self.stacks.iter_mut() {
+                for node in stack.iter_mut() {
+                    if node.push(other) {
+                        return true;
                     }
-                    return;
                 }
             }
-            unimplemented!("This case isn't needed yet");
-        } else {
-            unimplemented!("This case isn't needed yet");
+            false
         }
     }
 
@@ -227,9 +225,9 @@ fn merge_stack(lhs_nodes: &mut Vec<Node>, rhs_nodes: &mut Vec<Node>) {
             }
         }
         Some((index, itertools::EitherOrBoth::Right(_))) => {
-            // rhs is a superset, so we can append it to lhs
+            // rhs is a superset, so we can add it to lhs's stacks
             let remaining = rhs_nodes.split_off(index);
-            lhs_nodes.extend(remaining);
+            lhs_nodes.last_mut().unwrap().stacks.push(remaining);
             rhs_nodes.clear();
         }
         Some((_, itertools::EitherOrBoth::Left(_))) | None => {

src/graph/ops.rs

@@ -7,6 +7,9 @@ pub fn protect_branches(
 ) -> Result<(), git2::Error> {
     // Assuming the root is the base.  The base is not guaranteed to be a protected branch but
     // might be an ancestor of one.
+    //
+    // We can't use `descendant_protected` because a protect branch might not be in the
+    // descendants, depending on what Graph the user selected.
     for protected_oid in protected_branches.oids() {
         if let Some(merge_base_oid) = repo.merge_base(root.local_commit.id, protected_oid) {
             if merge_base_oid == root.local_commit.id {
@@ -17,13 +20,13 @@ pub fn protect_branches(
     }
 
     for stack in root.stacks.iter_mut() {
-        protect_branches_internal(stack, repo, protected_branches)?;
+        protect_branches_stack(stack, repo, protected_branches)?;
     }
 
     Ok(())
 }
 
-fn protect_branches_internal(
+fn protect_branches_stack(
     nodes: &mut Vec<Node>,
     repo: &dyn crate::git::Repo,
     protected_branches: &crate::git::Branches,
@@ -32,7 +35,7 @@ fn protect_branches_internal(
     for node in nodes.iter_mut().rev() {
         let mut stacks_protected = false;
         for stack in node.stacks.iter_mut() {
-            let stack_protected = protect_branches_internal(stack, repo, protected_branches)?;
+            let stack_protected = protect_branches_stack(stack, repo, protected_branches)?;
             stacks_protected |= stack_protected;
         }
         let self_protected = protected_branches.contains_oid(node.local_commit.id);
@@ -118,9 +121,12 @@ fn pushable_stack(nodes: &mut [Node]) -> Result<(), git2::Error> {
                 log::debug!("{} is pushable", branch.name);
                 node.pushable = true;
             }
+            // Bail out, only the first branch of a stack is up for consideration
             return Ok(());
-        } else if !node.stacks.is_empty() {
-            cause = Some("ambiguous which branch owns some commits");
+        } else {
+            for stack in node.stacks.iter_mut() {
+                pushable_stack(stack)?;
+            }
         }
     }
 
@@ -133,7 +139,7 @@ pub fn delinearize(node: &mut Node) {
     }
 }
 
-fn delinearize_stack(nodes: &mut Vec<Node>) {
+pub(crate) fn delinearize_stack(nodes: &mut Vec<Node>) {
     for node in nodes.iter_mut() {
         for stack in node.stacks.iter_mut() {
             delinearize_stack(stack);

tests/graph.rs

@@ -26,7 +26,6 @@ mod test_rebase {
         let mut root = Node::from_branches(&repo, graph_branches).unwrap();
         git_stack::graph::protect_branches(&mut root, &repo, &protected_branches).unwrap();
         git_stack::graph::rebase_branches(&mut root, master_commit.id).unwrap();
-        git_stack::graph::delinearize(&mut root);
         let script = git_stack::graph::to_script(&root);
 
         let mut executor = git_stack::git::Executor::new(&repo, false);
@@ -68,7 +67,6 @@ mod test_rebase {
         let mut root = Node::from_branches(&repo, graph_branches).unwrap();
         git_stack::graph::protect_branches(&mut root, &repo, &protected_branches).unwrap();
         git_stack::graph::rebase_branches(&mut root, master_commit.id).unwrap();
-        git_stack::graph::delinearize(&mut root);
         let script = git_stack::graph::to_script(&root);
 
         let mut executor = git_stack::git::Executor::new(&repo, false);