Another step towards removing git2

AGENTS.md

@@ -3,3 +3,5 @@
 When asked about general development workflow, project structure, building, testing, or contributing to GitButler, please read `@.github/copilot-instructions.md` for context.
 
 When working on the `but` CLI (Rust command-line tool in `crates/but`), please read `@crates/but/agents.md` for context on API usage patterns, output formatting, and testing conventions.
+
+When tests need to observe repository changes written to disk, prefer `repo.reload()` over reopening the repository.

crates/but-api/Cargo.toml

@@ -75,7 +75,6 @@ legacy = [
 but-serde.workspace = true
 but-path.workspace = true
 but-api-macros.workspace = true
-but-oxidize.workspace = true
 but-error.workspace = true
 but-core.workspace = true
 but-graph.workspace = true

crates/but-api/src/legacy/git.rs

@@ -3,7 +3,7 @@ use anyhow::{Context as _, Result};
 use bstr::ByteSlice;
 use but_api_macros::but_api;
 use but_core::git_config::{
-    open_user_global_config_for_editing, remove_config_value, set_config_value, write_config,
+    edit_config, open_global_config_for_reading, remove_config_value, set_config_value,
 };
 use gitbutler_reference::RemoteRefname;
 use gitbutler_repo_actions::RepoActionsExt as _;
@@ -80,25 +80,27 @@ pub fn delete_all_data() -> Result<()> {
 #[but_api]
 #[instrument(err(Debug))]
 pub fn git_set_global_config(key: String, value: String) -> Result<String> {
-    let (mut config, path) = open_user_global_config_for_editing()?;
-    set_config_value(&mut config, &key, &value)?;
-    write_config(&path, &config)?;
+    _ = edit_config(None, gix::config::Source::User, |config| {
+        set_config_value(config, &key, &value)?;
+        Ok(())
+    })?;
     Ok(value)
 }
 
 #[but_api]
 #[instrument(err(Debug))]
 pub fn git_remove_global_config(key: String) -> Result<()> {
-    let (mut config, path) = open_user_global_config_for_editing()?;
-    remove_config_value(&mut config, &key)?;
-    write_config(&path, &config)?;
+    _ = edit_config(None, gix::config::Source::User, |config| {
+        remove_config_value(config, &key)?;
+        Ok(())
+    })?;
     Ok(())
 }
 
 #[but_api]
 #[instrument(err(Debug))]
 pub fn git_get_global_config(key: String) -> Result<Option<String>> {
-    let (config, _) = open_user_global_config_for_editing()?;
+    let config = open_global_config_for_reading()?;
     Ok(get_config_string(&config, &key))
 }
 

crates/but-api/src/legacy/projects.rs

@@ -1,8 +1,9 @@
 use std::path::PathBuf;
 
-use anyhow::Result;
+use anyhow::{Result, anyhow};
 use but_api_macros::but_api;
 use but_ctx::{Context, ProjectHandleOrLegacyProjectId};
+use but_error::Code;
 use tracing::instrument;
 
 use super::legacy_project;
@@ -116,12 +117,27 @@ pub fn delete_project(project_id: ProjectHandleOrLegacyProjectId) -> Result<()>
 /// the legacy metadata view with the workspace currently present in Git. It is safe for activation
 /// paths because it avoids rewriting `gitbutler/workspace`.
 pub fn prepare_project_for_activation(ctx: &mut Context) -> Result<()> {
+    assure_repo_ownership(&*ctx.repo.get()?)?;
     let mut guard = ctx.exclusive_worktree_access();
     gitbutler_branch_actions::base::bootstrap_default_target_if_missing(ctx)?;
     super::meta::reconcile_in_workspace_state_of_vb_toml(ctx, guard.write_permission()).ok();
     Ok(())
 }
 
+// TODO(gix): remove this once there is no `git2` as `gix` provides safety by not trusting Git configuration instead.
+fn assure_repo_ownership(repo: &gix::Repository) -> Result<()> {
+    if repo.git_dir_trust() == gix::sec::Trust::Full {
+        return Ok(());
+    }
+
+    let path = repo.workdir().unwrap_or(repo.git_dir());
+    Err(anyhow!(
+        "The git directory is considered unsafe as it's not owned by the current user. Use `git config --global --add safe.directory '{}'` to allow it",
+        path.display()
+    )
+    .context(Code::RepoOwnership))
+}
+
 #[but_api]
 #[instrument(err(Debug))]
 pub fn is_gerrit(ctx: &but_ctx::Context) -> Result<bool> {

crates/but-api/src/legacy/repo.rs

@@ -4,7 +4,6 @@ use anyhow::{Context as _, Result};
 use but_api_macros::but_api;
 use but_core::DiffSpec;
 use but_ctx::Context;
-use but_oxidize::ObjectIdExt;
 use gitbutler_branch_actions::hooks;
 use gitbutler_repo::{
     FileInfo, RepoCommands,
@@ -54,7 +53,7 @@ pub fn get_commit_file(
     relative_path: PathBuf,
     commit_id: gix::ObjectId,
 ) -> Result<FileInfo> {
-    ctx.read_file_from_commit(commit_id.to_git2(), &relative_path)
+    ctx.read_file_from_commit(commit_id, &relative_path)
 }
 
 #[but_api]

crates/but-core/src/git_config.rs

@@ -1,54 +1,120 @@
 //! Utilities for mutating Git configuration entries by dotted key.
 
-use std::path::{Path, PathBuf};
+use std::path::PathBuf;
 
-use anyhow::{Context as _, Result};
+use anyhow::{Context as _, Result, bail};
 use bstr::ByteSlice as _;
 use gix::config::AsKey as _;
 
-/// Open the user-global Git config for editing, creating it first if needed.
-pub fn open_user_global_config_for_editing() -> Result<(gix::config::File<'static>, PathBuf)> {
-    let path = gix::config::Source::User
+fn config_path(repo: Option<&gix::Repository>, source: gix::config::Source) -> Result<PathBuf> {
+    let path = source
         .storage_location(&mut |name| std::env::var_os(name))
-        .context("failed to determine global git config location")?
-        .into_owned();
+        .with_context(|| format!("failed to determine {source:?} git config location"))?;
+    let path = if path.is_relative() {
+        let repo = repo.with_context(|| {
+            format!("determining the {source:?} git config location requires a repository")
+        })?;
+        if source == gix::config::Source::Local {
+            repo.common_dir().join(path.as_ref())
+        } else {
+            repo.git_dir().join(path.as_ref())
+        }
+    } else {
+        path.into_owned()
+    };
+    Ok(path)
+}
+
+/// Open the Git config for `source` for editing, creating it first if needed.
+/// `repo` is used to resolve repo-local paths, depending on `source`.
+/// Return `(config, lock)`.
+/// Write it back with [`write_locked_config()`].
+pub fn open_config_for_editing(
+    repo: Option<&gix::Repository>,
+    source: gix::config::Source,
+) -> Result<(gix::config::File<'static>, gix::lock::File)> {
+    let path = config_path(repo, source)?;
+    std::fs::create_dir_all(path.parent().context("git config path has no parent")?)?;
+    let lock = gix::lock::File::acquire_to_update_resource(
+        &path,
+        gix::lock::acquire::Fail::Immediately,
+        None,
+    )?;
     if !path.exists() {
-        std::fs::create_dir_all(
-            path.parent()
-                .context("global git config path has no parent")?,
-        )?;
         std::fs::File::create(&path)?;
     }
-    let config = gix::config::File::from_path_no_includes(path.clone(), gix::config::Source::User)
-        .with_context(|| format!("failed to open global git config at {}", path.display()))?;
-    Ok((config, path))
+    let config = gix::config::File::from_path_no_includes(path.clone(), source)
+        .with_context(|| format!("failed to open {source:?} git config at {}", path.display()))?;
+    Ok((config, lock))
+}
+
+/// Open the user-global Git config for reading without acquiring a write lock.
+///
+/// If the config file doesn't exist yet, an empty in-memory config is returned.
+pub fn open_global_config_for_reading() -> Result<gix::config::File<'static>> {
+    let path = config_path(None, gix::config::Source::User)?;
+    if !path.exists() {
+        return Ok(gix::config::File::new(gix::config::file::Metadata::from(
+            gix::config::Source::User,
+        )));
+    }
+    gix::config::File::from_path_no_includes(path.clone(), gix::config::Source::User)
+        .with_context(|| format!("failed to open User git config at {}", path.display()))
 }
 
-/// Serialize a Git `config` file back to disk at `path`.
-pub fn write_config(path: &Path, config: &gix::config::File<'_>) -> Result<()> {
-    let mut file = std::io::BufWriter::new(
-        std::fs::OpenOptions::new()
-            .write(true)
-            .truncate(true)
-            .create(true)
-            .open(path)
-            .with_context(|| {
-                format!(
-                    "failed to open git config for writing at {}",
-                    path.display()
-                )
-            })?,
-    );
+/// Serialize a Git `config` file back to disk at `lock`.
+pub fn write_locked_config(
+    config: &gix::config::File<'_>,
+    mut lock: gix::lock::File,
+) -> Result<()> {
+    let path = lock.resource_path();
     config
-        .write_to(&mut file)
+        .write_to(&mut lock)
         .with_context(|| format!("failed to serialize git config at {}", path.display()))?;
-    std::io::Write::flush(&mut file)
+    std::io::Write::flush(&mut lock)
         .with_context(|| format!("failed to flush git config at {}", path.display()))?;
+    lock.commit()
+        .map_err(|err| err.error)
+        .with_context(|| format!("failed to commit git config at {}", path.display()))?;
     Ok(())
 }
 
+/// Open the Git config for `source` using `repo` when needed, let `edit` mutate it, and
+/// write it back if the edited configuration differs from its original state.
+/// Return `true` if the file changed and was written, `false` otherwise.
+pub fn edit_config(
+    repo: Option<&gix::Repository>,
+    source: gix::config::Source,
+    edit: impl FnOnce(&mut gix::config::File<'static>) -> Result<()>,
+) -> Result<bool> {
+    let (mut config, lock) = open_config_for_editing(repo, source)?;
+    let previous_contents = config.to_bstring();
+    edit(&mut config)?;
+    let changed = config.to_bstring() != previous_contents;
+    if changed {
+        write_locked_config(&config, lock)?;
+    }
+    Ok(changed)
+}
+
+/// Open the Git config for `source` relative to `repo`, let `edit` mutate it, and write it back
+/// if the edited configuration differs from its original state.
+pub fn edit_repo_config(
+    repo: &gix::Repository,
+    source: gix::config::Source,
+    edit: impl FnOnce(&mut gix::config::File<'static>) -> Result<()>,
+) -> Result<bool> {
+    if matches!(
+        source,
+        gix::config::Source::System | gix::config::Source::GitInstallation
+    ) {
+        bail!("editing {source:?} config through a repository is not supported");
+    }
+    edit_config(Some(repo), source, edit)
+}
+
 /// Set the entry in `config` identified by the dotted `key` (like `section.value` or `section.subsection.value`) to `value`.
-/// This will create sections as needed.
+/// This will create sections as needed, and remove all previous values under the same section with the same name.
 pub fn set_config_value(
     config: &mut gix::config::File<'static>,
     key: &str,