diff --git a/Cargo.lock b/Cargo.lock index 71d52bab9c..5fe693fd8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -729,8 +729,11 @@ dependencies = [ "git2", "gitbutler-command-context", "gitbutler-commit", + "gitbutler-diff", "gitbutler-id", + "gitbutler-operating-modes", "gitbutler-oxidize", + "gitbutler-project", "gitbutler-repo", "gitbutler-serde", "gitbutler-stack", diff --git a/crates/but-workspace/Cargo.toml b/crates/but-workspace/Cargo.toml index a8c8576d17..f9aa84c4ea 100644 --- a/crates/but-workspace/Cargo.toml +++ b/crates/but-workspace/Cargo.toml @@ -19,13 +19,16 @@ gix = { workspace = true, features = [ "parallel", "serde", "status", - "revision" + "revision", ] } gitbutler-stack.workspace = true gitbutler-command-context.workspace = true gitbutler-oxidize.workspace = true gitbutler-commit.workspace = true gitbutler-repo.workspace = true +gitbutler-project.workspace = true +gitbutler-operating-modes.workspace = true +gitbutler-diff.workspace = true serde = { workspace = true, features = ["std"] } gitbutler-serde.workspace = true itertools = "0.14" diff --git a/crates/gitbutler-branch-actions/src/integration.rs b/crates/gitbutler-branch-actions/src/integration.rs index 37d3197090..6e2d546b37 100644 --- a/crates/gitbutler-branch-actions/src/integration.rs +++ b/crates/gitbutler-branch-actions/src/integration.rs @@ -1,26 +1,21 @@ use std::{path::PathBuf, vec}; use anyhow::{anyhow, Context, Result}; -use bstr::ByteSlice; -use gitbutler_branch::BranchCreateRequest; use gitbutler_branch::{self, GITBUTLER_WORKSPACE_REFERENCE}; use gitbutler_cherry_pick::RepositoryExt as _; use gitbutler_command_context::CommandContext; -use gitbutler_commit::commit_ext::CommitExt; use gitbutler_error::error::Marker; use gitbutler_operating_modes::OPEN_WORKSPACE_REFS; use gitbutler_oxidize::{git2_to_gix_object_id, gix_to_git2_oid, GixRepositoryExt}; use gitbutler_project::access::WorktreeWritePermission; -use gitbutler_repo::logging::{LogUntil, RepositoryExt as _}; -use gitbutler_repo::RepositoryExt; use gitbutler_repo::SignaturePurpose; use gitbutler_stack::{Stack, VirtualBranchesHandle}; use tracing::instrument; -use crate::{branch_manager::BranchManagerExt, conflicts, VirtualBranchesExt}; +use crate::{conflicts, workspace_commit::resolve_commits_above, VirtualBranchesExt}; const WORKSPACE_HEAD: &str = "Workspace Head"; -const GITBUTLER_INTEGRATION_COMMIT_TITLE: &str = "GitButler Integration Commit"; +pub const GITBUTLER_INTEGRATION_COMMIT_TITLE: &str = "GitButler Integration Commit"; pub const GITBUTLER_WORKSPACE_COMMIT_TITLE: &str = "GitButler Workspace Commit"; /// Creates and returns a merge commit of all active branch heads. @@ -292,7 +287,7 @@ pub fn update_workspace_commit( pub fn verify_branch(ctx: &CommandContext, perm: &mut WorktreeWritePermission) -> Result<()> { verify_current_branch_name(ctx) .and_then(verify_head_is_set) - .and_then(|()| verify_head_is_clean(ctx, perm)) + .and_then(|()| resolve_commits_above(ctx, perm)) .context(Marker::VerificationFailure)?; Ok(()) } @@ -322,100 +317,6 @@ fn verify_current_branch_name(ctx: &CommandContext) -> Result<&CommandContext> { } } -// TODO(ST): Probably there should not be an implicit vbranch creation here. -fn verify_head_is_clean(ctx: &CommandContext, perm: &mut WorktreeWritePermission) -> Result<()> { - let head_commit = ctx - .repo() - .head() - .context("failed to get head")? - .peel_to_commit() - .context("failed to peel to commit")?; - - let vb_handle = VirtualBranchesHandle::new(ctx.project().gb_dir()); - let default_target = vb_handle - .get_default_target() - .context("failed to get default target")?; - - let commits = ctx - .repo() - .log( - head_commit.id(), - LogUntil::Commit(default_target.sha), - false, - ) - .context("failed to get log")?; - - let workspace_index = commits - .iter() - .position(|commit| { - commit.message().is_some_and(|message| { - message.starts_with(GITBUTLER_WORKSPACE_COMMIT_TITLE) - || message.starts_with(GITBUTLER_INTEGRATION_COMMIT_TITLE) - }) - }) - .context("GitButler workspace commit not found")?; - let workspace_commit = &commits[workspace_index]; - let mut extra_commits = commits[..workspace_index].to_vec(); - extra_commits.reverse(); - - if extra_commits.is_empty() { - // no extra commits found, so we're good - return Ok(()); - } - - ctx.repo() - .reset(workspace_commit.as_object(), git2::ResetType::Soft, None) - .context("failed to reset to workspace commit")?; - - let branch_manager = ctx.branch_manager(); - let mut new_branch = branch_manager - .create_virtual_branch( - &BranchCreateRequest { - name: extra_commits - .last() - .map(|commit| commit.message_bstr().to_string()), - ..Default::default() - }, - perm, - ) - .context("failed to create virtual branch")?; - - // rebasing the extra commits onto the new branch - let mut head = new_branch.head(); - for commit in extra_commits { - let new_branch_head = ctx - .repo() - .find_commit(head) - .context("failed to find new branch head")?; - - let rebased_commit_oid = ctx - .repo() - .commit_with_signature( - None, - &commit.author(), - &commit.committer(), - &commit.message_bstr().to_str_lossy(), - &commit.tree().unwrap(), - &[&new_branch_head], - None, - ) - .context(format!( - "failed to rebase commit {} onto new branch", - commit.id() - ))?; - - let rebased_commit = ctx.repo().find_commit(rebased_commit_oid).context(format!( - "failed to find rebased commit {}", - rebased_commit_oid - ))?; - - new_branch.set_stack_head(ctx, rebased_commit.id(), Some(rebased_commit.tree_id()))?; - - head = rebased_commit.id(); - } - Ok(()) -} - fn invalid_head_err(head_name: &str) -> anyhow::Error { anyhow!( "project is on {head_name}. Please checkout {} to continue", diff --git a/crates/gitbutler-branch-actions/src/lib.rs b/crates/gitbutler-branch-actions/src/lib.rs index 2726408038..1f5e475f1f 100644 --- a/crates/gitbutler-branch-actions/src/lib.rs +++ b/crates/gitbutler-branch-actions/src/lib.rs @@ -39,6 +39,7 @@ pub mod upstream_integration; mod integration; pub use integration::{update_workspace_commit, verify_branch}; +pub mod workspace_commit; mod file; pub use file::{Get, RemoteBranchFile}; diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index 3c42543c91..612e505ed6 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -8,7 +8,8 @@ use crate::{ remote::branch_to_remote_branch, stack::stack_series, status::{get_applied_status, get_applied_status_cached}, - Get, RemoteBranchData, VirtualBranchHunkRange, VirtualBranchHunkRangeMap, VirtualBranchesExt, + verify_branch, Get, RemoteBranchData, VirtualBranchHunkRange, VirtualBranchHunkRangeMap, + VirtualBranchesExt, }; use anyhow::{anyhow, bail, Context, Result}; use bstr::{BString, ByteSlice}; @@ -326,6 +327,9 @@ pub fn list_virtual_branches_cached( ) -> Result { assure_open_workspace_mode(ctx) .context("Listing virtual branches requires open workspace mode")?; + // Make sure that the workspace commit is the head of the branch before listing. + verify_branch(ctx, perm)?; + let mut branches: Vec = Vec::new(); let vb_state = ctx.project().virtual_branches(); diff --git a/crates/gitbutler-branch-actions/src/workspace_commit.rs b/crates/gitbutler-branch-actions/src/workspace_commit.rs new file mode 100644 index 0000000000..90dc004b41 --- /dev/null +++ b/crates/gitbutler-branch-actions/src/workspace_commit.rs @@ -0,0 +1,209 @@ +use std::{cmp::Ordering, collections::HashMap}; + +use anyhow::{Context as _, Result}; +use but_workspace::StackId; +use gitbutler_branch::BranchCreateRequest; +use gitbutler_command_context::CommandContext; +use gitbutler_commit::commit_ext::CommitExt as _; +use gitbutler_diff::diff_files_into_hunks; +use gitbutler_operating_modes::{assure_open_workspace_mode, WORKSPACE_BRANCH_REF}; +use gitbutler_project::access::{WorktreeReadPermission, WorktreeWritePermission}; +use gitbutler_repo::{ + logging::{LogUntil, RepositoryExt as _}, + rebase::cherry_rebase_group, +}; +use gitbutler_stack::VirtualBranchesHandle; + +use crate::{ + compute_workspace_dependencies, integration::GITBUTLER_INTEGRATION_COMMIT_TITLE, + update_workspace_commit, BranchManagerExt as _, GITBUTLER_WORKSPACE_COMMIT_TITLE, +}; + +#[derive(Debug)] +pub enum WorkspaceCommitStatus { + /// gitbutler/workspace has a workspace commit, but it has extra commits + /// above it. + WorkspaceCommitBehind { + workspace_commit: git2::Oid, + extra_commits: Vec, + }, + /// gitbutler/workspace has a workspace commit, and the workspace commit is + /// the head of gitbutler/workspace + OnWorkspaceCommit { workspace_commit: git2::Oid }, + /// gitbutler/workspace does not have workspace_commit + NoWorkspaceCommit, +} + +/// Returns the current state of the workspace commit, whether it's non-existant +/// the head of gitbutler/workspace, or has commits above +pub fn workspace_commit_status( + ctx: &CommandContext, + _perm: &WorktreeReadPermission, +) -> Result { + assure_open_workspace_mode(ctx)?; + let repository = ctx.repo(); + let vb_handle = VirtualBranchesHandle::new(ctx.project().gb_dir()); + let default_target = vb_handle.get_default_target()?; + + let head_commit = repository + .find_reference(WORKSPACE_BRANCH_REF)? + .peel_to_commit()?; + let commits = repository.log( + head_commit.id(), + LogUntil::Commit(default_target.sha), + false, + )?; + + let Some(workspace_index) = commits.iter().position(|commit| { + commit.message().is_some_and(|message| { + message.starts_with(GITBUTLER_WORKSPACE_COMMIT_TITLE) + || message.starts_with(GITBUTLER_INTEGRATION_COMMIT_TITLE) + }) + }) else { + return Ok(WorkspaceCommitStatus::NoWorkspaceCommit); + }; + let workspace_commit = &commits[workspace_index]; + let extra_commits = commits[..workspace_index].to_vec(); + + if extra_commits.is_empty() { + // no extra commits found, so we're good + return Ok(WorkspaceCommitStatus::OnWorkspaceCommit { + workspace_commit: workspace_commit.id(), + }); + } + + Ok(WorkspaceCommitStatus::WorkspaceCommitBehind { + workspace_commit: workspace_commit.id(), + extra_commits: extra_commits + .iter() + .map(git2::Commit::id) + .collect::>(), + }) +} + +/// Resolves the situation if there are commits above the workspace merge commit. +/// +/// This function should only be run in open workspace mode. +/// +/// This function tries to move the commits into a branch into the workspace if +/// possible, or will remove the commits, leaving the changes in the working +/// directory. +/// +/// If there are no branches in the workspace this function will create a new +/// banch for them, rather than simply dropping them. +pub fn resolve_commits_above( + ctx: &CommandContext, + perm: &mut WorktreeWritePermission, +) -> Result<()> { + assure_open_workspace_mode(ctx)?; + let repository = ctx.repo(); + let head_commit = repository.head()?.peel_to_commit()?; + + let WorkspaceCommitStatus::WorkspaceCommitBehind { + workspace_commit, + extra_commits, + } = workspace_commit_status(ctx, perm.read_permission())? + else { + return Ok(()); + }; + + let best_stack_id = + find_or_create_branch_for_commit(ctx, perm, head_commit.id(), workspace_commit)?; + + if let Some(best_stack_id) = best_stack_id { + let vb_handle = VirtualBranchesHandle::new(ctx.project().gb_dir()); + let mut stack = vb_handle.get_stack_in_workspace(best_stack_id)?; + + let new_head = cherry_rebase_group(repository, stack.head(), &extra_commits, false)?; + + stack.set_stack_head( + ctx, + new_head, + Some(repository.find_commit(new_head)?.tree_id()), + )?; + + update_workspace_commit(&vb_handle, ctx)?; + } else { + // There is no stack which can hold the commits so we should just unroll those changes + repository.reference(WORKSPACE_BRANCH_REF, workspace_commit, true, "")?; + repository.set_head(WORKSPACE_BRANCH_REF)?; + } + + Ok(()) +} + +/// Tries to find a branch or create a branch that the commits can be moved into. +/// +/// Uses the following logic: +/// if there are no branches applied: +/// create a new branch +/// otherwise: +/// if the changes lock to 2 or more branches +/// there is no branch that can accept these commits +/// if the chances lock to 1 branch +/// return that branch +/// otherwise: +/// return the branch currently selected for changes +fn find_or_create_branch_for_commit( + ctx: &CommandContext, + perm: &mut WorktreeWritePermission, + head_commit: git2::Oid, + workspace_commit: git2::Oid, +) -> Result> { + let vb_state = VirtualBranchesHandle::new(ctx.project().gb_dir()); + let default_target = vb_state.get_default_target()?; + let repository = ctx.repo(); + let stacks = vb_state.list_stacks_in_workspace()?; + + let head_commit = repository.find_commit(head_commit)?; + + let diffs = gitbutler_diff::trees( + ctx.repo(), + &repository.find_commit(workspace_commit)?.tree()?, + &head_commit.tree()?, + true, + )?; + let base_diffs: HashMap<_, _> = diff_files_into_hunks(&diffs).collect(); + let workspace_dependencies = + compute_workspace_dependencies(ctx, &default_target.sha, &base_diffs, &stacks)?; + + match workspace_dependencies.commit_dependent_diffs.len().cmp(&1) { + Ordering::Greater => { + // The commits are locked to multiple stacks. We can't correctly assign it + // to any one stack, so the commits should be undone. + Ok(None) + } + Ordering::Equal => { + // There is one stack which the commits are locked to, so the commits + // should be added to that particular stack. + let stack_id = workspace_dependencies + .commit_dependent_diffs + .keys() + .next() + .expect("Values was asserted length 1 above"); + Ok(Some(*stack_id)) + } + Ordering::Less => { + // We should return the branch selected for changes, or create a new default branch. + let mut stacks = vb_state.list_stacks_in_workspace()?; + stacks.sort_by_key(|stack| stack.selected_for_changes.unwrap_or(0)); + + if let Some(stack) = stacks.last() { + return Ok(Some(stack.id)); + } + + let branch_manager = ctx.branch_manager(); + let new_stack = branch_manager + .create_virtual_branch( + &BranchCreateRequest { + name: Some(head_commit.message_bstr().to_string()), + ..Default::default() + }, + perm, + ) + .context("failed to create virtual branch")?; + + Ok(Some(new_stack.id)) + } + } +} diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/create_virtual_branch_from_branch.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/create_virtual_branch_from_branch.rs index 885079b9e4..763ff5e99e 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/create_virtual_branch_from_branch.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/create_virtual_branch_from_branch.rs @@ -382,6 +382,8 @@ mod conflict_cases { let branch_refname = gitbutler_branch_actions::save_and_unapply_virutal_branch(ctx, branch.id).unwrap(); + gitbutler_branch_actions::list_virtual_branches(ctx).unwrap(); + // Make X and set base branch to X let mut tree_builder = git_repository .treebuilder(Some( diff --git a/crates/gitbutler-stack/src/stack.rs b/crates/gitbutler-stack/src/stack.rs index 5ff6696f0f..92f5eda495 100644 --- a/crates/gitbutler-stack/src/stack.rs +++ b/crates/gitbutler-stack/src/stack.rs @@ -511,6 +511,9 @@ impl Stack { self.updated_timestamp_ms = gitbutler_time::time::now_ms(); #[allow(deprecated)] // this is the only place where this is allowed self.set_head(commit_id); + // TODO(CTO): Determine whether this does anything. If we're not + // calling `checkout_branch_trees` right after this, then it will + // likly get overridden by the next `list_virtual_branches`. if let Some(tree) = tree { self.tree = tree; } diff --git a/crates/gitbutler-watcher/src/handler.rs b/crates/gitbutler-watcher/src/handler.rs index 7b9a09f46b..69e671c27e 100644 --- a/crates/gitbutler-watcher/src/handler.rs +++ b/crates/gitbutler-watcher/src/handler.rs @@ -1,7 +1,12 @@ use std::{path::PathBuf, sync::Arc}; use anyhow::{Context, Result}; -use gitbutler_branch_actions::{internal::StackListResult, VirtualBranches}; +use gitbutler_branch_actions::{ + internal::StackListResult, + verify_branch, + workspace_commit::{workspace_commit_status, WorkspaceCommitStatus}, + VirtualBranches, +}; use gitbutler_command_context::CommandContext; use gitbutler_diff::DiffByPathMap; use gitbutler_error::error::Marker; @@ -196,6 +201,32 @@ impl Handler { Ok(()) } + fn handle_commited_workspace(&self, ctx: &CommandContext) -> Result<()> { + // Don't worry about non-workspace settings (yet) + if !in_open_workspace_mode(ctx) { + return Ok(()); + } + + let workspace_state = { + let guard = ctx.project().exclusive_worktree_access(); + let permission = guard.read_permission(); + workspace_commit_status(ctx, permission)? + }; + if matches!( + workspace_state, + WorkspaceCommitStatus::WorkspaceCommitBehind { .. } + ) { + { + let mut guard = ctx.project().exclusive_worktree_access(); + let permission = guard.write_permission(); + verify_branch(ctx, permission)?; + } + self.calculate_virtual_branches(ctx, None)?; + } + + Ok(()) + } + pub fn git_files_change(&self, paths: Vec, ctx: &CommandContext) -> Result<()> { for path in paths { let Some(file_name) = path.to_str() else { @@ -206,6 +237,7 @@ impl Handler { self.emit_app_event(Change::GitFetch(ctx.project().id))?; } "logs/HEAD" => { + self.handle_commited_workspace(ctx)?; self.emit_app_event(Change::GitActivity(ctx.project().id))?; } "index" => {