Merge pull request #10447 from Byron/next

V3 apply/unapply: MVP

Author: Byron

crates/but-core/src/ref_metadata.rs

@@ -1,4 +1,5 @@
 use crate::Id;
+use gix::refs::FullNameRef;
 
 /// Metadata about workspaces, associated with references that are designated to a workspace,
 /// i.e. `refs/heads/gitbutler/workspaces/<name>`.
@@ -35,7 +36,72 @@ pub struct Workspace {
     pub push_remote: Option<String>,
 }
 
+/// Mutations
+impl Workspace {
+    /// Insert `branch` as new stack if it's not yet contained in the workspace and if `order` is not `None` or push
+    /// it to the end of the stack list.
+    /// Note that `order` is only relevant at insertion time.
+    /// Returns `true` if the ref was newly added, or `false` if it already existed.
+    pub fn add_or_insert_new_stack_if_not_present(
+        &mut self,
+        branch: &FullNameRef,
+        order: Option<usize>,
+    ) -> bool {
+        if self.contains_ref(branch) {
+            return false;
+        };
+
+        let stack = WorkspaceStack {
+            id: StackId::generate(),
+            branches: vec![WorkspaceStackBranch {
+                ref_name: branch.to_owned(),
+                archived: false,
+            }],
+        };
+        match order.map(|idx| idx.min(self.stacks.len())) {
+            None => {
+                self.stacks.push(stack);
+            }
+            Some(existing_index) => {
+                self.stacks.insert(existing_index, stack);
+            }
+        }
+        true
+    }
+}
+
+/// Access
 impl Workspace {
+    /// Return the names of the tips of all stacks in the workspace.
+    pub fn stack_names(&self) -> impl Iterator<Item = &gix::refs::FullNameRef> {
+        self.stacks
+            .iter()
+            .filter_map(|s| s.ref_name().map(|rn| rn.as_ref()))
+    }
+
+    /// Return `true` if the branch with `name` is the workspace target or the targets local tracking branch,
+    /// using `repo` for the lookup of the local tracking branch.
+    pub fn is_branch_the_target_or_its_local_tracking_branch(
+        &self,
+        name: &gix::refs::FullNameRef,
+        repo: &gix::Repository,
+    ) -> anyhow::Result<bool> {
+        let Some(target_ref) = self.target_ref.as_ref() else {
+            return Ok(false);
+        };
+
+        if target_ref.as_ref() == name {
+            Ok(true)
+        } else {
+            let Some((local_tracking_branch, _remote_name)) =
+                repo.upstream_branch_and_remote_for_tracking_branch(target_ref.as_ref())?
+            else {
+                return Ok(false);
+            };
+            Ok(local_tracking_branch.as_ref() == name)
+        }
+    }
+
     /// Return `true` if `name` is a reference mentioned in our [stacks](Workspace::stacks).
     pub fn contains_ref(&self, name: &gix::refs::FullNameRef) -> bool {
         self.stacks
@@ -92,6 +158,18 @@ pub struct Branch {
     pub review: Review,
 }
 
+/// Mutations
+impl Branch {
+    /// Claim that we now updated the branch in some way, and possibly also set the created time
+    /// if `is_new_ref` is `true`
+    pub fn update_times(&mut self, is_new_ref: bool) {
+        self.ref_info.set_updated_to_now();
+        if is_new_ref {
+            self.ref_info.set_created_to_now();
+        }
+    }
+}
+
 impl std::fmt::Debug for Branch {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         const DEFAULT_IN_TESTSUITE: gix::date::Time = gix::date::Time {

crates/but-core/tests/core/main.rs

@@ -2,5 +2,6 @@ mod cmd;
 mod commit;
 mod diff;
 mod json_samples;
+mod ref_metadata;
 mod settings;
 mod unified_diff;

crates/but-core/tests/core/ref_metadata.rs

@@ -0,0 +1,26 @@
+mod workspace {
+    use but_core::ref_metadata::Workspace;
+
+    #[test]
+    fn add_new_stack_if_not_present() {
+        let mut ws = Workspace::default();
+        assert_eq!(ws.stacks.len(), 0);
+
+        let a_ref = r("refs/heads/A");
+        assert!(ws.add_or_insert_new_stack_if_not_present(a_ref, Some(100)));
+        assert!(!ws.add_or_insert_new_stack_if_not_present(a_ref, Some(200)));
+        assert_eq!(ws.stacks.len(), 1);
+
+        let b_ref = r("refs/heads/B");
+        assert!(ws.add_or_insert_new_stack_if_not_present(b_ref, Some(0)));
+        assert_eq!(ws.stack_names().collect::<Vec<_>>(), [b_ref, a_ref]);
+
+        let c_ref = r("refs/heads/C");
+        assert!(ws.add_or_insert_new_stack_if_not_present(c_ref, None));
+        assert_eq!(ws.stack_names().collect::<Vec<_>>(), [b_ref, a_ref, c_ref]);
+    }
+
+    fn r(name: &str) -> &gix::refs::FullNameRef {
+        name.try_into().expect("statically known ref")
+    }
+}

crates/but-graph/src/api.rs

@@ -63,6 +63,53 @@ impl Graph {
     }
 }
 
+/// Merge-base computation
+impl Graph {
+    /// Compute the lowest merge-base between two segments.
+    /// Such a merge-base is reachable from all possible paths from `a` and `b`.
+    ///
+    /// The segment representing the merge-base is expected to not be empty, as its first commit
+    /// is usually what one is interested in.
+    // TODO: should be multi, with extra segments as third parameter
+    // TODO: actually find the lowest merge-base, right now it just finds the first merge-base, but that's not
+    //       the lowest.
+    pub fn first_merge_base(&self, a: SegmentIndex, b: SegmentIndex) -> Option<SegmentIndex> {
+        // TODO(perf): improve this by allowing to set bitflags on the segments themselves, to allow
+        //       marking them accordingly, just like Git does.
+        //       Right now we 'emulate' bitflags on pre-allocated data with two data sets, expensive
+        //       in comparison.
+        //       And yes, let's avoid `gix::Repository::merge_base` as we have free
+        //       generation numbers here and can avoid work duplication.
+        let mut segments_reachable_by_b = BTreeSet::new();
+        self.visit_all_segments_including_start_until(b, Direction::Outgoing, |s| {
+            segments_reachable_by_b.insert(s.id);
+            // Collect everything, keep it simple.
+            // This is fast* as completely in memory.
+            // *means slow compared to an array traversal with memory locality.
+            false
+        });
+
+        let mut candidate = None;
+        self.visit_all_segments_including_start_until(a, Direction::Outgoing, |s| {
+            if candidate.is_some() {
+                return true;
+            }
+            let prune = segments_reachable_by_b.contains(&s.id);
+            if prune {
+                candidate = Some(s.id);
+            }
+            prune
+        });
+        if candidate.is_none() {
+            // TODO: improve this - workspaces shouldn't be like this but if they are, do we deal with it well?
+            tracing::warn!(
+                "Couldn't find merge-base between segments {a:?} and {b:?} - this might lead to unexpected results"
+            )
+        }
+        candidate
+    }
+}
+
 /// Query
 /// ‼️Useful only if one knows the graph traversal was started where one expects, or else the graph may be partial.
 impl Graph {
@@ -130,6 +177,11 @@ impl Graph {
         sidx_with_commits.and_then(|sidx| self[sidx].commits.first())
     }
 
+    /// The first commit reachable by skipping over empty segments starting at the entrypoint segment.
+    pub fn entrypoint_commit(&self) -> Option<&Commit> {
+        self.tip_skip_empty(self.entrypoint?.0)
+    }
+
     /// Visit the ancestry of `start` along the first parents, itself excluded, until `stop` returns `true`.
     /// Also return the segment that we stopped at.
     /// **Important**: `stop` is not called with `start`, this is a feature.
@@ -171,6 +223,12 @@ impl Graph {
         self.hard_limit_hit = true;
     }
 
+    /// Lookup the segment of `sidx` and then find its sibling segment, if it has one.
+    pub fn lookup_sibling_segment(&self, sidx: SegmentIndex) -> Option<&Segment> {
+        self.inner
+            .node_weight(self.inner.node_weight(sidx)?.sibling_segment_id?)
+    }
+
     /// Return the entry-point of the graph as configured during traversal.
     /// It's useful for when one wants to know which commit was used to discover the entire graph.
     ///

crates/but-graph/src/debug.rs

@@ -371,7 +371,7 @@ impl Graph {
                 .commit_id_by_index(e.dst)
                 .map(|c| c.to_hex_with_len(HEX).to_string())
                 .unwrap_or_else(|| "dst".into());
-            format!(", label = \"⚠️{src} → {dst} ({err})\", fontname = Courier")
+            format!(", label = \"⚠{src} → {dst} ({err})\", fontname = Courier")
         };
         let dot = petgraph::dot::Dot::with_attr_getters(&self.inner, &[], &edge_attrs, &node_attrs);
         format!("{dot:?}")

crates/but-graph/src/init/mod.rs

@@ -25,10 +25,11 @@ pub(crate) type Entrypoint = Option<(gix::ObjectId, Option<gix::refs::FullName>)
 
 /// A way to define information to be served from memory, instead of from the underlying data source, when
 /// [initializing](Graph::from_commit_traversal()) the graph.
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Clone)]
 pub struct Overlay {
     entrypoint: Entrypoint,
     nonoverriding_references: Vec<gix::refs::Reference>,
+    overriding_references: Vec<gix::refs::Reference>,
     meta_branches: Vec<(gix::refs::FullName, ref_metadata::Branch)>,
     workspace: Option<(gix::refs::FullName, ref_metadata::Workspace)>,
 }

crates/but-graph/src/init/overlay.rs

@@ -2,8 +2,9 @@ use crate::init::walk::RefsById;
 use crate::init::{Entrypoint, Overlay};
 use but_core::{RefMetadata, ref_metadata};
 use gix::prelude::ReferenceExt;
+use gix::refs::Target;
 use std::borrow::Cow;
-use std::collections::BTreeSet;
+use std::collections::{BTreeMap, BTreeSet};
 
 impl Overlay {
     /// Serve the given `refs` from memory, as if they would exist.
@@ -16,13 +17,27 @@ impl Overlay {
         self
     }
 
+    /// Serve the given `refs` from memory, which is like creating the reference or as if its value was set,
+    /// completely overriding the value in the repository.
+    pub fn with_references(mut self, refs: impl IntoIterator<Item = gix::refs::Reference>) -> Self {
+        self.overriding_references.extend(refs);
+        self
+    }
+
     /// Override the starting position of the traversal by setting it to `id`,
     /// and optionally, by providing the `ref_name` that points to `id`.
     pub fn with_entrypoint(
         mut self,
         id: gix::ObjectId,
         ref_name: Option<gix::refs::FullName>,
     ) -> Self {
+        if let Some(ref_name) = &ref_name {
+            self.overriding_references.push(gix::refs::Reference {
+                name: ref_name.to_owned(),
+                target: Target::Object(id),
+                peeled: Some(id),
+            })
+        }
         self.entrypoint = Some((id, ref_name));
         self
     }
@@ -59,13 +74,21 @@ impl Overlay {
     {
         let Overlay {
             nonoverriding_references,
+            overriding_references,
             meta_branches,
             workspace,
             entrypoint,
         } = self;
         (
             OverlayRepo {
-                nonoverriding_references,
+                nonoverriding_references: nonoverriding_references
+                    .into_iter()
+                    .map(|r| (r.name.clone(), r))
+                    .collect(),
+                overriding_references: overriding_references
+                    .into_iter()
+                    .map(|r| (r.name.clone(), r))
+                    .collect(),
                 inner: repo,
             },
             OverlayMetadata {
@@ -80,7 +103,8 @@ impl Overlay {
 
 pub(crate) struct OverlayRepo<'repo> {
     inner: &'repo gix::Repository,
-    nonoverriding_references: Vec<gix::refs::Reference>,
+    nonoverriding_references: BTreeMap<gix::refs::FullName, gix::refs::Reference>,
+    overriding_references: BTreeMap<gix::refs::FullName, gix::refs::Reference>,
 }
 
 /// Note that functions with `'repo` in their return value technically leak the bare repo, and it's
@@ -94,13 +118,11 @@ impl<'repo> OverlayRepo<'repo> {
         &self,
         ref_name: &gix::refs::FullNameRef,
     ) -> anyhow::Result<Option<gix::Reference<'repo>>> {
-        if let Some(rn) = self.inner.try_find_reference(ref_name)? {
+        if let Some(r) = self.overriding_references.get(ref_name) {
+            Ok(Some(r.clone().attach(self.inner)))
+        } else if let Some(rn) = self.inner.try_find_reference(ref_name)? {
             Ok(Some(rn))
-        } else if let Some(r) = self
-            .nonoverriding_references
-            .iter()
-            .find(|r| r.name.as_ref() == ref_name)
-        {
+        } else if let Some(r) = self.nonoverriding_references.get(ref_name) {
             Ok(Some(r.clone().attach(self.inner)))
         } else {
             Ok(None)
@@ -111,17 +133,16 @@ impl<'repo> OverlayRepo<'repo> {
         &self,
         ref_name: &gix::refs::FullNameRef,
     ) -> anyhow::Result<gix::Reference<'repo>> {
+        if let Some(r) = self.overriding_references.get(ref_name) {
+            return Ok(r.clone().attach(self.inner));
+        }
         Ok(self
             .inner
             .find_reference(ref_name)
             .or_else(|err| match err {
                 gix::reference::find::existing::Error::Find(_) => Err(err),
                 gix::reference::find::existing::Error::NotFound { .. } => {
-                    if let Some(r) = self
-                        .nonoverriding_references
-                        .iter()
-                        .find(|r| r.name.as_ref() == ref_name)
-                    {
+                    if let Some(r) = self.nonoverriding_references.get(ref_name) {
                         Ok(r.clone().attach(self.inner))
                     } else {
                         Err(err)
@@ -202,6 +223,18 @@ impl<'repo> OverlayRepo<'repo> {
             };
         let mut all_refs_by_id = gix::hashtable::HashMap::<_, Vec<_>>::default();
         for prefix in prefixes {
+            // apply overrides - they are seen first and take the spot of everything.
+            for (commit_id, git_reference) in self
+                .overriding_references
+                .values()
+                .filter(|rn| rn.name.as_bstr().starts_with(prefix.as_bytes()))
+                .filter_map(|rn| ref_filter(rn.clone().attach(self.inner)))
+            {
+                all_refs_by_id
+                    .entry(commit_id)
+                    .or_default()
+                    .push(git_reference);
+            }
             for (commit_id, git_reference) in self
                 .inner
                 .references()?
@@ -214,10 +247,10 @@ impl<'repo> OverlayRepo<'repo> {
                     .or_default()
                     .push(git_reference);
             }
-            // apply overrides
+            // apply overrides (new only)
             for (commit_id, git_reference) in self
                 .nonoverriding_references
-                .iter()
+                .values()
                 .filter(|rn| rn.name.as_bstr().starts_with(prefix.as_bytes()))
                 .filter_map(|rn| ref_filter(rn.clone().attach(self.inner)))
             {