From a7b01bd64115d36bb12db5178ae792864ce66914 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 22:46:03 +0200 Subject: [PATCH] Keep using `get_workspace_head()` when no cached result is used. That way, the application will not run into unexpected cases. --- crates/gitbutler-branch-actions/src/integration.rs | 12 ++++++++---- crates/gitbutler-branch-actions/src/status.rs | 11 +++++------ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/crates/gitbutler-branch-actions/src/integration.rs b/crates/gitbutler-branch-actions/src/integration.rs index 66a8e877f..d1d8f8f39 100644 --- a/crates/gitbutler-branch-actions/src/integration.rs +++ b/crates/gitbutler-branch-actions/src/integration.rs @@ -19,10 +19,14 @@ use crate::{branch_manager::BranchManagerExt, conflicts, VirtualBranchesExt}; const WORKSPACE_HEAD: &str = "Workspace Head"; const GITBUTLER_INTEGRATION_COMMIT_TITLE: &str = "GitButler Integration Commit"; -// Creates and returns a merge commit of all active branch heads. -// -// This is the base against which we diff the working directory to understand -// what files have been modified. +/// Creates and returns a merge commit of all active branch heads. +/// +/// This is the base against which we diff the working directory to understand +/// what files have been modified. +/// +/// This should be used to update the `gitbutler/workspace` ref with, which is usually +/// done from [`update_gitbutler_integration()`], after any of its input changes. +/// This is namely the conflicting state, or any head of the virtual branches. #[instrument(level = tracing::Level::DEBUG, skip(ctx))] pub(crate) fn get_workspace_head(ctx: &CommandContext) -> Result { let vb_state = ctx.project().virtual_branches(); diff --git a/crates/gitbutler-branch-actions/src/status.rs b/crates/gitbutler-branch-actions/src/status.rs index 65126359e..fc93857a9 100644 --- a/crates/gitbutler-branch-actions/src/status.rs +++ b/crates/gitbutler-branch-actions/src/status.rs @@ -1,5 +1,6 @@ use std::{collections::HashMap, path::PathBuf, vec}; +use crate::integration::get_workspace_head; use crate::{ conflicts::RepoConflictsExt, file::{virtual_hunks_into_virtual_files, VirtualBranchFile}, @@ -16,7 +17,6 @@ use gitbutler_command_context::CommandContext; use gitbutler_diff::{diff_files_into_hunks, GitHunk, Hunk, HunkHash}; use gitbutler_operating_modes::assure_open_workspace_mode; use gitbutler_project::access::WorktreeWritePermission; -use gitbutler_repo::RepositoryExt; use tracing::instrument; /// Represents the uncommitted status of the applied virtual branches in the workspace. @@ -52,11 +52,10 @@ pub fn get_applied_status_cached( .virtual_branches() .list_branches_in_workspace()?; let base_file_diffs = worktree_changes.map(Ok).unwrap_or_else(|| { - // TODO(ST): this was `get_workspace_head()`, which is slow and ideally, we don't dynamically - // calculate which should already be 'fixed' - why do we have the integration branch - // if we can't assume it's in the right state? So ideally, we assure that the code - // that affects the integration branch also updates it? - let integration_commit_id = ctx.repository().head_commit()?.id(); + // TODO(ST): Ideally, we can avoid calling `get_workspace_head()` as everyone who modifies + // any of its inputs will update the intragration commit right away. + // It's for another day though - right now the integration commit may be slightly stale. + let integration_commit_id = get_workspace_head(ctx)?; gitbutler_diff::workdir(ctx.repository(), &integration_commit_id.to_owned()) .context("failed to diff workdir") })?;