From dd724f310df161c08294fbdd6d7c76da6e620a15 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 27 Aug 2024 14:57:34 +0200 Subject: [PATCH 01/11] Also debug-trace information on `list_virtual_branches` calls. This can be useful for those who seek extra information, and while it's dominating refresh times. --- .../src/integration.rs | 2 ++ crates/gitbutler-branch-actions/src/status.rs | 2 ++ .../gitbutler-branch-actions/src/virtual.rs | 25 ++++++++++--------- crates/gitbutler-diff/src/diff.rs | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/crates/gitbutler-branch-actions/src/integration.rs b/crates/gitbutler-branch-actions/src/integration.rs index f776a2777e..66a8e877f6 100644 --- a/crates/gitbutler-branch-actions/src/integration.rs +++ b/crates/gitbutler-branch-actions/src/integration.rs @@ -12,6 +12,7 @@ use gitbutler_commit::commit_ext::CommitExt; use gitbutler_error::error::Marker; use gitbutler_project::access::WorktreeWritePermission; use gitbutler_repo::{LogUntil, RepoActionsExt, RepositoryExt}; +use tracing::instrument; use crate::{branch_manager::BranchManagerExt, conflicts, VirtualBranchesExt}; @@ -22,6 +23,7 @@ const GITBUTLER_INTEGRATION_COMMIT_TITLE: &str = "GitButler Integration Commit"; // // This is the base against which we diff the working directory to understand // what files have been modified. +#[instrument(level = tracing::Level::DEBUG, skip(ctx))] pub(crate) fn get_workspace_head(ctx: &CommandContext) -> Result { let vb_state = ctx.project().virtual_branches(); let target = vb_state diff --git a/crates/gitbutler-branch-actions/src/status.rs b/crates/gitbutler-branch-actions/src/status.rs index cffcd18747..910480e461 100644 --- a/crates/gitbutler-branch-actions/src/status.rs +++ b/crates/gitbutler-branch-actions/src/status.rs @@ -10,6 +10,7 @@ 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 tracing::instrument; use crate::{ conflicts::RepoConflictsExt, @@ -30,6 +31,7 @@ pub struct VirtualBranchesStatus { /// Returns branches and their associated file changes, in addition to a list /// of skipped files. // TODO(kv): make this side effect free +#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm))] pub fn get_applied_status( ctx: &CommandContext, perm: Option<&mut WorktreeWritePermission>, diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index 3b2afea7d4..ddcf24440c 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -5,6 +5,17 @@ use std::{ vec, }; +use crate::{ + branch_manager::BranchManagerExt, + commit::{commit_to_vbranch_commit, VirtualBranchCommit}, + conflicts::{self, RepoConflictsExt}, + file::VirtualBranchFile, + hunk::VirtualBranchHunk, + integration::get_workspace_head, + remote::{branch_to_remote_branch, RemoteBranch}, + status::get_applied_status, + Get, VirtualBranchesExt, +}; use anyhow::{anyhow, bail, Context, Result}; use bstr::ByteSlice; use git2_hooks::HookResult; @@ -27,18 +38,7 @@ use gitbutler_repo::{ }; use gitbutler_time::time::now_since_unix_epoch_ms; use serde::Serialize; - -use crate::{ - branch_manager::BranchManagerExt, - commit::{commit_to_vbranch_commit, VirtualBranchCommit}, - conflicts::{self, RepoConflictsExt}, - file::VirtualBranchFile, - hunk::VirtualBranchHunk, - integration::get_workspace_head, - remote::{branch_to_remote_branch, RemoteBranch}, - status::get_applied_status, - Get, VirtualBranchesExt, -}; +use tracing::instrument; // this struct is a mapping to the view `Branch` type in Typescript // found in src-tauri/src/routes/repo/[project_id]/types.ts @@ -255,6 +255,7 @@ fn resolve_old_applied_state( Ok(()) } +#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm))] pub fn list_virtual_branches( ctx: &CommandContext, // TODO(ST): this should really only shared access, but there is some internals diff --git a/crates/gitbutler-diff/src/diff.rs b/crates/gitbutler-diff/src/diff.rs index 058db17a1b..293f81af34 100644 --- a/crates/gitbutler-diff/src/diff.rs +++ b/crates/gitbutler-diff/src/diff.rs @@ -126,7 +126,7 @@ pub struct FileDiff { pub new_size_bytes: u64, } -#[instrument(skip(repo))] +#[instrument(level = tracing::Level::DEBUG, skip(repo))] pub fn workdir(repo: &git2::Repository, commit_oid: &git2::Oid) -> Result { let commit = repo .find_commit(*commit_oid) From 30cc6a48385d4552ad05ad26053915563186e08f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 27 Aug 2024 15:19:39 +0200 Subject: [PATCH 02/11] Add `branch status` to CLI It calls `list_virtual_branches()` which kind of has the effect of a status call. --- crates/gitbutler-cli/src/args.rs | 2 ++ crates/gitbutler-cli/src/command/vbranch.rs | 4 ++++ crates/gitbutler-cli/src/main.rs | 1 + 3 files changed, 7 insertions(+) diff --git a/crates/gitbutler-cli/src/args.rs b/crates/gitbutler-cli/src/args.rs index 4c3d2a6834..9b94fc0c5f 100644 --- a/crates/gitbutler-cli/src/args.rs +++ b/crates/gitbutler-cli/src/args.rs @@ -38,6 +38,8 @@ pub mod vbranch { #[derive(Debug, clap::Subcommand)] pub enum SubCommands { + /// Provide the current state of all applied virtual branches. + Status, /// Make the named branch the default so all worktree or index changes are associated with it automatically. SetDefault { /// The name of the new default virtual branch. diff --git a/crates/gitbutler-cli/src/command/vbranch.rs b/crates/gitbutler-cli/src/command/vbranch.rs index 685ab7886c..4dd7e75d6d 100644 --- a/crates/gitbutler-cli/src/command/vbranch.rs +++ b/crates/gitbutler-cli/src/command/vbranch.rs @@ -40,6 +40,10 @@ pub fn list(project: Project) -> Result<()> { Ok(()) } +pub fn status(project: Project) -> Result<()> { + debug_print(VirtualBranchActions.list_virtual_branches(&project)?) +} + pub fn unapply(project: Project, branch_name: String) -> Result<()> { let branch = branch_by_name(&project, &branch_name)?; debug_print(VirtualBranchActions.convert_to_real_branch(&project, branch.id)?) diff --git a/crates/gitbutler-cli/src/main.rs b/crates/gitbutler-cli/src/main.rs index a7585db56d..865cf2778b 100644 --- a/crates/gitbutler-cli/src/main.rs +++ b/crates/gitbutler-cli/src/main.rs @@ -20,6 +20,7 @@ fn main() -> Result<()> { args::Subcommands::Branch(vbranch::Platform { cmd }) => { let project = command::prepare::project_from_path(args.current_dir)?; match cmd { + Some(vbranch::SubCommands::Status) => command::vbranch::status(project), Some(vbranch::SubCommands::Unapply { name }) => { command::vbranch::unapply(project, name) } From bd2c19db0b7cccb5652a152e0a2da57aebc9fb66 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 08:36:53 +0200 Subject: [PATCH 03/11] Allow the `bench` profile to compile faster There we don't necessarily need every last bit of optimization like one would want in release profiles, but build performance is of the essence for developer productivity. --- Cargo.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 12e39fd637..6e52d9dc7f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,3 +90,8 @@ codegen-units = 1 # Compile crates one after another so the compiler can optimiz lto = true # Enables link to optimizations opt-level = "s" # Optimize for binary size debug = true # Enable debug symbols, for profiling + +[profile.bench] +codegen-units = 256 +lto = false +opt-level = 3 \ No newline at end of file From 7e0878a14f80f02642cd9310255b2b4b376c41f8 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 07:41:53 +0200 Subject: [PATCH 04/11] Use read-only worktree diff The worktree diff is essentially a 'git status' with a given tree, and that can be expressed with a diff of the workdir, the index and a tree. This comes at the expense of not being able to control which file sizes are diffed. --- .../tests/extra/mod.rs | 10 ++- .../convert_to_real_branch.rs | 14 ++-- .../tests/virtual_branches/mod.rs | 5 +- crates/gitbutler-diff/src/diff.rs | 67 +++++++------------ crates/gitbutler-testsupport/src/suite.rs | 2 +- .../gitbutler-testsupport/src/test_project.rs | 11 +-- 6 files changed, 50 insertions(+), 59 deletions(-) diff --git a/crates/gitbutler-branch-actions/tests/extra/mod.rs b/crates/gitbutler-branch-actions/tests/extra/mod.rs index 04b8ce13d8..ed2350480a 100644 --- a/crates/gitbutler-branch-actions/tests/extra/mod.rs +++ b/crates/gitbutler-branch-actions/tests/extra/mod.rs @@ -759,7 +759,7 @@ fn commit_id_can_be_generated_or_specified() -> Result<()> { #[test] fn merge_vbranch_upstream_clean_rebase() -> Result<()> { let suite = Suite::default(); - let Case { ctx, project, .. } = &suite.new_case(); + let Case { ctx, project, .. } = &mut suite.new_case(); // create a commit and set the target let file_path = Path::new("test.txt"); @@ -829,8 +829,14 @@ fn merge_vbranch_upstream_clean_rebase() -> Result<()> { // create the branch let (branches, _) = list_virtual_branches(ctx, guard.write_permission())?; + assert_eq!(branches.len(), 1); let branch1 = &branches[0]; - assert_eq!(branch1.files.len(), 1); + assert_eq!( + branch1.files.len(), + 1 + 1, + "'test' (modified compared to index) and 'test2' (untracked).\ + This is actually correct when looking at the git repository" + ); assert_eq!(branch1.commits.len(), 1); // assert_eq!(branch1.upstream.as_ref().unwrap().commits.len(), 1); diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs index a552ba764d..5ee9c23937 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs @@ -61,15 +61,20 @@ fn conflicting() { let (branches, _) = controller.list_virtual_branches(project).unwrap(); assert_eq!(branches.len(), 1); - assert!(branches[0].base_current); - assert!(branches[0].active); + let branch = &branches[0]; + assert_eq!( + branch.name, "Virtual branch", + "the auto-created branch gets the default name" + ); + assert!(branch.base_current); + assert!(branch.active); assert_eq!( - branches[0].files[0].hunks[0].diff, + branch.files[0].hunks[0].diff, "@@ -1 +1 @@\n-first\n\\ No newline at end of file\n+conflict\n\\ No newline at end of file\n" ); let unapplied_branch = controller - .convert_to_real_branch(project, branches[0].id) + .convert_to_real_branch(project, branch.id) .unwrap(); Refname::from_str(&unapplied_branch).unwrap() @@ -100,7 +105,6 @@ fn conflicting() { assert_eq!(branches.len(), 1); let branch = &branches[0]; - // assert!(!branch.base_current); assert!(branch.conflicted); assert_eq!( branch.files[0].hunks[0].diff, diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs index b1b36b6b64..65b4797b66 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs @@ -50,9 +50,8 @@ impl Test { /// Consume this instance and keep the temp directory that held the local repository, returning it. /// Best used inside a `dbg!(test.debug_local_repo())` #[allow(dead_code)] - pub fn debug_local_repo(mut self) -> PathBuf { - let repo = std::mem::take(&mut self.repository); - repo.debug_local_repo() + pub fn debug_local_repo(&mut self) -> Option { + self.repository.debug_local_repo() } } diff --git a/crates/gitbutler-diff/src/diff.rs b/crates/gitbutler-diff/src/diff.rs index 293f81af34..393f27c500 100644 --- a/crates/gitbutler-diff/src/diff.rs +++ b/crates/gitbutler-diff/src/diff.rs @@ -1,9 +1,4 @@ -use std::{ - borrow::Cow, - collections::HashMap, - path::{Path, PathBuf}, - str, -}; +use std::{borrow::Cow, collections::HashMap, path::PathBuf, str}; use anyhow::{Context, Result}; use bstr::{BStr, BString, ByteSlice, ByteVec}; @@ -133,34 +128,6 @@ pub fn workdir(repo: &git2::Repository, commit_oid: &git2::Oid) -> Result i32 { - let file_size = std::fs::metadata(path).map(|m| m.len()).unwrap_or(0); - if file_size > 50_000_000 { - skipped_files.insert( - path.to_path_buf(), - FileDiff { - old_path: None, - new_path: None, - hunks: Vec::new(), - skipped: true, - binary: true, - old_size_bytes: 0, - new_size_bytes: 0, - }, - ); - 1 //skips the entry - } else { - 0 - } - }; - workdir_index.add_all(["."], git2::IndexAddOption::DEFAULT, Some(cb))?; - let workdir_tree_id = workdir_index.write_tree()?; - - let new_tree = repo.find_tree(workdir_tree_id)?; - let mut diff_opts = git2::DiffOptions::new(); diff_opts .recurse_untracked_dirs(true) @@ -170,14 +137,27 @@ pub fn workdir(repo: &git2::Repository, commit_oid: &git2::Oid) -> Result = index + .conflicts()? + .filter_map(Result::ok) + .filter_map(|c| { + c.our + .or(c.their) + .or(c.ancestor) + .and_then(|c| c.path.into_string().ok()) + }) + .collect(); + for conflict_path_to_resolve in paths_to_add { + index.add_path(conflict_path_to_resolve.as_ref())?; + } + let diff = repo.diff_tree_to_workdir_with_index(Some(&old_tree), Some(&mut diff_opts))?; + // TODO(ST): bring back support for skipped (large) files. + hunks_by_filepath(Some(repo), &diff) } pub fn trees( @@ -194,9 +174,10 @@ pub fn trees( .context_lines(3) .show_untracked_content(true); + // This is not a content-based diff, but also considers modification times apparently, + // maybe related to racy-git. This is why empty diffs have ot be filtered. let diff = repository.diff_tree_to_tree(Some(old_tree), Some(new_tree), Some(&mut diff_opts))?; - hunks_by_filepath(None, &diff) } diff --git a/crates/gitbutler-testsupport/src/suite.rs b/crates/gitbutler-testsupport/src/suite.rs index 2384c50507..4a434ccca0 100644 --- a/crates/gitbutler-testsupport/src/suite.rs +++ b/crates/gitbutler-testsupport/src/suite.rs @@ -92,7 +92,7 @@ pub struct Case { pub ctx: CommandContext, pub credentials: Helper, /// The directory containing the `ctx` - project_tmp: Option, + pub project_tmp: Option, } impl Drop for Case { diff --git a/crates/gitbutler-testsupport/src/test_project.rs b/crates/gitbutler-testsupport/src/test_project.rs index f4cfbf8ce4..b26b8311ab 100644 --- a/crates/gitbutler-testsupport/src/test_project.rs +++ b/crates/gitbutler-testsupport/src/test_project.rs @@ -19,8 +19,8 @@ pub struct TestProject { impl Drop for TestProject { fn drop(&mut self) { if std::env::var_os(VAR_NO_CLEANUP).is_some() { - let _ = self.local_tmp.take().unwrap().into_path(); - let _ = self.remote_tmp.take().unwrap().into_path(); + let _ = self.local_tmp.take().map(|tmp| tmp.into_path()); + let _ = self.remote_tmp.take().map(|tmp| tmp.into_path()); } } } @@ -83,10 +83,11 @@ impl Default for TestProject { } impl TestProject { - /// Consume this instance and keep the temp directory that held the local repository, returning it. + /// Take the tmp directory holding the local repository and make sure it won't be deleted, + /// returning a path to it. /// Best used inside a `dbg!(test_project.debug_local_repo())` - pub fn debug_local_repo(mut self) -> PathBuf { - self.local_tmp.take().unwrap().into_path() + pub fn debug_local_repo(&mut self) -> Option { + self.local_tmp.take().map(|tmp| tmp.into_path()) } pub fn path(&self) -> &std::path::Path { self.local_repository.workdir().unwrap() From fc63929da35737436cdb1342a0855780936109c6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 11:19:17 +0200 Subject: [PATCH 05/11] Add a flag to allow performance logging: GITBUTLER_PERFORMANCE_LOG ```` GITBUTLER_PERFORMANCE_LOG=1 LOG_LEVEL=debug pnpm tauri dev ```` Additionally, some noise was removed by turning a warning into a trace. --- Cargo.lock | 1 + DEVELOPMENT.md | 6 ++++ crates/gitbutler-cli/src/main.rs | 11 +++++-- crates/gitbutler-command-context/src/lib.rs | 2 +- crates/gitbutler-tauri/Cargo.toml | 1 + crates/gitbutler-tauri/src/logs.rs | 36 ++++++++++++++------- crates/gitbutler-tauri/src/main.rs | 3 +- 7 files changed, 43 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 852078c590..a102bec305 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2565,6 +2565,7 @@ dependencies = [ "tokio", "tracing", "tracing-appender", + "tracing-forest", "tracing-subscriber", ] diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 5ff71668bb..11cba16e78 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -132,6 +132,12 @@ The app writes logs into: 1. `stdout` in development mode 2. The Tauri [logs](https://tauri.app/v1/api/js/path/#platform-specific) directory +One can get performance log when launching the application locally as follows: + +```bash +GITBUTLER_PERFORMANCE_LOG=1 LOG_LEVEL=debug pnpm tauri dev +``` + ### Tokio We are also collecting tokio's runtime tracing information that could be viewed using [tokio-console](https://github.com/tokio-rs/console#tokio-console-prototypes): diff --git a/crates/gitbutler-cli/src/main.rs b/crates/gitbutler-cli/src/main.rs index 865cf2778b..a2c1413229 100644 --- a/crates/gitbutler-cli/src/main.rs +++ b/crates/gitbutler-cli/src/main.rs @@ -77,14 +77,19 @@ fn main() -> Result<()> { } mod trace { + use tracing::metadata::LevelFilter; use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; + use tracing_subscriber::Layer; pub fn init() -> anyhow::Result<()> { tracing_subscriber::registry() - .with(tracing_forest::ForestLayer::from( - tracing_forest::printer::PrettyPrinter::new().writer(std::io::stderr), - )) + .with( + tracing_forest::ForestLayer::from( + tracing_forest::printer::PrettyPrinter::new().writer(std::io::stderr), + ) + .with_filter(LevelFilter::DEBUG), + ) .init(); Ok(()) } diff --git a/crates/gitbutler-command-context/src/lib.rs b/crates/gitbutler-command-context/src/lib.rs index 76d7ad812c..20ad66e748 100644 --- a/crates/gitbutler-command-context/src/lib.rs +++ b/crates/gitbutler-command-context/src/lib.rs @@ -28,7 +28,7 @@ impl CommandContext { Ok(false) => true, Ok(true) => false, Err(err) => { - tracing::warn!( + tracing::trace!( "failed to get gitbutler.didSetPrune for repository at {}; cannot disable gc: {}", project.path.display(), err diff --git a/crates/gitbutler-tauri/Cargo.toml b/crates/gitbutler-tauri/Cargo.toml index 45e1e2bb4c..39ea66b148 100644 --- a/crates/gitbutler-tauri/Cargo.toml +++ b/crates/gitbutler-tauri/Cargo.toml @@ -47,6 +47,7 @@ tokio = { workspace = true, features = ["rt-multi-thread", "parking_lot"] } tracing.workspace = true tracing-appender = "0.2.3" tracing-subscriber.workspace = true +tracing-forest = { version = "0.1.6" } gitbutler-watcher.workspace = true gitbutler-branch-actions.workspace = true gitbutler-oplog.workspace = true diff --git a/crates/gitbutler-tauri/src/logs.rs b/crates/gitbutler-tauri/src/logs.rs index 9449644eff..e0d973e9e3 100644 --- a/crates/gitbutler-tauri/src/logs.rs +++ b/crates/gitbutler-tauri/src/logs.rs @@ -5,7 +5,7 @@ use tracing::{instrument, metadata::LevelFilter, subscriber::set_global_default} use tracing_appender::rolling::{RollingFileAppender, Rotation}; use tracing_subscriber::{fmt::format::FmtSpan, layer::SubscriberExt, Layer}; -pub fn init(app_handle: &AppHandle) { +pub fn init(app_handle: &AppHandle, performance_logging: bool) { let logs_dir = app_handle .path_resolver() .app_log_dir() @@ -53,25 +53,37 @@ pub fn init(app_handle: &AppHandle) { .recording_path(logs_dir.join("tokio-console")) .spawn(), ) - .with( - // subscriber that writes spans to stdout - tracing_subscriber::fmt::layer() - .event_format(format_for_humans.clone()) - .with_ansi(use_colors_in_logs) - .with_span_events(FmtSpan::CLOSE) - .with_filter(log_level_filter), - ) .with( // subscriber that writes spans to a file tracing_subscriber::fmt::layer() - .event_format(format_for_humans) + .event_format(format_for_humans.clone()) .with_ansi(false) .with_span_events(FmtSpan::NEW | FmtSpan::CLOSE) .with_writer(file_writer) .with_filter(log_level_filter), ); - - set_global_default(subscriber).expect("failed to set subscriber"); + if performance_logging { + set_global_default( + subscriber.with( + tracing_forest::ForestLayer::from( + tracing_forest::printer::PrettyPrinter::new().writer(std::io::stdout), + ) + .with_filter(log_level_filter), + ), + ) + } else { + set_global_default( + subscriber.with( + // subscriber that writes spans to stdout + tracing_subscriber::fmt::layer() + .event_format(format_for_humans) + .with_ansi(use_colors_in_logs) + .with_span_events(FmtSpan::CLOSE) + .with_filter(log_level_filter), + ), + ) + } + .expect("failed to set subscriber"); } fn get_server_addr(app_handle: &AppHandle) -> (Ipv4Addr, u16) { diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index 9953e865cd..ea8a323e58 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -20,6 +20,7 @@ use tauri::{generate_context, Manager}; use tauri_plugin_log::LogTarget; fn main() { + let performance_logging = std::env::var_os("GITBUTLER_PERFORMANCE_LOG").is_some(); gitbutler_project::configure_git2(); let tauri_context = generate_context!(); gitbutler_secret::secret::set_application_namespace( @@ -60,7 +61,7 @@ fn main() { let app_handle = tauri_app.handle(); - logs::init(&app_handle); + logs::init(&app_handle, performance_logging); // On MacOS, in dev mode with debug assertions, we encounter popups each time // the binary is rebuilt. To counter that, use a git-credential based implementation. From 7ce143954f37b4fcd24387ee2d7bdbc43f4b7ea7 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 15:32:28 +0200 Subject: [PATCH 06/11] Do not dynamically calculate the head commit when listing virtual branches. That way, the head is static which allows to re-use the worktree status across multiple related calls. Also, add a way to quickly get the `head_commit()` from a `git2::Repository`. Note that this may cause breakage in the app, as now it will see the actual head, not a computed one, but that should also help us to find places that didn't properly update their state. --- crates/gitbutler-branch-actions/src/branch.rs | 8 ++---- crates/gitbutler-branch-actions/src/status.rs | 26 +++++++++++-------- .../tests/extra/mod.rs | 1 + .../convert_to_real_branch.rs | 6 ++--- .../tests/virtual_branches/mod.rs | 8 ++++-- .../virtual_branches/update_base_branch.rs | 3 +++ crates/gitbutler-oplog/src/oplog.rs | 3 +++ crates/gitbutler-repo/src/repository_ext.rs | 8 ++++++ 8 files changed, 41 insertions(+), 22 deletions(-) diff --git a/crates/gitbutler-branch-actions/src/branch.rs b/crates/gitbutler-branch-actions/src/branch.rs index e135a25db4..1be6eb22cc 100644 --- a/crates/gitbutler-branch-actions/src/branch.rs +++ b/crates/gitbutler-branch-actions/src/branch.rs @@ -8,6 +8,7 @@ use gitbutler_branch::{ use gitbutler_command_context::{CommandContext, GixRepositoryExt}; use gitbutler_project::access::WorktreeReadPermission; use gitbutler_reference::normalize_branch_name; +use gitbutler_repo::RepositoryExt; use gitbutler_serde::BStringForFrontend; use gix::object::tree::diff::Action; use gix::prelude::ObjectIdExt; @@ -27,12 +28,7 @@ pub(crate) fn get_uncommited_files( _permission: &WorktreeReadPermission, ) -> Result> { let repository = context.repository(); - let head_commit = repository - .head() - .context("Failed to get head")? - .peel_to_commit() - .context("Failed to get head commit")?; - + let head_commit = repository.head_commit()?; let files = gitbutler_diff::workdir(repository, &head_commit.id()) .context("Failed to list uncommited files")? .into_iter() diff --git a/crates/gitbutler-branch-actions/src/status.rs b/crates/gitbutler-branch-actions/src/status.rs index 910480e461..8e7b528e45 100644 --- a/crates/gitbutler-branch-actions/src/status.rs +++ b/crates/gitbutler-branch-actions/src/status.rs @@ -1,5 +1,11 @@ use std::{collections::HashMap, path::PathBuf, vec}; +use crate::{ + conflicts::RepoConflictsExt, + file::{virtual_hunks_into_virtual_files, VirtualBranchFile}, + hunk::{file_hunks_from_diffs, HunkLock, VirtualBranchHunk}, + BranchManagerExt, VirtualBranchesExt, +}; use anyhow::{bail, Context, Result}; use git2::Tree; use gitbutler_branch::{ @@ -10,16 +16,9 @@ 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; -use crate::{ - conflicts::RepoConflictsExt, - file::{virtual_hunks_into_virtual_files, VirtualBranchFile}, - hunk::{file_hunks_from_diffs, HunkLock, VirtualBranchHunk}, - integration::get_workspace_head, - BranchManagerExt, VirtualBranchesExt, -}; - /// Represents the uncommitted status of the applied virtual branches in the workspace. pub struct VirtualBranchesStatus { /// A collection of branches and their associated uncommitted file changes. @@ -37,13 +36,18 @@ pub fn get_applied_status( perm: Option<&mut WorktreeWritePermission>, ) -> Result { assure_open_workspace_mode(ctx).context("ng applied status requires open workspace mode")?; - let integration_commit = get_workspace_head(ctx)?; + // 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(); let mut virtual_branches = ctx .project() .virtual_branches() .list_branches_in_workspace()?; - let base_file_diffs = gitbutler_diff::workdir(ctx.repository(), &integration_commit.to_owned()) - .context("failed to diff workdir")?; + let base_file_diffs = + gitbutler_diff::workdir(ctx.repository(), &integration_commit_id.to_owned()) + .context("failed to diff workdir")?; let mut skipped_files: Vec = Vec::new(); for file_diff in base_file_diffs.values() { diff --git a/crates/gitbutler-branch-actions/tests/extra/mod.rs b/crates/gitbutler-branch-actions/tests/extra/mod.rs index ed2350480a..857adb5949 100644 --- a/crates/gitbutler-branch-actions/tests/extra/mod.rs +++ b/crates/gitbutler-branch-actions/tests/extra/mod.rs @@ -973,6 +973,7 @@ fn merge_vbranch_upstream_conflict() -> Result<()> { )?; // make gb see the conflict resolution + gitbutler_branch_actions::update_gitbutler_integration(&vb_state, ctx)?; let (branches, _) = list_virtual_branches(ctx, guard.write_permission())?; assert!(branches[0].conflicted); diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs index 5ee9c23937..945bb9bda5 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs @@ -1,6 +1,3 @@ -use gitbutler_branch::BranchCreateRequest; -use gitbutler_reference::Refname; - use super::*; #[test] @@ -101,6 +98,9 @@ fn conflicting() { "<<<<<<< ours\nconflict\n=======\nsecond\n>>>>>>> theirs\n" ); + let vb_state = VirtualBranchesHandle::new(project.gb_dir()); + let ctx = CommandContext::open(project).unwrap(); + update_gitbutler_integration(&vb_state, &ctx).unwrap(); let (branches, _) = controller.list_virtual_branches(project).unwrap(); assert_eq!(branches.len(), 1); diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs index 65b4797b66..c78e231084 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/mod.rs @@ -1,7 +1,8 @@ use std::{fs, path, path::PathBuf, str::FromStr}; -use gitbutler_branch::BranchCreateRequest; -use gitbutler_branch_actions::VirtualBranchActions; +use gitbutler_branch::{BranchCreateRequest, VirtualBranchesHandle}; +use gitbutler_branch_actions::{update_gitbutler_integration, VirtualBranchActions}; +use gitbutler_command_context::CommandContext; use gitbutler_error::error::Marker; use gitbutler_project::{self as projects, Project, ProjectId}; use gitbutler_reference::Refname; @@ -137,6 +138,9 @@ fn resolve_conflict_flow() { .create_virtual_branch_from_branch(project, &unapplied_branch, None) .unwrap(); + let vb_state = VirtualBranchesHandle::new(project.gb_dir()); + let ctx = CommandContext::open(project).unwrap(); + update_gitbutler_integration(&vb_state, &ctx).unwrap(); let (branches, _) = controller.list_virtual_branches(project).unwrap(); assert_eq!(branches.len(), 1); assert!(branches[0].active); diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/update_base_branch.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/update_base_branch.rs index 158628e244..fe2214437b 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/update_base_branch.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/update_base_branch.rs @@ -731,6 +731,9 @@ mod applied_branch { ) .unwrap(); + let vb_state = VirtualBranchesHandle::new(project.gb_dir()); + let ctx = CommandContext::open(project).unwrap(); + update_gitbutler_integration(&vb_state, &ctx).unwrap(); let (branches, _) = controller.list_virtual_branches(project).unwrap(); assert_eq!(branches.len(), 1); assert!(branches[0].active); diff --git a/crates/gitbutler-oplog/src/oplog.rs b/crates/gitbutler-oplog/src/oplog.rs index f0ce344309..a31ffa20b7 100644 --- a/crates/gitbutler-oplog/src/oplog.rs +++ b/crates/gitbutler-oplog/src/oplog.rs @@ -260,6 +260,7 @@ impl OplogExt for Project { restore_snapshot(self, snapshot_commit_id, guard.write_permission()) } + #[instrument(level = tracing::Level::DEBUG, skip(self), err(Debug))] fn should_auto_snapshot(&self, check_if_last_snapshot_older_than: Duration) -> Result { let last_snapshot_time = OplogHandle::new(&self.gb_dir()).modified_at()?; if last_snapshot_time.elapsed()? <= check_if_last_snapshot_older_than { @@ -711,6 +712,7 @@ fn write_conflicts_tree( /// Exclude files that are larger than the limit (eg. database.sql which may never be intended to be committed) /// TODO(ST): refactor this to be path-safe and ' ' save - the returned list is space separated (!!) +#[instrument(level = tracing::Level::DEBUG, skip(repo), err(Debug))] fn worktree_files_larger_than_limit_as_git2_ignore_rule( repo: &git2::Repository, worktree_dir: &Path, @@ -765,6 +767,7 @@ fn lines_since_snapshot(project: &Project, repo: &git2::Repository) -> Result Result>; fn remote_branches(&self) -> Result>; fn remotes_as_string(&self) -> Result>; /// Open a new in-memory repository and executes the provided closure using it. @@ -72,6 +73,13 @@ pub trait RepositoryExt { } impl RepositoryExt for git2::Repository { + fn head_commit(&self) -> Result> { + self.head() + .context("Failed to get head")? + .peel_to_commit() + .context("Failed to get head commit") + } + fn in_memory(&self, f: F) -> Result where F: FnOnce(&git2::Repository) -> Result, From 13deabdebdae929da1416e42ac6d5c82128baf5d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 15:33:37 +0200 Subject: [PATCH 07/11] find large files more quickly thanks to `gix` based dirwalk Also make sure that important `gix` crates are always compiled with optimization for proper performance. This is particularly useful when optimizing performance as not everything has to be done in release mode. However, it also causes CI to be slower as it compiles some dependencies longer. The problem realy is that `tauri dev` doesn't support custom profiles, so there is only `dev` and `release`. --- DEVELOPMENT.md | 17 +++++++++++++ crates/gitbutler-oplog/src/oplog.rs | 39 +++++++++++++++++------------ 2 files changed, 40 insertions(+), 16 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 11cba16e78..c65f635a18 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -138,6 +138,23 @@ One can get performance log when launching the application locally as follows: GITBUTLER_PERFORMANCE_LOG=1 LOG_LEVEL=debug pnpm tauri dev ``` +For more realistic performance logging, use local release builds with `--release`. + +```bash +GITBUTLER_PERFORMANCE_LOG=1 LOG_LEVEL=debug pnpm tauri dev --release +``` + +Since release builds are configured for public releases, they are very slow to compile. +Speed them up by sourcing the following file. + +```bash +export CARGO_PROFILE_RELEASE_DEBUG=0 +export CARGO_PROFILE_RELEASE_INCREMENTAL=false +export CARGO_PROFILE_RELEASE_LTO=false +export CARGO_PROFILE_RELEASE_CODEGEN_UNITS=256 +export CARGO_PROFILE_RELEASE_OPT_LEVEL=2 +``` + ### Tokio We are also collecting tokio's runtime tracing information that could be viewed using [tokio-console](https://github.com/tokio-rs/console#tokio-console-prototypes): diff --git a/crates/gitbutler-oplog/src/oplog.rs b/crates/gitbutler-oplog/src/oplog.rs index a31ffa20b7..00d9bf109a 100644 --- a/crates/gitbutler-oplog/src/oplog.rs +++ b/crates/gitbutler-oplog/src/oplog.rs @@ -15,6 +15,7 @@ use gitbutler_project::{ Project, }; use gitbutler_repo::RepositoryExt; +use gix::bstr::{BString, ByteSlice, ByteVec}; use tracing::instrument; use super::{ @@ -717,22 +718,28 @@ fn worktree_files_larger_than_limit_as_git2_ignore_rule( repo: &git2::Repository, worktree_dir: &Path, ) -> Result { - let statuses = repo.statuses(None)?; - let mut files_to_exclude = vec![]; - for entry in statuses.iter() { - let Some(rela_path) = entry.path() else { - continue; - }; - let full_path = worktree_dir.join(rela_path); - if let Ok(metadata) = fs::metadata(&full_path) { - if metadata.is_file() - && metadata.len() > SNAPSHOT_FILE_LIMIT_BYTES - && entry.status().is_wt_new() - { - files_to_exclude.push(rela_path.to_owned()); - } - } - } + let repo = gix::open(repo.path())?; + let files_to_exclude: Vec<_> = repo + .dirwalk_iter( + repo.index_or_empty()?, + None::, + Default::default(), + repo.dirwalk_options()? + .emit_ignored(None) + .emit_pruned(false) + .emit_untracked(gix::dir::walk::EmissionMode::Matching), + )? + .filter_map(Result::ok) + .filter_map(|item| { + let path = worktree_dir.join(gix::path::from_bstr(item.entry.rela_path.as_bstr())); + let file_is_too_large = path.metadata().map_or(false, |md| { + md.is_file() && md.len() > SNAPSHOT_FILE_LIMIT_BYTES + }); + file_is_too_large + .then(|| Vec::from(item.entry.rela_path).into_string().ok()) + .flatten() + }) + .collect(); Ok(files_to_exclude.join(" ")) } From 751e091f6f71cb3248fa0c5da3e99c37443916d2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 13:00:12 +0200 Subject: [PATCH 08/11] Reuse the worktree-diff during 'recalulate-everything' It's used for emission of changed files, but also for listing branch information. Make sure we only run the worktree-status once to save time. This also unifies the acquisition of the `HEAD^{commit}`, which one day will be a natural feature once `gix::Repository` is used. --- Cargo.lock | 1 + .../gitbutler-branch-actions/src/actions.rs | 55 ++++++++++++++----- crates/gitbutler-branch-actions/src/branch.rs | 16 ++++-- crates/gitbutler-branch-actions/src/status.rs | 20 +++++-- .../gitbutler-branch-actions/src/virtual.rs | 18 ++++-- crates/gitbutler-diff/src/lib.rs | 4 +- crates/gitbutler-watcher/Cargo.toml | 1 + crates/gitbutler-watcher/src/handler.rs | 42 +++++++++----- 8 files changed, 114 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a102bec305..b94b5110f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2627,6 +2627,7 @@ dependencies = [ "backoff", "gitbutler-branch-actions", "gitbutler-command-context", + "gitbutler-diff", "gitbutler-error", "gitbutler-notify-debouncer", "gitbutler-operating-modes", diff --git a/crates/gitbutler-branch-actions/src/actions.rs b/crates/gitbutler-branch-actions/src/actions.rs index 9a47e5dbc8..4629682228 100644 --- a/crates/gitbutler-branch-actions/src/actions.rs +++ b/crates/gitbutler-branch-actions/src/actions.rs @@ -1,8 +1,22 @@ +use super::r#virtual as branch; +use crate::branch::get_uncommited_files_raw; +use crate::{ + base::{ + get_base_branch_data, set_base_branch, set_target_push_remote, update_base_branch, + BaseBranch, + }, + branch::get_uncommited_files, + branch_manager::BranchManagerExt, + file::RemoteBranchFile, + remote::{get_branch_data, list_remote_branches, RemoteBranch, RemoteBranchData}, + VirtualBranchesExt, +}; use anyhow::{Context, Result}; use gitbutler_branch::{ BranchCreateRequest, BranchId, BranchOwnershipClaims, BranchUpdateRequest, ChangeReference, }; use gitbutler_command_context::CommandContext; +use gitbutler_diff::DiffByPathMap; use gitbutler_operating_modes::assure_open_workspace_mode; use gitbutler_oplog::{ entry::{OperationKind, SnapshotDetails}, @@ -13,19 +27,6 @@ use gitbutler_reference::{ReferenceName, Refname, RemoteRefname}; use gitbutler_repo::{credentials::Helper, RepoActionsExt, RepositoryExt}; use tracing::instrument; -use super::r#virtual as branch; -use crate::{ - base::{ - get_base_branch_data, set_base_branch, set_target_push_remote, update_base_branch, - BaseBranch, - }, - branch::get_uncommited_files, - branch_manager::BranchManagerExt, - file::RemoteBranchFile, - remote::{get_branch_data, list_remote_branches, RemoteBranch, RemoteBranchData}, - VirtualBranchesExt, -}; - #[derive(Clone, Copy, Default)] pub struct VirtualBranchActions; @@ -81,6 +82,24 @@ impl VirtualBranchActions { .map_err(Into::into) } + pub fn list_virtual_branches_cached( + &self, + project: &Project, + worktree_changes: Option, + ) -> Result<(Vec, Vec)> { + let ctx = open_with_verify(project)?; + + assure_open_workspace_mode(&ctx) + .context("Listing virtual branches requires open workspace mode")?; + + branch::list_virtual_branches_cached( + &ctx, + project.exclusive_worktree_access().write_permission(), + worktree_changes, + ) + .map_err(Into::into) + } + pub fn create_virtual_branch( &self, project: &Project, @@ -569,11 +588,17 @@ impl VirtualBranchActions { pub fn get_uncommited_files(&self, project: &Project) -> Result> { let context = CommandContext::open(project)?; - let guard = project.exclusive_worktree_access(); - get_uncommited_files(&context, guard.read_permission()) } + + /// Like [`get_uncommited_files()`], but returns a type that can be re-used with + /// [`crate::list_virtual_branches()`]. + pub fn get_uncommited_files_reusable(&self, project: &Project) -> Result { + let context = CommandContext::open(project)?; + let guard = project.exclusive_worktree_access(); + get_uncommited_files_raw(&context, guard.read_permission()) + } } fn open_with_verify(project: &Project) -> Result { diff --git a/crates/gitbutler-branch-actions/src/branch.rs b/crates/gitbutler-branch-actions/src/branch.rs index 1be6eb22cc..d775b6bca2 100644 --- a/crates/gitbutler-branch-actions/src/branch.rs +++ b/crates/gitbutler-branch-actions/src/branch.rs @@ -6,6 +6,7 @@ use gitbutler_branch::{ Branch as GitButlerBranch, BranchId, BranchIdentity, ReferenceExtGix, Target, }; use gitbutler_command_context::{CommandContext, GixRepositoryExt}; +use gitbutler_diff::DiffByPathMap; use gitbutler_project::access::WorktreeReadPermission; use gitbutler_reference::normalize_branch_name; use gitbutler_repo::RepositoryExt; @@ -23,14 +24,21 @@ use std::{ vec, }; -pub(crate) fn get_uncommited_files( +pub(crate) fn get_uncommited_files_raw( context: &CommandContext, _permission: &WorktreeReadPermission, -) -> Result> { +) -> Result { let repository = context.repository(); let head_commit = repository.head_commit()?; - let files = gitbutler_diff::workdir(repository, &head_commit.id()) - .context("Failed to list uncommited files")? + gitbutler_diff::workdir(repository, &head_commit.id()) + .context("Failed to list uncommited files") +} + +pub(crate) fn get_uncommited_files( + context: &CommandContext, + _permission: &WorktreeReadPermission, +) -> Result> { + let files = get_uncommited_files_raw(context, _permission)? .into_iter() .map(|(path, file)| { let binary = file.hunks.iter().any(|h| h.binary); diff --git a/crates/gitbutler-branch-actions/src/status.rs b/crates/gitbutler-branch-actions/src/status.rs index 8e7b528e45..dc2f5ae287 100644 --- a/crates/gitbutler-branch-actions/src/status.rs +++ b/crates/gitbutler-branch-actions/src/status.rs @@ -27,13 +27,24 @@ pub struct VirtualBranchesStatus { pub skipped_files: Vec, } +pub fn get_applied_status( + ctx: &CommandContext, + perm: Option<&mut WorktreeWritePermission>, +) -> Result { + get_applied_status_cached(ctx, perm, None) +} + /// Returns branches and their associated file changes, in addition to a list /// of skipped files. +/// `worktree_changes` are all changed files against the current `HEAD^{tree}` and index +/// against the current working tree directory, and it's used to avoid double-computing +/// this expensive information. // TODO(kv): make this side effect free -#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm))] -pub fn get_applied_status( +#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm, worktree_changes))] +pub fn get_applied_status_cached( ctx: &CommandContext, perm: Option<&mut WorktreeWritePermission>, + worktree_changes: Option, ) -> Result { assure_open_workspace_mode(ctx).context("ng applied status requires open workspace mode")?; // TODO(ST): this was `get_workspace_head()`, which is slow and ideally, we don't dynamically @@ -45,9 +56,10 @@ pub fn get_applied_status( .project() .virtual_branches() .list_branches_in_workspace()?; - let base_file_diffs = + let base_file_diffs = worktree_changes.map(Ok).unwrap_or_else(|| { gitbutler_diff::workdir(ctx.repository(), &integration_commit_id.to_owned()) - .context("failed to diff workdir")?; + .context("failed to diff workdir") + })?; let mut skipped_files: Vec = Vec::new(); for file_diff in base_file_diffs.values() { diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index ddcf24440c..d6713d9748 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -13,7 +13,7 @@ use crate::{ hunk::VirtualBranchHunk, integration::get_workspace_head, remote::{branch_to_remote_branch, RemoteBranch}, - status::get_applied_status, + status::{get_applied_status, get_applied_status_cached}, Get, VirtualBranchesExt, }; use anyhow::{anyhow, bail, Context, Result}; @@ -254,13 +254,23 @@ fn resolve_old_applied_state( Ok(()) } - -#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm))] pub fn list_virtual_branches( + ctx: &CommandContext, + perm: &mut WorktreeWritePermission, +) -> Result<(Vec, Vec)> { + list_virtual_branches_cached(ctx, perm, None) +} + +/// `worktree_changes` are all changed files against the current `HEAD^{tree}` and index +/// against the current working tree directory, and it's used to avoid double-computing +/// this expensive information. +#[instrument(level = tracing::Level::DEBUG, skip(ctx, perm, worktree_changes))] +pub fn list_virtual_branches_cached( ctx: &CommandContext, // TODO(ST): this should really only shared access, but there is some internals // that conditionally write things. perm: &mut WorktreeWritePermission, + worktree_changes: Option, ) -> Result<(Vec, Vec)> { assure_open_workspace_mode(ctx) .context("Listing virtual branches requires open workspace mode")?; @@ -274,7 +284,7 @@ pub fn list_virtual_branches( .get_default_target() .context("failed to get default target")?; - let status = get_applied_status(ctx, Some(perm))?; + let status = get_applied_status_cached(ctx, Some(perm), worktree_changes)?; let max_selected_for_changes = status .branches .iter() diff --git a/crates/gitbutler-diff/src/lib.rs b/crates/gitbutler-diff/src/lib.rs index e26858ac18..994acf2606 100644 --- a/crates/gitbutler-diff/src/lib.rs +++ b/crates/gitbutler-diff/src/lib.rs @@ -2,7 +2,7 @@ mod diff; mod hunk; pub mod write; pub use diff::{ - diff_files_into_hunks, hunks_by_filepath, reverse_hunk, trees, workdir, ChangeType, FileDiff, - GitHunk, + diff_files_into_hunks, hunks_by_filepath, reverse_hunk, trees, workdir, ChangeType, + DiffByPathMap, FileDiff, GitHunk, }; pub use hunk::{Hunk, HunkHash}; diff --git a/crates/gitbutler-watcher/Cargo.toml b/crates/gitbutler-watcher/Cargo.toml index b2ebed9dfc..2559a940be 100644 --- a/crates/gitbutler-watcher/Cargo.toml +++ b/crates/gitbutler-watcher/Cargo.toml @@ -24,6 +24,7 @@ gix = { workspace = true, features = [ ] } gitbutler-command-context.workspace = true gitbutler-project.workspace = true +gitbutler-diff.workspace = true gitbutler-user.workspace = true gitbutler-reference.workspace = true gitbutler-error.workspace = true diff --git a/crates/gitbutler-watcher/src/handler.rs b/crates/gitbutler-watcher/src/handler.rs index 53fd9d001e..392501f689 100644 --- a/crates/gitbutler-watcher/src/handler.rs +++ b/crates/gitbutler-watcher/src/handler.rs @@ -1,8 +1,10 @@ use std::{path::PathBuf, sync::Arc}; +use super::{events, Change}; use anyhow::{Context, Result}; -use gitbutler_branch_actions::{VirtualBranchActions, VirtualBranches}; +use gitbutler_branch_actions::{RemoteBranchFile, VirtualBranchActions, VirtualBranches}; use gitbutler_command_context::CommandContext; +use gitbutler_diff::DiffByPathMap; use gitbutler_error::error::Marker; use gitbutler_operating_modes::{ in_open_workspace_mode, in_outside_workspace_mode, operating_mode, @@ -18,8 +20,6 @@ use gitbutler_sync::cloud::{push_oplog, push_repo}; use gitbutler_user as users; use tracing::instrument; -use super::{events, Change}; - /// A type that contains enough state to make decisions based on changes in the filesystem, which themselves /// may trigger [Changes](Change) // NOTE: This is `Clone` as each incoming event is spawned onto a thread for processing. @@ -71,7 +71,7 @@ impl Handler { // This is only produced at the end of mutating Tauri commands to trigger a fresh state being served to the UI. events::InternalEvent::CalculateVirtualBranches(project_id) => self - .calculate_virtual_branches(project_id) + .calculate_virtual_branches(project_id, None) .context("failed to handle virtual branch event"), } } @@ -90,8 +90,12 @@ impl Handler { CommandContext::open(&project).context("Failed to create a command context") } - #[instrument(skip(self, project_id))] - fn calculate_virtual_branches(&self, project_id: ProjectId) -> Result<()> { + #[instrument(skip(self, project_id, worktree_changes))] + fn calculate_virtual_branches( + &self, + project_id: ProjectId, + worktree_changes: Option, + ) -> Result<()> { let ctx = self.open_command_context(project_id)?; // Skip if we're not on the open workspace mode if !in_open_workspace_mode(&ctx) { @@ -102,7 +106,7 @@ impl Handler { .projects .get(project_id) .context("failed to get project")?; - match VirtualBranchActions.list_virtual_branches(&project) { + match VirtualBranchActions.list_virtual_branches_cached(&project, worktree_changes) { Ok((branches, skipped_files)) => self.emit_app_event(Change::VirtualBranches { project_id: project.id, virtual_branches: VirtualBranches { @@ -126,26 +130,36 @@ impl Handler { fn recalculate_everything(&self, paths: Vec, project_id: ProjectId) -> Result<()> { let ctx = self.open_command_context(project_id)?; - self.emit_uncommited_files(ctx.project()); + let worktree_changes = self.emit_uncommited_files(ctx.project()).ok(); if in_open_workspace_mode(&ctx) { self.maybe_create_snapshot(project_id).ok(); - self.calculate_virtual_branches(project_id)?; + self.calculate_virtual_branches(project_id, worktree_changes)?; } Ok(()) } /// Try to emit uncommited files. Swollow errors if they arrise. - fn emit_uncommited_files(&self, project: &Project) { - let Ok(files) = VirtualBranchActions.get_uncommited_files(project) else { - return; - }; + fn emit_uncommited_files(&self, project: &Project) -> Result { + let files = VirtualBranchActions.get_uncommited_files_reusable(project)?; let _ = self.emit_app_event(Change::UncommitedFiles { project_id: project.id, - files, + files: files + .clone() + .into_iter() + .map(|(path, file)| { + let binary = file.hunks.iter().any(|h| h.binary); + RemoteBranchFile { + path, + hunks: file.hunks, + binary, + } + }) + .collect(), }); + Ok(files) } fn maybe_create_snapshot(&self, project_id: ProjectId) -> anyhow::Result<()> { From a0e236110a314fc412c30205c216f55d1ea2b3a6 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 21:56:55 +0200 Subject: [PATCH 09/11] refactor - move `gix` repository extension to where it belongs more naturally. - make sure that `get_wd_tree()` is named more closely to what it really does, and use it in more places. --- crates/gitbutler-branch-actions/src/branch.rs | 4 +-- .../src/branch_manager/branch_creation.rs | 2 +- .../gitbutler-branch-actions/src/virtual.rs | 6 ++-- crates/gitbutler-command-context/src/lib.rs | 15 ---------- crates/gitbutler-edit-mode/src/lib.rs | 29 ++----------------- crates/gitbutler-repo/src/lib.rs | 2 +- crates/gitbutler-repo/src/repository_ext.rs | 24 +++++++++++++-- 7 files changed, 31 insertions(+), 51 deletions(-) diff --git a/crates/gitbutler-branch-actions/src/branch.rs b/crates/gitbutler-branch-actions/src/branch.rs index d775b6bca2..dc45cd93ee 100644 --- a/crates/gitbutler-branch-actions/src/branch.rs +++ b/crates/gitbutler-branch-actions/src/branch.rs @@ -5,11 +5,11 @@ use core::fmt; use gitbutler_branch::{ Branch as GitButlerBranch, BranchId, BranchIdentity, ReferenceExtGix, Target, }; -use gitbutler_command_context::{CommandContext, GixRepositoryExt}; +use gitbutler_command_context::CommandContext; use gitbutler_diff::DiffByPathMap; use gitbutler_project::access::WorktreeReadPermission; use gitbutler_reference::normalize_branch_name; -use gitbutler_repo::RepositoryExt; +use gitbutler_repo::{GixRepositoryExt, RepositoryExt}; use gitbutler_serde::BStringForFrontend; use gix::object::tree::diff::Action; use gix::prelude::ObjectIdExt; diff --git a/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs b/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs index ec47113ca0..ba491b9226 100644 --- a/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs +++ b/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs @@ -454,7 +454,7 @@ impl BranchManager<'_> { vb_state.set_branch(branch.clone())?; } - let wd_tree = self.ctx.repository().get_wd_tree()?; + let wd_tree = self.ctx.repository().create_wd_tree()?; let branch_tree = repo .find_tree(branch.tree) diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index d6713d9748..ebba075f33 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -578,7 +578,7 @@ pub fn integrate_upstream_commits(ctx: &CommandContext, branch_id: BranchId) -> let new_head_tree = repo.find_commit(new_head)?.tree()?; let head_commit = repo.find_commit(new_head)?; - let wd_tree = ctx.repository().get_wd_tree()?; + let wd_tree = ctx.repository().create_wd_tree()?; let integration_tree = repo.find_commit(get_workspace_head(ctx)?)?.tree()?; let mut merge_index = repo.merge_trees(&integration_tree, &new_head_tree, &wd_tree, None)?; @@ -616,7 +616,7 @@ pub(crate) fn integrate_with_merge( upstream_commit: &git2::Commit, merge_base: git2::Oid, ) -> Result { - let wd_tree = ctx.repository().get_wd_tree()?; + let wd_tree = ctx.repository().create_wd_tree()?; let repo = ctx.repository(); let remote_tree = upstream_commit.tree().context("failed to get tree")?; let upstream_branch = branch.upstream.as_ref().context("upstream not found")?; @@ -1171,7 +1171,7 @@ pub fn is_remote_branch_mergeable( let base_tree = find_base_tree(ctx.repository(), &branch_commit, &target_commit)?; - let wd_tree = ctx.repository().get_wd_tree()?; + let wd_tree = ctx.repository().create_wd_tree()?; let branch_tree = branch_commit.tree().context("failed to find branch tree")?; let mergeable = !ctx diff --git a/crates/gitbutler-command-context/src/lib.rs b/crates/gitbutler-command-context/src/lib.rs index 20ad66e748..ca8a45422f 100644 --- a/crates/gitbutler-command-context/src/lib.rs +++ b/crates/gitbutler-command-context/src/lib.rs @@ -98,18 +98,3 @@ impl CommandContext { )?) } } - -// TODO(ST): put this into `gix`, the logic seems good, add unit-test for number generation. -pub trait GixRepositoryExt: Sized { - /// Configure the repository for diff operations between trees. - /// This means it needs an object cache relative to the amount of files in the repository. - fn for_tree_diffing(self) -> Result; -} - -impl GixRepositoryExt for gix::Repository { - fn for_tree_diffing(mut self) -> anyhow::Result { - let bytes = self.compute_object_cache_size_for_tree_diffs(&***self.index_or_empty()?); - self.object_cache_size_if_unset(bytes); - Ok(self) - } -} diff --git a/crates/gitbutler-edit-mode/src/lib.rs b/crates/gitbutler-edit-mode/src/lib.rs index a768758dd5..b90eb5230c 100644 --- a/crates/gitbutler-edit-mode/src/lib.rs +++ b/crates/gitbutler-edit-mode/src/lib.rs @@ -34,17 +34,7 @@ fn save_uncommited_files(ctx: &CommandContext) -> Result<()> { let repository = ctx.repository(); // Create a tree of all uncommited files - let mut index = repository.index().context("Failed to get index")?; - index - .add_all(["*"], git2::IndexAddOption::DEFAULT, None) - .context("Failed to add all to index")?; - index.write().context("Failed to write index")?; - let tree_oid = index - .write_tree() - .context("Failed to create tree from index")?; - let tree = repository - .find_tree(tree_oid) - .context("Failed to find tree")?; + let tree = repository.create_wd_tree()?; // Commit tree and reference it let author_signature = @@ -134,10 +124,7 @@ fn checkout_edit_branch(ctx: &CommandContext, commit: &git2::Commit) -> Result<( ), )?; - let mut index = repository.index()?; - index.add_all(["*"], git2::IndexAddOption::DEFAULT, None)?; - let tree = index.write_tree()?; - let tree = repository.find_tree(tree)?; + let tree = repository.create_wd_tree()?; let author_signature = signature(SignaturePurpose::Author)?; let committer_signature = signature(SignaturePurpose::Committer)?; @@ -265,17 +252,7 @@ pub(crate) fn save_and_return_to_workspace( }; // Recommit commit - let mut index = repository.index().context("Failed to get index")?; - index - .add_all(["*"].iter(), git2::IndexAddOption::DEFAULT, None) - .context("Failed to add all to index")?; - index.write().context("Failed to write index")?; - let tree_oid = index - .write_tree() - .context("Failed to create tree from index")?; - let tree = repository - .find_tree(tree_oid) - .context("Failed to find tree")?; + let tree = repository.create_wd_tree()?; let commit_headers = commit .gitbutler_headers() .map(|commit_headers| CommitHeadersV2 { diff --git a/crates/gitbutler-repo/src/lib.rs b/crates/gitbutler-repo/src/lib.rs index 46ba6fc802..baf7e0a5c5 100644 --- a/crates/gitbutler-repo/src/lib.rs +++ b/crates/gitbutler-repo/src/lib.rs @@ -7,7 +7,7 @@ mod commands; pub use commands::RepoCommands; mod repository_ext; -pub use repository_ext::RepositoryExt; +pub use repository_ext::{GixRepositoryExt, RepositoryExt}; pub mod credentials; diff --git a/crates/gitbutler-repo/src/repository_ext.rs b/crates/gitbutler-repo/src/repository_ext.rs index 7db1db7b9f..4317126a42 100644 --- a/crates/gitbutler-repo/src/repository_ext.rs +++ b/crates/gitbutler-repo/src/repository_ext.rs @@ -41,7 +41,7 @@ pub trait RepositoryExt { fn checkout_tree_builder<'a>(&'a self, tree: &'a git2::Tree<'a>) -> CheckoutTreeBuidler; fn find_branch_by_refname(&self, name: &Refname) -> Result>; /// Based on the index, add all data similar to `git add .` and create a tree from it, which is returned. - fn get_wd_tree(&self) -> Result; + fn create_wd_tree(&self) -> Result; /// Returns the `gitbutler/integration` branch if the head currently points to it, or fail otherwise. /// Use it before any modification to the repository, or extra defensively each time the @@ -136,10 +136,13 @@ impl RepositoryExt for git2::Repository { } } + /// Note that this will add all untracked files in the worktree to the index, + /// and write a tree from it. + /// The index won't be stored though. #[instrument(level = tracing::Level::DEBUG, skip(self), err(Debug))] - fn get_wd_tree(&self) -> Result { + fn create_wd_tree(&self) -> Result { let mut index = self.index()?; - index.add_all(["*"], git2::IndexAddOption::CHECK_PATHSPEC, None)?; + index.add_all(["*"], git2::IndexAddOption::DEFAULT, None)?; let oid = index.write_tree()?; self.find_tree(oid).map(Into::into).map_err(Into::into) } @@ -448,3 +451,18 @@ impl CheckoutIndexBuilder<'_> { .map_err(Into::into) } } + +// TODO(ST): put this into `gix`, the logic seems good, add unit-test for number generation. +pub trait GixRepositoryExt: Sized { + /// Configure the repository for diff operations between trees. + /// This means it needs an object cache relative to the amount of files in the repository. + fn for_tree_diffing(self) -> Result; +} + +impl GixRepositoryExt for gix::Repository { + fn for_tree_diffing(mut self) -> anyhow::Result { + let bytes = self.compute_object_cache_size_for_tree_diffs(&***self.index_or_empty()?); + self.object_cache_size_if_unset(bytes); + Ok(self) + } +} From 79798c74074886ef466f9cbf4d0114d1d90e1541 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Wed, 28 Aug 2024 21:52:48 +0200 Subject: [PATCH 10/11] Exclude big files when performing a worktree diff. This was lost previously when switching it over to a read-only implementation. Implementing it with an ignore list will take time, 400ms in the GitLab repository, but it's not slower than it was before and it is always preferred to not dump objects into the ODB unnecessarily. --- Cargo.lock | 2 + crates/gitbutler-branch-actions/src/status.rs | 10 ++-- crates/gitbutler-command-context/Cargo.toml | 1 + crates/gitbutler-command-context/src/lib.rs | 3 + .../src/repository_ext.rs | 50 +++++++++++++++++ crates/gitbutler-diff/src/diff.rs | 3 +- crates/gitbutler-oplog/Cargo.toml | 1 + crates/gitbutler-oplog/src/oplog.rs | 56 ++----------------- 8 files changed, 69 insertions(+), 57 deletions(-) create mode 100644 crates/gitbutler-command-context/src/repository_ext.rs diff --git a/Cargo.lock b/Cargo.lock index b94b5110f6..2991463947 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2211,6 +2211,7 @@ name = "gitbutler-command-context" version = "0.0.0" dependencies = [ "anyhow", + "bstr", "git2", "gitbutler-project", "gix", @@ -2374,6 +2375,7 @@ dependencies = [ "anyhow", "git2", "gitbutler-branch", + "gitbutler-command-context", "gitbutler-diff", "gitbutler-fs", "gitbutler-project", diff --git a/crates/gitbutler-branch-actions/src/status.rs b/crates/gitbutler-branch-actions/src/status.rs index dc2f5ae287..65126359ea 100644 --- a/crates/gitbutler-branch-actions/src/status.rs +++ b/crates/gitbutler-branch-actions/src/status.rs @@ -47,16 +47,16 @@ pub fn get_applied_status_cached( worktree_changes: Option, ) -> Result { assure_open_workspace_mode(ctx).context("ng applied status requires open workspace mode")?; - // 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(); let mut virtual_branches = ctx .project() .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(); gitbutler_diff::workdir(ctx.repository(), &integration_commit_id.to_owned()) .context("failed to diff workdir") })?; diff --git a/crates/gitbutler-command-context/Cargo.toml b/crates/gitbutler-command-context/Cargo.toml index df826abf78..cd5d076203 100644 --- a/crates/gitbutler-command-context/Cargo.toml +++ b/crates/gitbutler-command-context/Cargo.toml @@ -12,3 +12,4 @@ gix.workspace = true tracing.workspace = true gitbutler-project.workspace = true itertools = "0.13" +bstr = "1.10.0" diff --git a/crates/gitbutler-command-context/src/lib.rs b/crates/gitbutler-command-context/src/lib.rs index ca8a45422f..c4dc09dc6e 100644 --- a/crates/gitbutler-command-context/src/lib.rs +++ b/crates/gitbutler-command-context/src/lib.rs @@ -98,3 +98,6 @@ impl CommandContext { )?) } } + +mod repository_ext; +pub use repository_ext::RepositoryExtLite; diff --git a/crates/gitbutler-command-context/src/repository_ext.rs b/crates/gitbutler-command-context/src/repository_ext.rs new file mode 100644 index 0000000000..58841ea898 --- /dev/null +++ b/crates/gitbutler-command-context/src/repository_ext.rs @@ -0,0 +1,50 @@ +use anyhow::{Context, Result}; +use gix::bstr::{BString, ByteVec}; +use tracing::instrument; + +/// An extension trait that should avoid pulling in large amounts of dependency so it can be used +/// in more places without causing cycles. +/// `gitbutler_repo::RepositoryExt` may not be usable everywhere due to that. +pub trait RepositoryExtLite { + /// Exclude files that are larger than `limit_in_bytes` (eg. database.sql which may never be intended to be committed) + /// so they don't show up in the next diff. + fn ignore_large_files_in_diffs(&self, limit_in_bytes: u64) -> Result<()>; +} + +impl RepositoryExtLite for git2::Repository { + #[instrument(level = tracing::Level::DEBUG, skip(self), err(Debug))] + fn ignore_large_files_in_diffs(&self, limit_in_bytes: u64) -> Result<()> { + use gix::bstr::ByteSlice; + let repo = gix::open(self.path())?; + let worktree_dir = repo + .work_dir() + .context("All repos are expected to have a worktree")?; + let files_to_exclude: Vec<_> = repo + .dirwalk_iter( + repo.index_or_empty()?, + None::, + Default::default(), + repo.dirwalk_options()? + .emit_ignored(None) + .emit_pruned(false) + .emit_untracked(gix::dir::walk::EmissionMode::Matching), + )? + .filter_map(Result::ok) + .filter_map(|item| { + let path = worktree_dir.join(gix::path::from_bstr(item.entry.rela_path.as_bstr())); + let file_is_too_large = path + .metadata() + .map_or(false, |md| md.is_file() && md.len() > limit_in_bytes); + file_is_too_large + .then(|| Vec::from(item.entry.rela_path).into_string().ok()) + .flatten() + }) + .collect(); + // TODO(ST): refactor this to be path-safe and ' ' save - the returned list is space separated (!!) + // Just make sure this isn't needed anymore. + let ignore_list = files_to_exclude.join(" "); + // In-memory, libgit2 internal ignore rule + self.add_ignore_rule(&ignore_list)?; + Ok(()) + } +} diff --git a/crates/gitbutler-diff/src/diff.rs b/crates/gitbutler-diff/src/diff.rs index 393f27c500..0315d8cdb0 100644 --- a/crates/gitbutler-diff/src/diff.rs +++ b/crates/gitbutler-diff/src/diff.rs @@ -3,6 +3,7 @@ use std::{borrow::Cow, collections::HashMap, path::PathBuf, str}; use anyhow::{Context, Result}; use bstr::{BStr, BString, ByteSlice, ByteVec}; use gitbutler_cherry_pick::RepositoryExt; +use gitbutler_command_context::RepositoryExtLite; use gitbutler_serde::BStringForFrontend; use serde::{Deserialize, Serialize}; use tracing::instrument; @@ -155,8 +156,8 @@ pub fn workdir(repo: &git2::Repository, commit_oid: &git2::Oid) -> Result Result { - let repo = gix::open(repo.path())?; - let files_to_exclude: Vec<_> = repo - .dirwalk_iter( - repo.index_or_empty()?, - None::, - Default::default(), - repo.dirwalk_options()? - .emit_ignored(None) - .emit_pruned(false) - .emit_untracked(gix::dir::walk::EmissionMode::Matching), - )? - .filter_map(Result::ok) - .filter_map(|item| { - let path = worktree_dir.join(gix::path::from_bstr(item.entry.rela_path.as_bstr())); - let file_is_too_large = path.metadata().map_or(false, |md| { - md.is_file() && md.len() > SNAPSHOT_FILE_LIMIT_BYTES - }); - file_is_too_large - .then(|| Vec::from(item.entry.rela_path).into_string().ok()) - .flatten() - }) - .collect(); - Ok(files_to_exclude.join(" ")) -} - /// Returns the number of lines of code (added + removed) since the last snapshot in `project`. /// Includes untracked files. /// `repo` is an already opened project repository. @@ -752,13 +712,7 @@ fn lines_since_snapshot(project: &Project, repo: &git2::Repository) -> Result Date: Wed, 28 Aug 2024 22:46:03 +0200 Subject: [PATCH 11/11] 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 66a8e877f6..d1d8f8f397 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 65126359ea..fc93857a9f 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") })?;