Merge pull request #9682 from Byron/next

V3 of remove_branch

Author: Byron

crates/but-api/src/commands/stack.rs

@@ -1,15 +1,16 @@
+use crate::{App, error::Error};
 use anyhow::{Context, anyhow};
 use but_workspace::branch::{ReferenceAnchor, ReferencePosition};
 use gitbutler_branch_actions::internal::PushResult;
 use gitbutler_branch_actions::stack::CreateSeriesRequest;
 use gitbutler_command_context::CommandContext;
+use gitbutler_oplog::SnapshotExt;
 use gitbutler_project::ProjectId;
 use gitbutler_stack::StackId;
 use gitbutler_user::User;
 use gix::refs::Category;
 use serde::Deserialize;
-
-use crate::{App, error::Error};
+use std::borrow::Cow;
 
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
@@ -21,9 +22,8 @@ pub struct CreateBranchParams {
 
 pub fn create_branch(app: &App, params: CreateBranchParams) -> Result<(), Error> {
     let project = gitbutler_project::get(params.project_id)?;
-    let ws3_enabled = app.app_settings.get()?.feature_flags.ws3;
     let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?;
-    if ws3_enabled {
+    if app.app_settings.get()?.feature_flags.ws3 {
         use ReferencePosition::Above;
         let (repo, mut meta, graph) = ctx.graph_and_meta_and_repo()?;
         let ws = graph.to_workspace()?;
@@ -38,16 +38,18 @@ pub fn create_branch(app: &App, params: CreateBranchParams) -> Result<(), Error>
             .into());
         }
 
-        let _guard = project.exclusive_worktree_access();
-        but_workspace::branch::create_reference(
+        let mut guard = project.exclusive_worktree_access();
+        ctx.snapshot_create_dependent_branch(&params.request.name, guard.write_permission())
+            .ok();
+        _ = but_workspace::branch::create_reference(
             new_ref.as_ref(),
             {
                 let segment = stack.segments.first().context("BUG: no empty stacks")?;
                 segment
                     .ref_name
                     .as_ref()
                     .map(|rn| ReferenceAnchor::AtSegment {
-                        ref_name: rn.as_ref(),
+                        ref_name: Cow::Borrowed(rn.as_ref()),
                         position: Above,
                     })
                     .or_else(|| {
@@ -85,7 +87,31 @@ pub struct RemoveBranchParams {
 pub fn remove_branch(app: &App, params: RemoveBranchParams) -> Result<(), Error> {
     let project = gitbutler_project::get(params.project_id)?;
     let ctx = CommandContext::open(&project, app.app_settings.get()?.clone())?;
-    gitbutler_branch_actions::stack::remove_branch(&ctx, params.stack_id, params.branch_name)?;
+    if app.app_settings.get()?.feature_flags.ws3 {
+        let (repo, mut meta, graph) = ctx.graph_and_meta_and_repo()?;
+        let ws = graph.to_workspace()?;
+        let ref_name = Category::LocalBranch
+            .to_full_name(params.branch_name.as_str())
+            .map_err(anyhow::Error::from)?;
+        let mut guard = project.exclusive_worktree_access();
+        ctx.snapshot_remove_dependent_branch(&params.branch_name, guard.write_permission())
+            .ok();
+        but_workspace::branch::remove_reference(
+            ref_name.as_ref(),
+            &repo,
+            &ws,
+            &mut meta,
+            but_workspace::branch::remove_reference::Options {
+                avoid_anonymous_stacks: true,
+                // The UI kind of keeps it, but we can't do that somehow
+                // the object id is null, and stuff breaks. Fine for now.
+                // Delete is delete.
+                keep_metadata: false,
+            },
+        )?;
+    } else {
+        gitbutler_branch_actions::stack::remove_branch(&ctx, params.stack_id, params.branch_name)?;
+    }
     Ok(())
 }
 

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

@@ -646,6 +646,18 @@ impl Graph {
         let ref_name = self[tip_sidx].ref_name.clone();
         Graph::from_commit_traversal_inner(tip, &repo, ref_name, &meta, self.options.clone())
     }
+
+    /// Like [`Self::redo_traversal_with_overlay()`], but replaces this instance, without overlay, and returns
+    /// a newly computed workspace for it.
+    pub fn workspace_of_redone_traversal(
+        &mut self,
+        repo: &gix::Repository,
+        meta: &impl RefMetadata,
+    ) -> anyhow::Result<crate::projection::Workspace<'_>> {
+        let new = self.redo_traversal_with_overlay(repo, meta, Default::default())?;
+        *self = new;
+        self.to_workspace()
+    }
 }
 
 impl Graph {

crates/but-graph/src/lib.rs

@@ -225,6 +225,7 @@ pub type CommitIndex = usize;
 
 /// A graph of connected segments that represent a section of the actual commit-graph.
 #[derive(Default, Debug, Clone)]
+#[must_use]
 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/ref_metadata_legacy.rs

@@ -1,4 +1,4 @@
-use crate::virtual_branches_legacy_types::{Stack, StackBranch, VirtualBranches};
+use crate::virtual_branches_legacy_types::{CommitOrChangeId, Stack, StackBranch, VirtualBranches};
 use anyhow::{Context, bail};
 use but_core::RefMetadata;
 use but_core::ref_metadata::{
@@ -48,6 +48,17 @@ impl Snapshot {
     /// Instead of trying to maintain this, let's just fix it before writing.
     fn to_consistent_data(&self) -> VirtualBranches {
         let mut data = self.content.clone();
+        // EVIL HACK: assure we fill-in the CommitIDs of heads or else everything breaks.
+        //            this probably won't be needed once no old code is running, and by then
+        //            we should move away from this anyway and have a DB backed implementation.
+        let repo = gix::discover(self.path.parent().expect("at least a file"))
+            .ok()
+            .map(|repo| {
+                (
+                    repo,
+                    CommitOrChangeId::CommitId(gix::hash::Kind::Sha1.null().to_string()),
+                )
+            });
         for stack in data.branches.values_mut() {
             if stack.name.is_empty() {
                 stack.name = stack
@@ -58,6 +69,18 @@ impl Snapshot {
                     .unwrap_or_default()
                     .to_string();
             }
+            if let Some((repo, null_id)) = repo.as_ref() {
+                for segment in &mut stack.heads {
+                    if &segment.head == null_id {
+                        let Ok(mut r) = repo.find_reference(&segment.name) else {
+                            continue;
+                        };
+                        if let Ok(id) = r.peel_to_id_in_place() {
+                            segment.head = CommitOrChangeId::CommitId(id.to_string());
+                        }
+                    }
+                }
+            }
         }
         data
     }

crates/but-settings/assets/defaults.jsonc

@@ -22,6 +22,8 @@
 		"v3": false,
 		/// Enable the usage of V3 workspace APIs.
 		"ws3": false,
+		/// Enable undo/redo support.
+		"undo": false,
 		/// Enable the usage of GitButler Acitions.
 		"actions": false,
 		/// Enable the usage of the butbot chat.

crates/but-settings/src/app_settings.rs

@@ -32,6 +32,21 @@ pub struct FeatureFlags {
     /// Enable the usage of V3 workspace APIs.
     #[serde(default = "default_true")]
     pub ws3: bool,
+    /// Enable undo/redo support.
+    ///
+    /// ### Progression for implementation
+    ///
+    /// * use snapshot system in undo/redo queue
+    ///     - consider not referring to these objects by reference to `git gc` will catch them,
+    ///       or even purge them on shutdown. Alternatively, keep them in-memory with in-memory objects.
+    /// * add user-control to snapshot system to purge now, or purge after time X. That way data isn't stored forever.
+    /// * Finally, consider implementing undo/redo with invasive primitives that are undoable/redoable themselves for
+    ///   the most efficient solution, inherently in memory, i.e.
+    ///     - CRUD reference
+    ///     - CRUD metadata
+    ///     - CRUD workspace
+    ///     - CRUD files
+    pub undo: bool,
     /// Enable the usage of GitButler Acitions.
     pub actions: bool,
     /// Enable the usage of the butbot chat.

crates/but-testing/src/args.rs

@@ -205,6 +205,30 @@ pub enum Subcommands {
         #[clap(long, short = 'd')]
         description: Option<String>,
     },
+    /// Create a reference at the given position (dependent and independent)
+    CreateReference {
+        /// Create the branch above the given commit or branch short name.
+        #[arg(long, short = 'a', conflicts_with = "below")]
+        above: Option<String>,
+        /// Create the branch below the given commit or branch short name.
+        #[arg(long, short = 'b', conflicts_with = "above")]
+        below: Option<String>,
+        /// the short-name of the new branch.
+        short_name: String,
+    },
+    /// Delete the given workspace reference.
+    RemoveReference {
+        /// Allow stacks to be empty.
+        #[arg(long)]
+        permit_empty_stacks: bool,
+        /// Do not delete the branch metadata associated with the deleted reference.
+        ///
+        /// Useful if it might be recreated, afterwards.
+        #[arg(long)]
+        keep_metadata: bool,
+        /// the short-name of the reference to delete.
+        short_name: String,
+    },
 }
 
 #[cfg(test)]

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

@@ -3,10 +3,13 @@ use but_core::UnifiedDiff;
 use but_db::poll::ItemKind;
 use but_graph::VirtualBranchesTomlMetadata;
 use but_settings::AppSettings;
+use but_workspace::branch::{ReferenceAnchor, ReferencePosition};
 use but_workspace::{DiffSpec, HunkHeader};
 use gitbutler_project::{Project, ProjectId};
 use gix::bstr::{BString, ByteSlice};
 use gix::odb::store::RefreshMode;
+use gix::refs::Category;
+use std::borrow::Cow;
 use std::io::{Write, stdout};
 use std::mem::ManuallyDrop;
 use std::path::Path;
@@ -72,6 +75,21 @@ pub fn repo_and_maybe_project(
     Ok(res)
 }
 
+pub fn repo_and_maybe_project_and_graph(
+    args: &super::Args,
+    mode: RepositoryOpenMode,
+) -> anyhow::Result<(
+    gix::Repository,
+    Option<Project>,
+    but_graph::Graph,
+    ManuallyDrop<VirtualBranchesTomlMetadata>,
+)> {
+    let (repo, project) = repo_and_maybe_project(args, mode)?;
+    let meta = meta_from_maybe_project(project.as_ref())?;
+    let graph = but_graph::Graph::from_head(&repo, &*meta, meta.graph_options())?;
+    Ok((repo, project, graph, meta))
+}
+
 fn debug_print(this: impl std::fmt::Debug) -> anyhow::Result<()> {
     println!("{:#?}", this);
     Ok(())
@@ -318,14 +336,15 @@ pub mod stacks {
         description: Option<&str>,
         current_dir: &Path,
         use_json: bool,
+        ws3: bool,
     ) -> anyhow::Result<()> {
         let project = project_from_path(current_dir)?;
         // Enable v3 feature flags for the command context
         let app_settings = AppSettings {
             feature_flags: but_settings::app_settings::FeatureFlags {
                 v3: true,
-                // Keep this off until it caught up at least.
-                ws3: false,
+                ws3,
+                undo: false,
                 actions: false,
                 butbot: false,
                 rules: false,
@@ -493,26 +512,14 @@ pub fn graph(
             .collect(),
     };
 
-    let meta_with_drop;
-    let meta_without_drop;
-    let meta = match project {
-        None => {
-            meta_without_drop = ManuallyDrop::new(VirtualBranchesTomlMetadata::from_path(
-                "should-never-be-written-back.toml",
-            )?);
-            &meta_without_drop
-        }
-        Some(project) => {
-            meta_with_drop = ref_metadata_toml(&project)?;
-            &meta_with_drop
-        }
-    };
+    // Never drop - this is read-only.
+    let meta = meta_from_maybe_project(project.as_ref())?;
     let graph = match ref_name {
-        None => but_graph::Graph::from_head(&repo, meta, opts),
+        None => but_graph::Graph::from_head(&repo, &*meta, opts),
         Some(ref_name) => {
             let mut reference = repo.find_reference(ref_name)?;
             let id = reference.peel_to_id_in_place()?;
-            but_graph::Graph::from_commit_traversal(id, reference.name().to_owned(), meta, opts)
+            but_graph::Graph::from_commit_traversal(id, reference.name().to_owned(), &*meta, opts)
         }
     }?;
 
@@ -556,6 +563,81 @@ pub fn graph(
     Ok(())
 }
 
+/// NOTE: THis is a special case while vb.toml is used and while projects are somewhat special.
+fn meta_from_maybe_project(
+    project: Option<&Project>,
+) -> anyhow::Result<ManuallyDrop<VirtualBranchesTomlMetadata>> {
+    let meta = ManuallyDrop::new(match project {
+        None => VirtualBranchesTomlMetadata::from_path("should-never-be-written-back.toml")?,
+        Some(project) => ref_metadata_toml(project)?,
+    });
+    Ok(meta)
+}
+
+pub fn remove_reference(
+    args: &super::Args,
+    short_name: &str,
+    opts: but_workspace::branch::remove_reference::Options,
+) -> anyhow::Result<()> {
+    let (repo, _project, graph, mut meta) =
+        repo_and_maybe_project_and_graph(args, RepositoryOpenMode::General)?;
+
+    let ref_name = Category::LocalBranch.to_full_name(short_name)?;
+    let deleted = but_workspace::branch::remove_reference(
+        ref_name.as_ref(),
+        &repo,
+        &graph.to_workspace()?,
+        &mut *meta,
+        opts,
+    )?
+    .is_some();
+    if deleted {
+        eprintln!("Deleted");
+    } else {
+        eprintln!("Nothing deleted");
+    }
+    // write metadata if there are projects - this is a special case while we use vb.toml.
+    ManuallyDrop::into_inner(meta);
+    Ok(())
+}
+
+pub fn create_reference(
+    args: &super::Args,
+    short_name: &str,
+    above: Option<&str>,
+    below: Option<&str>,
+) -> anyhow::Result<()> {
+    let (repo, project, graph, mut meta) =
+        repo_and_maybe_project_and_graph(args, RepositoryOpenMode::General)?;
+    let resolve =
+        |spec: &str, position: ReferencePosition| -> anyhow::Result<ReferenceAnchor<'_>> {
+            Ok(match repo.try_find_reference(spec)? {
+                None => ReferenceAnchor::AtCommit {
+                    commit_id: repo.rev_parse_single(spec)?.detach(),
+                    position,
+                },
+                Some(rn) => ReferenceAnchor::AtSegment {
+                    ref_name: Cow::Owned(rn.inner.name),
+                    position,
+                },
+            })
+        };
+    let anchor = above
+        .map(|spec| resolve(spec, ReferencePosition::Above))
+        .or_else(|| below.map(|spec| resolve(spec, ReferencePosition::Below)))
+        .transpose()?;
+
+    let new_ref = Category::LocalBranch.to_full_name(short_name)?;
+    let ws = graph.to_workspace()?;
+    _ = but_workspace::branch::create_reference(new_ref.as_ref(), anchor, &repo, &ws, &mut *meta)?;
+
+    if project.is_some() {
+        // write metadata if there are projects - this is a special case while we use vb.toml.
+        ManuallyDrop::into_inner(meta);
+    }
+    Ok(())
+}
+
 fn indices_or_headers_to_hunk_headers(
     repo: &gix::Repository,
     indices_or_headers: Option<IndicesOrHeaders<'_>>,