Add new feature toggle for apply/unapply

Author: Byron

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

@@ -4,7 +4,7 @@ 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.
@@ -73,19 +73,22 @@ impl Overlay {
         T: RefMetadata,
     {
         let Overlay {
-            mut nonoverriding_references,
-            mut overriding_references,
+            nonoverriding_references,
+            overriding_references,
             meta_branches,
             workspace,
             entrypoint,
         } = self;
-        // Make sure that duplicates from later determine the value.
-        nonoverriding_references.reverse();
-        overriding_references.reverse();
         (
             OverlayRepo {
-                nonoverriding_references: nonoverriding_references.into_iter().collect(),
-                overriding_references: overriding_references.into_iter().collect(),
+                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 {
@@ -100,8 +103,8 @@ impl Overlay {
 
 pub(crate) struct OverlayRepo<'repo> {
     inner: &'repo gix::Repository,
-    nonoverriding_references: BTreeSet<gix::refs::Reference>,
-    overriding_references: BTreeSet<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
@@ -115,19 +118,11 @@ impl<'repo> OverlayRepo<'repo> {
         &self,
         ref_name: &gix::refs::FullNameRef,
     ) -> anyhow::Result<Option<gix::Reference<'repo>>> {
-        if let Some(r) = self
-            .overriding_references
-            .iter()
-            .find(|r| r.name.as_ref() == 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)
@@ -138,11 +133,7 @@ impl<'repo> OverlayRepo<'repo> {
         &self,
         ref_name: &gix::refs::FullNameRef,
     ) -> anyhow::Result<gix::Reference<'repo>> {
-        if let Some(r) = self
-            .overriding_references
-            .iter()
-            .find(|r| r.name.as_ref() == ref_name)
-        {
+        if let Some(r) = self.overriding_references.get(ref_name) {
             return Ok(r.clone().attach(self.inner));
         }
         Ok(self
@@ -151,11 +142,7 @@ impl<'repo> OverlayRepo<'repo> {
             .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)
@@ -239,7 +226,7 @@ impl<'repo> OverlayRepo<'repo> {
             // apply overrides - they are seen first and take the spot of everything.
             for (commit_id, git_reference) in self
                 .overriding_references
-                .iter()
+                .values()
                 .filter(|rn| rn.name.as_bstr().starts_with(prefix.as_bytes()))
                 .filter_map(|rn| ref_filter(rn.clone().attach(self.inner)))
             {
@@ -263,7 +250,7 @@ impl<'repo> OverlayRepo<'repo> {
             // 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)))
             {

crates/but-settings/assets/defaults.jsonc

@@ -18,6 +18,8 @@
 		"oauthClientId": "cd51880daa675d9e6452"
 	},
 	"featureFlags": {
+		/// Use the V3 version of apply and unapply.
+		"apply3": false,
 		// Enables the v3 safe checkout.
 		"cv3": false,
 		/// Enable the usage of V3 workspace APIs.

crates/but-settings/src/app_settings.rs

@@ -27,6 +27,8 @@ pub struct FeatureFlags {
     pub ws3: bool,
     /// Turn on the set a v3 version of checkout
     pub cv3: bool,
+    /// Use the V3 version of apply and unapply.
+    pub apply3: bool,
     /// Enable undo/redo support.
     ///
     /// ### Progression for implementation

crates/but-testing/src/command/mod.rs

@@ -372,6 +372,7 @@ pub mod stacks {
         let app_settings = AppSettings {
             feature_flags: but_settings::app_settings::FeatureFlags {
                 ws3,
+                apply3: false,
                 cv3: false,
                 undo: false,
                 actions: false,

crates/but-workspace/src/branch/apply.rs

@@ -79,14 +79,15 @@ pub(crate) mod function {
     use crate::ext::ObjectStorageExt;
     use crate::ref_info::WorkspaceExt;
     use anyhow::{Context, bail};
+    use but_core::RefMetadata;
     use but_core::ref_metadata::Workspace;
-    use but_core::{RefMetadata, ref_metadata};
     use but_graph::init::Overlay;
     use but_graph::projection::WorkspaceKind;
     use gitbutler_oxidize::GixRepositoryExt;
     use gix::refs::transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog};
     use gix::refs::{FullNameRef, Target};
     use std::borrow::Cow;
+    use tracing::instrument;
 
     /// Apply `branch` to the given `workspace`, and possibly create the workspace reference in `repo`
     /// along with its `meta`-data if it doesn't exist yet.
@@ -107,6 +108,7 @@ pub(crate) mod function {
     ///
     /// Note that options have no effect if `branch` is already in the workspace, so `apply` is *not* a way
     /// to alter certain aspects of the workspace by applying the same branch again.
+    #[instrument(level = tracing::Level::DEBUG, skip(workspace, repo, meta), err(Debug))]
     pub fn apply<'graph>(
         branch: &gix::refs::FullNameRef,
         workspace: &but_graph::projection::Workspace<'graph>,
@@ -258,7 +260,7 @@ pub(crate) mod function {
 
         let mut ws_md = meta.workspace(workspace_ref_name_to_update.as_ref())?;
         {
-            let ws_mut: &mut ref_metadata::Workspace = &mut ws_md;
+            let ws_mut: &mut Workspace = &mut ws_md;
             for rn in &branches_to_apply {
                 ws_mut.add_or_insert_new_stack_if_not_present(rn, order);
             }
@@ -370,11 +372,6 @@ pub(crate) mod function {
             overlay.with_entrypoint(new_head_id, Some(workspace_ref_name_to_update.clone())),
         )?;
         persist_metadata(meta, &branches_to_apply, &ws_md)?;
-        set_head_to_reference(
-            repo,
-            new_head_id,
-            (!ws_ref_exists).then_some(workspace_ref_name_to_update.as_ref()),
-        )?;
         crate::branch::safe_checkout(
             prev_head_id,
             new_head_id,
@@ -384,6 +381,11 @@ pub(crate) mod function {
             },
         )?;
 
+        set_head_to_reference(
+            repo,
+            new_head_id,
+            (!ws_ref_exists).then_some(workspace_ref_name_to_update.as_ref()),
+        )?;
         Ok(Outcome {
             graph: Cow::Owned(graph),
             workspace_ref_created: needs_ws_ref_creation,

crates/but-workspace/src/commit.rs

@@ -77,23 +77,15 @@ pub mod merge {
                 .filter_map(|s| s.branches.first())
                 .filter_map(|top_segment| {
                     let stack_tip_name = top_segment.ref_name.as_ref();
-                    match graph.named_segment_by_ref_name(stack_tip_name) {
+                    match graph.segment_and_commit_by_ref_name(stack_tip_name) {
                         None => {
                             missing_stacks.push(top_segment.ref_name.to_owned());
                             None
                         }
-                        Some(segment) => graph
-                            .tip_skip_empty(segment.id)
-                            .map(|c| Ok((stack_tip_name, c.id, segment.id)))
-                            .or_else(|| {
-                                Some(Err(anyhow::anyhow!(
-                                    "Base segment {id} didn't have a single commit reachable",
-                                    id = segment.id.index()
-                                )))
-                            }),
+                        Some((segment, commit)) => Some((stack_tip_name, commit.id, segment.id)),
                     }
                 })
-                .collect::<Result<_, _>>()?;
+                .collect();
 
             let conflicting_stacks = Vec::new();
             let mut prev_base_sidx = None;
@@ -139,7 +131,7 @@ pub mod merge {
                         merge_options.clone(),
                     )?;
                     if merge.has_unresolved_conflicts(conflict_kind) {
-                        todo!("conflict handling - hero or not: {hero_stack:?}");
+                        bail!("TODO: conflict handling with hero-special: {hero_stack:?}");
                     }
                     merge_tree_id = merge.tree.write()?.detach().into();
                 }

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

@@ -2,6 +2,9 @@ use super::BranchManager;
 use crate::r#virtual as vbranch;
 use crate::{hunk::VirtualBranchHunk, integration::update_workspace_commit, VirtualBranchesExt};
 use anyhow::{anyhow, bail, Context, Result};
+use but_workspace::branch::apply::{IntegrationMode, WorkspaceReferenceNaming};
+use but_workspace::branch::checkout::UncommitedWorktreeChanges;
+use but_workspace::branch::OnWorkspaceMergeConflict;
 use but_workspace::stack_ext::StackExt;
 use gitbutler_branch::BranchCreateRequest;
 use gitbutler_branch::{self, dedup};
@@ -139,6 +142,39 @@ impl BranchManager<'_> {
         pr_number: Option<usize>,
         perm: &mut WorktreeWritePermission,
     ) -> Result<(StackId, Vec<StackId>)> {
+        // Assume that this is always about 'apply' and hijack the entire method.
+        // That way we'd learn what's missing.
+        if self.ctx.app_settings().feature_flags.apply3 {
+            let (repo, mut meta, graph) = self.ctx.graph_and_meta_mut_and_repo(perm)?;
+            let ws = graph.to_workspace()?;
+            let target = target.to_string();
+            let branch_to_apply = target.as_str().try_into()?;
+            let out = but_workspace::branch::apply(
+                branch_to_apply,
+                &ws,
+                &repo,
+                &mut *meta,
+                but_workspace::branch::apply::Options {
+                    integration_mode: IntegrationMode::AlwaysMerge,
+                    on_workspace_conflict:
+                        OnWorkspaceMergeConflict::MaterializeAndReportConflictingStacks,
+                    workspace_reference_naming: WorkspaceReferenceNaming::Default,
+                    uncommitted_changes: UncommitedWorktreeChanges::KeepAndAbortOnConflict,
+                    order: None,
+                },
+            )?;
+            let ws = out.graph.to_workspace()?;
+            let unapplied_stacks_to_be_done = Vec::new();
+            let applied_branch_stack_id = ws
+                .find_segment_and_stack_by_refname(branch_to_apply)
+                .with_context(||
+                    format!("Failed to apply branch and we probably have to handle more of the outcome\n{out:?}")
+                )?
+                .0
+                .id
+                .context("BUG: newly applied stacks should always have a stack id")?;
+            return Ok((applied_branch_stack_id, unapplied_stacks_to_be_done));
+        }
         let old_cwd = (!self.ctx.app_settings().feature_flags.cv3)
             .then(|| self.ctx.repo().create_wd_tree(0).map(|tree| tree.id()))
             .transpose()?;