From 08d22de40e6036a4b04aedd76d5b4c08f437b5e8 Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Sun, 25 Feb 2024 21:35:21 +0100 Subject: [PATCH] if a project has not virtual branches migrate it to using diffs with context --- .../src/virtual_branches/commands.rs | 16 ++++- .../src/virtual_branches/controller.rs | 6 +- gitbutler-app/src/virtual_branches/tests.rs | 70 +++++++++---------- gitbutler-app/src/virtual_branches/virtual.rs | 16 +++-- .../caltulate_virtual_branches_handler.rs | 2 +- 5 files changed, 66 insertions(+), 44 deletions(-) diff --git a/gitbutler-app/src/virtual_branches/commands.rs b/gitbutler-app/src/virtual_branches/commands.rs index 7efb586e1..1335e8b7e 100644 --- a/gitbutler-app/src/virtual_branches/commands.rs +++ b/gitbutler-app/src/virtual_branches/commands.rs @@ -12,7 +12,7 @@ use crate::{ }; use super::{ - branch::BranchId, + branch::{self, BranchId}, controller::{Controller, ControllerError}, BaseBranch, RemoteBranchFile, }; @@ -80,11 +80,23 @@ pub async fn list_virtual_branches( code: Code::Validation, message: "Malformed project id".to_string(), })?; - let branches = handle + let (branches, uses_diff_context) = handle .state::() .list_virtual_branches(&project_id) .await?; + // Migration: If use_diff_context is not already set and if there are no vbranches, set use_diff_context to true + if !uses_diff_context && branches.is_empty() { + let _ = handle + .state::() + .update(&projects::UpdateRequest { + id: project_id, + use_diff_context: Some(true), + ..Default::default() + }) + .await; + } + let proxy = handle.state::(); let branches = proxy.proxy_virtual_branches(branches).await; Ok(branches) diff --git a/gitbutler-app/src/virtual_branches/controller.rs b/gitbutler-app/src/virtual_branches/controller.rs index a0f0cb21a..978f58fee 100644 --- a/gitbutler-app/src/virtual_branches/controller.rs +++ b/gitbutler-app/src/virtual_branches/controller.rs @@ -124,7 +124,8 @@ impl Controller { pub async fn list_virtual_branches( &self, project_id: &ProjectId, - ) -> Result, ControllerError> { + ) -> Result<(Vec, bool), ControllerError> + { self.inner(project_id) .await .list_virtual_branches(project_id) @@ -493,7 +494,8 @@ impl ControllerInner { pub async fn list_virtual_branches( &self, project_id: &ProjectId, - ) -> Result, ControllerError> { + ) -> Result<(Vec, bool), ControllerError> + { let _permit = self.semaphore.acquire().await; self.with_verify_branch(project_id, |gb_repository, project_repository, _| { diff --git a/gitbutler-app/src/virtual_branches/tests.rs b/gitbutler-app/src/virtual_branches/tests.rs index ec9cc583e..d1f090d4b 100644 --- a/gitbutler-app/src/virtual_branches/tests.rs +++ b/gitbutler-app/src/virtual_branches/tests.rs @@ -81,7 +81,7 @@ fn test_commit_on_branch_then_change_file_then_get_status() -> Result<()> { "line0\nline1\nline2\nline3\nline4\n", )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches[0]; assert_eq!(branch.files.len(), 1); assert_eq!(branch.commits.len(), 0); @@ -99,7 +99,7 @@ fn test_commit_on_branch_then_change_file_then_get_status() -> Result<()> { )?; // status (no files) - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches[0]; assert_eq!(branch.files.len(), 0); assert_eq!(branch.commits.len(), 1); @@ -110,7 +110,7 @@ fn test_commit_on_branch_then_change_file_then_get_status() -> Result<()> { )?; // should have just the last change now, the other line is committed - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches[0]; assert_eq!(branch.files.len(), 1); assert_eq!(branch.commits.len(), 1); @@ -170,7 +170,7 @@ fn test_signed_commit() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository).unwrap(); + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository).unwrap(); let commit_id = &branches[0].commits[0].id; let commit_obj = project_repository.git_repository.find_commit(*commit_id)?; // check the raw_header contains the string "SSH SIGNATURE" @@ -235,7 +235,7 @@ fn test_track_binary_files() -> Result<()> { let mut file = fs::File::create(std::path::Path::new(&project.path).join("image.bin"))?; file.write_all(&image_data)?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches[0]; assert_eq!(branch.files.len(), 2); let img_file = &branch @@ -262,7 +262,7 @@ fn test_track_binary_files() -> Result<()> { )?; // status (no files) - let branches = list_virtual_branches(&gb_repository, &project_repository).unwrap(); + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository).unwrap(); let commit_id = &branches[0].commits[0].id; let commit_obj = project_repository.git_repository.find_commit(*commit_id)?; let tree = commit_obj.tree()?; @@ -291,7 +291,7 @@ fn test_track_binary_files() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository).unwrap(); + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository).unwrap(); let commit_id = &branches[0].commits[0].id; // get tree from commit_id let commit_obj = project_repository.git_repository.find_commit(*commit_id)?; @@ -1040,7 +1040,7 @@ fn test_merge_vbranch_upstream_clean() -> Result<()> { .context("failed to write target branch after push")?; // create the branch - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches[0]; assert_eq!(branch1.files.len(), 1); assert_eq!(branch1.commits.len(), 1); @@ -1054,7 +1054,7 @@ fn test_merge_vbranch_upstream_clean() -> Result<()> { None, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches[0]; let contents = std::fs::read(std::path::Path::new(&project.path).join(file_path))?; @@ -1168,7 +1168,7 @@ fn test_merge_vbranch_upstream_conflict() -> Result<()> { .context("failed to write target branch after push")?; // create the branch - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches[0]; assert_eq!(branch1.files.len(), 1); @@ -1177,7 +1177,7 @@ fn test_merge_vbranch_upstream_conflict() -> Result<()> { merge_virtual_branch_upstream(&gb_repository, &project_repository, &branch1.id, None, None)?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches[0]; let contents = std::fs::read(std::path::Path::new(&project.path).join(file_path))?; @@ -1197,7 +1197,7 @@ fn test_merge_vbranch_upstream_conflict() -> Result<()> { )?; // make gb see the conflict resolution - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; assert!(branches[0].conflicted); // commit the merge resolution @@ -1212,7 +1212,7 @@ fn test_merge_vbranch_upstream_conflict() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches[0]; assert!(!branch1.conflicted); assert_eq!(branch1.files.len(), 0); @@ -1252,7 +1252,7 @@ fn test_unapply_ownership_partial() -> Result<()> { ) .expect("failed to create virtual branch"); - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; assert_eq!(branches.len(), 1); assert_eq!(branches[0].files.len(), 1); assert_eq!(branches[0].ownership.files.len(), 1); @@ -1270,7 +1270,7 @@ fn test_unapply_ownership_partial() -> Result<()> { ) .unwrap(); - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; assert_eq!(branches.len(), 1); assert_eq!(branches[0].files.len(), 0); assert_eq!(branches[0].ownership.files.len(), 0); @@ -1344,7 +1344,7 @@ fn test_apply_unapply_branch() -> Result<()> { let contents = std::fs::read(std::path::Path::new(&project.path).join(file_path2))?; assert_eq!("line5\nline6\n", String::from_utf8(contents)?); - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1); assert!(branch.active); @@ -1356,7 +1356,7 @@ fn test_apply_unapply_branch() -> Result<()> { let contents = std::fs::read(std::path::Path::new(&project.path).join(file_path2))?; assert_eq!("line5\nline6\n", String::from_utf8(contents)?); - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1); assert!(!branch.active); @@ -1370,7 +1370,7 @@ fn test_apply_unapply_branch() -> Result<()> { let contents = std::fs::read(std::path::Path::new(&project.path).join(file_path2))?; assert_eq!("line5\nline6\n", String::from_utf8(contents)?); - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1); assert!(branch.active); @@ -1626,7 +1626,7 @@ fn test_detect_mergeable_branch() -> Result<()> { }; branch_writer.write(&mut branch4)?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; assert_eq!(branches.len(), 4); let branch1 = &branches.iter().find(|b| b.id == branch1_id).unwrap(); @@ -1808,7 +1808,7 @@ fn test_upstream_integrated_vbranch() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert!(branch1.commits.iter().any(|c| c.is_integrated)); @@ -1855,7 +1855,7 @@ fn test_commit_same_hunk_twice() -> Result<()> { "line1\npatch1\nline2\nline3\nline4\nline5\nmiddle\nmiddle\nmiddle\nmiddle\nline6\nline7\nline8\nline9\nline10\nmiddle\nmiddle\nmiddle\nline11\nline12\n", )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1); @@ -1874,7 +1874,7 @@ fn test_commit_same_hunk_twice() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 0, "no files expected"); @@ -1894,7 +1894,7 @@ fn test_commit_same_hunk_twice() -> Result<()> { "line1\nPATCH1\nline2\nline3\nline4\nline5\nmiddle\nmiddle\nmiddle\nmiddle\nline6\nline7\nline8\nline9\nline10\nmiddle\nmiddle\nmiddle\nline11\nline12\n", )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1, "one file should be changed"); @@ -1911,7 +1911,7 @@ fn test_commit_same_hunk_twice() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!( @@ -1956,7 +1956,7 @@ fn test_commit_same_file_twice() -> Result<()> { "line1\npatch1\nline2\nline3\nline4\nline5\nmiddle\nmiddle\nmiddle\nmiddle\nline6\nline7\nline8\nline9\nline10\nmiddle\nmiddle\nmiddle\nline11\nline12\n", )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1); @@ -1975,7 +1975,7 @@ fn test_commit_same_file_twice() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 0, "no files expected"); @@ -1995,7 +1995,7 @@ fn test_commit_same_file_twice() -> Result<()> { "line1\npatch1\nline2\nline3\nline4\nline5\nmiddle\nmiddle\nmiddle\nmiddle\nline6\nline7\nline8\nline9\nline10\nmiddle\nmiddle\nmiddle\npatch2\nline11\nline12\n", )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1, "one file should be changed"); @@ -2012,7 +2012,7 @@ fn test_commit_same_file_twice() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!( @@ -2057,7 +2057,7 @@ fn test_commit_partial_by_hunk() -> Result<()> { "line1\npatch1\nline2\nline3\nline4\nline5\nmiddle\nmiddle\nmiddle\nmiddle\nline6\nline7\nline8\nline9\nline10\nmiddle\nmiddle\nmiddle\npatch2\nline11\nline12\n", )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1); @@ -2076,7 +2076,7 @@ fn test_commit_partial_by_hunk() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 1); @@ -2096,7 +2096,7 @@ fn test_commit_partial_by_hunk() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch = &branches.iter().find(|b| b.id == branch1_id).unwrap(); assert_eq!(branch.files.len(), 0); @@ -2163,7 +2163,7 @@ fn test_commit_partial_by_file() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches.iter().find(|b| b.id == branch1_id).unwrap(); // branch one test.txt has just the 1st and 3rd hunks applied @@ -2239,7 +2239,7 @@ fn test_commit_add_and_delete_files() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches.iter().find(|b| b.id == branch1_id).unwrap(); // branch one test.txt has just the 1st and 3rd hunks applied @@ -2310,7 +2310,7 @@ fn test_commit_executable_and_symlinks() -> Result<()> { false, )?; - let branches = list_virtual_branches(&gb_repository, &project_repository)?; + let (branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; let branch1 = &branches.iter().find(|b| b.id == branch1_id).unwrap(); let commit = &branch1.commits[0].id; @@ -2412,7 +2412,7 @@ fn test_verify_branch_commits_to_integration() -> Result<()> { integration::verify_branch(&gb_repository, &project_repository).unwrap(); // one virtual branch with two commits was created - let virtual_branches = list_virtual_branches(&gb_repository, &project_repository)?; + let virtual_(branches, _) = list_virtual_branches(&gb_repository, &project_repository)?; assert_eq!(virtual_branches.len(), 1); let branch = &virtual_branches.first().unwrap(); diff --git a/gitbutler-app/src/virtual_branches/virtual.rs b/gitbutler-app/src/virtual_branches/virtual.rs index 4a5bfcd1b..2f5bccc45 100644 --- a/gitbutler-app/src/virtual_branches/virtual.rs +++ b/gitbutler-app/src/virtual_branches/virtual.rs @@ -729,7 +729,7 @@ fn find_base_tree<'a>( pub fn list_virtual_branches( gb_repository: &gb_repository::Repository, project_repository: &project_repository::Repository, -) -> Result, errors::ListVirtualBranchesError> { +) -> Result<(Vec, bool), errors::ListVirtualBranchesError> { let mut branches: Vec = Vec::new(); let default_target = gb_repository @@ -908,7 +908,11 @@ pub fn list_virtual_branches( super::integration::update_gitbutler_integration(gb_repository, project_repository)?; - Ok(branches) + let uses_diff_context = project_repository + .project() + .use_diff_context + .unwrap_or(false); + Ok((branches, uses_diff_context)) } fn branches_with_large_files_abridged(mut branches: Vec) -> Vec { @@ -3851,8 +3855,12 @@ pub fn context_lines(project_repository: &project_repository::Repository) -> u32 .project() .use_diff_context .unwrap_or(false); - - if use_context { 3_u32 } else { 0_u32 } + + if use_context { + 3_u32 + } else { + 0_u32 + } } #[cfg(test)] diff --git a/gitbutler-app/src/watcher/handlers/caltulate_virtual_branches_handler.rs b/gitbutler-app/src/watcher/handlers/caltulate_virtual_branches_handler.rs index 8052ba3a8..23296f897 100644 --- a/gitbutler-app/src/watcher/handlers/caltulate_virtual_branches_handler.rs +++ b/gitbutler-app/src/watcher/handlers/caltulate_virtual_branches_handler.rs @@ -79,7 +79,7 @@ impl InnerHandler { .list_virtual_branches(project_id) .await { - Ok(branches) => Ok(vec![events::Event::Emit( + Ok((branches, _)) => Ok(vec![events::Event::Emit( app_events::Event::virtual_branches( project_id, &self.assets_proxy.proxy_virtual_branches(branches).await,