Add support for creating dependent and independent branches in workspaces

With and without workspace commits.

Author: Byron

Cargo.lock

@@ -58,6 +58,22 @@ impl Workspace {
             .iter()
             .find_map(|stack| stack.branches.iter().find(|b| b.ref_name.as_ref() == name))
     }
+
+    /// Find the `(stack_idx, branch_idx)` of `name` within our stack branches and return it,
+    /// for direct access like `ws.stacks[stack_idx].branches[branch_idx]`.
+    pub fn find_owner_indexes_by_name(
+        &self,
+        name: &gix::refs::FullNameRef,
+    ) -> Option<(usize, usize)> {
+        self.stacks
+            .iter()
+            .enumerate()
+            .find_map(|(stack_idx, stack)| {
+                stack.branches.iter().enumerate().find_map(|(seg_idx, b)| {
+                    (b.ref_name.as_ref() == name).then_some((stack_idx, seg_idx))
+                })
+            })
+    }
 }
 
 /// Metadata about branches, associated with any Git branch.

crates/but-core/src/ref_metadata.rs

@@ -34,5 +34,4 @@ gix-testtools.workspace = true
 insta = "1.43.1"
 termtree = "0.5.1"
 but-testsupport.workspace = true
-regex = "1.11.1"
 

crates/but-graph/Cargo.toml

@@ -27,6 +27,7 @@ mod post;
 pub struct Overlay {
     nonoverriding_references: Vec<gix::refs::Reference>,
     meta_branches: Vec<(gix::refs::FullName, ref_metadata::Branch)>,
+    workspace: Option<(gix::refs::FullName, ref_metadata::Workspace)>,
 }
 
 pub(super) type PetGraph = petgraph::stable_graph::StableGraph<Segment, Edge>;

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

@@ -9,7 +9,7 @@ use std::collections::BTreeSet;
 impl Overlay {
     /// Serve the given `refs` from memory, as if they would exist.
     /// This is true only, however, if a real reference doesn't exist.
-    pub fn with_references_non_overriding(
+    pub fn with_references_if_new(
         mut self,
         refs: impl IntoIterator<Item = gix::refs::Reference>,
     ) -> Self {
@@ -19,13 +19,23 @@ impl Overlay {
 
     /// Serve the given `branches` metadata from memory, as if they would exist,
     /// possibly overriding metadata of a ref that already exists.
-    pub fn with_branch_metadata_overriding(
+    pub fn with_branch_metadata_override(
         mut self,
         refs: impl IntoIterator<Item = (gix::refs::FullName, ref_metadata::Branch)>,
     ) -> Self {
         self.meta_branches = refs.into_iter().collect();
         self
     }
+
+    /// Serve the given workspace `metadata` from memory, as if they would exist,
+    /// possibly overriding metadata of a workspace at that place
+    pub fn with_workspace_metadata_override(
+        mut self,
+        metadata: Option<(gix::refs::FullName, ref_metadata::Workspace)>,
+    ) -> Self {
+        self.workspace = metadata;
+        self
+    }
 }
 
 impl Overlay {
@@ -40,6 +50,7 @@ impl Overlay {
         let Overlay {
             nonoverriding_references,
             meta_branches,
+            workspace,
         } = self;
         (
             OverlayRepo {
@@ -49,6 +60,7 @@ impl Overlay {
             OverlayMetadata {
                 inner: meta,
                 meta_branches,
+                workspace,
             },
         )
     }
@@ -209,6 +221,7 @@ impl<'repo> OverlayRepo<'repo> {
 pub(crate) struct OverlayMetadata<'meta, T> {
     inner: &'meta T,
     meta_branches: Vec<(gix::refs::FullName, ref_metadata::Branch)>,
+    workspace: Option<(gix::refs::FullName, ref_metadata::Workspace)>,
 }
 
 impl<T> OverlayMetadata<'_, T>
@@ -226,13 +239,30 @@ where
                     .ok()
                     .map(|ws| (ref_name, ws))
             })
-            .map(|(ref_name, ws)| (ref_name, (*ws).clone()))
+            .map(|(ref_name, ws)| {
+                if let Some((_ws_ref, ws_override)) = self
+                    .workspace
+                    .as_ref()
+                    .filter(|(ws_ref, _ws_data)| *ws_ref == ref_name)
+                {
+                    (ref_name, ws_override.clone())
+                } else {
+                    (ref_name, (*ws).clone())
+                }
+            })
     }
 
     pub fn workspace_opt(
         &self,
         ref_name: &gix::refs::FullNameRef,
     ) -> anyhow::Result<Option<ref_metadata::Workspace>> {
+        if let Some((_ws_ref, ws_meta)) = self
+            .workspace
+            .as_ref()
+            .filter(|(ws_ref, _ws_meta)| ws_ref.as_ref() == ref_name)
+        {
+            return Ok(Some(ws_meta.clone()));
+        }
         let opt = self.inner.workspace_opt(ref_name)?;
         Ok(opt.map(|ws_data| ws_data.clone()))
     }

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

@@ -473,7 +473,12 @@ impl Graph {
                 .iter()
                 .flat_map(|s| s.commits_by_segment.iter().map(|(sidx, _)| *sidx))
         }) {
-            let s = &self.inner[sidx];
+            // The workspace might be stale by now as we delete empty segments.
+            // Thus be careful, and ignore non-existing ones - after all our workspace
+            // is temporary, nothing to worry about.
+            let Some(s) = self.inner.node_weight(sidx) else {
+                continue;
+            };
             if s.ref_name.is_some() || s.sibling_segment_id.is_some() {
                 continue;
             }
@@ -702,6 +707,9 @@ fn find_all_desired_stack_refs_in_commit<'a>(
     })
 }
 
+/// **Warning**: this can make workspace stacks stale, i.e. let them refer to non-existing segments.
+///              all accesses from hereon must be done with care. On the other hand, we can ignore
+///              that as our workspace is just temporary.
 fn delete_anon_if_empty_and_reconnect(graph: &mut Graph, sidx: SegmentIndex) {
     let segment = &graph[sidx];
     let may_delete = segment.commits.is_empty() && segment.ref_name.is_none();
@@ -717,6 +725,11 @@ fn delete_anon_if_empty_and_reconnect(graph: &mut Graph, sidx: SegmentIndex) {
     if outgoing.next().is_some() {
         return;
     }
+
+    tracing::debug!(
+        ?sidx,
+        "Deleting seemingly isolated and now completely unused segment"
+    );
     // Reconnect
     let new_target = first_outgoing.target();
     let incoming: Vec<_> = graph

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

@@ -224,7 +224,7 @@ mod debug;
 pub type CommitIndex = usize;
 
 /// A graph of connected segments that represent a section of the actual commit-graph.
-#[derive(Default, Debug)]
+#[derive(Default, Debug, Clone)]
 pub struct Graph {
     inner: init::PetGraph,
     /// From where the graph was created. This is useful if one wants to focus on a subset of the graph.

crates/but-graph/src/lib.rs

@@ -10,7 +10,7 @@ use crate::{
     projection::{Stack, StackCommit, StackCommitFlags, StackSegment},
 };
 use anyhow::Context;
-use bstr::ByteSlice;
+use bstr::{BStr, ByteSlice};
 use but_core::ref_metadata;
 use but_core::ref_metadata::StackId;
 use gix::reference::Category;
@@ -130,7 +130,7 @@ impl Workspace<'_> {
             .with_context(|| {
                 format!(
                     "Couldn't find any stack that contained the branch named '{}'",
-                    name.as_bstr()
+                    name.shorten()
                 )
             })
     }
@@ -159,7 +159,7 @@ impl Workspace<'_> {
             .with_context(|| {
                 format!(
                     "Couldn't find any stack that contained the branch named '{}'",
-                    name.as_bstr()
+                    name.shorten()
                 )
             })
     }
@@ -959,7 +959,7 @@ impl Workspace<'_> {
 }
 
 /// Query
-impl Workspace<'_> {
+impl<'graph> Workspace<'graph> {
     /// Return `true` if this workspace is managed, meaning we control certain aspects of it.
     /// If `false`, we are more conservative and may not support all features.
     pub fn has_managed_ref(&self) -> bool {
@@ -978,8 +978,14 @@ impl Workspace<'_> {
     /// Return the name of the workspace reference by looking our segment up in `graph`.
     /// Note that for managed workspaces, this can be retrieved via [`WorkspaceKind::Managed`].
     /// Note that it can be expected to be set on any workspace, but the data would allow it to not be set.
-    pub fn ref_name<'a>(&self, graph: &'a Graph) -> Option<&'a gix::refs::FullNameRef> {
-        graph[self.id].ref_name.as_ref().map(|rn| rn.as_ref())
+    pub fn ref_name(&self) -> Option<&'graph gix::refs::FullNameRef> {
+        self.graph[self.id].ref_name.as_ref().map(|rn| rn.as_ref())
+    }
+
+    /// Like [`Self::ref_name()`], but return a generic `<anonymous>` name for unnamed workspaces.
+    pub fn ref_name_display(&self) -> &BStr {
+        self.ref_name()
+            .map_or("<anonymous>".into(), |rn| rn.as_bstr())
     }
 }
 

crates/but-graph/src/projection/workspace.rs

@@ -240,10 +240,9 @@ impl RefMetadata for VirtualBranchesTomlMetadata {
                     stack_id = *branch.stack_id.borrow();
                 } else if stack_id != *branch.stack_id.borrow() {
                     bail!(
-                        "Inconsistent stack detected, wanted {:?}, but got {:?}",
-                        stack_id,
-                        branch.stack_id.borrow()
-                    )
+                        "BUG: unexpected situation where branch has stack-id {:?} but is associated with stack {stack_id:?}",
+                        branch.stack_id()
+                    );
                 }
             }
 

crates/but-graph/src/ref_metadata_legacy.rs

@@ -2,7 +2,7 @@ use but_core::RefMetadata;
 use but_core::ref_metadata::{StackId, ValueInfo, WorkspaceStack, WorkspaceStackBranch};
 use but_graph::VirtualBranchesTomlMetadata;
 use but_testsupport::gix_testtools::tempfile::{TempDir, tempdir};
-use std::collections::HashMap;
+use but_testsupport::{debug_str, sanitize_uuids_and_timestamps_with_mapping};
 use std::ops::Deref;
 use std::path::PathBuf;
 
@@ -39,7 +39,7 @@ fn read_only() -> anyhow::Result<()> {
     let (mut store, _tmp) = vb_store_rw("virtual-branches-01")?;
     let ws = store.workspace("refs/heads/gitbutler/workspace".try_into()?)?;
     assert!(!ws.is_default(), "value read from file");
-    let (actual, uuids) = sanitize_uuids_and_timestamps(debug_str(&ws.stacks));
+    let (actual, uuids) = sanitize_uuids_and_timestamps_with_mapping(debug_str(&ws.stacks));
     insta::assert_snapshot!(actual, @r#"
     [
         WorkspaceStack {
@@ -421,7 +421,7 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
 
     // Assure `ws` is what we think it should be - a single stack with one branch.
     let mut ws = store.workspace(workspace_name.as_ref())?;
-    let (actual, uuids) = sanitize_uuids_and_timestamps(debug_str(&ws.stacks));
+    let (actual, uuids) = sanitize_uuids_and_timestamps_with_mapping(debug_str(&ws.stacks));
     insta::assert_snapshot!(actual, @r#"
     [
         WorkspaceStack {
@@ -461,7 +461,7 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
         .expect("This is the way to add branches");
 
     let mut ws = store.workspace(workspace_name.as_ref())?;
-    let (actual, uuids) = sanitize_uuids_and_timestamps(debug_str(&ws.stacks));
+    let (actual, uuids) = sanitize_uuids_and_timestamps_with_mapping(debug_str(&ws.stacks));
     insta::assert_snapshot!(actual, @r#"
     [
         WorkspaceStack {
@@ -491,7 +491,8 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
     drop(store);
 
     assert!(toml_path.exists(), "file was written due to change");
-    let (actual, uuids) = sanitize_uuids_and_timestamps(std::fs::read_to_string(&toml_path)?);
+    let (actual, uuids) =
+        sanitize_uuids_and_timestamps_with_mapping(std::fs::read_to_string(&toml_path)?);
     insta::assert_snapshot!(actual, @r#"
     [branch_targets]
 
@@ -538,7 +539,7 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
         ws.deref(),
         "It's still what it was before - it was persisted"
     );
-    let (actual, uuids) = sanitize_uuids_and_timestamps(debug_str(&new_ws.stacks));
+    let (actual, uuids) = sanitize_uuids_and_timestamps_with_mapping(debug_str(&new_ws.stacks));
     insta::assert_snapshot!(actual, @r#"
     [
         WorkspaceStack {
@@ -576,7 +577,7 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
     );
     store.set_workspace(&ws)?;
     let mut ws = store.workspace(workspace_name.as_ref())?;
-    let (actual, uuids) = sanitize_uuids_and_timestamps(debug_str(&ws.stacks));
+    let (actual, uuids) = sanitize_uuids_and_timestamps_with_mapping(debug_str(&ws.stacks));
     insta::assert_snapshot!(actual, @r#"
     [
         WorkspaceStack {
@@ -639,7 +640,7 @@ fn create_workspace_and_stacks_with_branches_from_scratch() -> anyhow::Result<()
     store.set_workspace(&ws)?;
     let mut ws = store.workspace(ws.as_ref())?;
     // Two stacks are present now.
-    let (actual, uuids) = sanitize_uuids_and_timestamps(debug_str(&ws.stacks));
+    let (actual, uuids) = sanitize_uuids_and_timestamps_with_mapping(debug_str(&ws.stacks));
     insta::assert_snapshot!(actual, @r#"
     [
         WorkspaceStack {
@@ -960,42 +961,3 @@ fn roundtrip_journey(metadata: &mut impl RefMetadata) -> anyhow::Result<()> {
     assert_eq!(metadata.iter().count(), 0, "Nothing is left after deletion");
     Ok(())
 }
-
-fn sanitize_uuids_and_timestamps(input: String) -> (String, HashMap<String, usize>) {
-    let uuid_regex = regex::Regex::new(
-        r"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}",
-    )
-    .unwrap();
-    let timestamp_regex = regex::Regex::new(r#""\d{13}""#).unwrap();
-
-    let mut uuid_map: HashMap<String, usize> = HashMap::new();
-    let mut uuid_counter = 1;
-
-    let mut timestamp_map: HashMap<String, usize> = HashMap::new();
-    let mut timestamp_counter = 12_345;
-
-    let result = uuid_regex.replace_all(&input, |caps: &regex::Captures| {
-        let uuid = caps.get(0).unwrap().as_str().to_string();
-        let entry = uuid_map.entry(uuid).or_insert_with(|| {
-            let num = uuid_counter;
-            uuid_counter += 1;
-            num
-        });
-        entry.to_string()
-    });
-    let result = timestamp_regex.replace_all(&result, |caps: &regex::Captures| {
-        let timestamp = caps.get(0).unwrap().as_str().to_string();
-        let entry = timestamp_map.entry(timestamp).or_insert_with(|| {
-            let num = timestamp_counter;
-            timestamp_counter += 1;
-            num
-        });
-        entry.to_string()
-    });
-
-    (result.to_string(), uuid_map)
-}
-
-fn debug_str(input: &dyn std::fmt::Debug) -> String {
-    format!("{:#?}", input)
-}