Merge pull request #13132 from gitbutlerapp/multi-process-safety

Multi-process safety

Author: Byron

crates/but-api/src/branch.rs

@@ -1,7 +1,10 @@
 use std::collections::BTreeMap;
 
 use but_api_macros::but_api;
-use but_core::{RefMetadata, ui::TreeChanges, worktree::checkout::UncommitedWorktreeChanges};
+use but_core::{
+    RefMetadata, sync::RepoExclusive, ui::TreeChanges,
+    worktree::checkout::UncommitedWorktreeChanges,
+};
 use but_ctx::Context;
 use but_oplog::legacy::{OperationKind, SnapshotDetails, Trailer};
 use but_rebase::graph_rebase::Editor;
@@ -86,13 +89,33 @@ pub mod json {
     }
 }
 
-/// Apply `existing_branch` to the workspace in the repository that `ctx` refers to, or create the workspace with default name.
+/// Applies a branch using the behavior described by [`apply_only_with_perm()`].
+///
+/// This acquires exclusive worktree access from `ctx` before applying
+/// `existing_branch`.
 pub fn apply_only(
     ctx: &mut but_ctx::Context,
     existing_branch: &gix::refs::FullNameRef,
+) -> anyhow::Result<but_workspace::branch::apply::Outcome<'static>> {
+    let mut guard = ctx.exclusive_worktree_access();
+    apply_only_with_perm(ctx, existing_branch, guard.write_permission())
+}
+
+/// Applies `existing_branch` to the current workspace under caller-held
+/// exclusive repository access.
+///
+/// It applies the branch with the default workspace-apply options, updates the
+/// in-memory workspace stored in `ctx` to the returned workspace state, and
+/// returns the apply outcome. This variant does not create an oplog
+/// entry. For lower-level implementation details, see
+/// [`but_workspace::branch::apply()`].
+pub fn apply_only_with_perm(
+    ctx: &mut but_ctx::Context,
+    existing_branch: &gix::refs::FullNameRef,
+    perm: &mut RepoExclusive,
 ) -> anyhow::Result<but_workspace::branch::apply::Outcome<'static>> {
     let mut meta = ctx.meta()?;
-    let (_guard, repo, mut ws, _) = ctx.workspace_mut_and_db()?;
+    let (repo, mut ws, _) = ctx.workspace_mut_and_db_with_perm(perm)?;
     let out = but_workspace::branch::apply(
         existing_branch,
         &ws,
@@ -115,74 +138,116 @@ pub fn apply_only(
     Ok(out)
 }
 
-/// Just like [apply_only()], but will create an oplog entry as well on success.
+/// Applies `existing_branch` using the behavior described by
+/// [`apply_with_perm()`].
+///
+/// This acquires exclusive worktree access from `ctx`, applies
+/// `existing_branch`, and records an oplog snapshot on success.
 #[but_api(napi, json::ApplyOutcome)]
 #[instrument(err(Debug))]
 pub fn apply(
     ctx: &mut but_ctx::Context,
     existing_branch: &gix::refs::FullNameRef,
+) -> anyhow::Result<but_workspace::branch::apply::Outcome<'static>> {
+    let mut guard = ctx.exclusive_worktree_access();
+    apply_with_perm(ctx, existing_branch, guard.write_permission())
+}
+
+/// Apply `existing_branch` to the workspace under caller-held exclusive
+/// repository access and record an oplog snapshot on success.
+///
+/// It behaves like [`apply_only_with_perm()`], but first prepares a best-effort
+/// oplog snapshot for a create-branch operation, annotated with the branch
+/// name, and commits that snapshot only if the apply succeeds. For lower-level
+/// implementation details, see [`but_workspace::branch::apply()`].
+pub fn apply_with_perm(
+    ctx: &mut but_ctx::Context,
+    existing_branch: &gix::refs::FullNameRef,
+    perm: &mut RepoExclusive,
 ) -> anyhow::Result<but_workspace::branch::apply::Outcome<'static>> {
     // NOTE: since this is optional by nature, the same would be true if snapshotting/undo would be disabled via `ctx` app settings, for instance.
-    let maybe_oplog_entry = but_oplog::UnmaterializedOplogSnapshot::from_details(
+    let maybe_oplog_entry = but_oplog::UnmaterializedOplogSnapshot::from_details_with_perm(
         ctx,
         SnapshotDetails::new(OperationKind::CreateBranch).with_trailers(vec![Trailer {
             key: "name".into(),
             value: existing_branch.to_string(),
         }]),
+        perm.read_permission(),
     )
     .ok();
 
-    let res = apply_only(ctx, existing_branch);
+    let res = apply_only_with_perm(ctx, existing_branch, perm);
     if let Some(snapshot) = maybe_oplog_entry.filter(|_| res.is_ok()) {
-        let mut guard = ctx.exclusive_worktree_access();
-        snapshot.commit(ctx, guard.write_permission()).ok();
+        snapshot.commit(ctx, perm).ok();
     }
     res
 }
 
-/// Gets the changes for a given branch.
-#[but_api(napi, TreeChanges)]
+/// Computes the worktree-visible diff for `branch` in the current workspace.
+///
+/// `branch` is resolved by name in the repository referenced by `ctx`, and the
+/// diff is computed against the current workspace state. For lower-level
+/// implementation details, see [`but_workspace::ui::diff::changes_in_branch()`].
+#[but_api(napi)]
 #[instrument(err(Debug))]
 pub fn branch_diff(ctx: &Context, branch: String) -> anyhow::Result<TreeChanges> {
     let (_guard, repo, ws, _) = ctx.workspace_and_db()?;
     let branch = repo.find_reference(&branch)?;
     but_workspace::ui::diff::changes_in_branch(&repo, &ws, branch.name())
 }
 
-/// Move a branch on top of another
+/// Moves a branch using the behavior described by [`move_branch_with_perm()`].
+///
+/// This acquires exclusive worktree access from `ctx`, moves `subject_branch`
+/// on top of `target_branch`, and records an oplog snapshot on success.
 #[but_api(napi, json::UIMoveBranchResult)]
 #[instrument(err(Debug))]
 pub fn move_branch(
     ctx: &mut but_ctx::Context,
     subject_branch: &gix::refs::FullNameRef,
     target_branch: &gix::refs::FullNameRef,
 ) -> anyhow::Result<MoveBranchResult> {
-    let maybe_oplog_entry = but_oplog::UnmaterializedOplogSnapshot::from_details(
+    let mut guard = ctx.exclusive_worktree_access();
+    move_branch_with_perm(ctx, subject_branch, target_branch, guard.write_permission())
+}
+
+/// Move `subject_branch` on top of `target_branch` under caller-held
+/// exclusive repository access and record an oplog snapshot on success.
+///
+/// It prepares a best-effort move-branch oplog snapshot, rebases the subject
+/// branch onto the target branch, updates workspace metadata, and commits the
+/// snapshot only if the move succeeds. The returned [`MoveBranchResult`]
+/// contains the commit-id mapping produced by materializing the rebase. For
+/// lower-level implementation details, see
+/// [`but_workspace::branch::move_branch()`].
+pub fn move_branch_with_perm(
+    ctx: &mut but_ctx::Context,
+    subject_branch: &gix::refs::FullNameRef,
+    target_branch: &gix::refs::FullNameRef,
+    perm: &mut RepoExclusive,
+) -> anyhow::Result<MoveBranchResult> {
+    let maybe_oplog_entry = but_oplog::UnmaterializedOplogSnapshot::from_details_with_perm(
         ctx,
         SnapshotDetails::new(OperationKind::MoveBranch),
+        perm.read_permission(),
     )
     .ok();
 
-    let move_branch_result = move_branch_impl(ctx, subject_branch, target_branch);
+    let move_branch_result = move_branch_impl_with_perm(ctx, subject_branch, target_branch, perm);
     if let Some(snapshot) = maybe_oplog_entry.filter(|_| move_branch_result.is_ok()) {
-        let mut guard = ctx.exclusive_worktree_access();
-        snapshot.commit(ctx, guard.write_permission()).ok();
+        snapshot.commit(ctx, perm).ok();
     }
     move_branch_result
 }
 
-/// Move the branch, updating the workspace and the metadata.
-///
-/// `subject_branch` - The branch to move.
-///
-/// `target_branch` - The branch to move `subject_branch` on top of.
-fn move_branch_impl(
+fn move_branch_impl_with_perm(
     ctx: &mut but_ctx::Context,
     subject_branch: &gix::refs::FullNameRef,
     target_branch: &gix::refs::FullNameRef,
+    perm: &mut RepoExclusive,
 ) -> anyhow::Result<MoveBranchResult> {
     let mut meta = ctx.meta()?;
-    let (_guard, repo, mut ws, _, _cache) = ctx.workspace_mut_and_db_and_cache()?;
+    let (repo, mut ws, _, _cache) = ctx.workspace_mut_and_db_and_cache_with_perm(perm)?;
     let editor = Editor::create(&mut ws, &mut meta, &repo)?;
     let but_workspace::branch::move_branch::Outcome { rebase, ws_meta } =
         but_workspace::branch::move_branch(editor, subject_branch, target_branch)?;
@@ -199,36 +264,55 @@ fn move_branch_impl(
     })
 }
 
-/// Take a branch out of a stack
+/// Tears off a branch using the behavior described by [`tear_off_branch_with_perm()`].
 ///
-/// `subject_branch` - The branch to take out of its stack, and create a new one out of.
+/// This acquires exclusive worktree access from `ctx`, tears `subject_branch`
+/// out of its current stack, and records an oplog snapshot on success.
 #[but_api(napi, json::UIMoveBranchResult)]
 #[instrument(err(Debug))]
 pub fn tear_off_branch(
     ctx: &mut but_ctx::Context,
     subject_branch: &gix::refs::FullNameRef,
 ) -> anyhow::Result<MoveBranchResult> {
-    let maybe_oplog_entry = but_oplog::UnmaterializedOplogSnapshot::from_details(
+    let mut guard = ctx.exclusive_worktree_access();
+    tear_off_branch_with_perm(ctx, subject_branch, guard.write_permission())
+}
+
+/// Removes `subject_branch` from its current stack, creating a new stack for
+/// it, under caller-held exclusive repository access.
+///
+/// It prepares a best-effort tear-off oplog snapshot, performs the tear-off
+/// rebase and workspace metadata update under `perm`, and commits the snapshot
+/// only if the mutation succeeds. The returned [`MoveBranchResult`] contains
+/// the commit-id mapping produced by materializing the rebase. For lower-level
+/// implementation details, see [`but_workspace::branch::tear_off_branch()`].
+pub fn tear_off_branch_with_perm(
+    ctx: &mut but_ctx::Context,
+    subject_branch: &gix::refs::FullNameRef,
+    perm: &mut RepoExclusive,
+) -> anyhow::Result<MoveBranchResult> {
+    let maybe_oplog_entry = but_oplog::UnmaterializedOplogSnapshot::from_details_with_perm(
         ctx,
         SnapshotDetails::new(OperationKind::TearOffBranch),
+        perm.read_permission(),
     )
     .ok();
 
-    let move_branch_result = tear_off_branch_impl(ctx, subject_branch);
+    let move_branch_result = tear_off_branch_impl_with_perm(ctx, subject_branch, perm);
     if let Some(snapshot) = maybe_oplog_entry.filter(|_| move_branch_result.is_ok()) {
-        let mut guard = ctx.exclusive_worktree_access();
-        snapshot.commit(ctx, guard.write_permission()).ok();
+        snapshot.commit(ctx, perm).ok();
     }
     move_branch_result
 }
 
 /// Move the branch, updating the workspace and the metadata.
-fn tear_off_branch_impl(
+fn tear_off_branch_impl_with_perm(
     ctx: &mut but_ctx::Context,
     subject_branch: &gix::refs::FullNameRef,
+    perm: &mut RepoExclusive,
 ) -> anyhow::Result<MoveBranchResult> {
     let mut meta = ctx.meta()?;
-    let (_guard, repo, mut ws, _, _cache) = ctx.workspace_mut_and_db_and_cache()?;
+    let (repo, mut ws, _, _cache) = ctx.workspace_mut_and_db_and_cache_with_perm(perm)?;
     let editor = Editor::create(&mut ws, &mut meta, &repo)?;
     let but_workspace::branch::move_branch::Outcome { rebase, ws_meta } =
         but_workspace::branch::tear_off_branch(editor, subject_branch, None)?;

crates/but-api/src/commit/amend.rs

@@ -8,7 +8,10 @@ use tracing::instrument;
 
 use super::types::CommitCreateResult;
 
-/// Amends an existing commit with selected changes.
+/// Amends the commit at `commit_id` with `changes`.
+///
+/// See [`but_workspace::commit::commit_amend()`] for lower-level implementation
+/// details.
 #[but_api(crate::commit::json::UICommitCreateResult)]
 #[instrument(err(Debug))]
 pub fn commit_amend_only(
@@ -27,7 +30,6 @@ pub fn commit_amend_only(
     )
 }
 
-/// Amends an existing commit with selected changes.
 pub(crate) fn commit_amend_only_impl(
     ctx: &mut but_ctx::Context,
     commit_id: gix::ObjectId,
@@ -62,7 +64,12 @@ pub(crate) fn commit_amend_only_impl(
     })
 }
 
-/// Amends an existing commit with selected changes, with oplog support.
+/// Amend the commit at `commit_id` with `changes` and record an oplog snapshot on success.
+///
+/// This performs the rewrite under exclusive worktree access and creates a
+/// best-effort `AmendCommit` oplog entry if the operation succeeds. For
+/// lower-level implementation details, see
+/// [`but_workspace::commit::commit_amend()`].
 #[but_api(napi, crate::commit::json::UICommitCreateResult)]
 #[instrument(err(Debug))]
 pub fn commit_amend(

crates/but-api/src/commit/create.rs

@@ -11,7 +11,12 @@ use tracing::instrument;
 
 use super::types::CommitCreateResult;
 
-/// Creates and inserts a commit relative to either a commit or a reference.
+/// Creates a commit from `changes` with `message`, inserted on `side` of
+/// `relative_to`.
+///
+/// This acquires exclusive worktree access from `ctx` before creating the
+/// commit. For lower-level implementation details, see
+/// [`but_workspace::commit::commit_create()`].
 #[but_api(crate::commit::json::UICommitCreateResult)]
 #[instrument(err(Debug))]
 pub fn commit_create_only(
@@ -78,7 +83,14 @@ pub(crate) fn commit_create_only_impl(
     })
 }
 
-/// Creates and inserts a commit relative to either a commit or a reference, with oplog support.
+/// Insert a new commit built from `changes` and record an oplog snapshot on
+/// success.
+///
+/// `relative_to` and `side` choose where the commit is inserted. `message` is
+/// the entire commit message text, not just the title. On success, this commits
+/// a best-effort `CreateCommit` oplog snapshot using the same lock. For
+/// lower-level implementation details, see
+/// [`but_workspace::commit::commit_create()`].
 #[but_api(napi, crate::commit::json::UICommitCreateResult)]
 #[instrument(skip_all, fields(relative_to, side, message), err(Debug))]
 pub fn commit_create(