Merge pull request #10402 from Byron/fix

Use the new workspace engine when listing branches if `ws3` is enabled

Author: Byron

crates/but-workspace/src/ui/ref_info.rs

@@ -39,7 +39,9 @@ pub struct RemoteTrackingReference {
 }
 
 impl RemoteTrackingReference {
-    fn for_ui(
+    /// Create a new instance from `ref_name` and `remote_names`, essentially splitting the remote
+    /// name off the short name.
+    pub fn for_ui(
         ref_name: gix::refs::FullName,
         remote_names: &gix::remote::Names,
     ) -> anyhow::Result<Self> {

crates/gitbutler-branch-actions/src/branch.rs

@@ -12,11 +12,10 @@ use gitbutler_project::access::WorktreeReadPermission;
 use gitbutler_reference::normalize_branch_name;
 use gitbutler_reference::RemoteRefname;
 use gitbutler_serde::BStringForFrontend;
-use gitbutler_stack::{Stack as GitButlerBranch, StackId, Target};
+use gitbutler_stack::{Stack, StackId, Target};
 use gix::object::tree::diff::Action;
-use gix::prelude::{ObjectIdExt, TreeDiffChangeExt};
+use gix::prelude::TreeDiffChangeExt;
 use gix::reference::Category;
-use itertools::Itertools;
 use serde::{Deserialize, Serialize};
 use std::borrow::Cow;
 use std::collections::BTreeSet;
@@ -59,7 +58,6 @@ pub fn list_branches(
     repo.object_cache_size_if_unset(1024 * 1024);
     let has_filter = filter.is_some();
     let filter = filter.unwrap_or_default();
-    let vb_handle = ctx.project().virtual_branches();
     let platform = repo.references()?;
     let mut branches: Vec<GroupBranch> = vec![];
     for reference in platform.all()?.filter_map(Result::ok) {
@@ -89,7 +87,36 @@ pub fn list_branches(
         });
     }
 
-    let stacks = vb_handle.list_all_stacks()?;
+    let vb_handle = ctx.project().virtual_branches();
+    let remote_names = repo.remote_names();
+    let stacks = if ctx.app_settings().feature_flags.ws3 {
+        if let Some(workspace_ref) = repo.try_find_reference("refs/heads/gitbutler/workspace")? {
+            // Let's get this here for convenience, and hope this isn't ever called by a writer (or there will be a deadlock).
+            let read_guard = ctx.project().shared_worktree_access();
+            let meta = ctx.meta(read_guard.read_permission())?;
+            let info = but_workspace::ref_info(
+                workspace_ref,
+                &*meta,
+                but_workspace::ref_info::Options {
+                    traversal: but_graph::init::Options::limited(),
+                    expensive_commit_info: false,
+                },
+            )?;
+            info.stacks
+                .into_iter()
+                .map(|s| GitButlerStack::new(s, &remote_names))
+                .collect::<Result<Vec<_>, _>>()?
+        } else {
+            Vec::new()
+        }
+    } else {
+        vb_handle
+            .list_all_stacks()?
+            .iter()
+            .map(|s| GitButlerStack::new_from_old(s, &remote_names))
+            .collect::<Result<Vec<_>, _>>()?
+    };
+
     branches.extend(stacks.iter().map(|s| GroupBranch::Virtual(s.clone())));
     let mut branches = combine_branches(branches, &repo, vb_handle.get_default_target()?)?;
 
@@ -130,13 +157,13 @@ pub fn list_branches(
     // To do this, we build up a list of all the branch identities that are
     // part of a stack and then filter out any branches that have been grouped
     // without a stack and match one of these identities.
-    let branch_identities_to_exclude = stacks
-        .iter()
+    let branch_identities_to_exclude: HashSet<BString> = stacks
+        .into_iter()
         .flat_map(|s| {
-            s.branches()
+            s.unarchived_segments
                 .into_iter()
-                .map(|b| BString::from(b.name().to_owned()))
-                .chain([BString::from(s.name.to_owned())])
+                .map(|b| b.short_name().into())
+                .chain(Some(s.name.into()))
         })
         .collect::<HashSet<_>>();
 
@@ -264,24 +291,20 @@ fn branch_group_to_branch(
     }
 
     // Virtual branch associated with this branch
-    let virtual_branch_reference = virtual_branch.map(|stack| StackReference {
-        given_name: stack.name.clone(),
-        id: stack.id,
-        in_workspace: stack.in_workspace,
-        branches: stack
-            .branches()
-            .iter()
-            .filter(|b| !b.archived)
-            .rev()
-            .map(|b| b.name())
-            .cloned()
-            .collect_vec(),
-        pull_requests: stack
-            .branches()
-            .iter()
-            .filter(|b| !b.archived)
-            .filter_map(|b| b.pr_number.map(|pr| (b.name().to_owned(), pr)))
-            .collect(),
+    let virtual_branch_reference = virtual_branch.map(|stack| {
+        let unarchived_branches = stack.unarchived_segments.iter();
+        StackReference {
+            given_name: stack.name.clone(),
+            id: stack.id,
+            in_workspace: stack.in_workspace,
+            branches: unarchived_branches
+                .clone()
+                .map(|b| b.short_name())
+                .collect(),
+            pull_requests: unarchived_branches
+                .filter_map(|b| b.pr_or_mr.map(|pr| (b.short_name().to_owned(), pr)))
+                .collect(),
+        }
     });
 
     let mut remotes: Vec<gix::remote::Name<'static>> = Vec::new();
@@ -298,7 +321,7 @@ fn branch_group_to_branch(
     // If there is a virtual branch let's get it's head. Alternatively, pick the first local branch and use it's head.
     // If there are no local branches, pick the first remote branch.
     let head_commit = if let Some(vbranch) = virtual_branch {
-        Some(vbranch.head_oid(repo)?.attach(repo))
+        Some(vbranch.head_oid(repo)?)
     } else if let Some(mut branch) = local_branches.into_iter().next() {
         branch.peel_to_id_in_place_packed(packed).ok()
     } else if let Some(mut branch) = remote_branches.into_iter().next() {
@@ -329,11 +352,141 @@ fn branch_group_to_branch(
 }
 
 /// A sum type of branch that can be a plain git branch or a virtual branch
-#[expect(clippy::large_enum_variant)]
 enum GroupBranch<'a> {
     Local(gix::Reference<'a>),
     Remote(gix::Reference<'a>),
-    Virtual(GitButlerBranch),
+    Virtual(GitButlerStack),
+}
+
+/// A type to just keep the parts we currently need.
+#[derive(Debug, Clone)]
+struct GitButlerStack {
+    id: StackId,
+    /// `true` if the stack is applied to the workspace.
+    in_workspace: bool,
+    /// The short name of the top-most segment.
+    name: String,
+    /// The full ref name of the top-most segment.
+    source_refname: Option<gix::refs::FullName>,
+    /// The full ref name of the remote tracking branch of the top-most segment.
+    upstream: Option<but_workspace::ui::ref_info::RemoteTrackingReference>,
+    /// The time at which anything in the stack was last updated.
+    updated_timestamp_ms: u128,
+    // All segments of the stack, as long as they are not archived.
+    // The tip comes first.
+    unarchived_segments: Vec<GitbutlerStackSegment>,
+}
+
+#[derive(Debug, Clone)]
+struct GitbutlerStackSegment {
+    /// The name of the segment, without support for these to be anonymous (which is a problem).
+    tip: gix::refs::FullName,
+    /// The PR or MR associated with it.
+    pr_or_mr: Option<usize>,
+}
+
+impl GitbutlerStackSegment {
+    fn short_name(&self) -> String {
+        self.tip.shorten().to_string()
+    }
+}
+
+impl GitButlerStack {
+    fn new(
+        stack: but_workspace::branch::Stack,
+        names: &gix::remote::Names,
+    ) -> anyhow::Result<Self> {
+        let first_segment = stack.segments.first();
+        Ok(GitButlerStack {
+            id: stack.id.context("Can't handle stacks without ID yet")?,
+            // The ones we have reachable are never
+            in_workspace: true,
+            name: stack
+                .name()
+                .map(|rn| rn.shorten().to_string())
+                // Hack it - the datastructure isn't suitable and this needs a `gitbutler->but` port.
+                .unwrap_or_default(),
+            source_refname: stack.ref_name().map(|rn| rn.to_owned()),
+            upstream: first_segment
+                .and_then(|s| {
+                    s.remote_tracking_ref_name.as_ref().map(|rn| {
+                        but_workspace::ui::ref_info::RemoteTrackingReference::for_ui(
+                            rn.clone(),
+                            names,
+                        )
+                    })
+                })
+                .transpose()?,
+            updated_timestamp_ms: first_segment
+                .and_then(|s| {
+                    let md = s.metadata.as_ref()?;
+                    Some(md.ref_info.updated_at?.seconds as u128 * 1_000)
+                })
+                .unwrap_or_default(),
+            unarchived_segments: stack
+                .segments
+                .iter()
+                .map(|s| GitbutlerStackSegment {
+                    tip: s.ref_name.clone().unwrap_or_else(|| {
+                        gix::refs::FullName::try_from(
+                            "refs/heads/unnamed-ref-and-we-fake-a-name-fix-me",
+                        )
+                        .expect("known to be valid statically")
+                    }),
+                    pr_or_mr: s.metadata.as_ref().and_then(|md| md.review.pull_request),
+                })
+                .collect(),
+        })
+    }
+    fn new_from_old(stack: &Stack, names: &gix::remote::Names) -> anyhow::Result<Self> {
+        Ok(GitButlerStack {
+            id: stack.id,
+            in_workspace: stack.in_workspace,
+            name: stack.name.clone(),
+            source_refname: stack
+                .source_refname
+                .as_ref()
+                .and_then(|r| r.to_string().try_into().ok()),
+            upstream: stack
+                .upstream
+                .as_ref()
+                .and_then(|r| {
+                    r.to_string().try_into().ok().map(|rn| {
+                        but_workspace::ui::ref_info::RemoteTrackingReference::for_ui(rn, names)
+                    })
+                })
+                .transpose()?,
+            updated_timestamp_ms: stack.updated_timestamp_ms,
+            unarchived_segments: stack
+                .branches()
+                .iter()
+                // The tip is at the bottom here.
+                .rev()
+                .filter(|s| !s.archived)
+                .map(|s| GitbutlerStackSegment {
+                    tip: s
+                        .full_name()
+                        .expect("full names are always valid, as their short names were valid"),
+                    pr_or_mr: s.pr_number,
+                })
+                .collect(),
+        })
+    }
+}
+
+impl GitButlerStack {
+    /// Return the top-most stack's commit id.
+    fn head_oid<'repo>(&self, repo: &'repo gix::Repository) -> anyhow::Result<gix::Id<'repo>> {
+        let tip_ref = self
+            .unarchived_segments
+            .iter()
+            .map(|s| s.tip.as_ref())
+            .next()
+            .with_context(|| format!("Stack {} didn't have a tip ref name", self.id))?;
+        repo.find_reference(tip_ref)?
+            .try_id()
+            .with_context(|| format!("'{}' was as symbolic reference", tip_ref.shorten()))
+    }
 }
 
 impl fmt::Debug for GroupBranch<'_> {
@@ -352,7 +505,7 @@ impl fmt::Debug for GroupBranch<'_> {
                 )
                 .finish(),
             GroupBranch::Virtual(branch) => formatter
-                .debug_struct("GroupBranch::Virtal")
+                .debug_struct("GroupBranch::Virtual")
                 .field("0", branch)
                 .finish(),
         }
@@ -370,12 +523,15 @@ impl GroupBranch<'_> {
             }
             // The identity of a Virtual branch is derived from the source refname, upstream or the branch given name, in that order
             GroupBranch::Virtual(branch) => {
-                let name_from_source = branch.source_refname.as_ref().and_then(|n| n.branch());
-                let name_from_upstream = branch.upstream.as_ref().map(|n| n.branch());
+                let name_from_source = branch.source_refname.as_ref().map(|n| n.shorten());
+                let name_from_upstream = branch
+                    .upstream
+                    .as_ref()
+                    .map(|n| n.display_name.as_str().into());
 
                 // If we have a source refname or upstream, use those directly
                 if let Some(name) = name_from_source.or(name_from_upstream) {
-                    return Some(name.into());
+                    return name.try_into().ok();
                 }
 
                 // Only fall back to the normalized rich name if no source/upstream is available
@@ -515,7 +671,7 @@ pub struct StackReference {
     /// List of branches that are part of the stack
     /// Ordered from newest to oldest (the most recent branch is first in the list)
     pub branches: Vec<String>,
-    /// Pull Request numbes by branch name associated with the stack
+    /// Pull Request numbers by branch name associated with the stack
     pub pull_requests: HashMap<String, usize>,
 }
 

crates/gitbutler-branch-actions/tests/virtual_branches/list.rs

@@ -45,7 +45,7 @@ fn one_vbranch_in_workspace_one_commit() -> Result<()> {
 #[test]
 fn two_vbranches_in_workspace_one_commit() -> Result<()> {
     init_env();
-    let ctx = project_ctx("two-vbranches-in-workspace-one-applied")?;
+    let ctx = project_ctx_without_ws3("two-vbranches-in-workspace-one-applied")?;
     let list = list_branches(
         &ctx,
         Some(BranchListingFilter {
@@ -92,7 +92,7 @@ fn two_vbranches_in_workspace_one_commit() -> Result<()> {
 #[test]
 fn one_feature_branch_and_one_vbranch_in_workspace_one_commit() -> Result<()> {
     init_env();
-    let ctx = project_ctx("a-vbranch-named-like-target-branch-short-name")?;
+    let ctx = project_ctx_without_ws3("a-vbranch-named-like-target-branch-short-name")?;
     let list = list_branches(&ctx, None)?;
     assert_eq!(
         list.len(),
@@ -118,7 +118,7 @@ fn one_feature_branch_and_one_vbranch_in_workspace_one_commit() -> Result<()> {
 #[test]
 fn one_branch_in_workspace_multiple_remotes() -> Result<()> {
     init_env();
-    let ctx = project_ctx("one-vbranch-in-workspace-two-remotes")?;
+    let ctx = project_ctx_without_ws3("one-vbranch-in-workspace-two-remotes")?;
     let list = list_branches(&ctx, None)?;
     assert_eq!(list.len(), 1, "a single virtual branch");
 
@@ -138,6 +138,8 @@ fn one_branch_in_workspace_multiple_remotes() -> Result<()> {
 
 mod util {
     use anyhow::Result;
+    use but_settings::app_settings::FeatureFlags;
+    use but_settings::AppSettings;
     use gitbutler_branch::BranchIdentity;
     use gitbutler_branch_actions::{BranchListing, BranchListingFilter};
     use gitbutler_command_context::CommandContext;
@@ -221,6 +223,17 @@ mod util {
         gitbutler_testsupport::read_only::fixture("for-listing.sh", name)
     }
 
+    pub fn project_ctx_without_ws3(name: &str) -> Result<CommandContext> {
+        gitbutler_testsupport::read_only::fixture_with_features(
+            "for-listing.sh",
+            name,
+            FeatureFlags {
+                ws3: false,
+                ..AppSettings::default().feature_flags
+            },
+        )
+    }
+
     pub fn list_branches(
         ctx: &CommandContext,
         filter: Option<BranchListingFilter>,
@@ -230,4 +243,5 @@ mod util {
         Ok(branches)
     }
 }
+use crate::virtual_branches::list::util::project_ctx_without_ws3;
 pub use util::{assert_equal, init_env, list_branches, project_ctx, ExpectedBranchListing};