conflict::*()functions don't impose UTF-8 on paths

That way it's compatible to more kinds of paths, on more filesystem.
It assumes that the content of the `conflict` file is never moved
across platforms.

Cargo.lock

@@ -2020,6 +2020,7 @@ dependencies = [
  "gitbutler-time",
  "gitbutler-url",
  "gitbutler-user",
+ "gix",
  "glob",
  "hex",
  "itertools 0.13.0",

crates/gitbutler-branch-actions/Cargo.toml

@@ -9,6 +9,7 @@ publish = false
 tracing = "0.1.40"
 anyhow = "1.0.86"
 git2.workspace = true
+gix.workspace = true
 tokio.workspace = true
 gitbutler-oplog.workspace = true
 gitbutler-repo.workspace = true

crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs

@@ -16,6 +16,7 @@ use gitbutler_project::access::WorktreeWritePermission;
 use gitbutler_reference::Refname;
 use gitbutler_repo::{rebase::cherry_rebase, RepoActionsExt, RepositoryExt};
 use gitbutler_time::time::now_since_unix_epoch_ms;
+use std::borrow::Cow;
 
 impl BranchManager<'_> {
     pub fn create_virtual_branch(
@@ -346,9 +347,7 @@ impl BranchManager<'_> {
                 let mut merge_conflicts = Vec::new();
                 for path in conflicts.flatten() {
                     if let Some(ours) = path.our {
-                        let path = std::str::from_utf8(&ours.path)
+                        let path = gix::path::try_from_bstr(Cow::Owned(ours.path.into()))?;
-                            .context("failed to convert path to utf8")?
-                            .to_string();
                         merge_conflicts.push(path);
                     }
                 }
@@ -472,9 +471,7 @@ impl BranchManager<'_> {
             let mut merge_conflicts = Vec::new();
             for path in conflicts.flatten() {
                 if let Some(ours) = path.our {
-                    let path = std::str::from_utf8(&ours.path)
+                    let path = gix::path::try_from_bstr(Cow::Owned(ours.path.into()))?;
-                        .context("failed to convert path to utf8")?
-                        .to_string();
                     merge_conflicts.push(path);
                 }
             }

crates/gitbutler-branch-actions/src/conflicts.rs

@@ -1,3 +1,8 @@
+use anyhow::{anyhow, Context, Result};
+use bstr::ByteSlice;
+use gitbutler_command_context::ProjectRepository;
+use gitbutler_error::error::Marker;
+use std::ffi::OsStr;
 /// stuff to manage merge conflict state.
 /// This is the dumbest possible way to do this, but it is a placeholder.
 /// Conflicts are stored one path per line in .git/conflicts.
@@ -5,18 +10,12 @@
 /// Conflicts are removed as they are resolved, the conflicts file is removed when there are no more conflicts
 /// or when the merge is complete.
 use std::{
-    io::{BufRead, Write},
+    io::Write,
     path::{Path, PathBuf},
 };
 
-use anyhow::{anyhow, Context, Result};
-use gitbutler_command_context::ProjectRepository;
-use itertools::Itertools;
-
-use gitbutler_error::error::Marker;
-
 pub(crate) fn mark<P: AsRef<Path>, A: AsRef<[P]>>(
-    repository: &ProjectRepository,
+    ctx: &ProjectRepository,
     paths: A,
     parent: Option<git2::Oid>,
 ) -> Result<()> {
@@ -30,21 +29,28 @@ pub(crate) fn mark<P: AsRef<Path>, A: AsRef<[P]>>(
         buf.write_all(path.as_ref().as_os_str().as_encoded_bytes())?;
         buf.write_all(b"\n")?;
     }
-    gitbutler_fs::write(repository.repo().path().join("conflicts"), buf)?;
+    gitbutler_fs::write(conflicts_path(ctx), &buf)?;
 
     if let Some(parent) = parent {
         // write all the file paths to a file on disk
-        gitbutler_fs::write(
+        gitbutler_fs::write(merge_parent_path(ctx), parent.to_string().as_bytes())?;
-            repository.repo().path().join("base_merge_parent"),
-            parent.to_string().as_bytes(),
-        )?;
     }
 
     Ok(())
 }
 
-pub(crate) fn merge_parent(repository: &ProjectRepository) -> Result<Option<git2::Oid>> {
+fn conflicts_path(ctx: &ProjectRepository) -> PathBuf {
-    let merge_path = repository.repo().path().join("base_merge_parent");
+    ctx.repo().path().join("conflicts")
+}
+
+fn merge_parent_path(ctx: &ProjectRepository) -> PathBuf {
+    ctx.repo().path().join("base_merge_parent")
+}
+
+pub(crate) fn merge_parent(ctx: &ProjectRepository) -> Result<Option<git2::Oid>> {
+    use std::io::BufRead;
+
+    let merge_path = merge_parent_path(ctx);
     if !merge_path.exists() {
         return Ok(None);
     }
@@ -61,78 +67,71 @@ pub(crate) fn merge_parent(repository: &ProjectRepository) -> Result<Option<git2
     }
 }
 
-pub fn resolve<P: AsRef<Path>>(repository: &ProjectRepository, path: P) -> Result<()> {
+pub fn resolve<P: AsRef<Path>>(ctx: &ProjectRepository, path_to_resolve: P) -> Result<()> {
-    let path = path.as_ref();
+    let path_to_resolve = path_to_resolve.as_ref();
-    let conflicts_path = repository.repo().path().join("conflicts");
+    let path_to_resolve = path_to_resolve.as_os_str().as_encoded_bytes();
-    let file = std::fs::File::open(conflicts_path.clone())?;
+    let conflicts_path = conflicts_path(ctx);
-    let reader = std::io::BufReader::new(file);
+    let path_per_line = std::fs::read(&conflicts_path)?;
-    let mut remaining = Vec::new();
+    let remaining: Vec<_> = path_per_line
-    for line in reader.lines().map_ok(PathBuf::from) {
+        .lines()
-        let line = line?;
+        .filter(|path| *path != path_to_resolve)
-        if line != path {
+        .map(|path| unsafe { OsStr::from_encoded_bytes_unchecked(path) })
-            remaining.push(line);
+        .collect();
-        }
-    }
 
     // re-write file if needed, otherwise remove file entirely
     if remaining.is_empty() {
         std::fs::remove_file(conflicts_path)?;
     } else {
-        mark(repository, &remaining, None)?;
+        mark(ctx, &remaining, None)?;
     }
     Ok(())
 }
 
-pub(crate) fn conflicting_files(repository: &ProjectRepository) -> Result<Vec<String>> {
+pub(crate) fn conflicting_files(ctx: &ProjectRepository) -> Result<Vec<PathBuf>> {
-    let conflicts_path = repository.repo().path().join("conflicts");
+    let conflicts_path = conflicts_path(ctx);
     if !conflicts_path.exists() {
         return Ok(vec![]);
     }
 
-    let file = std::fs::File::open(conflicts_path)?;
+    let path_per_line = std::fs::read(conflicts_path)?;
-    let reader = std::io::BufReader::new(file);
+    Ok(path_per_line
-    Ok(reader.lines().map_while(Result::ok).collect())
+        .lines()
+        .map(|path| unsafe { OsStr::from_encoded_bytes_unchecked(path) }.into())
+        .collect())
 }
 
 /// Check if `path` is conflicting in `repository`, or if `None`, check if there is any conflict.
 // TODO(ST): Should this not rather check the conflicting state in the index?
 pub(crate) fn is_conflicting(repository: &ProjectRepository, path: Option<&Path>) -> Result<bool> {
-    let conflicts_path = repository.repo().path().join("conflicts");
+    let conflicts_path = conflicts_path(repository);
     if !conflicts_path.exists() {
         return Ok(false);
     }
 
-    let file = std::fs::File::open(conflicts_path)?;
+    let path_per_line = std::fs::read(conflicts_path)?;
-    let reader = std::io::BufReader::new(file);
+    let mut files = path_per_line
-    // TODO(ST): This shouldn't work on UTF8 strings.
+        .lines()
-    let mut files = reader.lines().map_ok(PathBuf::from);
+        .map(|path| unsafe { OsStr::from_encoded_bytes_unchecked(path) });
-    if let Some(pathname) = path {
+    let is_in_conflicts_file_or_has_conflicts = if let Some(path) = path {
-        // check if pathname is one of the lines in conflicts_path file
+        files.any(|p| p == path)
-        for line in files {
-            let line = line?;
-
-            if line == pathname {
-                return Ok(true);
-            }
-        }
-        Ok(false)
     } else {
-        Ok(files.next().transpose().map(|x| x.is_some())?)
+        files.next().is_some()
-    }
+    };
+    Ok(is_in_conflicts_file_or_has_conflicts)
 }
 
 // is this project still in a resolving conflict state?
 // - could be that there are no more conflicts, but the state is not committed
-pub(crate) fn is_resolving(repository: &ProjectRepository) -> bool {
+pub(crate) fn is_resolving(ctx: &ProjectRepository) -> bool {
-    repository.repo().path().join("base_merge_parent").exists()
+    merge_parent_path(ctx).exists()
 }
 
-pub(crate) fn clear(repository: &ProjectRepository) -> Result<()> {
+pub(crate) fn clear(ctx: &ProjectRepository) -> Result<()> {
-    let merge_path = repository.repo().path().join("base_merge_parent");
+    let merge_path = merge_parent_path(ctx);
     std::fs::remove_file(merge_path)?;
 
-    for file in conflicting_files(repository)? {
+    for file in conflicting_files(ctx)? {
-        resolve(repository, &file)?;
+        resolve(ctx, &file)?;
     }
 
     Ok(())

crates/gitbutler-branch-actions/src/virtual.rs

@@ -12,7 +12,7 @@ use gitbutler_repo::credentials::Helper;
 use gitbutler_repo::{LogUntil, RepoActionsExt, RepositoryExt};
 use itertools::Itertools;
 use md5::Digest;
-use std::borrow::Borrow;
+use std::borrow::{Borrow, Cow};
 #[cfg(target_family = "unix")]
 use std::os::unix::prelude::PermissionsExt;
 use std::time::SystemTime;
@@ -835,8 +835,8 @@ pub(crate) fn integrate_with_merge(
         let merge_conflicts = conflicts
             .flatten()
             .filter_map(|c| c.our)
-            .map(|our| std::string::String::from_utf8_lossy(&our.path).to_string())
+            .map(|our| gix::path::try_from_bstr(Cow::Owned(our.path.into())))
-            .collect::<Vec<_>>();
+            .collect::<Result<Vec<_>, _>>()?;
         conflicts::mark(
             project_repository,
             merge_conflicts,
@@ -1386,7 +1386,7 @@ fn virtual_hunks_into_virtual_files(
         .map(|(path, hunks)| {
             let id = path.display().to_string();
             let conflicted =
-                conflicts::is_conflicting(project_repository, Some(id.as_ref())).unwrap_or(false);
+                conflicts::is_conflicting(project_repository, Some(&path)).unwrap_or(false);
             let binary = hunks.iter().any(|h| h.binary);
             let modified_at = hunks.iter().map(|h| h.modified_at).max().unwrap_or(0);
             debug_assert!(hunks.iter().all(|hunk| hunk.file_path == path));
@@ -2913,7 +2913,7 @@ fn update_conflict_markers(
     let conflicting_files = conflicts::conflicting_files(project_repository)?;
     for (file_path, non_commited_hunks) in files {
         let mut conflicted = false;
-        if conflicting_files.contains(&file_path.display().to_string()) {
+        if conflicting_files.contains(file_path) {
             // check file for conflict markers, resolve the file if there are none in any hunk
             for hunk in non_commited_hunks {
                 if hunk.diff_lines.contains_str(b"<<<<<<< ours") {
@@ -2924,7 +2924,7 @@ fn update_conflict_markers(
                 }
             }
             if !conflicted {
-                conflicts::resolve(project_repository, file_path.display().to_string()).unwrap();
+                conflicts::resolve(project_repository, file_path).unwrap();
             }
         }
     }