safe-checkout for 'update_uncommitted_changes()'

Author: Byron

Cargo.lock

@@ -37,7 +37,6 @@ This builds the **web** target, points it to the but-server on `http://localhost
 
 Open Chrome (let's not kid ourselves) and got to `http://localhost:1420` and enjoy
 
-
 ### Development
 
 #### Auto-build the server on Rust changes

apps/desktop/README.md

@@ -1,4 +1,4 @@
-use super::{checkout_remerged_head, BranchManager};
+use super::BranchManager;
 use crate::r#virtual as vbranch;
 use crate::{hunk::VirtualBranchHunk, integration::update_workspace_commit, VirtualBranchesExt};
 use anyhow::{anyhow, bail, Context, Result};
@@ -139,7 +139,9 @@ impl BranchManager<'_> {
         pr_number: Option<usize>,
         perm: &mut WorktreeWritePermission,
     ) -> Result<(StackId, Vec<StackId>)> {
-        let old_cwd = self.ctx.repo().create_wd_tree(0)?.id();
+        let old_cwd = (!self.ctx.app_settings().feature_flags.cv3)
+            .then(|| self.ctx.repo().create_wd_tree(0).map(|tree| tree.id()))
+            .transpose()?;
         let old_workspace = WorkspaceState::create(self.ctx, perm.read_permission())?;
         // only set upstream if it's not the default target
         let upstream_branch = match upstream_branch {
@@ -270,13 +272,7 @@ impl BranchManager<'_> {
         )?;
         self.ctx.add_branch_reference(&branch)?;
 
-        match self.apply_branch(
-            branch.id,
-            perm,
-            old_workspace,
-            old_cwd,
-            self.ctx.app_settings().feature_flags.cv3,
-        ) {
+        match self.apply_branch(branch.id, perm, old_workspace, old_cwd) {
             Ok((_, unapplied_stacks)) => Ok((branch.id, unapplied_stacks)),
             Err(err)
                 if err
@@ -299,8 +295,7 @@ impl BranchManager<'_> {
         stack_id: StackId,
         perm: &mut WorktreeWritePermission,
         workspace_state: WorkspaceState,
-        old_cwd: git2::Oid,
-        safe_checkout: bool,
+        old_cwd: Option<git2::Oid>,
     ) -> Result<(String, Vec<StackId>)> {
         let repo = self.ctx.repo();
 
@@ -351,6 +346,7 @@ impl BranchManager<'_> {
                     .filter(|branch| branch.id != stack_id)
                 {
                     unapplied_stacks.push(stack_to_unapply.id);
+                    let safe_checkout = old_cwd.is_none();
                     let res =
                         self.unapply(stack_to_unapply.id, perm, false, Vec::new(), safe_checkout);
                     if res.is_err() {
@@ -437,23 +433,19 @@ impl BranchManager<'_> {
 
         // Permissions here might be wonky, just go with it though.
         let new_workspace = WorkspaceState::create(self.ctx, perm.read_permission())?;
-        if safe_checkout {
-            let res = checkout_remerged_head(self.ctx, &gix_repo);
-            if res.is_err() {
-                stack.in_workspace = false;
-                vb_state.set_stack(stack.clone())?;
-            }
-            res?;
-        } else {
-            update_uncommited_changes_with_tree(
-                self.ctx,
-                workspace_state,
-                new_workspace,
-                old_cwd,
-                Some(true),
-                perm,
-            )?;
+        let res = update_uncommited_changes_with_tree(
+            self.ctx,
+            workspace_state,
+            new_workspace,
+            old_cwd,
+            Some(true),
+            perm,
+        );
+        if res.is_err() {
+            stack.in_workspace = false;
+            vb_state.set_stack(stack.clone())?;
         }
+        res?;
 
         update_workspace_commit(&vb_state, self.ctx)?;
 

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

@@ -289,7 +289,7 @@ pub(crate) fn save_and_return_to_workspace(
         ctx,
         old_workspace,
         new_workspace,
-        uncommtied_changes,
+        Some(uncommtied_changes),
         Some(true),
         perm,
     )?;

crates/gitbutler-edit-mode/src/lib.rs

@@ -18,4 +18,5 @@ gitbutler-commit.workspace = true
 gitbutler-oxidize.workspace = true
 gitbutler-command-context.workspace = true
 gitbutler-cherry-pick.workspace = true
+but-workspace.workspace = true
 but-rebase.workspace = true

crates/gitbutler-workspace/Cargo.toml

@@ -53,42 +53,52 @@ pub fn update_uncommited_changes(
     perm: &mut WorktreeWritePermission,
 ) -> Result<()> {
     let repo = ctx.repo();
-    let uncommited_changes = repo.create_wd_tree(0)?;
+    let uncommited_changes = (!ctx.app_settings().feature_flags.cv3)
+        .then(|| repo.create_wd_tree(0).map(|tree| tree.id()))
+        .transpose()?;
 
-    update_uncommited_changes_with_tree(ctx, old, new, uncommited_changes.id(), None, perm)
+    update_uncommited_changes_with_tree(ctx, old, new, uncommited_changes, None, perm)
 }
 
+/// `old_uncommitted_changes` is `None` if the `safe_checkout` feature is toggled on in `ctx`
 pub fn update_uncommited_changes_with_tree(
     ctx: &CommandContext,
     old: WorkspaceState,
     new: WorkspaceState,
-    old_uncommited_changes: git2::Oid,
+    old_uncommited_changes: Option<git2::Oid>,
     always_checkout: Option<bool>,
     _perm: &mut WorktreeWritePermission,
 ) -> Result<()> {
     let repo = ctx.repo();
-
-    let mut new_uncommited_changes =
-        move_tree_between_workspaces(repo, old_uncommited_changes, old, new)?;
-
-    // If the new tree and old tree are the same, then we don't need to do anything
-    if !new_uncommited_changes.has_conflicts() && !always_checkout.unwrap_or(false) {
-        let tree = new_uncommited_changes.write_tree_to(repo)?;
-        if tree == old_uncommited_changes {
-            return Ok(());
+    if let Some(worktree_id) = old_uncommited_changes {
+        let mut new_uncommited_changes = move_tree_between_workspaces(repo, worktree_id, old, new)?;
+
+        // If the new tree and old tree are the same, then we don't need to do anything
+        if !new_uncommited_changes.has_conflicts() && !always_checkout.unwrap_or(false) {
+            let tree = new_uncommited_changes.write_tree_to(repo)?;
+            if tree == worktree_id {
+                return Ok(());
+            }
         }
-    }
-
-    repo.checkout_index(
-        Some(&mut new_uncommited_changes),
-        Some(
-            git2::build::CheckoutBuilder::new()
-                .force()
-                .remove_untracked(true)
-                .conflict_style_diff3(true),
-        ),
-    )?;
 
+        repo.checkout_index(
+            Some(&mut new_uncommited_changes),
+            Some(
+                git2::build::CheckoutBuilder::new()
+                    .force()
+                    .remove_untracked(true)
+                    .conflict_style_diff3(true),
+            ),
+        )?;
+    } else {
+        let new_tree_id = merge_workspace(repo, new)?.to_gix();
+        let gix_repo = ctx.gix_repo_for_merging()?;
+        but_workspace::branch::safe_checkout_from_head(
+            new_tree_id,
+            &gix_repo,
+            but_workspace::branch::checkout::Options::default(),
+        )?;
+    }
     Ok(())
 }