gitbutlerapp/gitbutler
1
1
Fork 0
mirror of https://github.com/gitbutlerapp/gitbutler.git synced 2024-12-19 15:41:31 +03:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3123 from gitbutlerapp/Virtual-branch-7

Browse Source 
perform proper path handling and vectorized writes in conflict resolver
This commit is contained in:
Josh Junon 2024-03-13 18:50:29 +01:00 committed by GitHub
commit 16140c7b4a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 60 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
gitbutler-app/src/project_repository/conflicts.rs
View File

@ -5,15 +5,24 @@
// conflicts are removed as they are resolved, the conflicts file is removed when there are no more conflicts // conflicts are removed as they are resolved, the conflicts file is removed when there are no more conflicts
// the merge parent file is removed when the merge is complete // the merge parent file is removed when the merge is complete
use std::io::{BufRead, Write}; use std::{
io::{BufRead, IoSlice, Write},
path::{Path, PathBuf},
};
use anyhow::Result; use anyhow::Result;
use itertools::Itertools;
use crate::git; use crate::git;
use super::Repository; use super::Repository;
pub fn mark(repository: &Repository, paths: &[String], parent: Option<git::Oid>) -> Result<()> { pub fn mark<P: AsRef<Path>, A: AsRef<[P]>>(
repository: &Repository,
paths: A,
parent: Option<git::Oid>,
) -> Result<()> {
let paths = paths.as_ref();
if paths.is_empty() { if paths.is_empty() {
return Ok(()); return Ok(());
} }
@ -21,8 +30,8 @@ pub fn mark(repository: &Repository, paths: &[String], parent: Option<git::Oid>)
// write all the file paths to a file on disk // write all the file paths to a file on disk
let mut file = std::fs::File::create(conflicts_path)?; let mut file = std::fs::File::create(conflicts_path)?;
for path in paths { for path in paths {
file.write_all(path.as_bytes()).unwrap(); file.write_all(path.as_ref().as_os_str().as_encoded_bytes())?;
file.write_all(b"\n").unwrap(); file.write_all(b"\n")?;
} }
if let Some(parent) = parent { if let Some(parent) = parent {
@ -53,12 +62,14 @@ pub fn merge_parent(repository: &Repository) -> Result<Option<git::Oid>> {
} }
} }
pub fn resolve(repository: &Repository, path: &str) -> Result<()> { pub fn resolve<P: AsRef<Path>>(repository: &Repository, path: P) -> Result<()> {
let path = path.as_ref();
let conflicts_path = repository.git_repository.path().join("conflicts"); let conflicts_path = repository.git_repository.path().join("conflicts");
let file = std::fs::File::open(conflicts_path.clone())?; let file = std::fs::File::open(conflicts_path.clone())?;
let reader = std::io::BufReader::new(file); let reader = std::io::BufReader::new(file);
let mut remaining = Vec::new(); let mut remaining = Vec::new();
for line in reader.lines().map_while(Result::ok) { for line in reader.lines().map_ok(PathBuf::from) {
let line = line?;
if line != path { if line != path {
remaining.push(line); remaining.push(line);
} }
@ -85,7 +96,7 @@ pub fn conflicting_files(repository: &Repository) -> Result<Vec<String>> {
Ok(reader.lines().map_while(Result::ok).collect()) Ok(reader.lines().map_while(Result::ok).collect())
} }
pub fn is_conflicting(repository: &Repository, path: Option<&str>) -> Result<bool> { pub fn is_conflicting<P: AsRef<Path>>(repository: &Repository, path: Option<P>) -> Result<bool> {
let conflicts_path = repository.git_repository.path().join("conflicts"); let conflicts_path = repository.git_repository.path().join("conflicts");
if !conflicts_path.exists() { if !conflicts_path.exists() {
return Ok(false); return Ok(false);
@ -93,17 +104,21 @@ pub fn is_conflicting(repository: &Repository, path: Option<&str>) -> Result<boo
let file = std::fs::File::open(conflicts_path)?; let file = std::fs::File::open(conflicts_path)?;
let reader = std::io::BufReader::new(file); let reader = std::io::BufReader::new(file);
let files = reader.lines().map_while(Result::ok).collect::<Vec<_>>(); let mut files = reader.lines().map_ok(PathBuf::from);
if let Some(pathname) = path { if let Some(pathname) = path {
let pathname = pathname.as_ref();
// check if pathname is one of the lines in conflicts_path file // check if pathname is one of the lines in conflicts_path file
for line in files { for line in files {
let line = line?;
if line == pathname { if line == pathname {
return Ok(true); return Ok(true);
} }
} }
Ok(false) Ok(false)
} else { } else {
Ok(!files.is_empty()) Ok(files.next().transpose().map(|x| x.is_some())?)
} }
} }

68
gitbutler-app/src/virtual_branches/virtual.rs
View File

@ -1,4 +1,8 @@
use std::{collections::HashMap, path, time, vec}; use std::{
collections::HashMap,
path::{Path, PathBuf},
time, vec,
};
#[cfg(target_family = "unix")] #[cfg(target_family = "unix")]
use std::os::unix::prelude::*; use std::os::unix::prelude::*;
@ -28,12 +32,12 @@ use super::{
branch_to_remote_branch, context, errors, target, Iterator, RemoteBranch, branch_to_remote_branch, context, errors, target, Iterator, RemoteBranch,
}; };
type AppliedStatuses = Vec<(branch::Branch, HashMap<path::PathBuf, Vec<diff::Hunk>>)>; type AppliedStatuses = Vec<(branch::Branch, HashMap<PathBuf, Vec<diff::Hunk>>)>;
#[derive(Debug, thiserror::Error)] #[derive(Debug, thiserror::Error)]
pub enum Error { pub enum Error {
#[error("path contains invalid utf-8 characters: {0}")] #[error("path contains invalid utf-8 characters: {0}")]
InvalidUnicodePath(path::PathBuf), InvalidUnicodePath(PathBuf),
} }
// this struct is a mapping to the view `Branch` type in Typescript // this struct is a mapping to the view `Branch` type in Typescript
@ -106,7 +110,7 @@ pub struct VirtualBranchCommit {
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub struct VirtualBranchFile { pub struct VirtualBranchFile {
pub id: String, pub id: String,
pub path: path::PathBuf, pub path: PathBuf,
pub hunks: Vec<VirtualBranchHunk>, pub hunks: Vec<VirtualBranchHunk>,
pub modified_at: u128, pub modified_at: u128,
pub conflicted: bool, pub conflicted: bool,
@ -128,7 +132,7 @@ pub struct VirtualBranchHunk {
pub id: String, pub id: String,
pub diff: String, pub diff: String,
pub modified_at: u128, pub modified_at: u128,
pub file_path: path::PathBuf, pub file_path: PathBuf,
pub hash: String, pub hash: String,
pub old_start: u32, pub old_start: u32,
pub start: u32, pub start: u32,
@ -497,7 +501,7 @@ pub fn unapply_ownership(
let hunks_to_unapply = applied_statuses let hunks_to_unapply = applied_statuses
.iter() .iter()
.map( .map(
|(branch, branch_files)| -> Result<Vec<(std::path::PathBuf, diff::Hunk)>> { |(branch, branch_files)| -> Result<Vec<(PathBuf, diff::Hunk)>> {
let branch_files = calculate_non_commited_diffs( let branch_files = calculate_non_commited_diffs(
project_repository, project_repository,
branch, branch,
@ -595,14 +599,14 @@ pub fn reset_files(
let repo = &project_repository.git_repository; let repo = &project_repository.git_repository;
let index = repo.index().context("failed to get index")?; let index = repo.index().context("failed to get index")?;
for file in files { for file in files {
let entry = index.get_path(path::Path::new(file), 0); let entry = index.get_path(Path::new(file), 0);
if entry.is_some() { if entry.is_some() {
repo.checkout_index_path(path::Path::new(file)) repo.checkout_index_path(Path::new(file))
.context("failed to checkout index")?; .context("failed to checkout index")?;
} else { } else {
// find the project root // find the project root
let project_root = &project_repository.project().path; let project_root = &project_repository.project().path;
let path = path::Path::new(file); let path = Path::new(file);
//combine the project root with the file path //combine the project root with the file path
let path = &project_root.join(path); let path = &project_root.join(path);
std::fs::remove_file(path).context("failed to remove file")?; std::fs::remove_file(path).context("failed to remove file")?;
@ -1122,8 +1126,8 @@ pub fn calculate_non_commited_diffs(
project_repository: &project_repository::Repository, project_repository: &project_repository::Repository,
branch: &branch::Branch, branch: &branch::Branch,
default_target: &target::Target, default_target: &target::Target,
files: &HashMap<path::PathBuf, Vec<diff::Hunk>>, files: &HashMap<PathBuf, Vec<diff::Hunk>>,
) -> Result<HashMap<path::PathBuf, Vec<diff::Hunk>>> { ) -> Result<HashMap<PathBuf, Vec<diff::Hunk>>> {
if default_target.sha == branch.head && !branch.applied { if default_target.sha == branch.head && !branch.applied {
return Ok(files.clone()); return Ok(files.clone());
}; };
@ -1369,7 +1373,7 @@ pub fn merge_virtual_branch_upstream(
signing_key: Option<&keys::PrivateKey>, signing_key: Option<&keys::PrivateKey>,
user: Option<&users::User>, user: Option<&users::User>,
) -> Result<(), errors::MergeVirtualBranchUpstreamError> { ) -> Result<(), errors::MergeVirtualBranchUpstreamError> {
if conflicts::is_conflicting(project_repository, None)? { if conflicts::is_conflicting::<&Path>(project_repository, None)? {
return Err(errors::MergeVirtualBranchUpstreamError::Conflict( return Err(errors::MergeVirtualBranchUpstreamError::Conflict(
errors::ProjectConflictError { errors::ProjectConflictError {
project_id: project_repository.project().id, project_id: project_repository.project().id,
@ -1802,7 +1806,7 @@ fn set_ownership(
Ok(()) Ok(())
} }
fn get_mtime(cache: &mut HashMap<path::PathBuf, u128>, file_path: &path::PathBuf) -> u128 { fn get_mtime(cache: &mut HashMap<PathBuf, u128>, file_path: &PathBuf) -> u128 {
if let Some(mtime) = cache.get(file_path) { if let Some(mtime) = cache.get(file_path) {
*mtime *mtime
} else { } else {
@ -1836,10 +1840,10 @@ fn diff_hash(diff: &str) -> String {
} }
pub fn virtual_hunks_by_filepath( pub fn virtual_hunks_by_filepath(
project_path: &path::Path, project_path: &Path,
diff: &HashMap<path::PathBuf, Vec<diff::Hunk>>, diff: &HashMap<PathBuf, Vec<diff::Hunk>>,
) -> HashMap<path::PathBuf, Vec<VirtualBranchHunk>> { ) -> HashMap<PathBuf, Vec<VirtualBranchHunk>> {
let mut mtimes: HashMap<path::PathBuf, u128> = HashMap::new(); let mut mtimes: HashMap<PathBuf, u128> = HashMap::new();
diff.iter() diff.iter()
.map(|(file_path, hunks)| { .map(|(file_path, hunks)| {
let hunks = hunks let hunks = hunks
@ -1864,7 +1868,7 @@ pub fn virtual_hunks_by_filepath(
.collect::<HashMap<_, _>>() .collect::<HashMap<_, _>>()
} }
pub type BranchStatus = HashMap<path::PathBuf, Vec<diff::Hunk>>; pub type BranchStatus = HashMap<PathBuf, Vec<diff::Hunk>>;
// list the virtual branches and their file statuses (statusi?) // list the virtual branches and their file statuses (statusi?)
#[allow(clippy::type_complexity)] #[allow(clippy::type_complexity)]
@ -1937,7 +1941,7 @@ fn get_non_applied_status(
virtual_branches virtual_branches
.into_iter() .into_iter()
.map( .map(
|branch| -> Result<(branch::Branch, HashMap<path::PathBuf, Vec<diff::Hunk>>)> { |branch| -> Result<(branch::Branch, HashMap<PathBuf, Vec<diff::Hunk>>)> {
if branch.applied { if branch.applied {
bail!("branch {} is applied", branch.name); bail!("branch {} is applied", branch.name);
} }
@ -1983,7 +1987,7 @@ fn get_applied_status(
) )
.context("failed to diff workdir")?; .context("failed to diff workdir")?;
let mut diff: HashMap<path::PathBuf, Vec<git::diff::Hunk>> = HashMap::new(); let mut diff: HashMap<PathBuf, Vec<git::diff::Hunk>> = HashMap::new();
let mut skipped_files: Vec<diff::DiffFile> = Vec::new(); let mut skipped_files: Vec<diff::DiffFile> = Vec::new();
for (file_path, diff_file) in diff_files { for (file_path, diff_file) in diff_files {
if diff_file.skipped { if diff_file.skipped {
@ -2010,7 +2014,7 @@ fn get_applied_status(
// - update shifted hunks // - update shifted hunks
// - remove non existent hunks // - remove non existent hunks
let mut hunks_by_branch_id: HashMap<BranchId, HashMap<path::PathBuf, Vec<diff::Hunk>>> = let mut hunks_by_branch_id: HashMap<BranchId, HashMap<PathBuf, Vec<diff::Hunk>>> =
virtual_branches virtual_branches
.iter() .iter()
.map(|branch| (branch.id, HashMap::new())) .map(|branch| (branch.id, HashMap::new()))
@ -2176,7 +2180,7 @@ fn virtual_hunks_to_virtual_files(
) -> Vec<VirtualBranchFile> { ) -> Vec<VirtualBranchFile> {
hunks hunks
.iter() .iter()
.fold(HashMap::<path::PathBuf, Vec<_>>::new(), |mut acc, hunk| { .fold(HashMap::<PathBuf, Vec<_>>::new(), |mut acc, hunk| {
acc.entry(hunk.file_path.clone()) acc.entry(hunk.file_path.clone())
.or_default() .or_default()
.push(hunk.clone()); .push(hunk.clone());
@ -2258,7 +2262,7 @@ pub fn reset_branch(
fn diffs_to_virtual_files( fn diffs_to_virtual_files(
project_repository: &project_repository::Repository, project_repository: &project_repository::Repository,
diffs: &HashMap<path::PathBuf, Vec<diff::Hunk>>, diffs: &HashMap<PathBuf, Vec<diff::Hunk>>,
) -> Vec<VirtualBranchFile> { ) -> Vec<VirtualBranchFile> {
let hunks_by_filepath = virtual_hunks_by_filepath(&project_repository.project().path, diffs); let hunks_by_filepath = virtual_hunks_by_filepath(&project_repository.project().path, diffs);
virtual_hunks_to_virtual_files( virtual_hunks_to_virtual_files(
@ -2277,7 +2281,7 @@ fn diffs_to_virtual_files(
pub fn write_tree( pub fn write_tree(
project_repository: &project_repository::Repository, project_repository: &project_repository::Repository,
target: &target::Target, target: &target::Target,
files: &HashMap<path::PathBuf, Vec<diff::Hunk>>, files: &HashMap<PathBuf, Vec<diff::Hunk>>,
) -> Result<git::Oid> { ) -> Result<git::Oid> {
write_tree_onto_commit(project_repository, target.sha, files) write_tree_onto_commit(project_repository, target.sha, files)
} }
@ -2285,7 +2289,7 @@ pub fn write_tree(
pub fn write_tree_onto_commit( pub fn write_tree_onto_commit(
project_repository: &project_repository::Repository, project_repository: &project_repository::Repository,
commit_oid: git::Oid, commit_oid: git::Oid,
files: &HashMap<path::PathBuf, Vec<diff::Hunk>>, files: &HashMap<PathBuf, Vec<diff::Hunk>>,
) -> Result<git::Oid> { ) -> Result<git::Oid> {
// read the base sha into an index // read the base sha into an index
let git_repository = &project_repository.git_repository; let git_repository = &project_repository.git_repository;
@ -2299,14 +2303,14 @@ pub fn write_tree_onto_commit(
pub fn write_tree_onto_tree( pub fn write_tree_onto_tree(
project_repository: &project_repository::Repository, project_repository: &project_repository::Repository,
base_tree: &git::Tree, base_tree: &git::Tree,
files: &HashMap<path::PathBuf, Vec<diff::Hunk>>, files: &HashMap<PathBuf, Vec<diff::Hunk>>,
) -> Result<git::Oid> { ) -> Result<git::Oid> {
let git_repository = &project_repository.git_repository; let git_repository = &project_repository.git_repository;
let mut builder = git_repository.treebuilder(Some(base_tree)); let mut builder = git_repository.treebuilder(Some(base_tree));
// now update the index with content in the working directory for each file // now update the index with content in the working directory for each file
for (filepath, hunks) in files { for (filepath, hunks) in files {
// convert this string to a Path // convert this string to a Path
let rel_path = std::path::Path::new(&filepath); let rel_path = Path::new(&filepath);
let full_path = project_repository.path().join(rel_path); let full_path = project_repository.path().join(rel_path);
let is_submodule = let is_submodule =
@ -2503,7 +2507,7 @@ pub fn commit(
})?; })?;
let files = calculate_non_commited_diffs(project_repository, branch, &default_target, files)?; let files = calculate_non_commited_diffs(project_repository, branch, &default_target, files)?;
if conflicts::is_conflicting(project_repository, None)? { if conflicts::is_conflicting::<&Path>(project_repository, None)? {
return Err(errors::CommitError::Conflicted( return Err(errors::CommitError::Conflicted(
errors::ProjectConflictError { errors::ProjectConflictError {
project_id: project_repository.project().id, project_id: project_repository.project().id,
@ -2887,7 +2891,7 @@ pub fn amend(
branch_id: &BranchId, branch_id: &BranchId,
target_ownership: &Ownership, target_ownership: &Ownership,
) -> Result<git::Oid, errors::AmendError> { ) -> Result<git::Oid, errors::AmendError> {
if conflicts::is_conflicting(project_repository, None)? { if conflicts::is_conflicting::<&Path>(project_repository, None)? {
return Err(errors::AmendError::Conflict(errors::ProjectConflictError { return Err(errors::AmendError::Conflict(errors::ProjectConflictError {
project_id: project_repository.project().id, project_id: project_repository.project().id,
})); }));
@ -3056,7 +3060,7 @@ pub fn cherry_pick(
branch_id: &BranchId, branch_id: &BranchId,
target_commit_oid: git::Oid, target_commit_oid: git::Oid,
) -> Result<Option<git::Oid>, errors::CherryPickError> { ) -> Result<Option<git::Oid>, errors::CherryPickError> {
if conflicts::is_conflicting(project_repository, None)? { if conflicts::is_conflicting::<&Path>(project_repository, None)? {
return Err(errors::CherryPickError::Conflict( return Err(errors::CherryPickError::Conflict(
errors::ProjectConflictError { errors::ProjectConflictError {
project_id: project_repository.project().id, project_id: project_repository.project().id,
@ -3245,7 +3249,7 @@ pub fn squash(
branch_id: &BranchId, branch_id: &BranchId,
commit_oid: git::Oid, commit_oid: git::Oid,
) -> Result<(), errors::SquashError> { ) -> Result<(), errors::SquashError> {
if conflicts::is_conflicting(project_repository, None)? { if conflicts::is_conflicting::<&Path>(project_repository, None)? {
return Err(errors::SquashError::Conflict( return Err(errors::SquashError::Conflict(
errors::ProjectConflictError { errors::ProjectConflictError {
project_id: project_repository.project().id, project_id: project_repository.project().id,
@ -3437,7 +3441,7 @@ pub fn update_commit_message(
return Err(errors::UpdateCommitMessageError::EmptyMessage); return Err(errors::UpdateCommitMessageError::EmptyMessage);
} }
if conflicts::is_conflicting(project_repository, None)? { if conflicts::is_conflicting::<&Path>(project_repository, None)? {
return Err(errors::UpdateCommitMessageError::Conflict( return Err(errors::UpdateCommitMessageError::Conflict(
errors::ProjectConflictError { errors::ProjectConflictError {
project_id: project_repository.project().id, project_id: project_repository.project().id,