diff --git a/AGENTS.md b/AGENTS.md index 27268eb4822..6a89aca55b4 100644 --- a/AGENTS.md +++ b/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. diff --git a/Cargo.lock b/Cargo.lock index dced4815584..1a24a979edf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -946,7 +946,6 @@ dependencies = [ "but-hunk-assignment", "but-hunk-dependency", "but-oplog", - "but-oxidize", "but-path", "but-rebase", "but-rules", @@ -1132,7 +1131,6 @@ dependencies = [ "anyhow", "but-core", "but-db", - "but-error", "but-graph", "but-meta", "but-path", @@ -1665,31 +1663,15 @@ dependencies = [ "but-core", "but-ctx", "but-db", - "but-fs", "but-graph", "but-meta", - "but-oxidize", - "but-project-handle", "but-settings", - "but-workspace", - "git2", - "gitbutler-branch-actions", - "gitbutler-project", - "gitbutler-reference", - "gitbutler-repo", - "gitbutler-stack", - "gitbutler-url", - "gitbutler-user", "gix", "gix-testtools", - "keyring", "regex", - "serde_json", "shell-words", "snapbox", - "tempfile", "termtree", - "uuid", ] [[package]] @@ -1759,7 +1741,6 @@ dependencies = [ "but-error", "but-graph", "but-meta", - "but-oxidize", "but-rebase", "but-schemars", "but-serde", @@ -3996,11 +3977,8 @@ name = "gitbutler-cherry-pick" version = "0.0.0" dependencies = [ "anyhow", - "but-oxidize", - "git2", "gitbutler-commit", "gix", - "tracing", ] [[package]] @@ -4031,7 +4009,6 @@ version = "0.0.0" dependencies = [ "bstr", "but-core", - "git2", "gix", ] @@ -4205,7 +4182,6 @@ dependencies = [ "bstr", "but-core", "but-ctx", - "but-error", "but-gerrit", "but-oxidize", "but-settings", @@ -4223,6 +4199,7 @@ dependencies = [ "infer", "insta", "itertools", + "keyring", "resolve-path", "scopeguard", "serde", @@ -4241,7 +4218,6 @@ dependencies = [ "but-core", "but-ctx", "but-error", - "but-oxidize", "git2", "gitbutler-git", "gitbutler-project", @@ -4266,12 +4242,10 @@ dependencies = [ "but-db", "but-error", "but-meta", - "but-oxidize", "but-rebase", "but-testsupport", "ctor 0.6.3", "filetime", - "git2", "gitbutler-reference", "gitbutler-repo", "gitbutler-repo-actions", diff --git a/crates/but-api/Cargo.toml b/crates/but-api/Cargo.toml index a293fe116db..7b4c04f343d 100644 --- a/crates/but-api/Cargo.toml +++ b/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 diff --git a/crates/but-api/src/legacy/git.rs b/crates/but-api/src/legacy/git.rs index f3c08304eab..38ede9a03cb 100644 --- a/crates/but-api/src/legacy/git.rs +++ b/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 { - 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> { - let (config, _) = open_user_global_config_for_editing()?; + let config = open_global_config_for_reading()?; Ok(get_config_string(&config, &key)) } diff --git a/crates/but-api/src/legacy/projects.rs b/crates/but-api/src/legacy/projects.rs index f63c404cb59..c091b3ee7af 100644 --- a/crates/but-api/src/legacy/projects.rs +++ b/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 { diff --git a/crates/but-api/src/legacy/repo.rs b/crates/but-api/src/legacy/repo.rs index 50bfda8c280..13a22e75ac1 100644 --- a/crates/but-api/src/legacy/repo.rs +++ b/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 { - ctx.read_file_from_commit(commit_id.to_git2(), &relative_path) + ctx.read_file_from_commit(commit_id, &relative_path) } #[but_api] diff --git a/crates/but-core/src/git_config.rs b/crates/but-core/src/git_config.rs index 653b6e4c82a..e814ebad77b 100644 --- a/crates/but-core/src/git_config.rs +++ b/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 { + 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> { + 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 { + 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 { + 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, diff --git a/crates/but-core/src/repo_ext.rs b/crates/but-core/src/repo_ext.rs index a096ef32b39..b8d8cac097e 100644 --- a/crates/but-core/src/repo_ext.rs +++ b/crates/but-core/src/repo_ext.rs @@ -74,12 +74,20 @@ pub trait RepositoryExt: Sized { /// Return all signatures that would be needed to perform a commit as configured in Git: `(author, committer)`. fn commit_signatures(&self) -> anyhow::Result<(gix::actor::Signature, gix::actor::Signature)>; - /// Return the configuration freshly loaded from `.git/config` so that it can be changed in memory, - /// and possibly written back with [`Self::write_local_common_config()`]. - fn local_common_config_for_editing(&self) -> anyhow::Result>; - /// Write the given `local_config` to `.git/config` of the common repository. - /// Note that we never write linked worktree-local configuration. - fn write_local_common_config(&self, local_config: &gix::config::File) -> anyhow::Result<()>; + /// Return the configuration freshly loaded from `.git/config` together with an acquired lock + /// for that file so it can be changed in memory and safely written back without another writer + /// racing the read-modify-write cycle. + fn local_common_config_for_editing( + &self, + ) -> anyhow::Result<(gix::config::File<'static>, gix::lock::File)>; + /// Write the given `local_config` to the file at `lock` of the while consuming + /// the lock previously acquired with [`Self::local_common_config_for_editing()`]. + /// Note that only local configuraiton is written, so it's safe to use it with `repo.config_snapshot_mut()`. + fn write_locked_config( + &self, + local_config: &gix::config::File, + lock: gix::lock::File, + ) -> anyhow::Result<()>; /// Cherry-pick the changes in the tree of `to_rebase_commit_id` onto `new_base_commit_id`. /// This method deals with the presence of conflicting commits to select the correct trees /// for the cheery-pick merge. @@ -184,37 +192,28 @@ impl RepositoryExt for gix::Repository { Ok((author.into(), committer)) } - fn local_common_config_for_editing(&self) -> anyhow::Result> { + fn local_common_config_for_editing( + &self, + ) -> anyhow::Result<(gix::config::File<'static>, gix::lock::File)> { let local_config_path = self.common_dir().join("config"); + let lock = gix::lock::File::acquire_to_update_resource( + &local_config_path, + gix::lock::acquire::Fail::Immediately, + None, + )?; let config = gix::config::File::from_path_no_includes( local_config_path.clone(), gix::config::Source::Local, )?; - Ok(config) + Ok((config, lock)) } - fn write_local_common_config(&self, local_config: &gix::config::File) -> anyhow::Result<()> { - use std::io::Write; - // Note: we don't use a lock file here to not risk changing the mode, and it's what Git does. - // But we lock the file so there is no raciness. - let local_config_path = self.common_dir().join("config"); - let _lock = gix::lock::Marker::acquire_to_hold_resource( - &local_config_path, - gix::lock::acquire::Fail::Immediately, - None, - )?; - let mut config_file = std::io::BufWriter::new( - std::fs::File::options() - .write(true) - .truncate(true) - .create(false) - .open(local_config_path)?, - ); - local_config.write_to_filter(&mut config_file, |section| { - section.meta().source.kind() == gix::config::source::Kind::Repository - })?; - config_file.flush()?; - Ok(()) + fn write_locked_config( + &self, + local_config: &gix::config::File, + lock: gix::lock::File, + ) -> anyhow::Result<()> { + crate::git_config::write_locked_config(local_config, lock) } fn cherry_pick_commits_to_tree( diff --git a/crates/but-core/src/settings.rs b/crates/but-core/src/settings.rs index c6837f2257f..9b98d4ca333 100644 --- a/crates/but-core/src/settings.rs +++ b/crates/but-core/src/settings.rs @@ -5,6 +5,8 @@ pub mod git { use anyhow::Result; use bstr::{BStr, BString, ByteSlice, ByteVec}; + use crate::git_config::edit_repo_config; + const GIT_SIGN_COMMITS: &str = "commit.gpgsign"; const GITBUTLER_SIGN_COMMITS: &str = "gitbutler.signCommits"; const GITBUTLER_GERRIT_MODE: &str = "gitbutler.gerritMode"; @@ -145,8 +147,6 @@ pub mod git { } use types::GitConfigSettings; - use crate::RepositoryExt; - impl GitConfigSettings { /// Read all settings from the given snapshot. pub fn try_from_snapshot(config: &gix::config::Snapshot<'_>) -> anyhow::Result { @@ -188,52 +188,56 @@ pub mod git { pub fn persist_to_local_config(&self, repo: &gix::Repository) -> Result<()> { // TODO: make this easier in `gix`. Could use config-snapshot-mut, but there is no way to // auto-reload it/assure it's up-to-date. - let mut config = repo.local_common_config_for_editing()?; - if let Some(sign_commits) = self.gitbutler_sign_commits { - config.set_raw_value( - GITBUTLER_SIGN_COMMITS, - if sign_commits { "true" } else { "false" }, - )?; - }; - if let Some(gerrit_mode) = self.gitbutler_gerrit_mode { - config.set_raw_value( - GITBUTLER_GERRIT_MODE, - if gerrit_mode { "true" } else { "false" }, - )?; - }; - if let Some(forge_template_path) = &self.gitbutler_forge_review_template_path { - config - .set_raw_value(GITBUTLER_FORGE_TEMPLATE_PATH, forge_template_path.as_bstr())?; - }; - if let Some(signing_key) = &self.signing_key { - config.set_raw_value(SIGNING_KEY, signing_key.as_bstr())?; - }; - if let Some(signing_format) = &self.signing_format { - config.set_raw_value(SIGNING_FORMAT, signing_format.as_bstr())?; - } - if let Some(gpg_program) = self.gpg_program.as_ref().and_then(osstring_into_bstring) { - config.set_raw_value(GPG_PROGRAM, gpg_program.as_bstr())?; - } - if let Some(gpg_ssh_program) = self - .gpg_ssh_program - .as_ref() - .and_then(osstring_into_bstring) - { - config.set_raw_value(GPG_SSH_PROGRAM, gpg_ssh_program.as_bstr())?; - } - if let Some(gitlab_project_id) = self.gitbutler_gitlab_project_id.as_deref() { - config.set_raw_value(GITBUTLER_GITLAB_PROJECT_ID, gitlab_project_id)?; - } - if let Some(gitlab_upstream_project_id) = - self.gitbutler_gitlab_upstream_project_id.as_deref() - { - config.set_raw_value( - GITBUTLER_GITLAB_UPSTREAM_PROJECT_ID, - gitlab_upstream_project_id, - )?; - } + edit_repo_config(repo, gix::config::Source::Local, |config| { + if let Some(sign_commits) = self.gitbutler_sign_commits { + config.set_raw_value( + GITBUTLER_SIGN_COMMITS, + if sign_commits { "true" } else { "false" }, + )?; + }; + if let Some(gerrit_mode) = self.gitbutler_gerrit_mode { + config.set_raw_value( + GITBUTLER_GERRIT_MODE, + if gerrit_mode { "true" } else { "false" }, + )?; + }; + if let Some(forge_template_path) = &self.gitbutler_forge_review_template_path { + config.set_raw_value( + GITBUTLER_FORGE_TEMPLATE_PATH, + forge_template_path.as_bstr(), + )?; + }; + if let Some(signing_key) = &self.signing_key { + config.set_raw_value(SIGNING_KEY, signing_key.as_bstr())?; + }; + if let Some(signing_format) = &self.signing_format { + config.set_raw_value(SIGNING_FORMAT, signing_format.as_bstr())?; + } + if let Some(gpg_program) = self.gpg_program.as_ref().and_then(osstring_into_bstring) + { + config.set_raw_value(GPG_PROGRAM, gpg_program.as_bstr())?; + } + if let Some(gpg_ssh_program) = self + .gpg_ssh_program + .as_ref() + .and_then(osstring_into_bstring) + { + config.set_raw_value(GPG_SSH_PROGRAM, gpg_ssh_program.as_bstr())?; + } + if let Some(gitlab_project_id) = self.gitbutler_gitlab_project_id.as_deref() { + config.set_raw_value(GITBUTLER_GITLAB_PROJECT_ID, gitlab_project_id)?; + } + if let Some(gitlab_upstream_project_id) = + self.gitbutler_gitlab_upstream_project_id.as_deref() + { + config.set_raw_value( + GITBUTLER_GITLAB_UPSTREAM_PROJECT_ID, + gitlab_upstream_project_id, + )?; + } - repo.write_local_common_config(&config)?; + Ok(()) + })?; Ok(()) } } diff --git a/crates/but-core/tests/core/git_config.rs b/crates/but-core/tests/core/git_config.rs new file mode 100644 index 00000000000..c62570f0972 --- /dev/null +++ b/crates/but-core/tests/core/git_config.rs @@ -0,0 +1,51 @@ +mod local { + use but_core::git_config::{edit_repo_config, remove_config_value, set_config_value}; + use but_testsupport::{invoke_bash, writable_scenario}; + + #[test] + fn writes_back_local_config_when_requested() -> anyhow::Result<()> { + let (mut repo, _tmp) = writable_scenario("git-config-empty"); + + assert!(edit_repo_config( + &repo, + gix::config::Source::Local, + |config| { + set_config_value(config, "gitbutler.testValue", "set")?; + Ok(()) + } + )?); + + repo.reload()?; + assert_eq!( + repo.config_snapshot() + .string("gitbutler.testValue") + .map(|value| value.to_string()), + Some("set".to_owned()) + ); + Ok(()) + } + + #[test] + fn writes_back_local_config_when_value_is_removed() -> anyhow::Result<()> { + let (mut repo, _tmp) = writable_scenario("git-config-empty"); + invoke_bash("git config --local gitbutler.testValue kept", &repo); + + assert!(edit_repo_config( + &repo, + gix::config::Source::Local, + |config| { + remove_config_value(config, "gitbutler.testValue")?; + Ok(()) + } + )?); + + repo.reload()?; + assert_eq!( + repo.config_snapshot() + .string("gitbutler.testValue") + .map(|value| value.to_string()), + None + ); + Ok(()) + } +} diff --git a/crates/but-core/tests/core/main.rs b/crates/but-core/tests/core/main.rs index 3d5cc326fa9..0808726bb69 100644 --- a/crates/but-core/tests/core/main.rs +++ b/crates/but-core/tests/core/main.rs @@ -4,6 +4,7 @@ mod cmd; mod commit; mod diff; mod extract_remote_name_and_short_name; +mod git_config; mod json_samples; mod ref_metadata; mod settings; diff --git a/crates/but-core/tests/fixtures/scenario/git-config-empty.sh b/crates/but-core/tests/fixtures/scenario/git-config-empty.sh new file mode 100644 index 00000000000..b7316a9aeeb --- /dev/null +++ b/crates/but-core/tests/fixtures/scenario/git-config-empty.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +### Description +# A newly initialized git repository used for git-config mutation tests. +git init diff --git a/crates/but-ctx/Cargo.toml b/crates/but-ctx/Cargo.toml index 145a51e1ed9..43f11478cca 100644 --- a/crates/but-ctx/Cargo.toml +++ b/crates/but-ctx/Cargo.toml @@ -30,7 +30,6 @@ gitbutler-project = { workspace = true, optional = true } tracing.workspace = true anyhow.workspace = true -but-error.workspace = true gix.workspace = true git2.workspace = true diff --git a/crates/but-ctx/src/legacy.rs b/crates/but-ctx/src/legacy.rs index 349e2572bcb..08f0d3f1cb4 100644 --- a/crates/but-ctx/src/legacy.rs +++ b/crates/but-ctx/src/legacy.rs @@ -35,6 +35,10 @@ impl Context { /// Open the repository identified by `legacy_project` and `settings`, while controlling /// how the repository sources configuration via `repo_open_mode`. + #[allow( + deprecated, + reason = "Context owns the deprecated boundary cache and must initialize it." + )] pub fn new_from_legacy_project_and_settings_with_repo_open_mode( legacy_project: &gitbutler_project::Project, settings: AppSettings, diff --git a/crates/but-ctx/src/lib.rs b/crates/but-ctx/src/lib.rs index f3918fec25c..c4add9a0f0a 100644 --- a/crates/but-ctx/src/lib.rs +++ b/crates/but-ctx/src/lib.rs @@ -13,7 +13,6 @@ use but_core::{ RepositoryExt, sync::{RepoExclusive, RepoExclusiveGuard, RepoShared, RepoSharedGuard}, }; -use but_error::Code; use but_settings::AppSettings; use tracing::instrument; @@ -110,7 +109,15 @@ pub struct Context { /// Note that the standard repository comes with a decently sized object cache, but further optimization can /// be performed by using [`Self::clone_repo_for_merging()`]. pub repo: OnDemand, - /// The most recently opened `git2` repository of the project. + /// A lazily opened legacy `git2` repository for the project. + /// + /// Keep new uses confined to the residual `git2` boundary: + /// checkout/worktree materialization, staged tree/index materialization, + /// and deliberate compatibility adapters that still require libgit2. + /// Activation and read-side flows should use [`Self::repo`] instead. + #[deprecated( + note = "Boundary-only escape hatch: use Context::repo unless you are crossing the remaining libgit2 checkout/index, hook, or transport/auth adapter boundary." + )] pub git2_repo: OnDemand, /// An open handle to the database. It's initialized lazily upon first access. /// It is also what makes this type non-Clone, which is fair. @@ -151,6 +158,10 @@ pub struct ThreadSafeContext { } impl From for Context { + #[allow( + deprecated, + reason = "Context owns the deprecated boundary cache and must initialize it." + )] fn from(value: ThreadSafeContext) -> Self { let ThreadSafeContext { settings, @@ -264,6 +275,10 @@ impl Context { /// Just like [`Context::new()`], but allows controlling how the repository /// sources configuration via `repo_open_mode`. + #[allow( + deprecated, + reason = "Context owns the deprecated boundary cache and must initialize it." + )] pub fn new_with_repo_open_mode( gitdir: impl Into, app_config_dir: impl AsRef, @@ -369,6 +384,10 @@ impl Context { Self::from_repo_with_legacy_support(repo, repo_open_mode) } + #[allow( + deprecated, + reason = "Context owns the deprecated boundary cache and must initialize it." + )] fn from_repo_with_legacy_support( repo: gix::Repository, repo_open_mode: RepoOpenMode, @@ -429,6 +448,10 @@ impl Context { /// /// Particularly useful in testing, which might start off with just a Git repository. /// **Note that it does not have support for legacy projects to encourage single-branch compatible code.** + #[allow( + deprecated, + reason = "Context owns the deprecated boundary cache and must initialize it." + )] pub fn from_repo(repo: gix::Repository) -> anyhow::Result { let gitdir = repo.git_dir().to_owned(); let project_data_dir = repo.gitbutler_storage_path()?; @@ -955,6 +978,10 @@ impl Context { } /// Take all copyable values and place them in an instance that can pass across thread boundaries. + #[allow( + deprecated, + reason = "Context owns the deprecated boundary cache and must move it out internally." + )] pub fn into_sync(self) -> ThreadSafeContext { let Context { settings, @@ -1060,34 +1087,6 @@ impl Context { self.clone_repo_for_merging() .map(|repo| repo.with_object_memory()) } - - /// Eagerly open the legacy `git2` repository, preserving repo-ownership error tagging used - /// by activation entrypoints before any database work begins. - /// - /// This is needed as parts of the code depend on the error carrying this context code - /// to indicate ownership issues, related to `safe.directories` or the lack thereof. - /// `gix::Repository` doesn't have that problem as it functions differently, - /// not by refusing to open the repository but by not trusting its configuration in particular when - /// programs are involved. - pub fn eagerly_populate_git2_repo_cache(&mut self) -> anyhow::Result<()> { - { - let existing = self.git2_repo.get_opt(); - if existing.is_some() { - return Ok(()); - } - } - let repo = git2::Repository::open(&self.gitdir).map_err(|err| { - let code = err.code(); - let err = anyhow::Error::from(err); - if code == git2::ErrorCode::Owner { - err.context(Code::RepoOwnership) - } else { - err - } - })?; - self.git2_repo.assign(repo); - Ok(()) - } } /// For now, always make sure we have object caches setup to make diffs fast in the common case. diff --git a/crates/but-napi/src/lib.rs b/crates/but-napi/src/lib.rs index ca0b721935d..0f21b3705ce 100644 --- a/crates/but-napi/src/lib.rs +++ b/crates/but-napi/src/lib.rs @@ -84,8 +84,6 @@ fn app_settings_sync() -> anyhow::Result { fn open_prepared_context(project_id: &ProjectHandleOrLegacyProjectId) -> anyhow::Result { // We don't get a validated legacy object here, but that's fine as we're only opening repos/watchers. let mut ctx: Context = project_id.clone().try_into()?; - // Keep this before any database usage on `ctx`. - ctx.eagerly_populate_git2_repo_cache()?; but_api::legacy::projects::prepare_project_for_activation(&mut ctx)?; Ok(ctx) } diff --git a/crates/but-secret/Cargo.toml b/crates/but-secret/Cargo.toml index a469ea30c43..7a905e8dce3 100644 --- a/crates/but-secret/Cargo.toml +++ b/crates/but-secret/Cargo.toml @@ -5,7 +5,6 @@ edition.workspace = true authors.workspace = true publish = false rust-version.workspace = true -autotests = false [lib] doctest = false @@ -19,10 +18,6 @@ serde.workspace = true gix = { workspace = true, features = ["dirwalk", "credentials", "parallel"] } keyring.workspace = true -[[test]] -name="secret" -path = "tests/mod.rs" - [dev-dependencies] [lints] diff --git a/crates/but-secret/tests/mod.rs b/crates/but-secret/tests/secret.rs similarity index 100% rename from crates/but-secret/tests/mod.rs rename to crates/but-secret/tests/secret.rs diff --git a/crates/but-testsupport/Cargo.toml b/crates/but-testsupport/Cargo.toml index 692943f31ce..4e36f2acf66 100644 --- a/crates/but-testsupport/Cargo.toml +++ b/crates/but-testsupport/Cargo.toml @@ -12,29 +12,6 @@ test = false doctest = false [features] -## Provide legacy GitButler test helpers that are still being migrated into `but-*` crates. -legacy = [ - "dep:but-ctx", - "dep:but-fs", - "dep:but-oxidize", - "dep:but-project-handle", - "dep:but-settings", - "dep:but-workspace", - "dep:gitbutler-branch-actions", - "dep:gitbutler-project", - "dep:gitbutler-reference", - "dep:gitbutler-repo", - "dep:gitbutler-stack", - "dep:gitbutler-url", - "dep:gitbutler-user", - "dep:git2", - "dep:keyring", - "dep:serde_json", - "dep:tempfile", - "dep:uuid", - "but-core/legacy", - "but-ctx/legacy" -] ## Make `snapbox` utilities available, generally useful for CLI testing. snapbox = ["dep:snapbox"] ## Provide a writable sandbox for testing of higher-level functions, with everything *but* the `Context`, useful even for plumbing. @@ -60,28 +37,6 @@ but-ctx = { workspace = true, optional = true } shell-words = { workspace = true, optional = true } -# for gitbutler-testtools -# Goal is to replace these with `but-testsupport` helpers next -but-fs = { workspace = true, optional = true, features = ["legacy"] } -but-oxidize = { workspace = true, optional = true } -but-project-handle = { workspace = true, optional = true } -but-workspace = { workspace = true, optional = true, features = ["legacy"] } - -gitbutler-branch-actions = { workspace = true, optional = true } -gitbutler-project = { workspace = true, optional = true } -gitbutler-reference = { workspace = true, optional = true } -gitbutler-repo = { workspace = true, optional = true } -gitbutler-stack = { workspace = true, optional = true } -gitbutler-url = { workspace = true, optional = true } -gitbutler-user = { workspace = true, optional = true } - -git2 = { workspace = true, optional = true } -keyring = { workspace = true, optional = true } -serde_json = { workspace = true, optional = true } -tempfile = { workspace = true, optional = true } -uuid = { workspace = true, optional = true } -# END for gitbutler-testtools - gix-testtools.workspace = true gix.workspace = true anyhow.workspace = true diff --git a/crates/but-testsupport/src/legacy/mod.rs b/crates/but-testsupport/src/legacy/mod.rs deleted file mode 100644 index 78aa47fbb7b..00000000000 --- a/crates/but-testsupport/src/legacy/mod.rs +++ /dev/null @@ -1,102 +0,0 @@ -use but_ctx::Context; -use but_oxidize::OidExt; -use but_workspace::{legacy::StacksFilter, ui::StackDetails}; -use gitbutler_stack::StackId; -use gix::prelude::ObjectIdExt; - -pub const VAR_NO_CLEANUP: &str = "GITBUTLER_TESTS_NO_CLEANUP"; - -mod test_project; -pub use test_project::TestProject; - -mod suite; -pub use suite::*; - -pub mod testing_repository; - -pub mod paths { - use tempfile::TempDir; - - use super::temp_dir; - - pub fn data_dir() -> TempDir { - temp_dir() - } -} - -pub mod virtual_branches { - use but_ctx::Context; - use but_oxidize::OidExt; - use gitbutler_stack::{Target, VirtualBranchesHandle}; - - use super::empty_bare_repository; - - pub fn set_test_target(ctx: &Context) -> anyhow::Result<()> { - let mut vb_state = VirtualBranchesHandle::new(ctx.project_data_dir()); - let (remote_repo, _tmp) = empty_bare_repository(); - let git2_repo = &*ctx.git2_repo.get()?; - let mut remote = git2_repo - .remote("origin", remote_repo.path().to_str().unwrap()) - .expect("failed to add remote"); - remote.push(&["refs/heads/master:refs/heads/master"], None)?; - - vb_state - .set_default_target(Target { - branch: "refs/remotes/origin/master".parse().unwrap(), - remote_url: remote_repo.path().to_str().unwrap().parse().unwrap(), - sha: remote_repo.head().unwrap().target().unwrap().to_gix(), - push_remote_name: None, - }) - .expect("failed to write target"); - - gitbutler_branch_actions::update_workspace_commit_with_vb_state(&vb_state, ctx, false) - .expect("failed to update workspace"); - - Ok(()) - } -} - -pub fn init_opts() -> git2::RepositoryInitOptions { - let mut opts = git2::RepositoryInitOptions::new(); - opts.initial_head("master"); - opts -} - -pub fn init_opts_bare() -> git2::RepositoryInitOptions { - let mut opts = init_opts(); - opts.bare(true); - opts -} - -pub fn visualize_git2_tree(tree_id: git2::Oid, repo: &git2::Repository) -> termtree::Tree { - let repo = gix::open_opts(repo.path(), gix::open::Options::isolated()).unwrap(); - crate::visualize_tree(tree_id.to_gix().attach(&repo)) -} - -pub fn stack_details(ctx: &Context) -> Vec<(StackId, StackDetails)> { - let repo = ctx.clone_repo_for_merging_non_persisting().unwrap(); - let stacks = { - let meta = ctx.legacy_meta().unwrap(); - let mut cache = ctx.cache.get_cache_mut().unwrap(); - but_workspace::legacy::stacks_v3(&repo, &meta, StacksFilter::default(), None, &mut cache) - } - .unwrap(); - let mut details = vec![]; - for stack in stacks { - let stack_id = stack - .id - .expect("BUG(opt-stack-id): test code shouldn't trigger this"); - details.push(( - stack_id, - { - let meta = ctx.legacy_meta().unwrap(); - let mut cache = ctx.cache.get_cache_mut().unwrap(); - but_workspace::legacy::stack_details_v3(stack_id.into(), &repo, &meta, &mut cache) - } - .unwrap(), - )); - } - details -} - -pub mod secrets; diff --git a/crates/but-testsupport/src/legacy/secrets.rs b/crates/but-testsupport/src/legacy/secrets.rs deleted file mode 100644 index 0c0148c8e11..00000000000 --- a/crates/but-testsupport/src/legacy/secrets.rs +++ /dev/null @@ -1,52 +0,0 @@ -use std::any::Any; - -use keyring::Result; - -pub fn setup_blackhole_store() { - keyring::set_default_credential_builder(Box::new(BlackholeBuilder)) -} - -struct BlackholeBuilder; - -struct BlackholeCredential; - -impl keyring::credential::CredentialApi for BlackholeCredential { - fn set_password(&self, _password: &str) -> keyring::Result<()> { - Ok(()) - } - - fn set_secret(&self, _password: &[u8]) -> Result<()> { - unreachable!("unused") - } - - fn get_password(&self) -> keyring::Result { - Err(keyring::Error::NoEntry) - } - - fn get_secret(&self) -> Result> { - unreachable!("unused") - } - - fn delete_credential(&self) -> keyring::Result<()> { - Ok(()) - } - - fn as_any(&self) -> &dyn Any { - self - } -} - -impl keyring::credential::CredentialBuilderApi for BlackholeBuilder { - fn build( - &self, - _target: Option<&str>, - _service: &str, - _user: &str, - ) -> keyring::Result> { - Ok(Box::new(BlackholeCredential)) - } - - fn as_any(&self) -> &dyn Any { - self - } -} diff --git a/crates/but-testsupport/src/legacy/suite.rs b/crates/but-testsupport/src/legacy/suite.rs deleted file mode 100644 index 65027d52069..00000000000 --- a/crates/but-testsupport/src/legacy/suite.rs +++ /dev/null @@ -1,197 +0,0 @@ -use std::{ - collections::HashMap, - fs, - path::{Path, PathBuf}, -}; - -use but_ctx::{Context, RepoOpenMode}; -use but_settings::AppSettings; -use gitbutler_repo::RepositoryExt; -use tempfile::{TempDir, tempdir}; - -use super::{VAR_NO_CLEANUP, init_opts, init_opts_bare, test_project::setup_config}; - -pub struct Suite { - pub local_app_data: Option, - pub storage: but_fs::Storage, -} - -impl Drop for Suite { - fn drop(&mut self) { - if std::env::var_os(VAR_NO_CLEANUP).is_some() { - let _ = self.local_app_data.take().unwrap().keep(); - } - } -} - -impl Default for Suite { - fn default() -> Self { - let local_app_data = temp_dir(); - let storage = but_fs::Storage::new(local_app_data.path()); - Self { - storage, - local_app_data: Some(local_app_data), - } - } -} - -impl Suite { - pub fn local_app_data(&self) -> &Path { - self.local_app_data.as_ref().unwrap().path() - } - pub fn sign_in(&self) -> gitbutler_user::User { - crate::legacy::secrets::setup_blackhole_store(); - let user: gitbutler_user::User = - serde_json::from_str(include_str!("../fixtures/user/minimal.v1")) - .expect("valid v1 user file"); - gitbutler_user::set_user(&user).expect("failed to add user"); - user - } - - fn project(&self, fs: HashMap) -> (gitbutler_project::Project, TempDir) { - let (repo, tmp) = test_repository(); - for (path, contents) in fs { - if let Some(parent) = path.parent() { - fs::create_dir_all(repo.path().parent().unwrap().join(parent)) - .expect("failed to create dir"); - } - fs::write( - repo.path().parent().unwrap().join(&path), - contents.as_bytes(), - ) - .expect("failed to write file"); - } - commit_all(&repo); - - let outcome = gitbutler_project::add_at_app_data_dir( - self.local_app_data(), - repo.path().parent().unwrap(), - ); - - let project = outcome.expect("failed to add project").unwrap_project(); - - (project, tmp) - } - - pub fn new_case_with_files(&self, fs: HashMap) -> Case { - let (project, project_tmp) = self.project(fs); - Case::new(project, project_tmp) - } - - pub fn new_case(&self) -> Case { - self.new_case_with_files(HashMap::new()) - } -} - -pub struct Case { - pub project: gitbutler_project::Project, - pub ctx: Context, - pub project_tmp: Option, -} - -impl Drop for Case { - fn drop(&mut self) { - if let Some(tmp) = self - .project_tmp - .take() - .filter(|_| std::env::var_os(VAR_NO_CLEANUP).is_some()) - { - let _ = tmp.keep(); - } - } -} - -impl Case { - fn new(project: gitbutler_project::Project, project_tmp: TempDir) -> Case { - let ctx = Context::new_from_legacy_project_and_settings_with_repo_open_mode( - &project, - AppSettings::default(), - RepoOpenMode::Isolated, - ) - .expect("can create context"); - Case { - project, - ctx, - project_tmp: Some(project_tmp), - } - } - - pub fn refresh(mut self, _suite: &Suite) -> Self { - let project = - gitbutler_project::get(self.project.id.clone()).expect("failed to get project"); - let ctx = Context::new_from_legacy_project_and_settings_with_repo_open_mode( - &project, - AppSettings::default(), - RepoOpenMode::Isolated, - ) - .expect("can create context"); - Self { - ctx, - project, - project_tmp: self.project_tmp.take(), - } - } -} - -pub fn temp_dir() -> TempDir { - tempdir().unwrap() -} - -pub fn empty_bare_repository() -> (git2::Repository, TempDir) { - let tmp = temp_dir(); - ( - git2::Repository::init_opts(&tmp, &init_opts_bare()).expect("failed to init repository"), - tmp, - ) -} - -pub fn test_repository() -> (git2::Repository, TempDir) { - let tmp = temp_dir(); - let git2_repo = - git2::Repository::init_opts(&tmp, &init_opts()).expect("failed to init repository"); - setup_config(&git2_repo.config().unwrap()).unwrap(); - let mut index = git2_repo.index().expect("failed to get index"); - let oid = index.write_tree().expect("failed to write tree"); - let signature = git2::Signature::now("test", "test@email.com").unwrap(); - git2_repo - .commit_with_signature( - Some(&"refs/heads/master".parse().unwrap()), - &signature, - &signature, - "Initial commit", - &git2_repo.find_tree(oid).expect("failed to find tree"), - &[], - None, - ) - .expect("failed to commit"); - (git2_repo, tmp) -} - -pub fn commit_all(repository: &git2::Repository) -> git2::Oid { - let mut index = repository.index().expect("failed to get index"); - index - .add_all(["."], git2::IndexAddOption::DEFAULT, None) - .expect("failed to add all"); - index.write().expect("failed to write index"); - let oid = index.write_tree().expect("failed to write tree"); - let signature = git2::Signature::now("test", "test@email.com").unwrap(); - let head = repository.head().expect("failed to get head"); - let repo: &git2::Repository = repository; - - repo.commit_with_signature( - Some(&head.name().map(|name| name.parse().unwrap()).unwrap()), - &signature, - &signature, - "some commit", - &repository.find_tree(oid).expect("failed to find tree"), - &[&repository - .find_commit( - repository - .refname_to_id("HEAD") - .expect("failed to get head"), - ) - .expect("failed to find commit")], - None, - ) - .expect("failed to commit") -} diff --git a/crates/but-testsupport/src/legacy/test_project.rs b/crates/but-testsupport/src/legacy/test_project.rs deleted file mode 100644 index 7e2a6a44900..00000000000 --- a/crates/but-testsupport/src/legacy/test_project.rs +++ /dev/null @@ -1,383 +0,0 @@ -use std::{fs, path, path::PathBuf}; - -use but_oxidize::{ObjectIdExt, OidExt}; -use gitbutler_reference::{LocalRefname, Refname}; -use gitbutler_repo::RepositoryExt; -use tempfile::TempDir; - -use super::{VAR_NO_CLEANUP, init_opts}; - -pub fn temp_dir() -> TempDir { - tempfile::tempdir().unwrap() -} - -pub struct TestProject { - pub local_repo: git2::Repository, - local_tmp: Option, - remote_repo: git2::Repository, - remote_tmp: Option, -} - -impl Drop for TestProject { - fn drop(&mut self) { - if std::env::var_os(VAR_NO_CLEANUP).is_some() { - let _ = self.local_tmp.take().map(|tmp| tmp.keep()); - let _ = self.remote_tmp.take().map(|tmp| tmp.keep()); - } - } -} - -impl Default for TestProject { - fn default() -> Self { - let local_tmp = temp_dir(); - let local_repository = git2::Repository::init_opts(local_tmp.path(), &init_opts()) - .expect("failed to init repository"); - setup_config(&local_repository.config().unwrap()).unwrap(); - let mut index = local_repository.index().expect("failed to get index"); - let oid = index.write_tree().expect("failed to write tree"); - let signature = git2::Signature::now("test", "test@email.com").unwrap(); - let repo: &git2::Repository = &local_repository; - repo.commit_with_signature( - Some(&"refs/heads/master".parse().unwrap()), - &signature, - &signature, - "Initial commit", - &local_repository - .find_tree(oid) - .expect("failed to find tree"), - &[], - None, - ) - .expect("failed to commit"); - - let remote_tmp = temp_dir(); - let remote_repository = git2::Repository::init_opts( - remote_tmp.path(), - git2::RepositoryInitOptions::new() - .bare(true) - .external_template(false), - ) - .expect("failed to init repository"); - setup_config(&remote_repository.config().unwrap()).unwrap(); - - { - let mut remote = local_repository - .remote( - "origin", - remote_repository - .path() - .to_str() - .expect("failed to convert path to str"), - ) - .expect("failed to add remote"); - remote - .push(&["refs/heads/master:refs/heads/master"], None) - .expect("failed to push"); - } - - Self { - local_repo: local_repository, - local_tmp: Some(local_tmp), - remote_repo: remote_repository, - remote_tmp: Some(remote_tmp), - } - } -} - -impl TestProject { - pub fn debug_local_repo(&mut self) -> Option { - self.local_tmp.take().map(|tmp| tmp.keep()) - } - pub fn path(&self) -> &std::path::Path { - self.local_repo.workdir().unwrap() - } - - pub fn push_branch(&self, branch: &LocalRefname) { - let mut origin = self.local_repo.find_remote("origin").unwrap(); - origin.push(&[&format!("{branch}:{branch}")], None).unwrap(); - } - - pub fn push(&self) { - let mut origin = self.local_repo.find_remote("origin").unwrap(); - origin - .push(&["refs/heads/master:refs/heads/master"], None) - .unwrap(); - } - - pub fn reset_hard(&self, oid: Option) { - let mut index = self.local_repo.index().expect("failed to get index"); - index - .add_all(["."], git2::IndexAddOption::DEFAULT, None) - .expect("failed to add all"); - index.write().expect("failed to write index"); - - let head = self.local_repo.head().unwrap(); - let commit = oid.map_or(head.peel_to_commit().unwrap(), |oid| { - self.local_repo.find_commit(oid).unwrap() - }); - - let head_ref = head.name().unwrap(); - self.local_repo.find_reference(head_ref).unwrap(); - - self.local_repo - .reset(commit.as_object(), git2::ResetType::Hard, None) - .unwrap(); - } - - pub fn fetch(&self) { - let mut remote = self.local_repo.find_remote("origin").unwrap(); - remote - .fetch(&["+refs/heads/*:refs/remotes/origin/*"], None, None) - .unwrap(); - } - - pub fn rebase_and_merge(&self, branch_name: &Refname) { - let branch_name: Refname = match branch_name { - Refname::Local(local) => format!("refs/heads/{}", local.branch()).parse().unwrap(), - Refname::Remote(remote) => format!("refs/heads/{}", remote.branch()).parse().unwrap(), - _ => "INVALID".parse().unwrap(), - }; - let branch = self - .remote_repo - .maybe_find_branch_by_refname(&branch_name) - .unwrap(); - let branch_commit = branch.unwrap().get().peel_to_commit().unwrap(); - - let master_branch = { - let name: Refname = "refs/heads/master".parse().unwrap(); - self.remote_repo - .maybe_find_branch_by_refname(&name) - .unwrap() - }; - let master_branch_commit = master_branch.unwrap().get().peel_to_commit().unwrap(); - - let mut rebase_options = git2::RebaseOptions::new(); - rebase_options.quiet(true); - rebase_options.inmemory(true); - - let mut rebase = self - .remote_repo - .rebase( - Some( - &self - .remote_repo - .find_annotated_commit(branch_commit.id()) - .unwrap(), - ), - Some( - &self - .remote_repo - .find_annotated_commit(master_branch_commit.id()) - .unwrap(), - ), - None, - Some(&mut rebase_options), - ) - .unwrap(); - - let mut rebase_success = true; - let mut last_rebase_head = branch_commit.id(); - while let Some(Ok(op)) = rebase.next() { - let commit = self.remote_repo.find_commit(op.id()).unwrap(); - let index = rebase.inmemory_index().unwrap(); - if index.has_conflicts() { - rebase_success = false; - break; - } - - if let Ok(commit_id) = rebase.commit(None, &commit.committer(), None) { - last_rebase_head = commit_id; - } else { - rebase_success = false; - break; - }; - } - - if rebase_success { - self.remote_repo - .reference( - "refs/heads/master", - last_rebase_head, - true, - &format!("rebase: {branch_name}"), - ) - .unwrap(); - } else { - rebase.abort().unwrap(); - } - } - - pub fn merge(&self, branch_name: &Refname) -> anyhow::Result<()> { - let branch_name: Refname = match branch_name { - Refname::Local(local) => format!("refs/heads/{}", local.branch()).parse()?, - Refname::Remote(remote) => format!("refs/heads/{}", remote.branch()).parse()?, - _ => "INVALID".parse()?, - }; - let branch = self - .remote_repo - .maybe_find_branch_by_refname(&branch_name)? - .expect("branch exists"); - let branch_commit = branch.get().peel_to_commit()?; - - let master_branch = { - let name: Refname = "refs/heads/master".parse()?; - self.remote_repo.maybe_find_branch_by_refname(&name)? - }; - let master_branch_commit = master_branch - .as_ref() - .expect("master branch exists") - .get() - .peel_to_commit()?; - - let gix_repo = gix::open_opts(self.remote_repo.path(), gix::open::Options::isolated())?; - let merge_tree = { - use but_core::RepositoryExt; - let mut merge_result = gix_repo.merge_commits( - master_branch_commit.id().to_gix(), - branch.get().peel_to_commit()?.id().to_gix(), - gix_repo.default_merge_labels(), - gix::merge::commit::Options::default(), - )?; - assert!( - !merge_result - .tree_merge - .has_unresolved_conflicts(Default::default()), - "test-merges should have non-conflicting trees" - ); - let tree_id = merge_result.tree_merge.tree.write()?; - self.remote_repo.find_tree(tree_id.to_git2())? - }; - - let repo: &git2::Repository = &self.remote_repo; - repo.commit_with_signature( - Some(&"refs/heads/master".parse()?), - &branch_commit.author(), - &branch_commit.committer(), - &format!("Merge pull request from {branch_name}"), - &merge_tree, - &[&master_branch_commit, &branch_commit], - None, - )?; - Ok(()) - } - - pub fn find_commit(&self, oid: git2::Oid) -> Result, git2::Error> { - self.local_repo.find_commit(oid) - } - - pub fn checkout_commit(&self, commit_oid: git2::Oid) { - let commit = self.local_repo.find_commit(commit_oid).unwrap(); - let commit_tree = commit.tree().unwrap(); - - self.local_repo.set_head_detached(commit_oid).unwrap(); - self.local_repo - .checkout_tree_builder(&commit_tree) - .force() - .checkout() - .unwrap(); - } - - pub fn checkout(&self, branch: &LocalRefname) { - let refname: Refname = branch.into(); - let head_commit = self.local_repo.head().unwrap().peel_to_commit().unwrap(); - let tree = match self.local_repo.maybe_find_branch_by_refname(&refname) { - Ok(branch) => match branch { - Some(branch) => branch.get().peel_to_tree().unwrap(), - None => { - self.local_repo - .reference(&refname.to_string(), head_commit.id(), false, "new branch") - .unwrap(); - head_commit.tree().unwrap() - } - }, - Err(error) => panic!("{error:?}"), - }; - self.local_repo.set_head(&refname.to_string()).unwrap(); - self.local_repo - .checkout_tree_builder(&tree) - .force() - .checkout() - .unwrap(); - } - - pub fn commit_all(&self, message: &str) -> git2::Oid { - let head = self.local_repo.head().unwrap(); - let mut index = self.local_repo.index().expect("failed to get index"); - index - .add_all(["."], git2::IndexAddOption::DEFAULT, None) - .expect("failed to add all"); - index.write().expect("failed to write index"); - let oid = index.write_tree().expect("failed to write tree"); - let signature = git2::Signature::now("test", "test@email.com").unwrap(); - let refname: Refname = head.name().unwrap().parse().unwrap(); - let repo: &git2::Repository = &self.local_repo; - repo.commit_with_signature( - Some(&refname), - &signature, - &signature, - message, - &self.local_repo.find_tree(oid).expect("failed to find tree"), - &[&self - .local_repo - .find_commit( - self.local_repo - .refname_to_id("HEAD") - .expect("failed to get head"), - ) - .expect("failed to find commit")], - None, - ) - .expect("failed to commit") - } - - pub fn references(&self) -> Vec> { - self.local_repo - .references() - .expect("failed to get references") - .collect::, _>>() - .expect("failed to read references") - } - - pub fn add_submodule(&self, url: &gitbutler_url::Url, path: &path::Path) { - let mut submodule = self - .local_repo - .submodule(&url.to_string(), path.as_ref(), false) - .unwrap(); - let repo = submodule.open().unwrap(); - - repo.find_remote("origin") - .unwrap() - .fetch(&["+refs/heads/*:refs/heads/*"], None, None) - .unwrap(); - let reference = repo.find_reference("refs/heads/master").unwrap(); - let reference_head = repo.find_commit(reference.target().unwrap()).unwrap(); - repo.checkout_tree(reference_head.tree().unwrap().as_object(), None) - .unwrap(); - - repo.set_head("refs/heads/master").unwrap(); - submodule.add_finalize().unwrap(); - } - - pub fn write_file(&self, path: &str, lines: &[String]) { - fs::write(self.path().join(path), lines.join("\n")).unwrap() - } - - pub fn gen_file(&self, path: &str, line_count: i32) -> Vec { - let lines: Vec<_> = (0_i32..line_count).map(|i| format!("line {i}")).collect(); - self.write_file(path, &lines); - lines - } -} - -pub(crate) fn setup_config(config: &git2::Config) -> anyhow::Result<()> { - match config.open_level(git2::ConfigLevel::Local) { - Ok(mut local) => { - local.set_str("commit.gpgsign", "false")?; - local.set_str("user.name", "gitbutler-test")?; - local.set_str("user.email", "gitbutler-test@example.com")?; - local.set_str(but_project_handle::storage_path_config_key(), "gitbutler")?; - Ok(()) - } - Err(err) => Err(err.into()), - } -} diff --git a/crates/but-testsupport/src/legacy/testing_repository.rs b/crates/but-testsupport/src/legacy/testing_repository.rs deleted file mode 100644 index 91b5750f32d..00000000000 --- a/crates/but-testsupport/src/legacy/testing_repository.rs +++ /dev/null @@ -1,244 +0,0 @@ -use std::fs; - -use but_core::{ChangeId, commit::Headers}; -use but_oxidize::OidExt; -use gitbutler_repo::RepositoryExt; -use gix_testtools::bstr::ByteSlice as _; -use tempfile::{TempDir, tempdir}; -use uuid::Uuid; - -use super::init_opts; - -pub struct TestingRepository { - pub repository: git2::Repository, - pub tempdir: TempDir, -} - -impl TestingRepository { - pub fn open() -> Self { - let tempdir = tempdir().unwrap(); - let repo = git2::Repository::init_opts(tempdir.path(), &init_opts()).unwrap(); - let signature = git2::Signature::now("Caleb", "caleb@gitbutler.com").unwrap(); - let empty_tree_id = repo.treebuilder(None).unwrap().write().unwrap(); - repo.commit( - Some("refs/heads/master"), - &signature, - &signature, - "init to prevent load index failure", - &repo.find_tree(empty_tree_id).unwrap(), - &[], - ) - .unwrap(); - - let config = repo.config().unwrap(); - match config.open_level(git2::ConfigLevel::Local) { - Ok(mut local) => { - local.set_str("commit.gpgsign", "false").unwrap(); - local.set_str("user.name", "gitbutler-test").unwrap(); - local - .set_str("user.email", "gitbutler-test@example.com") - .unwrap(); - } - Err(err) => panic!("{}", err), - } - - Self { - tempdir, - repository: repo, - } - } - - pub fn gix_repository(&self) -> gix::Repository { - gix::open(self.repository.path()).unwrap() - } - - pub fn open_with_initial_commit(files: &[(&str, &str)]) -> Self { - let tempdir = tempdir().unwrap(); - let repo = git2::Repository::init_opts(tempdir.path(), &init_opts()).unwrap(); - - let config = repo.config().unwrap(); - match config.open_level(git2::ConfigLevel::Local) { - Ok(mut local) => { - local.set_str("commit.gpgsign", "false").unwrap(); - local.set_str("user.name", "gitbutler-test").unwrap(); - local - .set_str("user.email", "gitbutler-test@example.com") - .unwrap(); - } - Err(err) => panic!("{}", err), - } - - let repo = Self { - tempdir, - repository: repo, - }; - { - let commit = repo.commit_tree(None, files); - repo.repository.branch("master", &commit, true).unwrap(); - } - repo - } - - pub fn commit_tree_with_change_id<'a>( - &'a self, - parent: Option<&git2::Commit<'_>>, - change_id: &str, - files: &[(&str, &str)], - ) -> git2::Commit<'a> { - self.commit_tree_inner(parent, &Uuid::new_v4().to_string(), files, Some(change_id)) - } - - pub fn commit_tree<'a>( - &'a self, - parent: Option<&git2::Commit<'_>>, - files: &[(&str, &str)], - ) -> git2::Commit<'a> { - self.commit_tree_inner(parent, &Uuid::new_v4().to_string(), files, None) - } - - pub fn commit_tree_with_message<'a>( - &'a self, - parent: Option<&git2::Commit<'_>>, - message: &str, - files: &[(&str, &str)], - ) -> git2::Commit<'a> { - self.commit_tree_inner(parent, message, files, None) - } - - pub fn commit_tree_inner<'a>( - &'a self, - parent: Option<&git2::Commit<'_>>, - message: &str, - files: &[(&str, &str)], - change_id: Option<&str>, - ) -> git2::Commit<'a> { - for entry in fs::read_dir(self.tempdir.path()).unwrap() { - let entry = entry.unwrap(); - if entry.file_name() != ".git" { - let path = entry.path(); - if path.is_dir() { - fs::remove_dir_all(path).unwrap(); - } else { - fs::remove_file(path).unwrap(); - } - } - } - for (file_name, contents) in files { - let path = self.tempdir.path().join(file_name); - let parent = path.parent().unwrap(); - - if !parent.exists() { - fs::create_dir_all(parent).unwrap(); - } - - fs::write(path, contents).unwrap(); - } - - let mut index = self.repository.index().unwrap(); - index.read(true).unwrap(); - index - .add_all(["*"], git2::IndexAddOption::DEFAULT, None) - .unwrap(); - index.write().unwrap(); - - let signature = git2::Signature::now("Caleb", "caleb@gitbutler.com").unwrap(); - let commit_headers = - change_id.map_or(Headers::new_with_random_change_id(), |change_id| Headers { - change_id: Some(ChangeId::from(change_id.as_bytes().as_bstr())), - conflicted: None, - }); - - let commit = self - .repository - .commit_with_signature( - None, - &signature, - &signature, - message, - &self - .repository - .find_tree(index.write_tree().unwrap()) - .unwrap(), - parent.map(|c| vec![c]).unwrap_or_default().as_slice(), - Some(commit_headers), - ) - .unwrap(); - - self.repository.find_commit(commit).unwrap() - } - - pub fn persist(self) { - let path = self.tempdir.keep(); - - println!("Persisting test repository at {path:?}"); - } -} - -pub fn assert_commit_tree_matches<'a>( - repository: &'a git2::Repository, - commit: &git2::Commit<'a>, - files: &[(&str, &[u8])], -) { - assert_tree_matches(repository, &commit.tree().unwrap(), files); -} - -pub fn assert_tree_matches<'a>( - repository: &'a git2::Repository, - tree: &git2::Tree<'a>, - files: &[(&str, &[u8])], -) { - assert_tree_matches_with_mode( - repository, - tree.id(), - &files - .iter() - .map(|(path, content)| (*path, *content, &[] as &[EntryAttribute])) - .collect::>(), - ); -} - -#[derive(Debug, Copy, Clone)] -pub enum EntryAttribute { - Tree, - Commit, - Link, - Blob, - Executable, -} - -pub fn assert_tree_matches_with_mode( - repository: &git2::Repository, - tree: git2::Oid, - files: &[(&str, &[u8], &[EntryAttribute])], -) { - let gix_repository = gix::open(repository.path()).unwrap(); - - let tree = gix_repository.find_tree(tree.to_gix()).unwrap(); - - for (path, content, entry_attributes) in files { - let tree_entry = tree.lookup_entry_by_path(path).unwrap().unwrap(); - let object = gix_repository.find_object(tree_entry.id()).unwrap(); - assert_eq!( - object.data, - *content, - "{}: expect {} == {}", - path, - object.data.to_str_lossy(), - content.to_str_lossy() - ); - - for entry_attribute in *entry_attributes { - match entry_attribute { - EntryAttribute::Tree => assert!(tree_entry.mode().is_tree(), "{path} is a tree"), - EntryAttribute::Commit => { - assert!(tree_entry.mode().is_commit(), "{path} is a commit") - } - EntryAttribute::Link => assert!(tree_entry.mode().is_link(), "{path} is a link"), - EntryAttribute::Blob => assert!(tree_entry.mode().is_blob(), "{path} is a blob"), - EntryAttribute::Executable => { - assert!(tree_entry.mode().is_executable(), "{path} is executable") - } - } - } - } -} diff --git a/crates/but-testsupport/src/lib.rs b/crates/but-testsupport/src/lib.rs index f5424783dd2..17d784319c5 100644 --- a/crates/but-testsupport/src/lib.rs +++ b/crates/but-testsupport/src/lib.rs @@ -15,11 +15,6 @@ use gix_testtools::{Creation, FixtureState, PostResult, tempfile}; mod in_memory_meta; pub use in_memory_meta::{InMemoryRefMetadata, InMemoryRefMetadataHandle, StackState}; -/// A module with the complete `gitbutler-testsupport` crate, with the goal of replacing its implementation with `but-testsupport` utilities. -#[cfg(feature = "legacy")] -#[allow(missing_docs)] -pub mod legacy; - #[cfg(feature = "sandbox")] mod sandbox; #[cfg(feature = "sandbox")] diff --git a/crates/but-workspace/Cargo.toml b/crates/but-workspace/Cargo.toml index 739d14fbdf6..08b3a464879 100644 --- a/crates/but-workspace/Cargo.toml +++ b/crates/but-workspace/Cargo.toml @@ -35,7 +35,6 @@ but-graph.workspace = true but-rebase.workspace = true but-error.workspace = true but-serde.workspace = true -but-oxidize.workspace = true but-meta = { workspace = true, features = ["legacy"], optional = true } but-ctx = { workspace = true, optional = true } diff --git a/crates/but-workspace/src/branch/apply.rs b/crates/but-workspace/src/branch/apply.rs index bc3b96a9b64..8651f71b8ab 100644 --- a/crates/but-workspace/src/branch/apply.rs +++ b/crates/but-workspace/src/branch/apply.rs @@ -367,7 +367,7 @@ pub(crate) mod function { let (local_tracking_config_and_ref_info, commit_to_create_branch_at) = if incoming_branch_is_remote_tracking_without_local_tracking { setup_local_tracking_configuration(repo, branch.as_ref(), branch_orig)? - .map(|(config, commit)| (Some(config), Some(commit))) + .map(|(config, lock, commit)| (Some((config, lock)), Some(commit))) .unwrap_or_default() } else { (None, None) @@ -402,11 +402,12 @@ pub(crate) mod function { .is_some_and(|(_stack, segment)| !segment.is_projected_from_outside(&ws.graph)) }); let needs_ws_ref_creation = !ws_ref_exists; - let local_tracking_config_and_ref_info = - local_tracking_config_and_ref_info.zip(commit_to_create_branch_at.map({ + let local_tracking_config_and_ref_info = local_tracking_config_and_ref_info + .zip(commit_to_create_branch_at.map({ let branch = branch.clone(); |commit| (branch, branch_orig, commit.attach(repo)) - })); + })) + .map(|((config, lock), ref_info)| (config, lock, ref_info)); let applied_branches = branches_to_apply .iter() .map(|rn| (*rn).to_owned()) @@ -825,7 +826,7 @@ pub(crate) mod function { repo: &gix::Repository, local_tracking_ref: &FullNameRef, remote_tracking_ref: &FullNameRef, - ) -> anyhow::Result, gix::ObjectId)>> { + ) -> anyhow::Result, gix::lock::File, gix::ObjectId)>> { let remote_tracking_commit_id = repo .find_reference(remote_tracking_ref)? .peel_to_commit()? @@ -834,7 +835,7 @@ pub(crate) mod function { // TODO(gix): Make config refreshes possible, and use the higher level API, and add a way // to only write back what changed and of course to add local sections more obviously. // Make it way easier to work with sections. - let mut config = repo.local_common_config_for_editing()?; + let (mut config, lock) = repo.local_common_config_for_editing()?; let mut section = config.section_mut_or_create_new("branch", Some(local_tracking_ref.shorten()))?; // Only edit the configuration if truly empty, let's not overwrite user data. @@ -852,7 +853,7 @@ pub(crate) mod function { Some(local_tracking_ref.as_bstr()), ); } - Ok(Some((config, remote_tracking_commit_id.into()))) + Ok(Some((config, lock, remote_tracking_commit_id.into()))) } fn add_branch_as_stack_forcefully( @@ -916,7 +917,8 @@ pub(crate) mod function { branches_to_apply: &[gix::refs::FullName], ws_md: &T::Handle, config_and_ref: Option<( - gix::config::File, + gix::config::File<'static>, + gix::lock::File, (gix::refs::FullName, &gix::refs::FullNameRef, gix::Id), )>, ) -> anyhow::Result<()> { @@ -930,10 +932,11 @@ pub(crate) mod function { meta.set_branch(&md)?; } - if let Some((config, (ref_to_create, remote_tracking_ref, ref_target_id))) = config_and_ref + if let Some((config, lock, (ref_to_create, remote_tracking_ref, ref_target_id))) = + config_and_ref { let repo = ref_target_id.repo; - repo.write_local_common_config(&config)?; + repo.write_locked_config(&config, lock)?; repo.reference( ref_to_create, diff --git a/crates/but-workspace/src/legacy/stacks.rs b/crates/but-workspace/src/legacy/stacks.rs index 6399f710765..334babcea2a 100644 --- a/crates/but-workspace/src/legacy/stacks.rs +++ b/crates/but-workspace/src/legacy/stacks.rs @@ -498,12 +498,10 @@ pub fn stack_branches(stack_id: StackId, ctx: &Context) -> anyhow::Result = Vec::new(); @@ -175,7 +173,7 @@ pub fn split_into_dependent_branch( .get()? .reference( new_branch_ref_name.clone(), - branch_head.to_gix(), + branch_head, gix::refs::transaction::PreviousValue::Any, new_branch_log_message.clone(), )? @@ -183,8 +181,7 @@ pub fn split_into_dependent_branch( // Remove all but the specified changes from the new branch let repo = ctx.repo.get()?; - let new_branch_commits = - first_parent_commit_ids_until(&repo, branch_head.to_gix(), merge_base)?; + let new_branch_commits = first_parent_commit_ids_until(&repo, branch_head, merge_base)?; // Branch as rebase steps let mut dependent_branch_steps: Vec = Vec::new(); @@ -226,7 +223,7 @@ pub fn split_into_dependent_branch( source_stack.add_series( ctx, - StackBranch::new(branch_head.to_gix(), new_branch_name, &repo)?, + StackBranch::new(branch_head, new_branch_name, &repo)?, Some(source_branch_name), )?; diff --git a/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs b/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs index 15ee944324b..9e3d7f2b0fb 100644 --- a/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs +++ b/crates/but-workspace/tests/workspace/branch/apply_unapply_commit_uncommit.rs @@ -1,6 +1,6 @@ use bstr::ByteSlice; use but_core::{ - RefMetadata, RepositoryExt, ref_metadata, + RefMetadata, ref_metadata, ref_metadata::{StackId, WorkspaceCommitRelation::Outside}, worktree::checkout::UncommitedWorktreeChanges, }; @@ -115,7 +115,7 @@ fn ws_ref_no_ws_commit_two_virtual_stacks_on_same_commit_apply_dependent_first() #[test] fn main_with_advanced_remote_tracking_branch() -> anyhow::Result<()> { - let (_tmp, graph, repo, _vb_version_cannot_have_remotes, _description) = + let (_tmp, graph, mut repo, _vb_version_cannot_have_remotes, _description) = named_writable_scenario_with_description_and_graph( "main-with-advanced-remote", |_meta| {}, @@ -194,7 +194,8 @@ fn main_with_advanced_remote_tracking_branch() -> anyhow::Result<()> { * 3183e43 (main) M1 "); - let config = repo.local_common_config_for_editing()?; + repo.reload()?; + let config = repo.config_snapshot(); let section = config.section("branch", Some("feature".into()))?; insta::assert_snapshot!(section.to_bstring(), @r#" [branch "feature"] diff --git a/crates/but-worktrees/Cargo.toml b/crates/but-worktrees/Cargo.toml index c0904996fda..524d9a98af5 100644 --- a/crates/but-worktrees/Cargo.toml +++ b/crates/but-worktrees/Cargo.toml @@ -14,7 +14,6 @@ but-workspace.workspace = true but-meta = { workspace = true, features = ["legacy"] } but-rebase.workspace = true but-core.workspace = true -but-oxidize.workspace = true but-ctx.workspace = true gitbutler-stack.workspace = true diff --git a/crates/but-worktrees/src/integrate.rs b/crates/but-worktrees/src/integrate.rs index 033bc90e98d..28882c99d09 100644 --- a/crates/but-worktrees/src/integrate.rs +++ b/crates/but-worktrees/src/integrate.rs @@ -5,13 +5,12 @@ use but_ctx::{ Context, access::{RepoExclusive, RepoShared}, }; -use but_oxidize::ObjectIdExt; use but_rebase::{Rebase, RebaseOutput, RebaseStep}; use but_workspace::legacy::stack_ext::StackExt; use gitbutler_branch_actions::update_workspace_commit; use gitbutler_stack::{Stack, VirtualBranchesHandle}; use gitbutler_workspace::branch_trees::{ - WorkspaceState, merge_workspace, move_tree, update_uncommitted_changes, + WorkspaceState, merge_workspace, move_tree_has_conflicts, update_uncommitted_changes, }; use gix::prelude::ObjectIdExt as _; use serde::{Deserialize, Serialize}; @@ -244,13 +243,12 @@ fn worktree_integration_inner( let wd_tree = repo.create_wd_tree(0)?; let working_dir_conflicts = { - let gix_repo = ctx.clone_repo_for_merging()?; let before_heads = stacks .iter() .map(|s| s.head_oid(ctx)) .collect::>>()?; let before = WorkspaceState::create_from_heads(ctx, perm, &before_heads)?; - let before = merge_workspace(&gix_repo, &before)?.to_git2(); + let before = merge_workspace(&repo, &before)?; let mut after_heads = stacks .iter() .filter(|s| s.id != stack.id) @@ -258,10 +256,9 @@ fn worktree_integration_inner( .collect::>>()?; after_heads.push(output.top_commit); let after = WorkspaceState::create_from_heads(ctx, perm, &after_heads)?; - let after = merge_workspace(&gix_repo, &after)?.to_git2(); - let index = move_tree(&*ctx.git2_repo.get()?, wd_tree.to_git2(), before, after)?; + let after = merge_workspace(&repo, &after)?; - index.has_conflicts() + move_tree_has_conflicts(ctx, wd_tree, before, after)? }; Ok(( diff --git a/crates/but/src/command/alias.rs b/crates/but/src/command/alias.rs index 6b8f65e86c0..08ae7605762 100644 --- a/crates/but/src/command/alias.rs +++ b/crates/but/src/command/alias.rs @@ -256,7 +256,7 @@ pub fn add( let repo = ctx.repo.get()?; edit_git_config(&repo, global, |config| { set_alias(config, name, value)?; - Ok(true) + Ok(()) })?; if let Some(out) = out.for_human() { @@ -291,7 +291,10 @@ pub fn remove( ) -> Result<()> { let is_global: bool = global.into(); let repo = ctx.repo.get()?; - let success = edit_git_config(&repo, global, |config| Ok(remove_alias(config, name)))?; + let success = edit_git_config(&repo, global, |config| { + remove_alias(config, name); + Ok(()) + })?; if let Some(out) = out.for_human() { if !success { @@ -314,12 +317,11 @@ pub fn remove( Ok(()) } -fn remove_alias(config: &mut gix::config::File<'_>, name: &str) -> bool { +fn remove_alias(config: &mut gix::config::File<'_>, name: &str) { config .section_mut("but", Some("alias".into())) .ok() - .and_then(|mut section| section.remove(name)) - .is_some() + .and_then(|mut section| section.remove(name)); } fn set_alias(config: &mut gix::config::File<'static>, name: &str, value: &str) -> Result<()> { diff --git a/crates/but/src/command/config.rs b/crates/but/src/command/config.rs index 089b10ef12e..5271af1a225 100644 --- a/crates/but/src/command/config.rs +++ b/crates/but/src/command/config.rs @@ -443,7 +443,7 @@ async fn user_config( let git_key = key.to_git_key(); edit_git_config(&repo, global.into(), |config| { set_config_value(config, git_key, &value)?; - Ok(true) + Ok(()) })?; if let Some(out) = out.for_human() { @@ -471,7 +471,7 @@ async fn user_config( let git_key = key.to_git_key(); edit_git_config(&repo, global.into(), |config| { remove_config_value(config, git_key)?; - Ok(true) + Ok(()) })?; if let Some(out) = out.for_human() { @@ -1370,15 +1370,11 @@ fn maybe_set_secret(handle: &str, secret_value: Option>) -> Re fn edit_ai_git_config( repo: Option<&gix::Repository>, scope: AiScope, - edit: impl FnOnce(&mut gix::config::File<'static>) -> Result, + edit: impl FnOnce(&mut gix::config::File<'static>) -> Result<()>, ) -> Result<()> { match scope { AiScope::Global => { - let (mut config, path) = but_core::git_config::open_user_global_config_for_editing()?; - let changed = edit(&mut config)?; - if changed { - but_core::git_config::write_config(&path, &config)?; - } + but_core::git_config::edit_config(None, gix::config::Source::User, edit)?; Ok(()) } AiScope::Local => { @@ -1417,7 +1413,7 @@ fn apply_openai_config( set_config_value(config, AI_OPENAI_KEY_OPTION_KEY, key_option.as_git_value())?; set_optional_config_value(config, AI_OPENAI_MODEL_NAME_KEY, model)?; set_optional_config_value(config, AI_OPENAI_CUSTOM_ENDPOINT_KEY, endpoint)?; - Ok(true) + Ok(()) })?; if matches!(key_option, AiKeyOption::BringYourOwn) { @@ -1445,7 +1441,7 @@ fn apply_anthropic_config( key_option.as_git_value(), )?; set_optional_config_value(config, AI_ANTHROPIC_MODEL_NAME_KEY, model)?; - Ok(true) + Ok(()) })?; if matches!(key_option, AiKeyOption::BringYourOwn) { @@ -1468,7 +1464,7 @@ fn apply_ollama_config( )?; set_optional_config_value(config, AI_OLLAMA_ENDPOINT_KEY, endpoint)?; set_optional_config_value(config, AI_OLLAMA_MODEL_NAME_KEY, model)?; - Ok(true) + Ok(()) }) } @@ -1486,7 +1482,7 @@ fn apply_lmstudio_config( )?; set_optional_config_value(config, AI_LMSTUDIO_ENDPOINT_KEY, endpoint)?; set_optional_config_value(config, AI_LMSTUDIO_MODEL_NAME_KEY, model)?; - Ok(true) + Ok(()) }) } @@ -1698,7 +1694,7 @@ fn ui_config(ctx: &mut Context, out: &mut OutputChannel, cmd: Option changed` to find out if the passed configuration was indeed changed, -/// to persist the file if that was the case. +/// It compares the edited config to its previous value to determine whether to persist it. pub(crate) fn edit_git_config( repo: &gix::Repository, global: EditGlobalConfig, - edit: impl FnOnce(&mut gix::config::File<'static>) -> Result, + edit: impl FnOnce(&mut gix::config::File<'static>) -> Result<()>, ) -> Result { - if global.into() { - let (mut config, path) = open_user_global_config_for_editing()?; - let changed = edit(&mut config)?; - if changed { - write_config(&path, &config)?; - } - Ok(changed) + let source = if global.into() { + gix::config::Source::User } else { - let mut config = repo.local_common_config_for_editing()?; - let changed = edit(&mut config)?; - if changed { - repo.write_local_common_config(&config)?; - } - Ok(changed) - } + gix::config::Source::Local + }; + edit_repo_config(repo, source, edit) } diff --git a/crates/but/src/command/legacy/resolve.rs b/crates/but/src/command/legacy/resolve.rs index 6b5dc3c2754..2c5348b46dd 100644 --- a/crates/but/src/command/legacy/resolve.rs +++ b/crates/but/src/command/legacy/resolve.rs @@ -219,8 +219,8 @@ fn show_conflicted_files(ctx: &mut Context, out: &mut OutputChannel) -> Result anyhow )?; } - let mut config = repo.local_common_config_for_editing()?; - let mut section = config.section_mut_or_create_new("remote", Some("gb-local".into()))?; - section.push("url".try_into()?, Some(repo_url.into())); - repo.write_local_common_config(&config)?; + edit_repo_config(repo, gix::config::Source::Local, |config| { + let mut section = config.section_mut_or_create_new("remote", Some("gb-local".into()))?; + section.push("url".try_into()?, Some(repo_url.into())); + Ok(()) + })?; // Figure out what local branch is probably the default target let mut head_ref = repo.head()?; diff --git a/crates/but/tests/but/command/alias.rs b/crates/but/tests/but/command/alias.rs index 8ee9721c821..ae11d028cf2 100644 --- a/crates/but/tests/but/command/alias.rs +++ b/crates/but/tests/but/command/alias.rs @@ -5,6 +5,10 @@ fn local_alias_roundtrip_uses_repo_config() -> anyhow::Result<()> { let env = Sandbox::empty()?; env.invoke_bash("git init repo"); + env.invoke_git_fails( + "-C repo config --local --get but.alias.st", + "no local alias is set", + ); env.but("-C repo alias add st status").assert().success(); assert_eq!( env.invoke_git("-C repo config --local --get but.alias.st"), diff --git a/crates/gitbutler-branch-actions/Cargo.toml b/crates/gitbutler-branch-actions/Cargo.toml index b12a1301359..e3fb9759798 100644 --- a/crates/gitbutler-branch-actions/Cargo.toml +++ b/crates/gitbutler-branch-actions/Cargo.toml @@ -64,7 +64,7 @@ but-schemars = { workspace = true, optional = true } [dev-dependencies] pretty_assertions = "1.4" but-graph.workspace = true -but-testsupport = { workspace = true, features = ["legacy"] } +but-testsupport.workspace = true gitbutler-workspace.workspace = true gix = { workspace = true, features = [] } glob = "0.3.3" diff --git a/crates/gitbutler-branch-actions/src/base.rs b/crates/gitbutler-branch-actions/src/base.rs index 3b7dcadd021..a5acf60c599 100644 --- a/crates/gitbutler-branch-actions/src/base.rs +++ b/crates/gitbutler-branch-actions/src/base.rs @@ -2,7 +2,7 @@ use std::{path::Path, time}; use anyhow::{Context as _, Result, anyhow}; use but_core::{ - RepositoryExt as _, git_config::ensure_config_value, + git_config::{edit_repo_config, ensure_config_value}, worktree::checkout::UncommitedWorktreeChanges, }; use but_ctx::Context; @@ -11,7 +11,7 @@ use but_oxidize::ObjectIdExt; use gitbutler_branch::GITBUTLER_WORKSPACE_REFERENCE; use gitbutler_project::FetchResult; use gitbutler_reference::{Refname, RemoteRefname}; -use gitbutler_repo::{RepositoryExt, first_parent_commit_ids_until}; +use gitbutler_repo::first_parent_commit_ids_until; use gitbutler_repo_actions::RepoActionsExt; use gitbutler_stack::{Stack, Target, VirtualBranchesHandle, canned_branch_name}; use serde::Serialize; @@ -156,12 +156,14 @@ fn go_back_to_integration(ctx: &Context, default_target: &Target) -> Result R fn set_exclude_decoration(ctx: &Context) -> Result<()> { let repo = ctx.repo.get()?; - let mut config = repo.local_common_config_for_editing()?; - let changed = ensure_config_value(&mut config, "log.excludeDecoration", "refs/gitbutler") - .context("failed to set log.excludeDecoration")?; - if changed { - repo.write_local_common_config(&config)?; - } + edit_repo_config(&repo, gix::config::Source::Local, |config| { + ensure_config_value(config, "log.excludeDecoration", "refs/gitbutler") + .context("failed to set log.excludeDecoration")?; + Ok(()) + })?; Ok(()) } @@ -509,7 +510,7 @@ fn first_parent_commit_ids_with_limit( pub(crate) fn push(ctx: &Context, with_force: bool) -> Result<()> { let target = default_target(&ctx.project_data_dir())?; let _ = ctx.push( - target.sha.to_git2(), + target.sha, &target.branch, with_force, ctx.legacy_project.force_push_protection, 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 de55c282dec..46f3dc9c8b9 100644 --- a/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs +++ b/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs @@ -2,7 +2,6 @@ use anyhow::{Context as _, Result, anyhow, bail}; use but_core::{RepositoryExt, worktree::checkout::UncommitedWorktreeChanges}; use but_ctx::access::RepoExclusive; use but_error::Marker; -use but_oxidize::ObjectIdExt; use but_workspace::{ branch::{ OnWorkspaceMergeConflict, @@ -345,7 +344,7 @@ impl BranchManager<'_> { self.ctx, workspace_state, new_workspace, - old_cwd.map(|id| id.to_git2()), + old_cwd, Some(true), perm, ); diff --git a/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs b/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs index bfed6d47fae..7ed0f075db7 100644 --- a/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs +++ b/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs @@ -3,7 +3,6 @@ use but_core::{DiffSpec, RepositoryExt}; use but_ctx::access::RepoExclusive; use but_oxidize::ObjectIdExt; use gitbutler_cherry_pick::GixRepositoryExt as _; -use gitbutler_repo::RepositoryExt as _; use gitbutler_repo_actions::RepoActionsExt; use gitbutler_stack::StackId; use gitbutler_workspace::workspace_base; @@ -35,6 +34,7 @@ impl BranchManager<'_> { .to_string()); } + #[expect(deprecated, reason = "checkout/materialization boundary")] let git2_repo = self.ctx.git2_repo.get()?; // Commit any assigned diffspecs if such exist so that it will be part of the unapplied branch. @@ -106,9 +106,10 @@ impl BranchManager<'_> { git2_repo.find_tree(merge.tree.write()?.to_git2())?; git2_repo - .checkout_tree_builder(&new_workspace_tree_with_worktree_changes) - .force() - .checkout() + .checkout_tree( + new_workspace_tree_with_worktree_changes.as_object(), + Some(git2::build::CheckoutBuilder::new().force()), + ) .context("failed to checkout tree")?; } diff --git a/crates/gitbutler-branch-actions/src/integration.rs b/crates/gitbutler-branch-actions/src/integration.rs index 903d7ee6d7b..8a5cff4b2db 100644 --- a/crates/gitbutler-branch-actions/src/integration.rs +++ b/crates/gitbutler-branch-actions/src/integration.rs @@ -80,6 +80,7 @@ pub fn update_workspace_commit_with_vb_state( .get_default_target() .context("failed to get target")?; + #[expect(deprecated, reason = "workspace checkout/index boundary")] let repo = &*ctx.git2_repo.get()?; let gix_repo = ctx.repo.get()?.clone(); @@ -263,6 +264,7 @@ fn verify_current_branch_name(ctx: &Context) -> Result<&Context> { // TODO(ST): Probably there should not be an implicit vbranch creation here. fn verify_head_is_clean(ctx: &Context, perm: &mut RepoExclusive) -> Result<()> { + #[expect(deprecated, reason = "soft reset/index boundary")] let git2_repo = &*ctx.git2_repo.get()?; let gix_repo = ctx.repo.get()?.clone(); let head_commit_id = gix_repo.head_id()?.detach(); diff --git a/crates/gitbutler-branch-actions/src/stack.rs b/crates/gitbutler-branch-actions/src/stack.rs index 7af79767339..e3e4cb46723 100644 --- a/crates/gitbutler-branch-actions/src/stack.rs +++ b/crates/gitbutler-branch-actions/src/stack.rs @@ -1,7 +1,6 @@ use anyhow::{Context as _, Result}; use but_core::RepositoryExt; use but_ctx::Context; -use but_oxidize::OidExt; use gitbutler_operating_modes::ensure_open_workspace_mode; use gitbutler_oplog::{ OplogExt, SnapshotExt, @@ -237,7 +236,7 @@ pub fn push_stack( &gix_repo, &remote_name, &url, - push_details.head.to_gix(), + push_details.head, &push_details.remote_refname, ctx.legacy_project.husky_hooks_enabled, )? { diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs b/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs index c25a9d3f427..ed18cdc29fc 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs @@ -2,21 +2,22 @@ use std::fs; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; -use but_testsupport::legacy::{Case, Suite}; use gitbutler_repo::hooks::{ErrorData, HookResult, MessageData, MessageHookResult}; +use crate::support::hook_case; + #[test] fn post_commit_hook_rejection() -> anyhow::Result<()> { - let suite = Suite::default(); - let mut case = suite.new_case(); + let mut case = hook_case()?; case.ctx.legacy_project.husky_hooks_enabled = true; - let Case { ctx, .. } = &case; + let ctx = &case.ctx; let hook = b" #!/bin/sh echo 'rejected' exit 1 "; + #[expect(deprecated, reason = "hook compatibility coverage")] git2_hooks::create_hook(&*ctx.git2_repo.get()?, git2_hooks::HOOK_POST_COMMIT, hook); assert_eq!( @@ -30,16 +31,16 @@ exit 1 #[test] fn message_hook_rejection() -> anyhow::Result<()> { - let suite = Suite::default(); - let mut case = suite.new_case(); + let mut case = hook_case()?; case.ctx.legacy_project.husky_hooks_enabled = true; - let Case { ctx, .. } = &case; + let ctx = &case.ctx; let hook = b" #!/bin/sh echo 'rejected' exit 1 "; + #[expect(deprecated, reason = "hook compatibility coverage")] git2_hooks::create_hook(&*ctx.git2_repo.get()?, git2_hooks::HOOK_COMMIT_MSG, hook); let message = "commit message".to_owned(); @@ -54,15 +55,15 @@ exit 1 #[test] fn rewrite_message() -> anyhow::Result<()> { - let suite = Suite::default(); - let mut case = suite.new_case(); + let mut case = hook_case()?; case.ctx.legacy_project.husky_hooks_enabled = true; - let Case { ctx, .. } = &case; + let ctx = &case.ctx; let hook = b" #!/bin/sh echo 'rewritten message' > $1 "; + #[expect(deprecated, reason = "hook compatibility coverage")] git2_hooks::create_hook(&*ctx.git2_repo.get()?, git2_hooks::HOOK_COMMIT_MSG, hook); let message = "commit message".to_owned(); @@ -77,15 +78,15 @@ echo 'rewritten message' > $1 #[test] fn keep_message() -> anyhow::Result<()> { - let suite = Suite::default(); - let mut case = suite.new_case(); + let mut case = hook_case()?; case.ctx.legacy_project.husky_hooks_enabled = true; - let Case { ctx, .. } = &case; + let ctx = &case.ctx; let hook = b" #!/bin/sh echo 'commit message' > $1 "; + #[expect(deprecated, reason = "hook compatibility coverage")] git2_hooks::create_hook(&*ctx.git2_repo.get()?, git2_hooks::HOOK_COMMIT_MSG, hook); let message = "commit message\n".to_owned(); @@ -98,10 +99,10 @@ echo 'commit message' > $1 #[test] fn husky_hooks_disabled_even_if_present() -> anyhow::Result<()> { - let suite = Suite::default(); - let case = suite.new_case(); - let Case { ctx, .. } = &case; + let case = hook_case()?; + let ctx = &case.ctx; + #[expect(deprecated, reason = "hook compatibility coverage")] let repo = ctx.git2_repo.get()?; let husky_dir = repo .path() diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/main.rs b/crates/gitbutler-branch-actions/tests/branch-actions/main.rs index 7d4bbc5fbfe..f2292b5b7ed 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/main.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/main.rs @@ -3,5 +3,6 @@ mod driverless; mod hooks; mod reorder; mod squash; +mod support; mod virtual_branches; mod workspace_commit; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs b/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs index 28ebb21cae4..07ca0d48bc9 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs @@ -2,15 +2,13 @@ use std::collections::HashMap; use anyhow::Result; use but_ctx::Context; -use but_oxidize::ObjectIdExt; -use but_testsupport::legacy::testing_repository::assert_commit_tree_matches; use gitbutler_branch_actions::{StackOrder, reorder::SeriesOrder, reorder_stack}; use gitbutler_commit::commit_ext::CommitMessageBstr as _; use gitbutler_stack::VirtualBranchesHandle; use itertools::Itertools; use tempfile::TempDir; -use crate::driverless; +use crate::{driverless, support}; #[test] fn noop_reorder_errors() -> Result<()> { @@ -347,21 +345,18 @@ fn conflicting_reorder_stack() -> Result<()> { // assert!(commits[1].timestamps().windows(2).all(|w| w[0] >= w[1])); // commit timestamps in descending order NB: This assertion started failing after switching to ws3 { - let repo = &*ctx.git2_repo.get()?; - let commit_1_prime = repo.find_commit(commits[1].ids()[0].to_git2())?; - assert_commit_tree_matches(repo, &commit_1_prime, &[("file", b"x\n")]); - - let commit_2_prime = repo.find_commit(commits[1].ids()[1].to_git2())?; + let repo = ctx.repo.get()?; + assert_commit_tree_matches(&repo, commits[1].ids()[0], &[("file", b"x\n")])?; assert_commit_tree_matches( - repo, - &commit_2_prime, + &repo, + commits[1].ids()[1], &[ (".auto-resolution/file", b"a\n"), (".conflict-base-0/file", b"x\n"), (".conflict-side-0/file", b"a\n"), (".conflict-side-1/file", b"y\n"), ], - ); + )?; } // Reordered the commits back to the original order @@ -384,12 +379,9 @@ fn conflicting_reorder_stack() -> Result<()> { assert!(commits[1].timestamps().windows(2).all(|w| w[0] >= w[1])); // commit timestamps in descending order { - let repo = &*ctx.git2_repo.get()?; - let commit_2_prime_prime = repo.find_commit(commits[1].ids()[0].to_git2())?; - assert_commit_tree_matches(repo, &commit_2_prime_prime, &[("file", b"y\n")]); - - let commit_1_prime_prime = repo.find_commit(commits[1].ids()[1].to_git2())?; - assert_commit_tree_matches(repo, &commit_1_prime_prime, &[("file", b"x\n")]); + let repo = ctx.repo.get()?; + assert_commit_tree_matches(&repo, commits[1].ids()[0], &[("file", b"y\n")])?; + assert_commit_tree_matches(&repo, commits[1].ids()[1], &[("file", b"x\n")])?; } Ok(()) @@ -436,7 +428,7 @@ impl CommitHelpers for Vec<(gix::ObjectId, String, bool, u128)> { /// Commits from list_virtual_branches fn vb_commits(ctx: &Context) -> Vec> { - let details = but_testsupport::legacy::stack_details(ctx); + let details = support::stack_details(ctx); let (_, my_stack) = details .iter() .find(|(_, d)| d.derived_name == "top-series") @@ -459,12 +451,31 @@ fn vb_commits(ctx: &Context) -> Vec> { } fn file(ctx: &Context, commit_id: gix::ObjectId) -> String { - let repo = &*ctx.git2_repo.get().unwrap(); - let commit = repo.find_commit(commit_id.to_git2()).unwrap(); - let tree = commit.tree().unwrap(); - let entry = tree.get_name("file").unwrap(); - let blob = repo.find_blob(entry.id()).unwrap(); - String::from_utf8(blob.content().to_vec()).unwrap() + let repo = ctx.repo.get().unwrap(); + let tree = repo.find_commit(commit_id).unwrap().tree().unwrap(); + let entry = tree.lookup_entry_by_path("file").unwrap().unwrap(); + let blob = entry.object().unwrap().into_blob(); + String::from_utf8(blob.data.to_vec()).unwrap() +} + +fn assert_commit_tree_matches( + repo: &gix::Repository, + commit_id: gix::ObjectId, + files: &[(&str, &[u8])], +) -> Result<()> { + let tree = repo.find_commit(commit_id)?.tree()?; + for (path, expected) in files { + let entry = tree + .lookup_entry_by_path(path)? + .unwrap_or_else(|| panic!("expected {path} in commit {commit_id}")); + let blob = entry.object()?.into_blob(); + assert_eq!( + blob.data.as_slice(), + *expected, + "unexpected blob contents for {path} in commit {commit_id}" + ); + } + Ok(()) } fn command_ctx(name: &str) -> Result<(Context, TempDir)> { diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs b/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs index c84425d832e..0be10f10c3a 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs @@ -7,7 +7,7 @@ use gitbutler_stack::{StackBranch, VirtualBranchesHandle}; use itertools::Itertools; use tempfile::TempDir; -use crate::driverless; +use crate::{driverless, support}; // Squash commit into it's parent without affecting stack heads // @@ -492,7 +492,7 @@ struct Branch { /// Stack branches from the API fn list_branches(ctx: &Context) -> Result { - let details = but_testsupport::legacy::stack_details(ctx); + let details = support::stack_details(ctx); let (_, details) = details.first().unwrap(); let branches: Vec = details .branch_details diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/support.rs b/crates/gitbutler-branch-actions/tests/branch-actions/support.rs new file mode 100644 index 00000000000..892dfec6e01 --- /dev/null +++ b/crates/gitbutler-branch-actions/tests/branch-actions/support.rs @@ -0,0 +1,62 @@ +use anyhow::Result; +use but_ctx::{Context, RepoOpenMode}; +use but_settings::AppSettings; +use but_testsupport::gix_testtools::{Creation, scripted_fixture_writable_with_args}; +use but_workspace::{legacy::StacksFilter, ui::StackDetails}; +use gitbutler_stack::StackId; +use tempfile::{TempDir, tempdir}; + +pub struct HookCase { + pub ctx: Context, + _app_data_dir: TempDir, + _repo_dir: TempDir, +} + +pub fn hook_case() -> Result { + let repo_dir = scripted_fixture_writable_with_args( + "scenario/repo-with-origin.sh", + None::, + Creation::Execute, + ) + .map_err(anyhow::Error::from_boxed)?; + let local_repo_dir = repo_dir.path().join("local"); + let app_data_dir = tempdir()?; + let project = gitbutler_project::add_at_app_data_dir(app_data_dir.path(), &local_repo_dir)? + .unwrap_project(); + let ctx = Context::new_from_legacy_project_and_settings_with_repo_open_mode( + &project, + AppSettings::default(), + RepoOpenMode::Isolated, + )?; + Ok(HookCase { + ctx, + _app_data_dir: app_data_dir, + _repo_dir: repo_dir, + }) +} + +pub fn stack_details(ctx: &Context) -> Vec<(StackId, StackDetails)> { + let repo = ctx.clone_repo_for_merging_non_persisting().unwrap(); + let stacks = { + let meta = ctx.legacy_meta().unwrap(); + let mut cache = ctx.cache.get_cache_mut().unwrap(); + but_workspace::legacy::stacks_v3(&repo, &meta, StacksFilter::default(), None, &mut cache) + } + .unwrap(); + + stacks + .into_iter() + .map(|stack| { + let stack_id = stack + .id + .expect("BUG(opt-stack-id): test code shouldn't trigger this"); + let details = { + let meta = ctx.legacy_meta().unwrap(); + let mut cache = ctx.cache.get_cache_mut().unwrap(); + but_workspace::legacy::stack_details_v3(stack_id.into(), &repo, &meta, &mut cache) + } + .unwrap(); + (stack_id, details) + }) + .collect() +} diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/amend.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/amend.rs index 7b41601a2ac..99e83d52cd3 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/amend.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/amend.rs @@ -1,5 +1,4 @@ use but_core::{DiffSpec, HunkHeader}; -use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use super::{list_commit_files, *}; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/apply_virtual_branch.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/apply_virtual_branch.rs index 88f9c2faf5f..669d2f4e003 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/apply_virtual_branch.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/apply_virtual_branch.rs @@ -1,7 +1,6 @@ use std::collections::HashMap; use but_forge::ForgeReview; -use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use gitbutler_branch_actions::upstream_integration::{BranchStatus, StackStatuses, TreeStatus}; use gitbutler_reference::Refname; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/create_virtual_branch_from_branch.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/create_virtual_branch_from_branch.rs index 6b4ef152b18..16dc5c3b293 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/create_virtual_branch_from_branch.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/create_virtual_branch_from_branch.rs @@ -1,4 +1,3 @@ -use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use gitbutler_reference::LocalRefname; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/init.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/init.rs index b861bd6643a..333e05691eb 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/init.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/init.rs @@ -1,12 +1,12 @@ -use but_testsupport::legacy::stack_details; +use but_core::git_config::{edit_config, set_config_value}; use super::*; #[test] fn twice() { - let data_dir = paths::data_dir(); + let data_dir = tempfile::tempdir().unwrap(); - let test_project = TestProject::default(); + let test_project = TestRepo::default(); { let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), test_project.path()) @@ -204,7 +204,7 @@ fn commit_on_target() { fn submodule() { let Test { repo, ctx, .. } = &mut Test::default(); - let test_project = TestProject::default(); + let test_project = TestRepo::default(); let submodule_url: gitbutler_url::Url = test_project.path().display().to_string().parse().unwrap(); repo.add_submodule(&submodule_url, path::Path::new("submodule")); @@ -244,18 +244,23 @@ fn bootstrap_missing_target_preserves_existing_workspace_ref() -> anyhow::Result )?; drop(guard); - let original_workspace_ref_target = ctx - .git2_repo - .get()? - .find_reference("refs/heads/gitbutler/workspace")? - .target() - .expect("workspace ref should point to a commit"); + let repo = ctx.repo.get()?; + let original_workspace_ref_target = repo + .try_find_reference("refs/heads/gitbutler/workspace")? + .expect("workspace ref should exist") + .peel_to_id()? + .detach(); let expected_stack_name = stack_details(ctx)[0].1.derived_name.clone(); - ctx.git2_repo.get()?.config()?.set_str( - but_project_handle::storage_path_config_key(), - "gitbutler-alt", - )?; + edit_config(Some(&repo), gix::config::Source::Local, |config| { + set_config_value( + config, + but_project_handle::storage_path_config_key(), + "gitbutler-alt", + )?; + Ok(()) + })?; + drop(repo); let mut reopened: Context = project_id.clone().try_into()?; assert!( @@ -273,11 +278,12 @@ fn bootstrap_missing_target_preserves_existing_workspace_ref() -> anyhow::Result drop(guard); let workspace_ref_target_after_activation = reopened - .git2_repo + .repo .get()? - .find_reference("refs/heads/gitbutler/workspace")? - .target() - .expect("workspace ref should still point to a commit"); + .try_find_reference("refs/heads/gitbutler/workspace")? + .expect("workspace ref should still exist") + .peel_to_id()? + .detach(); assert_eq!( workspace_ref_target_after_activation, original_workspace_ref_target diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/mod.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/mod.rs index 43b1db63aee..9e792f02e9e 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/mod.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/mod.rs @@ -2,17 +2,203 @@ use std::{fs, path, path::PathBuf, str::FromStr}; use but_ctx::{Context, ProjectHandleOrLegacyProjectId, RepoOpenMode}; use but_error::Marker; +use but_project_handle::storage_path_config_key; use but_settings::AppSettings; -use but_testsupport::legacy::{TestProject, VAR_NO_CLEANUP, paths}; +use but_testsupport::gix_testtools::{Creation, scripted_fixture_writable_with_args}; use gitbutler_branch::BranchCreateRequest; use gitbutler_branch_actions::GITBUTLER_WORKSPACE_COMMIT_TITLE; use gitbutler_oplog::{OplogExt, SnapshotExt}; use gitbutler_project::{self as projects}; +use gitbutler_reference::{LocalRefname, Refname}; use gitbutler_stack::StackId; -use tempfile::TempDir; +use tempfile::{TempDir, tempdir}; + +pub(crate) use crate::support::stack_details; + +const VAR_NO_CLEANUP: &str = "GITBUTLER_TESTS_NO_CLEANUP"; + +struct TestRepo { + local_repo: git2::Repository, + temp_dir: Option, +} + +impl Default for TestRepo { + fn default() -> Self { + let temp_dir = scripted_fixture_writable_with_args( + "scenario/repo-with-origin.sh", + None::, + Creation::Execute, + ) + .map_err(anyhow::Error::from_boxed) + .expect("fixture should materialize"); + let local_repo = + git2::Repository::open(temp_dir.path().join("local")).expect("local repo opens"); + setup_config(&local_repo.config().expect("config exists")).expect("config is writable"); + local_repo + .remote_set_url( + "origin", + temp_dir + .path() + .join("remote") + .to_str() + .expect("remote path is valid UTF-8"), + ) + .expect("origin remote URL can be normalized"); + sync_origin_refs(&local_repo).expect("origin refs are available locally"); + + Self { + local_repo, + temp_dir: Some(temp_dir), + } + } +} + +impl Drop for TestRepo { + fn drop(&mut self) { + if std::env::var_os(VAR_NO_CLEANUP).is_some() { + let _ = self.temp_dir.take().map(|tmp| tmp.keep()); + } + } +} + +impl TestRepo { + pub fn debug_local_repo(&mut self) -> Option { + self.temp_dir.take().map(|tmp| tmp.keep().join("local")) + } + + pub fn path(&self) -> &std::path::Path { + self.local_repo.workdir().expect("non-bare worktree") + } + + pub fn push_branch(&self, branch: &LocalRefname) { + let mut origin = self.local_repo.find_remote("origin").unwrap(); + origin.push(&[&format!("{branch}:{branch}")], None).unwrap(); + sync_origin_refs(&self.local_repo).unwrap(); + } + + pub fn push(&self) { + let mut origin = self.local_repo.find_remote("origin").unwrap(); + origin + .push(&["refs/heads/master:refs/heads/master"], None) + .unwrap(); + sync_origin_refs(&self.local_repo).unwrap(); + } + + pub fn reset_hard(&self, oid: Option) { + let mut index = self.local_repo.index().expect("failed to get index"); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .expect("failed to add all"); + index.write().expect("failed to write index"); + + let head = self.local_repo.head().unwrap(); + let commit = oid.map_or(head.peel_to_commit().unwrap(), |oid| { + self.local_repo.find_commit(oid).unwrap() + }); + + self.local_repo + .reset(commit.as_object(), git2::ResetType::Hard, None) + .unwrap(); + } + + pub fn checkout_commit(&self, commit_oid: git2::Oid) { + let commit = self.local_repo.find_commit(commit_oid).unwrap(); + let commit_tree = commit.tree().unwrap(); + + self.local_repo.set_head_detached(commit_oid).unwrap(); + self.local_repo + .checkout_tree( + commit_tree.as_object(), + Some(git2::build::CheckoutBuilder::new().force()), + ) + .unwrap(); + } + + pub fn checkout(&self, branch: &LocalRefname) { + let refname: Refname = branch.into(); + let head_commit = self.local_repo.head().unwrap().peel_to_commit().unwrap(); + let tree = match maybe_find_branch_by_refname(&self.local_repo, &refname) { + Ok(branch) => match branch { + Some(branch) => branch.get().peel_to_tree().unwrap(), + None => { + self.local_repo + .reference(&refname.to_string(), head_commit.id(), false, "new branch") + .unwrap(); + head_commit.tree().unwrap() + } + }, + Err(error) => panic!("{error:?}"), + }; + self.local_repo.set_head(&refname.to_string()).unwrap(); + self.local_repo + .checkout_tree( + tree.as_object(), + Some(git2::build::CheckoutBuilder::new().force()), + ) + .unwrap(); + } + + pub fn commit_all(&self, message: &str) -> git2::Oid { + let head = self.local_repo.head().unwrap(); + let mut index = self.local_repo.index().expect("failed to get index"); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .expect("failed to add all"); + index.write().expect("failed to write index"); + let tree_id = index.write_tree().expect("failed to write tree"); + let tree = self.local_repo.find_tree(tree_id).expect("tree exists"); + let signature = git2::Signature::now("test", "test@email.com").unwrap(); + let parent = self + .local_repo + .find_commit( + self.local_repo + .refname_to_id("HEAD") + .expect("failed to get head"), + ) + .expect("failed to find parent commit"); + self.local_repo + .commit( + Some(head.name().expect("head has name")), + &signature, + &signature, + message, + &tree, + &[&parent], + ) + .expect("failed to commit") + } + + pub fn add_submodule(&self, url: &gitbutler_url::Url, path: &path::Path) { + let mut submodule = self + .local_repo + .submodule(&url.to_string(), path.as_ref(), false) + .unwrap(); + let repo = submodule.open().unwrap(); + + repo.find_remote("origin") + .unwrap() + .fetch(&["+refs/heads/*:refs/heads/*"], None, None) + .unwrap(); + let reference = repo.find_reference("refs/heads/master").unwrap(); + let reference_head = repo.find_commit(reference.target().unwrap()).unwrap(); + repo.checkout_tree(reference_head.tree().unwrap().as_object(), None) + .unwrap(); + + repo.set_head("refs/heads/master").unwrap(); + submodule.add_finalize().unwrap(); + } + + pub fn references(&self) -> Vec> { + self.local_repo + .references() + .expect("failed to get references") + .collect::, _>>() + .expect("failed to read references") + } +} struct Test { - repo: TestProject, + repo: TestRepo, project_id: ProjectHandleOrLegacyProjectId, data_dir: Option, ctx: Context, @@ -20,9 +206,9 @@ struct Test { impl Test { pub fn new_with_settings(change_settings: fn(&mut AppSettings)) -> Self { - let data_dir = paths::data_dir(); + let data_dir = tempdir().expect("tempdir exists"); - let test_project = TestProject::default(); + let test_project = TestRepo::default(); let outcome = gitbutler_project::add_at_app_data_dir(data_dir.as_ref(), test_project.path()) .expect("failed to add project"); @@ -67,6 +253,64 @@ impl Test { } } +fn setup_config(config: &git2::Config) -> anyhow::Result<()> { + match config.open_level(git2::ConfigLevel::Local) { + Ok(mut local) => { + local.set_str("commit.gpgsign", "false")?; + local.set_str("user.name", "gitbutler-test")?; + local.set_str("user.email", "gitbutler-test@example.com")?; + local.set_str(storage_path_config_key(), "gitbutler")?; + Ok(()) + } + Err(err) => Err(err.into()), + } +} + +fn sync_origin_refs(repo: &git2::Repository) -> anyhow::Result<()> { + let remote = repo.find_remote("origin")?; + let url = remote + .url() + .ok_or_else(|| anyhow::anyhow!("origin must have a URL"))?; + let remote_path = if let Some(path) = url.strip_prefix("file://") { + PathBuf::from(path) + } else { + repo.path().parent().expect("git dir in worktree").join(url) + }; + let remote_repo = git2::Repository::open(remote_path)?; + for reference in remote_repo.references_glob("refs/heads/*")? { + let reference = reference?; + let name = reference.name().expect("reference has name"); + let short = name + .strip_prefix("refs/heads/") + .expect("head ref prefix is present"); + repo.reference( + &format!("refs/remotes/origin/{short}"), + reference.target().expect("direct ref target"), + true, + "sync origin refs for tests", + )?; + } + Ok(()) +} + +fn maybe_find_branch_by_refname<'repo>( + repo: &'repo git2::Repository, + name: &Refname, +) -> anyhow::Result>> { + let branch = repo.find_branch( + &name.simple_name(), + match name { + Refname::Virtual(_) | Refname::Local(_) | Refname::Other(_) => git2::BranchType::Local, + Refname::Remote(_) => git2::BranchType::Remote, + }, + ); + match branch { + Ok(branch) => Ok(Some(branch)), + Err(err) if err.code() == git2::ErrorCode::NotFound => Ok(None), + Err(err) => Err(err.into()), + } +} + mod amend; mod apply_virtual_branch; mod create_virtual_branch_from_branch; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/move_commit_to_vbranch.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/move_commit_to_vbranch.rs index 058c03b19c8..13c444b76bf 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/move_commit_to_vbranch.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/move_commit_to_vbranch.rs @@ -1,11 +1,10 @@ use std::str::FromStr; use bstr::ByteSlice; -use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use gitbutler_stack::StackId; -use super::Test; +use super::{Test, stack_details}; #[test] fn no_diffs() { diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/oplog.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/oplog.rs index 4dafe4b4952..c18f6cd5760 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/oplog.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/oplog.rs @@ -1,7 +1,7 @@ use std::{io::Write, path::Path}; +use bstr::ByteSlice as _; use but_core::{GitConfigSettings, RepositoryExt as _}; -use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use gitbutler_oplog::OplogExt; use gitbutler_oplog::entry::{OperationKind, SnapshotDetails}; @@ -117,6 +117,17 @@ fn has_signature(repo: &gix::Repository, id: gix::ObjectId) -> anyhow::Result anyhow::Result { + Ok(ctx + .repo + .get()? + .find_commit(id)? + .message()? + .summary() + .to_str_lossy() + .into_owned()) +} + #[test] fn basic_oplog() -> anyhow::Result<()> { let Test { repo, ctx, .. } = &mut Test::default(); @@ -342,15 +353,11 @@ fn restores_gitbutler_workspace() -> anyhow::Result<()> { let _commit1_id = super::create_commit(ctx, stack_entry.id, "commit one")?; // check the workspace commit - let commit1_id = { - let git2_repo = ctx.git2_repo.get()?; - let head = git2_repo.head().expect("never unborn"); - let commit = &head.peel_to_commit()?; - let commit1_id = commit.id(); - let message = commit.summary().unwrap(); - assert_eq!(message, GITBUTLER_WORKSPACE_COMMIT_TITLE); - commit1_id - }; + let commit1_id = ctx.repo.get()?.head_id()?.detach(); + assert_eq!( + commit_summary(ctx, commit1_id)?, + GITBUTLER_WORKSPACE_COMMIT_TITLE + ); // create second commit fs::write(repo.path().join("file.txt"), "changed content")?; @@ -358,11 +365,8 @@ fn restores_gitbutler_workspace() -> anyhow::Result<()> { // check the workspace commit changed { - let git2_repo = ctx.git2_repo.get()?; - let head = git2_repo.head().expect("never unborn"); - let commit = &head.peel_to_commit()?; - let commit2_id = commit.id(); - let message = commit.summary().unwrap(); + let commit2_id = ctx.repo.get()?.head_id()?.detach(); + let message = commit_summary(ctx, commit2_id)?; assert_eq!(message, GITBUTLER_WORKSPACE_COMMIT_TITLE); assert_ne!(commit1_id, commit2_id); } @@ -380,16 +384,11 @@ fn restores_gitbutler_workspace() -> anyhow::Result<()> { .expect("can restore the most recent snapshot, to undo commit 2, resetting to commit 1"); drop(guard); - { - let git2_repo = ctx.git2_repo.get()?; - let head = git2_repo.head().expect("never unborn"); - let current_commit = &head.peel_to_commit()?; - let id_of_restored_commit = current_commit.id(); - assert_eq!( - commit1_id, id_of_restored_commit, - "head now points to the first commit, it's not commit 2 anymore" - ); - } + assert_eq!( + commit1_id, + ctx.repo.get()?.head_id()?.detach(), + "head now points to the first commit, it's not commit 2 anymore" + ); let stacks = VirtualBranchesHandle::new(ctx.project_data_dir()).list_stacks_in_workspace()?; assert_eq!( diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/save_and_unapply_virtual_branch.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/save_and_unapply_virtual_branch.rs index 9b057ce7aee..2f9e4df99a9 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/save_and_unapply_virtual_branch.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/save_and_unapply_virtual_branch.rs @@ -1,6 +1,5 @@ use but_core::DiffSpec; use but_hunk_assignment::HunkAssignmentRequest; -use but_testsupport::legacy::stack_details; use super::*; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/set_base_branch.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/set_base_branch.rs index d9d330d8f35..0ba00b921c6 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/set_base_branch.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/set_base_branch.rs @@ -37,7 +37,6 @@ mod error { } mod go_back_to_workspace { - use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use pretty_assertions::assert_eq; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/unapply_without_saving_virtual_branch.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/unapply_without_saving_virtual_branch.rs index c6a7a3f39e9..5d682aad950 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/unapply_without_saving_virtual_branch.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/unapply_without_saving_virtual_branch.rs @@ -1,5 +1,3 @@ -use but_testsupport::legacy::stack_details; - use super::*; #[test] diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/undo_commit.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/undo_commit.rs index e4cba3e8264..89612fb4375 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/undo_commit.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/undo_commit.rs @@ -1,4 +1,3 @@ -use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use super::*; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/update_commit_message.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/update_commit_message.rs index e51a1d63199..fae6210bc3f 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/update_commit_message.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/update_commit_message.rs @@ -1,4 +1,3 @@ -use but_testsupport::legacy::stack_details; use but_workspace::ui::PushStatus; use gitbutler_branch::BranchCreateRequest; use gitbutler_commit::commit_ext::CommitExt; diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/workspace_migration.rs b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/workspace_migration.rs index 82208c494e2..b8da599014b 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/workspace_migration.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/workspace_migration.rs @@ -10,11 +10,12 @@ use gitbutler_operating_modes::{ fn works_on_integration_branch() -> anyhow::Result<()> { let (ctx, _temp_dir) = crate::driverless::writable_context("for-workspace-migration.sh", "workspace-migration")?; + let repo = ctx.repo.get()?; // Check that we are on the old `gitbutler/integration` branch. assert_eq!( - ctx.git2_repo.get()?.head()?.name(), - Some(INTEGRATION_BRANCH_REF) + repo.head_name()?.map(|name| name.to_string()), + Some(INTEGRATION_BRANCH_REF.to_owned()) ); // Should not throw verification error until migration is complete. @@ -25,8 +26,8 @@ fn works_on_integration_branch() -> anyhow::Result<()> { // Updating workspace commit should put us on the workspace branch. update_workspace_commit(&ctx, false)?; assert_eq!( - ctx.git2_repo.get()?.head()?.name(), - Some(WORKSPACE_BRANCH_REF) + repo.head_name()?.map(|name| name.to_string()), + Some(WORKSPACE_BRANCH_REF.to_owned()) ); Ok(()) } diff --git a/crates/gitbutler-branch-actions/tests/fixtures/scenario/repo-with-origin.sh b/crates/gitbutler-branch-actions/tests/fixtures/scenario/repo-with-origin.sh new file mode 100755 index 00000000000..192e93518d5 --- /dev/null +++ b/crates/gitbutler-branch-actions/tests/fixtures/scenario/repo-with-origin.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -eu -o pipefail + +git init -b master local +(cd local + git config user.name gitbutler-test + git config user.email gitbutler-test@example.com + git commit --allow-empty -m "Initial commit" +) + +git init --bare remote +(cd local + git remote add origin ../remote + git push origin master +) diff --git a/crates/gitbutler-branch/Cargo.toml b/crates/gitbutler-branch/Cargo.toml index f815f123644..9f4d6affb7a 100644 --- a/crates/gitbutler-branch/Cargo.toml +++ b/crates/gitbutler-branch/Cargo.toml @@ -5,7 +5,6 @@ edition.workspace = true authors.workspace = true publish = false rust-version.workspace = true -autotests = false [features] export-schema = ["dep:schemars", "dep:but-schemars"] @@ -25,9 +24,5 @@ lazy_static = "1.4.0" schemars = { workspace = true, optional = true } but-schemars = { workspace = true, optional = true } -[[test]] -name = "branch" -path = "tests/mod.rs" - [lints] workspace = true diff --git a/crates/gitbutler-branch/tests/mod.rs b/crates/gitbutler-branch/tests/mod.rs deleted file mode 100644 index 8b137891791..00000000000 --- a/crates/gitbutler-branch/tests/mod.rs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/crates/gitbutler-cherry-pick/Cargo.toml b/crates/gitbutler-cherry-pick/Cargo.toml index 0b286967c62..2d4e0ddfb31 100644 --- a/crates/gitbutler-cherry-pick/Cargo.toml +++ b/crates/gitbutler-cherry-pick/Cargo.toml @@ -10,14 +10,10 @@ rust-version.workspace = true doctest = false [dependencies] -but-oxidize.workspace = true - gitbutler-commit.workspace = true -git2.workspace = true gix.workspace = true anyhow.workspace = true -tracing.workspace = true [lints] workspace = true diff --git a/crates/gitbutler-cherry-pick/src/lib.rs b/crates/gitbutler-cherry-pick/src/lib.rs index 5766a6e2f0b..c7042533c42 100644 --- a/crates/gitbutler-cherry-pick/src/lib.rs +++ b/crates/gitbutler-cherry-pick/src/lib.rs @@ -1,12 +1,8 @@ use std::ops::Deref; use anyhow::{Context as _, Result}; -use but_oxidize::{ObjectIdExt as _, OidExt, RepoExt}; use gitbutler_commit::commit_ext::CommitExt; -mod repository_ext; -pub use repository_ext::RepositoryExtLite; - #[derive(Default)] pub enum ConflictedTreeKey { /// The commit we're rebasing onto "head" @@ -36,19 +32,6 @@ impl Deref for ConflictedTreeKey { } } -pub trait RepositoryExt { - /// Find the real tree of a commit, which is the tree of the commit if it's not in a conflicted state - /// or the tree according to `side` if it is conflicted. - /// - /// Unless you want to find a particular side, you likely want to pass Default::default() - /// as the [`side`](ConflictedTreeKey) which will give the automatically resolved resolution - fn find_real_tree( - &self, - commit: &git2::Commit, - side: ConflictedTreeKey, - ) -> Result>; -} - pub trait GixRepositoryExt { /// Find the real tree of a commit, which is the tree of the commit if it's not in a conflicted state /// or the tree according to `side` if it is conflicted. @@ -62,19 +45,6 @@ pub trait GixRepositoryExt { ) -> Result>; } -impl RepositoryExt for git2::Repository { - fn find_real_tree( - &self, - commit: &git2::Commit, - side: ConflictedTreeKey, - ) -> Result> { - let gix_repo = self.to_isolated_gix_repo()?; - let gix_commit = gix_repo.find_commit(commit.id().to_gix())?; - let tree_id = gix_repo.find_real_tree(&gix_commit, side)?.to_git2(); - Ok(self.find_tree(tree_id)?) - } -} - impl GixRepositoryExt for gix::Repository { fn find_real_tree<'repo>( &'repo self, diff --git a/crates/gitbutler-cherry-pick/src/repository_ext.rs b/crates/gitbutler-cherry-pick/src/repository_ext.rs deleted file mode 100644 index 1a70582bd0f..00000000000 --- a/crates/gitbutler-cherry-pick/src/repository_ext.rs +++ /dev/null @@ -1,55 +0,0 @@ -//! NOTE: This module is here because it's convenient, dependency-wise, not because it's the best place necessarily. -use anyhow::{Context as _, 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. - /// If `0` this method will have no effect. - fn ignore_large_files_in_diffs(&self, limit_in_bytes: u64) -> Result<()>; -} - -impl RepositoryExtLite for git2::Repository { - #[instrument(level = "debug", skip(self), err(Debug))] - fn ignore_large_files_in_diffs(&self, limit_in_bytes: u64) -> Result<()> { - if limit_in_bytes == 0 { - return Ok(()); - } - use gix::bstr::ByteSlice; - let repo = gix::open(self.path())?; - let worktree_dir = repo - .workdir() - .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() - .is_ok_and(|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-commit/Cargo.toml b/crates/gitbutler-commit/Cargo.toml index a7ef3aeb227..57fdd9b1e96 100644 --- a/crates/gitbutler-commit/Cargo.toml +++ b/crates/gitbutler-commit/Cargo.toml @@ -14,7 +14,6 @@ doctest = false testing = [] [dependencies] -git2.workspace = true gix.workspace = true bstr.workspace = true but-core.workspace = true diff --git a/crates/gitbutler-commit/src/commit_ext.rs b/crates/gitbutler-commit/src/commit_ext.rs index 8e18a2b1147..ac5f4a9dd99 100644 --- a/crates/gitbutler-commit/src/commit_ext.rs +++ b/crates/gitbutler-commit/src/commit_ext.rs @@ -1,4 +1,4 @@ -use bstr::{BStr, ByteSlice}; +use bstr::BStr; use but_core::{ChangeId, commit::Headers}; /// Extension trait for `gix::Commit`. @@ -43,9 +43,3 @@ impl CommitMessageBstr for gix::Commit<'_> { .expect("valid commit that can be parsed: TODO - allow it to return errors?") } } - -impl CommitMessageBstr for git2::Commit<'_> { - fn message_bstr(&self) -> &BStr { - self.message_bytes().as_bstr() - } -} diff --git a/crates/gitbutler-edit-mode/src/lib.rs b/crates/gitbutler-edit-mode/src/lib.rs index 2a7b64fee62..42a1edb9801 100644 --- a/crates/gitbutler-edit-mode/src/lib.rs +++ b/crates/gitbutler-edit-mode/src/lib.rs @@ -10,10 +10,10 @@ use but_ctx::{ Context, access::{RepoExclusive, RepoShared}, }; -use but_oxidize::{ObjectIdExt as _, OidExt, gix_to_git2_index}; +use but_oxidize::{ObjectIdExt as _, gix_to_git2_index}; use but_rebase::graph_rebase::{Editor, Pick, Step}; use git2::build::CheckoutBuilder; -use gitbutler_cherry_pick::{ConflictedTreeKey, GixRepositoryExt as _, RepositoryExt as _}; +use gitbutler_cherry_pick::{ConflictedTreeKey, GixRepositoryExt as _}; use gitbutler_commit::commit_ext::CommitExt; use gitbutler_operating_modes::{ EDIT_BRANCH_REF, EditModeMetadata, INTEGRATION_BRANCH_REF, OperatingMode, WORKSPACE_BRANCH_REF, @@ -58,11 +58,9 @@ fn get_commit_index(ctx: &Context, commit_id: gix::ObjectId) -> Result Result { fn checkout_edit_branch(ctx: &Context, commit_id: gix::ObjectId) -> Result<()> { let repo = &*ctx.repo.get()?; + #[expect(deprecated, reason = "checkout/index materialization boundary")] let git2_repo = &*ctx.git2_repo.get()?; let commit = git2_repo.find_commit(commit_id.to_git2())?; @@ -234,6 +233,7 @@ pub(crate) fn abort_and_return_to_workspace( ); } + #[expect(deprecated, reason = "checkout/materialization boundary")] let repo = &*ctx.git2_repo.get()?; // Checkout gitbutler workspace branch @@ -253,6 +253,7 @@ pub(crate) fn abort_and_return_to_workspace( pub(crate) fn save_and_return_to_workspace(ctx: &Context, perm: &mut RepoExclusive) -> Result<()> { let edit_mode_metadata = read_edit_mode_metadata(ctx).context("Failed to read metadata")?; + #[expect(deprecated, reason = "checkout/index materialization boundary")] let git2_repo = &*ctx.git2_repo.get()?; let repo = &*ctx.repo.get()?; @@ -330,7 +331,7 @@ pub(crate) fn save_and_return_to_workspace(ctx: &Context, perm: &mut RepoExclusi ctx, old_workspace, new_workspace, - Some(uncommtied_changes.to_git2()), + Some(uncommtied_changes), Some(true), perm, )?; @@ -360,15 +361,20 @@ pub(crate) fn starting_index_state( bail!("Starting index state can only be fetched while in edit mode") }; - let git2_repo = &*ctx.git2_repo.get()?; let repo = &*ctx.repo.get()?; - - let commit = git2_repo.find_commit(metadata.commit_oid.to_git2())?; - let gix_commit = repo.find_commit(commit.id().to_gix())?; - let commit_parent_tree = if gix_commit.is_conflicted() { - git2_repo.find_real_tree(&commit, ConflictedTreeKey::Base)? + let commit = repo.find_commit(metadata.commit_oid)?; + let commit_parent_tree = if commit.is_conflicted() { + repo.find_real_tree(&commit, ConflictedTreeKey::Base)? + .detach() } else { - commit.parent(0)?.tree()? + commit + .parent_ids() + .next() + .context("edited commit had no parent")? + .object()? + .try_into_commit()? + .tree_id()? + .detach() }; let index = get_commit_index(ctx, metadata.commit_oid)?; @@ -402,11 +408,9 @@ pub(crate) fn starting_index_state( let tree_changes = but_core::diff::tree_changes( &repo, - Some(commit_parent_tree.id().to_gix()), - git2_repo - .find_real_tree(&commit, ConflictedTreeKey::Theirs)? - .id() - .to_gix(), + Some(commit_parent_tree), + repo.find_real_tree(&commit, ConflictedTreeKey::Theirs)? + .detach(), )?; let outcome = tree_changes diff --git a/crates/gitbutler-edit-mode/tests/edit_mode.rs b/crates/gitbutler-edit-mode/tests/edit_mode.rs index 21f2df9e6e6..29978feb779 100644 --- a/crates/gitbutler-edit-mode/tests/edit_mode.rs +++ b/crates/gitbutler-edit-mode/tests/edit_mode.rs @@ -227,6 +227,7 @@ fn conficted_entries_get_written_when_leaving_edit_mode() -> Result<()> { let stack_id = stack_id(&ctx)?; enter_edit_mode(&mut ctx, foobar, stack_id)?; + #[expect(deprecated, reason = "checkout/index boundary coverage")] let repo = ctx.git2_repo.get()?; let init = repo.find_reference("refs/heads/main")?.peel_to_commit()?; let left = repo.find_reference("refs/heads/left")?.peel_to_commit()?; diff --git a/crates/gitbutler-operating-modes/Cargo.toml b/crates/gitbutler-operating-modes/Cargo.toml index 3f877489f20..161070d8407 100644 --- a/crates/gitbutler-operating-modes/Cargo.toml +++ b/crates/gitbutler-operating-modes/Cargo.toml @@ -30,7 +30,7 @@ uuid.workspace = true schemars.workspace = true [dev-dependencies] -but-testsupport = { workspace = true, features = ["legacy"] } +but-testsupport.workspace = true [lints] workspace = true diff --git a/crates/gitbutler-operating-modes/tests/fixtures/scenario/repo-with-main.sh b/crates/gitbutler-operating-modes/tests/fixtures/scenario/repo-with-main.sh new file mode 100755 index 00000000000..1f967ec35f9 --- /dev/null +++ b/crates/gitbutler-operating-modes/tests/fixtures/scenario/repo-with-main.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash + +set -eu -o pipefail + +git init -b main +git config user.name test +git config user.email test@example.com + +touch tracked-file +git add tracked-file +git commit -m "initial commit" diff --git a/crates/gitbutler-operating-modes/tests/operating_modes.rs b/crates/gitbutler-operating-modes/tests/operating_modes.rs index fbd8e67de7d..e1413cda8d0 100644 --- a/crates/gitbutler-operating-modes/tests/operating_modes.rs +++ b/crates/gitbutler-operating-modes/tests/operating_modes.rs @@ -1,10 +1,33 @@ use but_ctx::Context; +use but_testsupport::{ + gix_testtools::{Creation, scripted_fixture_writable_with_args, tempfile::TempDir}, + open_repo, +}; use gitbutler_operating_modes::{EditModeMetadata, write_edit_mode_metadata}; use gix::refs::{ Target, transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog}, }; +struct Case { + ctx: Context, + _tmp: TempDir, +} + +fn new_case() -> Case { + let tmp = scripted_fixture_writable_with_args( + "scenario/repo-with-main.sh", + None::, + Creation::CopyFromReadOnly, + ) + .unwrap(); + let repo = open_repo(tmp.path()).unwrap(); + let ctx = Context::from_repo(repo).unwrap(); + std::fs::create_dir_all(ctx.project_data_dir()).unwrap(); + + Case { ctx, _tmp: tmp } +} + /// Creates a branch from the head commit fn create_and_checkout_branch(ctx: &Context, branch_name: &str) { let repo = &*ctx.repo.get().unwrap(); @@ -64,15 +87,14 @@ mod operating_modes { } mod open_workspace_mode { - use but_testsupport::legacy::{Case, Suite}; use gitbutler_operating_modes::{ensure_open_workspace_mode, in_open_workspace_mode}; - use crate::create_and_checkout_branch; + use crate::{create_and_checkout_branch, new_case}; #[test] fn in_open_workspace_mode_true_when_in_gitbutler_workspace() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/workspace"); @@ -83,8 +105,8 @@ mod operating_modes { #[test] fn in_open_workspace_mode_false_when_in_gitbutler_edit() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); @@ -95,8 +117,8 @@ mod operating_modes { #[test] fn in_open_workspace_mode_false_when_on_other_branches() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "testeroni"); @@ -107,8 +129,8 @@ mod operating_modes { #[test] fn assure_open_workspace_mode_ok_when_on_gitbutler_workspace() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/workspace"); @@ -118,8 +140,8 @@ mod operating_modes { #[test] fn assure_open_workspace_mode_err_when_on_gitbutler_edit() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); @@ -129,8 +151,8 @@ mod operating_modes { #[test] fn assure_open_workspace_mode_err_when_on_other_branch() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "testeroni"); @@ -140,15 +162,14 @@ mod operating_modes { } mod outside_workspace_mode { - use but_testsupport::legacy::{Case, Suite}; use gitbutler_operating_modes::{ensure_outside_workspace_mode, in_outside_workspace_mode}; - use crate::{create_and_checkout_branch, create_edit_mode_metadata}; + use crate::{create_and_checkout_branch, create_edit_mode_metadata, new_case}; #[test] fn in_outside_workspace_mode_true_when_in_other_branches() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "testeroni"); @@ -160,8 +181,8 @@ mod operating_modes { #[test] fn in_outside_workspace_mode_false_when_on_gitbutler_edit() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); create_edit_mode_metadata(ctx); @@ -174,8 +195,8 @@ mod operating_modes { #[test] fn in_outside_workspace_mode_false_when_on_gitbutler_workspace() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/workspace"); @@ -187,8 +208,8 @@ mod operating_modes { #[test] fn assure_outside_workspace_mode_ok_when_on_other_branches() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "testeroni"); @@ -198,8 +219,8 @@ mod operating_modes { #[test] fn assure_outside_workspace_mode_err_when_on_gitbutler_edit() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); create_edit_mode_metadata(ctx); @@ -210,8 +231,8 @@ mod operating_modes { #[test] fn assure_outside_workspace_mode_err_when_on_gitbutler_workspace() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/workspace"); @@ -221,15 +242,14 @@ mod operating_modes { } mod edit_mode { - use but_testsupport::legacy::{Case, Suite}; use gitbutler_operating_modes::{ensure_edit_mode, in_edit_mode}; - use crate::{create_and_checkout_branch, create_edit_mode_metadata}; + use crate::{create_and_checkout_branch, create_edit_mode_metadata, new_case}; #[test] fn in_edit_mode_true_when_in_edit_mode() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); create_edit_mode_metadata(ctx); @@ -241,8 +261,8 @@ mod operating_modes { #[test] fn in_edit_mode_false_when_in_edit_mode_with_no_metadata() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); @@ -253,8 +273,8 @@ mod operating_modes { #[test] fn in_edit_mode_false_when_on_other_branches() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "testeroni"); create_edit_mode_metadata(ctx); @@ -266,8 +286,8 @@ mod operating_modes { #[test] fn assert_edit_mode_ok_when_in_edit_mode() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); create_edit_mode_metadata(ctx); @@ -278,8 +298,8 @@ mod operating_modes { #[test] fn assert_edit_mode_err_when_in_edit_mode_with_no_metadata() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "gitbutler/edit"); @@ -289,8 +309,8 @@ mod operating_modes { #[test] fn assert_edit_mode_err_when_on_other_branches() { - let suite = Suite::default(); - let Case { ctx, .. } = &suite.new_case(); + let case = new_case(); + let ctx = &case.ctx; create_and_checkout_branch(ctx, "testeroni"); create_edit_mode_metadata(ctx); diff --git a/crates/gitbutler-oplog/src/oplog.rs b/crates/gitbutler-oplog/src/oplog.rs index 52799dbe183..f80fd4819d1 100644 --- a/crates/gitbutler-oplog/src/oplog.rs +++ b/crates/gitbutler-oplog/src/oplog.rs @@ -13,11 +13,16 @@ use but_ctx::{ }; use but_meta::virtual_branches_legacy_types; use but_oxidize::{ObjectIdExt as _, OidExt}; -use gitbutler_cherry_pick::{GixRepositoryExt as _, RepositoryExtLite}; +use gitbutler_cherry_pick::GixRepositoryExt as _; use gitbutler_repo::{SignaturePurpose, commit_without_signature_gix, signature_gix}; use gitbutler_stack::{VirtualBranchesHandle, VirtualBranchesState}; use gix::objs::Write as _; -use gix::{ObjectId, bstr::ByteSlice, object::tree::EntryKind, prelude::ObjectIdExt}; +use gix::{ + ObjectId, + bstr::{BString, ByteSlice, ByteVec}, + object::tree::EntryKind, + prelude::ObjectIdExt, +}; use tracing::instrument; use super::{ @@ -352,8 +357,73 @@ fn get_workdir_tree( } } -pub fn prepare_snapshot(ctx: &Context, _shared_access: &RepoShared) -> Result { +fn write_index_tree(ctx: &Context) -> Result { + #[expect(deprecated, reason = "index materialization boundary")] + let git2_repo = ctx.git2_repo.get()?; + let mut index = git2_repo.index()?; + Ok(index.write_tree()?.to_gix()) +} + +fn checkout_workdir_tree(ctx: &Context, workdir_tree_id: gix::ObjectId) -> Result<()> { + #[expect(deprecated, reason = "checkout/materialization boundary")] + let git2_repo = ctx.git2_repo.get()?; + ignore_large_files_in_diffs(&git2_repo, AUTO_TRACK_LIMIT_BYTES)?; + let workdir_tree = git2_repo.find_tree(workdir_tree_id.to_git2())?; + let mut checkout_builder = git2::build::CheckoutBuilder::new(); + checkout_builder.remove_untracked(true); + checkout_builder.force(); + git2_repo.checkout_tree(workdir_tree.as_object(), Some(&mut checkout_builder))?; + Ok(()) +} + +fn ignore_large_files_in_diffs(repo: &git2::Repository, limit_in_bytes: u64) -> Result<()> { + if limit_in_bytes == 0 { + return Ok(()); + } + let gix_repo = gix::open(repo.path())?; + let worktree_dir = gix_repo + .workdir() + .context("All repos are expected to have a worktree")?; + let files_to_exclude: Vec<_> = gix_repo + .dirwalk_iter( + gix_repo.index_or_empty()?, + None::, + Default::default(), + gix_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() + .is_ok_and(|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(); + let ignore_list = files_to_exclude.join("\n"); + repo.add_ignore_rule(&ignore_list)?; + Ok(()) +} + +fn reset_index_to_tree(ctx: &Context, tree_id: gix::ObjectId) -> Result<()> { + #[expect(deprecated, reason = "index materialization boundary")] let git2_repo = ctx.git2_repo.get()?; + let tree = git2_repo + .find_tree(tree_id.to_git2()) + .context("failed to convert index tree entry to tree")?; + let mut index = git2_repo.index()?; + index.read_tree(&tree)?; + index.write()?; + Ok(()) +} + +pub fn prepare_snapshot(ctx: &Context, _shared_access: &RepoShared) -> Result { let repo = ctx.repo.get()?; let empty_tree_id = repo.empty_tree().id; @@ -370,12 +440,11 @@ pub fn prepare_snapshot(ctx: &Context, _shared_access: &RepoShared) -> Result Result { - let git2_repo = ctx.git2_repo.get()?; // Use a separate repo without caching so we are sure the 'has commit' checks pick up all changes. let repo = ctx.repo.get()?; @@ -598,8 +666,6 @@ fn restore_snapshot( let gix_repo = ctx.clone_repo_for_merging()?; let workdir_tree_id = get_workdir_tree(None, snapshot_commit_id, &gix_repo, ctx)?; - git2_repo.ignore_large_files_in_diffs(AUTO_TRACK_LIMIT_BYTES)?; - // Define the checkout builder if ctx.settings.feature_flags.cv3 { but_core::worktree::safe_checkout_from_head( @@ -608,12 +674,7 @@ fn restore_snapshot( but_core::worktree::checkout::Options::default(), )?; } else { - let workdir_tree = git2_repo.find_tree(workdir_tree_id.to_git2())?; - let mut checkout_builder = git2::build::CheckoutBuilder::new(); - checkout_builder.remove_untracked(true); - checkout_builder.force(); - // Checkout the tree - git2_repo.checkout_tree(workdir_tree.as_object(), Some(&mut checkout_builder))?; + checkout_workdir_tree(ctx, workdir_tree_id)?; } // Update virtual_branches.toml with the state from the snapshot @@ -636,11 +697,7 @@ fn restore_snapshot( let index_tree_entry = snapshot_tree .lookup_entry_by_path("index")? .context("failed to get virtual_branches.toml blob")?; - let index_tree = git2_repo - .find_tree(index_tree_entry.id().to_git2()) - .context("failed to convert index tree entry to tree")?; - let mut index = git2_repo.index()?; - index.read_tree(&index_tree)?; + reset_index_to_tree(ctx, index_tree_entry.id().detach())?; let restored_operation = snapshot_commit .message_raw()? diff --git a/crates/gitbutler-project/Cargo.toml b/crates/gitbutler-project/Cargo.toml index 022c8c5df15..650ec5d006b 100644 --- a/crates/gitbutler-project/Cargo.toml +++ b/crates/gitbutler-project/Cargo.toml @@ -38,7 +38,7 @@ resolve-path = "0.1.0" schemars = { workspace = true, optional = true } [dev-dependencies] -but-testsupport = { workspace = true, features = ["legacy"] } +but-testsupport.workspace = true tempfile.workspace = true insta.workspace = true diff --git a/crates/gitbutler-project/tests/add_cwd_as_project.rs b/crates/gitbutler-project/tests/add_cwd_as_project.rs index e9b4c3ec50c..c698b49dc37 100644 --- a/crates/gitbutler-project/tests/add_cwd_as_project.rs +++ b/crates/gitbutler-project/tests/add_cwd_as_project.rs @@ -1,9 +1,10 @@ -use but_testsupport::legacy::paths; +#[path = "project/support.rs"] +mod support; #[test] fn current_directory_dot() -> anyhow::Result<()> { - let tmp = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let tmp = support::data_dir(); + let repo = support::TestProject::default(); let repo_path = repo.path(); // Change to the repository directory and add "." as the path diff --git a/crates/gitbutler-project/tests/fixtures/repo-with-origin.sh b/crates/gitbutler-project/tests/fixtures/repo-with-origin.sh new file mode 100755 index 00000000000..192e93518d5 --- /dev/null +++ b/crates/gitbutler-project/tests/fixtures/repo-with-origin.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -eu -o pipefail + +git init -b master local +(cd local + git config user.name gitbutler-test + git config user.email gitbutler-test@example.com + git commit --allow-empty -m "Initial commit" +) + +git init --bare remote +(cd local + git remote add origin ../remote + git push origin master +) diff --git a/crates/gitbutler-project/tests/project/main.rs b/crates/gitbutler-project/tests/project/main.rs index bb0a7c272c6..cb4de8087a0 100644 --- a/crates/gitbutler-project/tests/project/main.rs +++ b/crates/gitbutler-project/tests/project/main.rs @@ -2,11 +2,13 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use but_core::RepositoryExt; -use but_testsupport::{CommandExt, git_at_dir, legacy::paths}; +use but_testsupport::{CommandExt, git_at_dir}; use tempfile::TempDir; +mod support; + pub fn new() -> TempDir { - paths::data_dir() + support::data_dir() } fn repo_path_at(name: &str) -> PathBuf { @@ -32,7 +34,8 @@ fn set_storage_path_config( let key = but_project_handle::storage_path_config_key(); repo.config_snapshot_mut() .set_raw_value(key, gix::path::os_str_into_bstr(value.as_ref())?)?; - repo.write_local_common_config(&repo.config_snapshot())?; + let (_config, lock) = repo.local_common_config_for_editing()?; + repo.write_locked_config(&repo.config_snapshot(), lock)?; Ok(repo) } @@ -41,8 +44,8 @@ mod add { #[test] fn success() -> anyhow::Result<()> { - let tmp = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let tmp = support::data_dir(); + let repo = support::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(tmp.path(), path) .unwrap() @@ -56,10 +59,13 @@ mod add { #[test] fn creates_configured_storage_dir() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let configured_repo = set_storage_path_config(repo.path(), "gitbutler-custom")?; - let expected_gb_dir = configured_repo.git_dir().join("gitbutler-custom"); + let expected_gb_dir = configured_repo + .git_dir() + .canonicalize()? + .join("gitbutler-custom"); assert!(!expected_gb_dir.exists()); let project = @@ -72,10 +78,13 @@ mod add { #[test] fn get_recreates_configured_storage_dir() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let configured_repo = set_storage_path_config(repo.path(), "gitbutler-custom")?; - let expected_gb_dir = configured_repo.git_dir().join("gitbutler-custom"); + let expected_gb_dir = configured_repo + .git_dir() + .canonicalize()? + .join("gitbutler-custom"); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), repo.path())?.unwrap_project(); @@ -91,7 +100,7 @@ mod add { #[test] fn submodule_is_added_as_project() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let fixture = writable_fixture(); let superproject = fixture.path().join("with-submodule").canonicalize()?; let submodule = superproject.join("submodule"); @@ -118,7 +127,7 @@ mod add { #[test] fn best_effort_adds_submodule_even_if_superproject_exists() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let fixture = writable_fixture(); let superproject = fixture.path().join("with-submodule").canonicalize()?; let submodule = superproject.join("submodule"); @@ -144,8 +153,8 @@ mod add { #[test] fn best_effort_adds_parent_repo_from_nested_directory() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let nested_dir = repo.path().join("nested/inside"); let expected_worktree_dir = repo.path().canonicalize()?; let expected_git_dir = repo_git_dir(repo.path())?; @@ -169,8 +178,8 @@ mod add { /// Used in deep-links for instance #[test] fn best_effort_finds_existing_project_from_file_path() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), repo.path())?.unwrap_project(); let file_path = repo.path().join("nested/inside/file.txt"); @@ -199,7 +208,7 @@ mod add { #[test] fn non_bare_without_worktree() { - let tmp = paths::data_dir(); + let tmp = support::data_dir(); let root = repo_path_at("non-bare-without-worktree"); let outcome = gitbutler_project::add_at_app_data_dir(tmp.path(), root.as_path()).unwrap(); @@ -208,7 +217,7 @@ mod add { #[test] fn missing() { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let tmp = tempfile::tempdir().unwrap(); let outcome = gitbutler_project::add_at_app_data_dir(data_dir.path(), tmp.path().join("missing")) @@ -218,7 +227,7 @@ mod add { #[test] fn directory_without_git() { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let tmp = tempfile::tempdir().unwrap(); let path = tmp.path(); std::fs::write(path.join("file.txt"), "hello world").unwrap(); @@ -228,7 +237,7 @@ mod add { #[test] fn empty() { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let tmp = tempfile::tempdir().unwrap(); let outcome = gitbutler_project::add_at_app_data_dir(data_dir.path(), tmp.path()).unwrap(); @@ -237,8 +246,8 @@ mod add { #[test] fn nested_directory_inside_repo_is_not_added_by_exact_path_but_is_by_best_effort() { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let nested_dir = repo.path().join("nested/inside"); std::fs::create_dir_all(&nested_dir).unwrap(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), repo.path()) @@ -266,8 +275,8 @@ mod add { #[test] fn twice() { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let path = repo.path(); gitbutler_project::add_at_app_data_dir(data_dir.path(), path).unwrap(); @@ -277,7 +286,7 @@ mod add { #[test] fn bare() { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let tmp = tempfile::tempdir().unwrap(); let repo_dir = tmp.path().join("bare"); @@ -294,7 +303,7 @@ mod add { #[test] fn worktree() { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let tmp = tempfile::tempdir().unwrap(); let main_worktree_dir = tmp.path().join("main"); let worktree_dir = tmp.path().join("worktree"); @@ -321,8 +330,8 @@ mod delete { use super::*; #[test] fn success() { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path) .unwrap() @@ -344,7 +353,7 @@ mod delete { #[test] fn submodule_success_without_accidentally_removing_submodule() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); + let data_dir = support::data_dir(); let fixture = writable_fixture(); let submodule = fixture .path() @@ -366,8 +375,8 @@ mod delete { #[test] fn deletes_gitbutler_references() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path)?.unwrap_project(); @@ -425,8 +434,8 @@ mod delete { #[test] fn deletes_project_without_gitbutler_references() -> anyhow::Result<()> { // This test ensures that deletion works even when there are no gitbutler references - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path)?.unwrap_project(); @@ -467,8 +476,8 @@ mod delete { #[test] fn removes_configured_storage_dir() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path)?.unwrap_project(); @@ -482,8 +491,8 @@ mod delete { #[test] fn refuses_to_delete_git_dir_when_storage_path_points_to_dot_git() -> anyhow::Result<()> { - let data_dir = paths::data_dir(); - let repo = but_testsupport::legacy::TestProject::default(); + let data_dir = support::data_dir(); + let repo = support::TestProject::default(); let git_dir = repo_git_dir(repo.path())?; let repo_after_config = set_storage_path_config(repo.path(), ".")?; assert!( diff --git a/crates/gitbutler-project/tests/project/support.rs b/crates/gitbutler-project/tests/project/support.rs new file mode 100644 index 00000000000..642bb39c83c --- /dev/null +++ b/crates/gitbutler-project/tests/project/support.rs @@ -0,0 +1,39 @@ +use std::path::{Path, PathBuf}; + +use but_testsupport::gix_testtools::{scripted_fixture_writable, tempfile::TempDir}; + +pub fn data_dir() -> TempDir { + tempfile::tempdir().expect("tempdir can be created") +} + +pub struct TestProject { + _tmp: TempDir, + local_path: PathBuf, +} + +impl Default for TestProject { + fn default() -> Self { + let tmp = scripted_fixture_writable("repo-with-origin.sh").expect("fixture is valid"); + let local_path = tmp.path().join("local"); + let repo = but_testsupport::open_repo(&local_path).expect("valid git repo in fixture"); + but_core::git_config::edit_repo_config(&repo, gix::config::Source::Local, |config| { + let key = but_project_handle::storage_path_config_key(); + config.set_raw_value(key, "gitbutler")?; + Ok(()) + }) + .expect("config can be persisted"); + let local_path = local_path + .canonicalize() + .expect("local fixture path is valid"); + Self { + _tmp: tmp, + local_path, + } + } +} + +impl TestProject { + pub fn path(&self) -> &Path { + &self.local_path + } +} diff --git a/crates/gitbutler-repo-actions/Cargo.toml b/crates/gitbutler-repo-actions/Cargo.toml index 9741e58a477..2113dc2cb87 100644 --- a/crates/gitbutler-repo-actions/Cargo.toml +++ b/crates/gitbutler-repo-actions/Cargo.toml @@ -12,7 +12,6 @@ doctest = false [dependencies] but-error.workspace = true but-core.workspace = true -but-oxidize.workspace = true but-ctx.workspace = true gitbutler-stack.workspace = true diff --git a/crates/gitbutler-repo-actions/src/repository.rs b/crates/gitbutler-repo-actions/src/repository.rs index 5540d613db7..195adaed99b 100644 --- a/crates/gitbutler-repo-actions/src/repository.rs +++ b/crates/gitbutler-repo-actions/src/repository.rs @@ -1,16 +1,11 @@ use std::{str::FromStr, time::UNIX_EPOCH}; use anyhow::{Context as _, Result, anyhow, bail}; -use but_core::commit::Headers; use but_ctx::Context; use but_error::Code; -use but_oxidize::{ObjectIdExt, OidExt}; use gitbutler_project::AuthKey; use gitbutler_reference::{Refname, RemoteRefname}; -use gitbutler_repo::{ - SignaturePurpose, commit_with_signature_gix, credentials, first_parent_commit_ids_until, - signature_gix, -}; +use gitbutler_repo::{credentials, first_parent_commit_ids_until}; use gitbutler_stack::{Stack, StackId}; use crate::askpass; @@ -20,7 +15,7 @@ pub trait RepoActionsExt { /// Returns the stderr output of the git executable if used. fn push( &self, - head: git2::Oid, + head: gix::ObjectId, branch: &RemoteRefname, with_force: bool, force_push_protection: bool, @@ -28,14 +23,7 @@ pub trait RepoActionsExt { askpass_broker: Option>, push_opts: Vec, ) -> Result; - fn commit( - &self, - message: &str, - tree: &git2::Tree, - parents: &[&git2::Commit], - commit_headers: Option, - ) -> Result; - fn distance(&self, from: git2::Oid, to: git2::Oid) -> Result; + fn distance(&self, from: gix::ObjectId, to: gix::ObjectId) -> Result; fn delete_branch_reference(&self, stack: &Stack) -> Result<()>; fn add_branch_reference(&self, stack: &Stack) -> Result<()>; fn git_test_push( @@ -71,7 +59,7 @@ impl RepoActionsExt for Context { .try_find_reference(&target_branch_refname.to_string())? .ok_or(anyhow!("failed to find branch {target_branch_refname}"))?; - let commit_id = branch.peel_to_commit()?.id.to_git2(); + let commit_id = branch.peel_to_commit()?.id; let now = now_ms(); let branch_name = format!("test-push-{now}"); @@ -139,43 +127,15 @@ impl RepoActionsExt for Context { } // returns the number of commits between the first oid to the second oid - fn distance(&self, from: git2::Oid, to: git2::Oid) -> Result { + fn distance(&self, from: gix::ObjectId, to: gix::ObjectId) -> Result { let repo = self.repo.get()?; - let oids = first_parent_commit_ids_until(&repo, from.to_gix(), to.to_gix())?; + let oids = first_parent_commit_ids_until(&repo, from, to)?; Ok(oids.len().try_into()?) } - fn commit( - &self, - message: &str, - tree: &git2::Tree, - parents: &[&git2::Commit], - commit_headers: Option, - ) -> Result { - let repo = self.repo.get()?; - let author = signature_gix(SignaturePurpose::Author); - let committer = signature_gix(SignaturePurpose::Committer); - let parent_ids = parents - .iter() - .map(|parent| parent.id().to_gix()) - .collect::>(); - commit_with_signature_gix( - &repo, - None, - author, - committer, - message.into(), - tree.id().to_gix(), - &parent_ids, - commit_headers, - ) - .map(|id| id.to_git2()) - .context("failed to commit") - } - fn push( &self, - head: git2::Oid, + head: gix::ObjectId, branch: &RemoteRefname, with_force: bool, force_push_protection: bool, @@ -243,6 +203,7 @@ impl RepoActionsExt for Context { } } } else { + #[expect(deprecated, reason = "libgit2 transport/auth adapter")] let git2_repo = self.git2_repo.get()?; let auth_flows = credentials::help(&git2_repo, &self.legacy_project, branch.remote())?; for (mut remote, callbacks) in auth_flows { @@ -336,6 +297,7 @@ impl RepoActionsExt for Context { .map_err(Into::into); } + #[expect(deprecated, reason = "libgit2 transport/auth adapter")] let git2_repo = self.git2_repo.get()?; let auth_flows = credentials::help(&git2_repo, &self.legacy_project, remote_name)?; for (mut remote, callbacks) in auth_flows { diff --git a/crates/gitbutler-repo/Cargo.toml b/crates/gitbutler-repo/Cargo.toml index 268435ba1a9..651672a12f5 100644 --- a/crates/gitbutler-repo/Cargo.toml +++ b/crates/gitbutler-repo/Cargo.toml @@ -5,13 +5,11 @@ edition.workspace = true authors.workspace = true publish = false rust-version.workspace = true -autotests = false [lib] doctest = false [dependencies] -but-error.workspace = true but-core = { workspace = true, features = ["legacy"] } but-gerrit.workspace = true but-oxidize.workspace = true @@ -40,16 +38,13 @@ scopeguard = "1.2.0" fuzzy-matcher = "0.3.7" ignore = "0.4.25" -[[test]] -name = "repo" -path = "tests/mod.rs" - [dev-dependencies] -but-testsupport = { workspace = true, features = ["legacy"] } +but-testsupport.workspace = true gitbutler-user.workspace = true but-settings.workspace = true serde_json = { workspace = true, features = ["arbitrary_precision"] } insta.workspace = true +keyring.workspace = true [lints] workspace = true diff --git a/crates/gitbutler-repo/src/commands.rs b/crates/gitbutler-repo/src/commands.rs index 3ba8063e31b..4e53b04df12 100644 --- a/crates/gitbutler-repo/src/commands.rs +++ b/crates/gitbutler-repo/src/commands.rs @@ -3,17 +3,16 @@ use std::{path::Path, sync::Mutex}; use anyhow::{Result, bail}; use base64::engine::Engine as _; use but_core::commit::sign_buffer; +use but_core::git_config::{edit_repo_config, ensure_config_value}; use but_ctx::Context; -use but_oxidize::{ObjectIdExt as _, OidExt as _}; use fuzzy_matcher::{FuzzyMatcher, skim::SkimMatcherV2}; -use git2::Oid; use ignore::WalkBuilder; use infer::MatcherType; use itertools::Itertools; use serde::Serialize; use tracing::warn; -use crate::{RepositoryExt, remote::GitRemote}; +use crate::remote::GitRemote; #[derive(Default, Debug, Serialize)] #[serde(rename_all = "camelCase")] @@ -158,7 +157,7 @@ pub trait RepoCommands { /// /// Bails when given an absolute path since that would suggest we are looking for a file in /// the workspace. Returns `FileInfo::default()` if file could not be found. - fn read_file_from_commit(&self, commit_id: Oid, path: &Path) -> Result; + fn read_file_from_commit(&self, commit_id: gix::ObjectId, path: &Path) -> Result; /// Read `path` in the following order: /// @@ -204,7 +203,7 @@ impl RepoCommands for Context { } fn add_remote(&self, name: &str, url: &str) -> Result<()> { - let repo = self.git2_repo.get()?; + let repo = self.open_isolated_repo()?; // Bail if remote with given name already exists. if repo.find_remote(name).is_ok() { @@ -212,20 +211,45 @@ impl RepoCommands for Context { } // Bail if remote with given url already exists. - if repo - .remotes_as_string()? + if let Some(remote_name) = repo + .remote_names() .iter() - .map(|name| repo.find_remote(name)) - .any(|result| result.is_ok_and(|remote| remote.url() == Some(url))) + .filter_map(|name| repo.find_remote(name.as_ref()).ok()) + .find_map(|remote| { + remote + .url(gix::remote::Direction::Fetch) + .and_then(|remote_url| { + if remote_url.to_bstring() == url { + remote + .name() + .and_then(|n| n.as_symbol().map(ToOwned::to_owned)) + } else { + None + } + }) + }) { - bail!("Remote with url '{url}' already exists"); + bail!("Remote with url '{url}' already exists at '{remote_name}'"); } - repo.remote(name, url)?; + edit_repo_config(&repo, gix::config::Source::Local, |config| { + let mut section = config.section_mut_or_create_new("remote", Some(name.into()))?; + section.push("url".try_into()?, Some(url.into())); + ensure_config_value( + config, + &format!("remote.{name}.fetch"), + &format!("+refs/heads/*:refs/remotes/{name}/*"), + )?; + Ok(()) + })?; Ok(()) } - fn read_file_from_commit(&self, commit_id: Oid, relative_path: &Path) -> Result { + fn read_file_from_commit( + &self, + commit_id: gix::ObjectId, + relative_path: &Path, + ) -> Result { if !relative_path.is_relative() { bail!( "Refusing to read '{relative_path:?}' from commit {commit_id:?} as it's not relative to the worktree" @@ -233,7 +257,7 @@ impl RepoCommands for Context { } let repo = self.repo.get()?; - let tree = repo.find_commit(commit_id.to_gix())?.tree()?; + let tree = repo.find_commit(commit_id)?.tree()?; Ok(match tree.lookup_entry_by_path(relative_path)? { Some(entry) => { @@ -297,10 +321,7 @@ impl RepoCommands for Context { } // Read file that has been deleted and staged for commit. Note that file not // found returns FileInfo::default() rather than an error. - None => self.read_file_from_commit( - repo.head_id()?.detach().to_git2(), - &relative_path, - )?, + None => self.read_file_from_commit(repo.head_id()?.detach(), &relative_path)?, } } Err(err) => return Err(err.into()), diff --git a/crates/gitbutler-repo/src/config.rs b/crates/gitbutler-repo/src/config.rs deleted file mode 100644 index 438db9a1b50..00000000000 --- a/crates/gitbutler-repo/src/config.rs +++ /dev/null @@ -1,23 +0,0 @@ -use anyhow::Result; - -pub struct Config<'a> { - repo: &'a gix::Repository, -} - -impl<'a> From<&'a gix::Repository> for Config<'a> { - fn from(value: &'a gix::Repository) -> Self { - Self { repo: value } - } -} - -// TODO: Remove this in favor of gitbutler-core::config::git::GitConfig -impl Config<'_> { - pub fn user_real_comitter(&self) -> Result { - let commit_as_gitbutler = self - .repo - .config_snapshot() - .boolean("gitbutler.gitbutlerCommitter") - .unwrap_or(false); - Ok(!commit_as_gitbutler) - } -} diff --git a/crates/gitbutler-repo/src/hooks.rs b/crates/gitbutler-repo/src/hooks.rs index 9860aeb646d..bd91790fca0 100644 --- a/crates/gitbutler-repo/src/hooks.rs +++ b/crates/gitbutler-repo/src/hooks.rs @@ -55,6 +55,7 @@ fn husky_search_paths(ctx: &Context) -> Option<&'static [&'static str]> { pub fn commit_msg(ctx: &Context, mut message: String) -> Result { let original_message = message.clone(); + #[expect(deprecated, reason = "libgit2 hook adapter boundary")] match git2_hooks::hooks_commit_msg( &*ctx.git2_repo.get()?, husky_search_paths(ctx), @@ -78,6 +79,7 @@ pub fn commit_msg(ctx: &Context, mut message: String) -> Result Result { + #[expect(deprecated, reason = "libgit2 hook/index adapter boundary")] let repo = &*ctx.git2_repo.get()?; let original_tree = repo.index()?.write_tree()?; @@ -94,7 +96,7 @@ pub fn pre_commit_with_tree(ctx: &Context, tree_id: gix::ObjectId) -> Result HookResult::Success, H::NoHookFound => HookResult::NotConfigured, H::RunNotSuccessful { @@ -117,6 +119,7 @@ pub fn pre_commit_with_tree(ctx: &Context, tree_id: gix::ObjectId) -> Result Result { + #[expect(deprecated, reason = "libgit2 hook adapter boundary")] match git2_hooks::hooks_post_commit(&*ctx.git2_repo.get()?, husky_search_paths(ctx))? { H::Ok { hook: _ } => Ok(HookResult::Success), H::NoHookFound => Ok(HookResult::NotConfigured), diff --git a/crates/gitbutler-repo/src/lib.rs b/crates/gitbutler-repo/src/lib.rs index f62fc8f4a04..395dda60073 100644 --- a/crates/gitbutler-repo/src/lib.rs +++ b/crates/gitbutler-repo/src/lib.rs @@ -1,5 +1,3 @@ -use gix::date::parse::TimeBuf; - pub mod rebase; mod commands; @@ -29,12 +27,9 @@ pub use commands::{FileInfo, RepoCommands}; pub use remote::GitRemote; mod repository_ext; -pub use repository_ext::{RepositoryExt, commit_with_signature_gix, commit_without_signature_gix}; +pub use repository_ext::{commit_with_signature_gix, commit_without_signature_gix}; pub mod credentials; - -mod config; -use config::Config; pub mod hooks; pub mod managed_hooks; mod remote; @@ -42,7 +37,6 @@ pub mod staging; pub mod commit_message; -use but_oxidize::gix_to_git2_signature; pub const GITBUTLER_COMMIT_AUTHOR_NAME: &str = "GitButler"; pub const GITBUTLER_COMMIT_AUTHOR_EMAIL: &str = "gitbutler@gitbutler.com"; @@ -51,13 +45,6 @@ pub enum SignaturePurpose { Committer, } -/// Provide a signature with the GitButler author, and the current time or the time overridden -/// depending on the value for `purpose`. -pub fn signature(purpose: SignaturePurpose) -> anyhow::Result> { - let signature = signature_gix(purpose); - gix_to_git2_signature(signature.to_ref(&mut TimeBuf::default())) -} - /// Provide a `gix` signature with the GitButler author and the current or overridden time. pub fn signature_gix(purpose: SignaturePurpose) -> gix::actor::Signature { gix::actor::Signature { diff --git a/crates/gitbutler-repo/src/rebase.rs b/crates/gitbutler-repo/src/rebase.rs index 08fe9a94d03..b0c78151146 100644 --- a/crates/gitbutler-repo/src/rebase.rs +++ b/crates/gitbutler-repo/src/rebase.rs @@ -6,9 +6,7 @@ use but_core::{ commit::{ConflictEntries, Headers}, }; use but_oxidize::{ObjectIdExt as _, OidExt as _}; -use gitbutler_cherry_pick::{ConflictedTreeKey, RepositoryExt as _}; - -use crate::RepositoryExt as _; +use gitbutler_cherry_pick::{ConflictedTreeKey, GixRepositoryExt as _}; fn extract_conflicted_files( merged_tree_id: gix::Id<'_>, @@ -87,25 +85,26 @@ pub fn merge_commits( incoming_commit: gix::ObjectId, resulting_name: &str, ) -> Result { - let repo = git2::Repository::open(gix_repository.path())?; - let target_commit = repo.find_commit(target_commit.to_git2())?; - let incoming_commit = repo.find_commit(incoming_commit.to_git2())?; - let merge_base = repo.merge_base(target_commit.id(), incoming_commit.id())?; - let merge_base = repo.find_commit(merge_base)?; + let target_commit = gix_repository.find_commit(target_commit)?; + let incoming_commit = gix_repository.find_commit(incoming_commit)?; + let merge_base = gix_repository.merge_base(target_commit.id, incoming_commit.id)?; + let merge_base = gix_repository.find_commit(merge_base.detach())?; - let base_tree = repo.find_real_tree(&merge_base, Default::default())?; + let base_tree = gix_repository.find_real_tree(&merge_base, Default::default())?; // We want to use the auto-resolution when computing the merge, but for // reconstructing it later, we want the "theirsiest" and "oursiest" trees - let target_tree = repo.find_real_tree(&target_commit, ConflictedTreeKey::Theirs)?; - let incoming_tree = repo.find_real_tree(&incoming_commit, ConflictedTreeKey::Ours)?; + let target_tree = gix_repository.find_real_tree(&target_commit, ConflictedTreeKey::Theirs)?; + let incoming_tree = gix_repository.find_real_tree(&incoming_commit, ConflictedTreeKey::Ours)?; - let target_merge_tree = repo.find_real_tree(&target_commit, Default::default())?; - let incoming_merge_tree = repo.find_real_tree(&incoming_commit, Default::default())?; - let gix_repo = but_core::open_repo_for_merging(repo.path())?; + let target_merge_tree = gix_repository.find_real_tree(&target_commit, Default::default())?; + let incoming_merge_tree = + gix_repository.find_real_tree(&incoming_commit, Default::default())?; + let repo = git2::Repository::open(gix_repository.path())?; + let gix_repo = gix_repository.clone().for_tree_diffing()?; let mut merge_result = gix_repo.merge_trees( - base_tree.id().to_gix(), - incoming_merge_tree.id().to_gix(), - target_merge_tree.id().to_gix(), + base_tree.detach(), + incoming_merge_tree.detach(), + target_merge_tree.detach(), gix_repo.default_merge_labels(), gix_repo.merge_options_force_ours()?, )?; @@ -126,9 +125,21 @@ pub fn merge_commits( let mut tree_writer = repo.treebuilder(Some(&auto_resolution_tree))?; // save the state of the conflict, so we can recreate it later - tree_writer.insert(&*ConflictedTreeKey::Ours, incoming_tree.id(), 0o040000)?; - tree_writer.insert(&*ConflictedTreeKey::Theirs, target_tree.id(), 0o040000)?; - tree_writer.insert(&*ConflictedTreeKey::Base, base_tree.id(), 0o040000)?; + tree_writer.insert( + &*ConflictedTreeKey::Ours, + incoming_tree.detach().to_git2(), + 0o040000, + )?; + tree_writer.insert( + &*ConflictedTreeKey::Theirs, + target_tree.detach().to_git2(), + 0o040000, + )?; + tree_writer.insert( + &*ConflictedTreeKey::Base, + base_tree.detach().to_git2(), + 0o040000, + )?; tree_writer.insert( &*ConflictedTreeKey::AutoResolution, merged_tree_id.to_git2(), @@ -153,37 +164,20 @@ pub fn merge_commits( Headers::new_with_random_change_id() }; - let (author, committer) = repo.signatures()?; - let commit_oid = crate::RepositoryExt::commit_with_signature( - &repo, + let (author, committer) = gix_repository.commit_signatures()?; + let commit_oid = crate::commit_with_signature_gix( + gix_repository, None, - &author, - &committer, - resulting_name, - &repo.find_tree(tree_oid).context("failed to find tree")?, - &[&target_commit, &incoming_commit], + author, + committer, + resulting_name.into(), + tree_oid.to_gix(), + &[target_commit.id, incoming_commit.id], Some(commit_headers), ) .context("failed to create commit")?; - Ok(commit_oid.to_gix()) -} -pub fn gitbutler_merge_commits<'repo>( - repo: &'repo git2::Repository, - target_commit: git2::Commit<'repo>, - incoming_commit: git2::Commit<'repo>, - target_branch_name: &str, - incoming_branch_name: &str, -) -> Result> { - let gix_repo = gix::open(repo.path())?; - let result_oid = merge_commits( - &gix_repo, - target_commit.id().to_gix(), - incoming_commit.id().to_gix(), - &format!("Merge `{incoming_branch_name}` into `{target_branch_name}`"), - )?; - - Ok(repo.find_commit(result_oid.to_git2())?) + Ok(commit_oid) } trait ToHeaders { diff --git a/crates/gitbutler-repo/src/remote.rs b/crates/gitbutler-repo/src/remote.rs index 16eec0b6fdf..a5382cdac27 100644 --- a/crates/gitbutler-repo/src/remote.rs +++ b/crates/gitbutler-repo/src/remote.rs @@ -8,15 +8,6 @@ pub struct GitRemote { pub url: Option, } -impl From> for GitRemote { - fn from(value: git2::Remote) -> Self { - GitRemote { - name: value.name().map(|name| name.to_owned()), - url: value.url().map(|url| url.to_owned()), - } - } -} - impl GitRemote { pub fn from_gix(name: String, remote: &gix::Remote<'_>) -> Self { GitRemote { diff --git a/crates/gitbutler-repo/src/repository_ext.rs b/crates/gitbutler-repo/src/repository_ext.rs index 1d069dd98b4..5cb485e1687 100644 --- a/crates/gitbutler-repo/src/repository_ext.rs +++ b/crates/gitbutler-repo/src/repository_ext.rs @@ -1,66 +1,15 @@ -use std::str; - -use anyhow::{Context as _, Result, anyhow, bail}; -use bstr::{BStr, BString}; +use anyhow::Result; +use bstr::BStr; use but_core::{ RepositoryExt as RepositoryExtGix, commit::{Headers, SignCommit}, }; -use but_error::Code; -use but_oxidize::{ - ObjectIdExt as _, OidExt, git2_signature_to_gix_signature, gix_to_git2_signature, -}; -use gitbutler_reference::{Refname, RemoteRefname}; - -use crate::{Config, SignaturePurpose}; - -/// Extension trait for `git2::Repository`. -/// -/// For now, it collects useful methods from `gitbutler-core::git::Repository` -pub trait RepositoryExt { - fn find_branch_by_refname(&self, name: &Refname) -> Result>; - /// Returns the common ancestor of the given commit Oids. - /// - /// This is like `git merge-base --octopus`. - /// - /// This method is called `merge_base_octopussy` so that it doesn't - /// conflict with the libgit2 binding I upstreamed when it eventually - /// gets merged. - fn merge_base_octopussy(&self, ids: &[git2::Oid]) -> Result; - fn signatures(&self) -> Result<(git2::Signature<'_>, git2::Signature<'_>)>; - - fn remote_branches(&self) -> Result>; - fn remotes_as_string(&self) -> Result>; - /// `buffer` is the commit object to sign, but in theory could be anything to compute the signature for. - /// Returns the computed signature. - fn sign_buffer(&self, buffer: &[u8]) -> Result; - fn checkout_tree_builder<'a>(&'a self, tree: &'a git2::Tree<'a>) -> CheckoutTreeBuidler<'a>; - fn maybe_find_branch_by_refname(&self, name: &Refname) -> Result>>; - /// Returns the `gitbutler/workspace` 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 - /// workspace is needed. - /// - /// This is for safety to assure the repository actually is in 'gitbutler mode'. - fn workspace_ref_from_head(&self) -> Result>; - - #[expect(clippy::too_many_arguments)] - fn commit_with_signature( - &self, - update_ref: Option<&Refname>, - author: &git2::Signature<'_>, - committer: &git2::Signature<'_>, - message: &str, - tree: &git2::Tree<'_>, - parents: &[&git2::Commit<'_>], - commit_headers: Option, - ) -> Result; -} /// Create a commit with GitButler signing and trailer behavior using `gix`-native inputs. #[expect(clippy::too_many_arguments)] fn commit_gix( repo: &gix::Repository, - update_ref: Option<&Refname>, + update_ref: Option<&gitbutler_reference::Refname>, author: gix::actor::Signature, committer: gix::actor::Signature, message: &BStr, @@ -98,7 +47,7 @@ fn commit_gix( #[expect(clippy::too_many_arguments)] pub fn commit_with_signature_gix( repo: &gix::Repository, - update_ref: Option<&Refname>, + update_ref: Option<&gitbutler_reference::Refname>, author: gix::actor::Signature, committer: gix::actor::Signature, message: &BStr, @@ -123,7 +72,7 @@ pub fn commit_with_signature_gix( #[expect(clippy::too_many_arguments)] pub fn commit_without_signature_gix( repo: &gix::Repository, - update_ref: Option<&Refname>, + update_ref: Option<&gitbutler_reference::Refname>, author: gix::actor::Signature, committer: gix::actor::Signature, message: &BStr, @@ -143,178 +92,3 @@ pub fn commit_without_signature_gix( SignCommit::No, ) } - -impl RepositoryExt for git2::Repository { - fn checkout_tree_builder<'a>(&'a self, tree: &'a git2::Tree<'a>) -> CheckoutTreeBuidler<'a> { - CheckoutTreeBuidler { - tree, - repo: self, - checkout_builder: git2::build::CheckoutBuilder::new(), - } - } - - fn maybe_find_branch_by_refname(&self, name: &Refname) -> Result>> { - let branch = self.find_branch( - &name.simple_name(), - match name { - Refname::Virtual(_) | Refname::Local(_) | Refname::Other(_) => { - git2::BranchType::Local - } - Refname::Remote(_) => git2::BranchType::Remote, - }, - ); - match branch { - Ok(branch) => Ok(Some(branch)), - Err(e) if e.code() == git2::ErrorCode::NotFound => Ok(None), - Err(e) => Err(e.into()), - } - } - - fn find_branch_by_refname(&self, name: &Refname) -> Result> { - let branch = self.find_branch( - &name.simple_name(), - match name { - Refname::Virtual(_) | Refname::Local(_) | Refname::Other(_) => { - git2::BranchType::Local - } - Refname::Remote(_) => git2::BranchType::Remote, - }, - )?; - - Ok(branch) - } - - fn workspace_ref_from_head(&self) -> Result> { - let head_ref = self.head().context("BUG: head must point to a reference")?; - if head_ref.name_bytes() == b"refs/heads/gitbutler/workspace" { - Ok(head_ref) - } else { - Err(anyhow!( - "Unexpected state: cannot perform operation on non-workspace branch" - )) - } - } - - fn commit_with_signature( - &self, - update_ref: Option<&Refname>, - author: &git2::Signature<'_>, - committer: &git2::Signature<'_>, - message: &str, - tree: &git2::Tree<'_>, - parents: &[&git2::Commit<'_>], - commit_headers: Option, - ) -> Result { - let repo = gix::open(self.path())?; - commit_with_signature_gix( - &repo, - update_ref, - git2_signature_to_gix_signature(author), - git2_signature_to_gix_signature(committer), - message.into(), - tree.id().to_gix(), - &parents - .iter() - .map(|commit| commit.id().to_gix()) - .collect::>(), - commit_headers, - ) - .map(|oid| oid.to_git2()) - } - - fn sign_buffer(&self, buffer: &[u8]) -> Result { - but_core::commit::sign_buffer(&gix::open(self.path())?, buffer) - } - - fn remotes_as_string(&self) -> Result> { - Ok(gix::open(self.path())? - .remote_names() - .iter() - .map(|name| name.to_string()) - .collect()) - } - - fn remote_branches(&self) -> Result> { - use bstr::ByteSlice; - - let repo = gix::open_opts(self.path(), gix::open::Options::isolated())?; - repo.references()? - .remote_branches()? - .filter_map(Result::ok) - // TODO: the question is if we really need this? Probably not, but it's part - // of the `gix` migration and we'd rather play it safe. Goal is for `gitbutler-` crates - // to not exist anyway. - .filter(|reference| !reference.name().as_bstr().ends_with_str("/HEAD")) - .map(|reference| { - reference - .name() - .to_string() - .parse() - .context("failed to convert branch to remote name") - }) - .collect() - } - - fn signatures(&self) -> Result<(git2::Signature<'_>, git2::Signature<'_>)> { - let repo = gix::open(self.path())?; - - let author = repo - .author() - .transpose()? - .map(gix_to_git2_signature) - .transpose()? - .context("No author is configured in Git") - .context(Code::AuthorMissing)?; - - let config: Config = (&repo).into(); - let committer = if config.user_real_comitter()? { - repo.committer() - .transpose()? - .map(gix_to_git2_signature) - .unwrap_or_else(|| crate::signature(SignaturePurpose::Committer)) - } else { - crate::signature(SignaturePurpose::Committer) - }?; - - Ok((author, committer)) - } - - fn merge_base_octopussy(&self, ids: &[git2::Oid]) -> Result { - if ids.len() < 2 { - bail!("Merge base octopussy requires at least two commit ids to operate on"); - }; - - let first_oid = ids[0]; - - let output = ids[1..].iter().try_fold(first_oid, |base, oid| { - self.merge_base(base, *oid) - .context("Failed to find merge base") - })?; - - Ok(output) - } -} - -pub struct CheckoutTreeBuidler<'a> { - repo: &'a git2::Repository, - tree: &'a git2::Tree<'a>, - checkout_builder: git2::build::CheckoutBuilder<'a>, -} - -impl CheckoutTreeBuidler<'_> { - pub fn force(&mut self) -> &mut Self { - self.checkout_builder.force(); - self - } - - pub fn remove_untracked(&mut self) -> &mut Self { - self.checkout_builder.remove_untracked(true); - self - } - - pub fn checkout(&mut self) -> Result<()> { - self.repo - .checkout_tree(self.tree.as_object(), Some(&mut self.checkout_builder)) - .map_err(Into::into) - } -} diff --git a/crates/gitbutler-repo/tests/merge_base_octopussy.rs b/crates/gitbutler-repo/tests/merge_base_octopussy.rs deleted file mode 100644 index 4f1bab12e10..00000000000 --- a/crates/gitbutler-repo/tests/merge_base_octopussy.rs +++ /dev/null @@ -1,136 +0,0 @@ -use but_testsupport::legacy::testing_repository::TestingRepository; -use gitbutler_repo::RepositoryExt as _; -use itertools::Itertools; - -#[test] -fn less_than_two_commits() { - let test_repository = TestingRepository::open(); - - let commit = test_repository.commit_tree(None, &[]); - - assert!( - test_repository - .repository - .merge_base_octopussy(&[commit.id()]) - .is_err() - ); -} - -#[test] -fn merge_base_of_two_linear_commits() { - // Setup: - // Base -> (A) -> (B) - // Expected merge base: A - - let test_repository = TestingRepository::open(); - - let base = test_repository.commit_tree(None, &[]); - let a = test_repository.commit_tree(Some(&base), &[]); - let b = test_repository.commit_tree(Some(&a), &[]); - - for permutation in [a.id(), b.id()].into_iter().permutations(2) { - let merge_base = test_repository - .repository - .merge_base_octopussy(&permutation) - .unwrap(); - - assert_eq!(merge_base, a.id()); - } -} - -#[test] -fn merge_base_of_three_linear_commits() { - // Setup: - // Base -> (A) -> (B) -> (C) - // Expected merge base: A - - let test_repository = TestingRepository::open(); - - let base = test_repository.commit_tree(None, &[]); - let a = test_repository.commit_tree(Some(&base), &[]); - let b = test_repository.commit_tree(Some(&a), &[]); - let c = test_repository.commit_tree(Some(&b), &[]); - - for permutation in [a.id(), b.id(), c.id()].into_iter().permutations(3) { - let merge_base = test_repository - .repository - .merge_base_octopussy(&permutation) - .unwrap(); - - assert_eq!(merge_base, a.id()); - } -} - -#[test] -fn merge_base_of_two_parallel_commits() { - // Setup: - // Base -> (A) - // \-> (B) - // Expected merge base: Base - - let test_repository = TestingRepository::open(); - - let base = test_repository.commit_tree(None, &[]); - let a = test_repository.commit_tree(Some(&base), &[]); - let b = test_repository.commit_tree(Some(&base), &[]); - - for permutation in [a.id(), b.id()].into_iter().permutations(2) { - let merge_base = test_repository - .repository - .merge_base_octopussy(&permutation) - .unwrap(); - - assert_eq!(merge_base, base.id()); - } -} - -#[test] -fn merge_base_of_three_parallel_commits() { - // Setup: - // Base -> (A) - // \ \-> (B) - // \--> (C) - // Expected merge base: Base - - let test_repository = TestingRepository::open(); - - let base = test_repository.commit_tree(None, &[]); - let a = test_repository.commit_tree(Some(&base), &[]); - let b = test_repository.commit_tree(Some(&base), &[]); - let c = test_repository.commit_tree(Some(&base), &[]); - - for permutation in [a.id(), b.id(), c.id()].into_iter().permutations(3) { - let merge_base = test_repository - .repository - .merge_base_octopussy(&permutation) - .unwrap(); - - assert_eq!(merge_base, base.id()); - } -} - -#[test] -fn merge_base_of_three_forked_commits() { - // Setup: - // Base -> x -> (A) - // \ \-> (B) - // \--> (C) - // Expected merge base: Base - - let test_repository = TestingRepository::open(); - - let base = test_repository.commit_tree(None, &[]); - let x = test_repository.commit_tree(Some(&base), &[]); - let a = test_repository.commit_tree(Some(&x), &[]); - let b = test_repository.commit_tree(Some(&x), &[]); - let c = test_repository.commit_tree(Some(&base), &[]); - - for permutation in [a.id(), b.id(), c.id()].into_iter().permutations(3) { - let merge_base = test_repository - .repository - .merge_base_octopussy(&permutation) - .unwrap(); - - assert_eq!(merge_base, base.id()); - } -} diff --git a/crates/gitbutler-repo/tests/create_wd_tree.rs b/crates/gitbutler-repo/tests/repo/create_wd_tree.rs similarity index 98% rename from crates/gitbutler-repo/tests/create_wd_tree.rs rename to crates/gitbutler-repo/tests/repo/create_wd_tree.rs index 206100f5d7d..338823e0bb6 100644 --- a/crates/gitbutler-repo/tests/create_wd_tree.rs +++ b/crates/gitbutler-repo/tests/repo/create_wd_tree.rs @@ -3,12 +3,9 @@ use std::{ path::Path, }; +use crate::support::testing_repository::TestingRepository; use but_core::RepositoryExt as _; -use but_testsupport::{ - gix_testtools::scripted_fixture_read_only, legacy::testing_repository::TestingRepository, - open_repo, visualize_tree, -}; -use gitbutler_repo::RepositoryExt as _; +use but_testsupport::{gix_testtools::scripted_fixture_read_only, open_repo, visualize_tree}; use gix::prelude::ObjectIdExt as _; const MAX_SIZE: u64 = 20; @@ -294,11 +291,14 @@ fn should_be_empty_after_checking_out_empty_tree() -> anyhow::Result<()> { { let tree_oid = test.repository.treebuilder(None)?.write()?; let tree = test.repository.find_tree(tree_oid)?; - test.repository - .checkout_tree_builder(&tree) - .force() - .remove_untracked() - .checkout()?; + test.repository.checkout_tree( + tree.as_object(), + Some( + git2::build::CheckoutBuilder::new() + .force() + .remove_untracked(true), + ), + )?; } assert!(!test.tempdir.path().join("file1.txt").exists()); diff --git a/crates/gitbutler-repo/tests/credentials.rs b/crates/gitbutler-repo/tests/repo/credentials.rs similarity index 94% rename from crates/gitbutler-repo/tests/credentials.rs rename to crates/gitbutler-repo/tests/repo/credentials.rs index 9791981f37a..f7e7f989184 100644 --- a/crates/gitbutler-repo/tests/credentials.rs +++ b/crates/gitbutler-repo/tests/repo/credentials.rs @@ -1,8 +1,8 @@ use std::{path::PathBuf, str}; +use crate::support::{setup_blackhole_store, test_repository}; use but_ctx::{Context, RepoOpenMode}; use but_settings::AppSettings; -use but_testsupport::legacy::test_repository; use gitbutler_project as projects; use gitbutler_repo::credentials::{Credential, SshCredential, help}; use gitbutler_user as users; @@ -16,11 +16,11 @@ struct TestCase<'a> { impl TestCase<'_> { fn run(&self) -> Vec<(String, Vec)> { - but_testsupport::legacy::secrets::setup_blackhole_store(); + setup_blackhole_store(); let user: users::User = serde_json::from_str(if self.with_github_login { - include_str!("../tests/fixtures/users/with-github.v1") + include_str!("../../tests/fixtures/users/with-github.v1") } else { - include_str!("../tests/fixtures/users/login-only.v1") + include_str!("../../tests/fixtures/users/login-only.v1") }) .expect("valid v1 sample user"); gitbutler_user::set_user(&user).unwrap(); @@ -38,6 +38,7 @@ impl TestCase<'_> { ) .expect("can create context"); + #[expect(deprecated, reason = "transport/auth compatibility coverage")] let git2_repo = &*ctx.git2_repo.get().unwrap(); let flow = help(git2_repo, &ctx.legacy_project, "origin").unwrap(); flow.into_iter() diff --git a/crates/gitbutler-repo/tests/hooks.rs b/crates/gitbutler-repo/tests/repo/hooks.rs similarity index 94% rename from crates/gitbutler-repo/tests/hooks.rs rename to crates/gitbutler-repo/tests/repo/hooks.rs index a893e2f8416..b087803aee9 100644 --- a/crates/gitbutler-repo/tests/hooks.rs +++ b/crates/gitbutler-repo/tests/repo/hooks.rs @@ -2,12 +2,13 @@ use std::fs; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; -use but_testsupport::{legacy::TestProject, open_repo}; +use crate::support::RepoWithOrigin; +use but_testsupport::open_repo; use gitbutler_repo::hooks::{HookResult, pre_push}; #[test] fn pre_push_hook_not_configured() -> anyhow::Result<()> { - let test_project = TestProject::default(); + let test_project = RepoWithOrigin::default(); let repo = open_repo(test_project.local_repo.path())?; let result = pre_push( @@ -25,7 +26,7 @@ fn pre_push_hook_not_configured() -> anyhow::Result<()> { #[test] fn pre_push_hook_success() -> anyhow::Result<()> { - let test_project = TestProject::default(); + let test_project = RepoWithOrigin::default(); let repo = open_repo(test_project.local_repo.path())?; let hooks_dir = repo.path().join("hooks"); @@ -61,7 +62,7 @@ fn pre_push_hook_success() -> anyhow::Result<()> { #[test] fn pre_push_hook_failure() -> anyhow::Result<()> { - let test_project = TestProject::default(); + let test_project = RepoWithOrigin::default(); let repo = open_repo(test_project.local_repo.path())?; let hooks_dir = repo.path().join("hooks"); @@ -98,7 +99,7 @@ fn pre_push_hook_failure() -> anyhow::Result<()> { #[test] fn pre_push_ignores_husky_core_hooks_path_when_disabled() -> anyhow::Result<()> { - let test_project = TestProject::default(); + let test_project = RepoWithOrigin::default(); let mut repo = open_repo(test_project.local_repo.path())?; let workdir = repo.workdir().expect("non-bare").to_path_buf(); @@ -140,7 +141,7 @@ fn pre_push_ignores_husky_core_hooks_path_when_disabled() -> anyhow::Result<()> #[test] fn pre_push_resolves_relative_core_hooks_path_against_workdir() -> anyhow::Result<()> { - let test_project = TestProject::default(); + let test_project = RepoWithOrigin::default(); let mut repo = open_repo(test_project.local_repo.path())?; let workdir = repo.workdir().expect("non-bare").to_path_buf(); diff --git a/crates/gitbutler-repo/tests/mod.rs b/crates/gitbutler-repo/tests/repo/main.rs similarity index 82% rename from crates/gitbutler-repo/tests/mod.rs rename to crates/gitbutler-repo/tests/repo/main.rs index 3641609d4aa..a85e72979ce 100644 --- a/crates/gitbutler-repo/tests/mod.rs +++ b/crates/gitbutler-repo/tests/repo/main.rs @@ -2,6 +2,7 @@ mod create_wd_tree; mod credentials; mod hooks; mod managed_hooks_tests; -mod merge_base_octopussy; mod read_file_from_workspace_security; mod rebase; +mod remotes; +mod support; diff --git a/crates/gitbutler-repo/tests/managed_hooks_tests.rs b/crates/gitbutler-repo/tests/repo/managed_hooks_tests.rs similarity index 100% rename from crates/gitbutler-repo/tests/managed_hooks_tests.rs rename to crates/gitbutler-repo/tests/repo/managed_hooks_tests.rs diff --git a/crates/gitbutler-repo/tests/read_file_from_workspace_security.rs b/crates/gitbutler-repo/tests/repo/read_file_from_workspace_security.rs similarity index 98% rename from crates/gitbutler-repo/tests/read_file_from_workspace_security.rs rename to crates/gitbutler-repo/tests/repo/read_file_from_workspace_security.rs index 33bf2d17b9f..a8bf9b3ed46 100644 --- a/crates/gitbutler-repo/tests/read_file_from_workspace_security.rs +++ b/crates/gitbutler-repo/tests/repo/read_file_from_workspace_security.rs @@ -1,8 +1,8 @@ use std::{fs, path::Path}; +use crate::support::{commit_all, test_repository}; use but_ctx::{Context, RepoOpenMode}; use but_settings::AppSettings; -use but_testsupport::legacy::{commit_all, test_repository}; use gitbutler_project as projects; use gitbutler_repo::RepoCommands; diff --git a/crates/gitbutler-repo/tests/rebase.rs b/crates/gitbutler-repo/tests/repo/rebase.rs similarity index 73% rename from crates/gitbutler-repo/tests/rebase.rs rename to crates/gitbutler-repo/tests/repo/rebase.rs index 8b0844f8484..7d5f2b92247 100644 --- a/crates/gitbutler-repo/tests/rebase.rs +++ b/crates/gitbutler-repo/tests/repo/rebase.rs @@ -1,8 +1,27 @@ mod gitbutler_merge_commits { - use but_testsupport::legacy::testing_repository::{ - TestingRepository, assert_commit_tree_matches, - }; - use gitbutler_repo::rebase::gitbutler_merge_commits; + use crate::support::testing_repository::{TestingRepository, assert_commit_tree_matches}; + use but_oxidize::{ObjectIdExt as _, OidExt as _}; + use gitbutler_repo::rebase::merge_commits; + + fn gitbutler_merge_commits<'repo>( + test_repository: &'repo TestingRepository, + target_commit: git2::Commit<'repo>, + incoming_commit: git2::Commit<'repo>, + target_branch_name: &str, + incoming_branch_name: &str, + ) -> anyhow::Result> { + let gix_repo = test_repository.gix_repository(); + let result_oid = merge_commits( + &gix_repo, + target_commit.id().to_gix(), + incoming_commit.id().to_gix(), + &format!("Merge `{incoming_branch_name}` into `{target_branch_name}`"), + )?; + + Ok(test_repository + .repository + .find_commit(result_oid.to_git2())?) + } #[test] fn unconflicting_merge() { @@ -13,9 +32,7 @@ mod gitbutler_merge_commits { let b = test_repository.commit_tree(Some(&a), &[("foo.txt", "b")]); let c = test_repository.commit_tree(Some(&a), &[("foo.txt", "a"), ("bar.txt", "a")]); - let result = - gitbutler_merge_commits(&test_repository.repository, b, c, "master", "feature") - .unwrap(); + let result = gitbutler_merge_commits(&test_repository, b, c, "master", "feature").unwrap(); assert_commit_tree_matches( &test_repository.repository, @@ -33,9 +50,7 @@ mod gitbutler_merge_commits { let b = test_repository.commit_tree(Some(&a), &[("foo.txt", "b")]); let c = test_repository.commit_tree(Some(&a), &[("foo.txt", "c")]); - let result = - gitbutler_merge_commits(&test_repository.repository, b, c, "master", "feature") - .unwrap(); + let result = gitbutler_merge_commits(&test_repository, b, c, "master", "feature").unwrap(); assert_commit_tree_matches( &test_repository.repository, @@ -60,17 +75,10 @@ mod gitbutler_merge_commits { let d = test_repository.commit_tree(Some(&a), &[("foo.txt", "a"), ("bar.txt", "a")]); let bc_result = - gitbutler_merge_commits(&test_repository.repository, b, c, "master", "feature") - .unwrap(); + gitbutler_merge_commits(&test_repository, b, c, "master", "feature").unwrap(); - let result = gitbutler_merge_commits( - &test_repository.repository, - bc_result, - d, - "master", - "feature", - ) - .unwrap(); + let result = + gitbutler_merge_commits(&test_repository, bc_result, d, "master", "feature").unwrap(); // While its based on a conflicted commit, merging `bc_result` and `d` // should not conflict, because the auto-resolution of `bc_result`, @@ -98,21 +106,14 @@ mod gitbutler_merge_commits { let e = test_repository.commit_tree(Some(&a), &[("foo.txt", "a"), ("bar.txt", "c")]); let bc_result = - gitbutler_merge_commits(&test_repository.repository, b, c, "master", "feature") - .unwrap(); + gitbutler_merge_commits(&test_repository, b, c, "master", "feature").unwrap(); let de_result = - gitbutler_merge_commits(&test_repository.repository, d, e, "master", "feature") - .unwrap(); + gitbutler_merge_commits(&test_repository, d, e, "master", "feature").unwrap(); - let result = gitbutler_merge_commits( - &test_repository.repository, - bc_result, - de_result, - "master", - "feature", - ) - .unwrap(); + let result = + gitbutler_merge_commits(&test_repository, bc_result, de_result, "master", "feature") + .unwrap(); // We don't expect result to be conflicted, because we've chosen the // setup such that the auto-resolution of `bc_result` and `de_result` @@ -145,21 +146,14 @@ mod gitbutler_merge_commits { let e = test_repository.commit_tree(Some(&a), &[("foo.txt", "f")]); let bc_result = - gitbutler_merge_commits(&test_repository.repository, b, c, "master", "feature") - .unwrap(); + gitbutler_merge_commits(&test_repository, b, c, "master", "feature").unwrap(); let de_result = - gitbutler_merge_commits(&test_repository.repository, d, e, "master", "feature") - .unwrap(); + gitbutler_merge_commits(&test_repository, d, e, "master", "feature").unwrap(); - let result = gitbutler_merge_commits( - &test_repository.repository, - bc_result, - de_result, - "master", - "feature", - ) - .unwrap(); + let result = + gitbutler_merge_commits(&test_repository, bc_result, de_result, "master", "feature") + .unwrap(); // bc_result auto-resoultion tree: // foo.txt: c diff --git a/crates/gitbutler-repo/tests/repo/remotes.rs b/crates/gitbutler-repo/tests/repo/remotes.rs new file mode 100644 index 00000000000..2e01398a95c --- /dev/null +++ b/crates/gitbutler-repo/tests/repo/remotes.rs @@ -0,0 +1,82 @@ +use crate::support::test_repository; +use but_ctx::{Context, RepoOpenMode}; +use but_settings::AppSettings; +use gitbutler_project as projects; +use gitbutler_repo::RepoCommands; +use tempfile::TempDir; + +struct TestCtx { + ctx: Context, + _tmp: TempDir, +} + +fn ctx() -> TestCtx { + let (repo, tmp) = test_repository(); + let project = projects::Project::new_for_gitbutler_repo( + repo.workdir().unwrap().to_path_buf(), + projects::AuthKey::SystemExecutable, + ); + TestCtx { + ctx: Context::new_from_legacy_project_and_settings_with_repo_open_mode( + &project, + AppSettings::default(), + RepoOpenMode::Isolated, + ) + .expect("can create context"), + _tmp: tmp, + } +} + +#[test] +fn add_remote_writes_fetch_url_to_local_config() -> anyhow::Result<()> { + let test = ctx(); + let ctx = &test.ctx; + + ctx.add_remote("origin", "https://example.com/repo.git")?; + + let repo = ctx.open_isolated_repo()?; + let remote = repo.find_remote("origin")?; + assert_eq!( + remote + .url(gix::remote::Direction::Fetch) + .map(|url| url.to_bstring().to_string()), + Some("https://example.com/repo.git".to_owned()) + ); + let config = repo.config_snapshot(); + assert_eq!( + config + .string("remote.origin.fetch") + .map(|value| value.to_string()), + Some("+refs/heads/*:refs/remotes/origin/*".to_owned()) + ); + Ok(()) +} + +#[test] +fn add_remote_rejects_duplicate_name() { + let test = ctx(); + let ctx = &test.ctx; + ctx.add_remote("origin", "https://example.com/repo.git") + .expect("first add succeeds"); + + let err = ctx + .add_remote("origin", "https://example.com/other.git") + .expect_err("duplicate name should fail"); + assert_eq!(err.to_string(), "Remote name 'origin' already exists"); +} + +#[test] +fn add_remote_rejects_duplicate_url() { + let test = ctx(); + let ctx = &test.ctx; + ctx.add_remote("origin", "https://example.com/repo.git") + .expect("first add succeeds"); + + let err = ctx + .add_remote("upstream", "https://example.com/repo.git") + .expect_err("duplicate url should fail"); + assert_eq!( + err.to_string(), + "Remote with url 'https://example.com/repo.git' already exists at 'origin'" + ); +} diff --git a/crates/gitbutler-repo/tests/repo/support/mod.rs b/crates/gitbutler-repo/tests/repo/support/mod.rs new file mode 100644 index 00000000000..d4de197d7cc --- /dev/null +++ b/crates/gitbutler-repo/tests/repo/support/mod.rs @@ -0,0 +1,182 @@ +use std::any::Any; + +use tempfile::{TempDir, tempdir}; + +pub mod testing_repository; + +fn init_opts() -> git2::RepositoryInitOptions { + let mut opts = git2::RepositoryInitOptions::new(); + opts.initial_head("master"); + opts +} + +fn init_opts_bare() -> git2::RepositoryInitOptions { + let mut opts = init_opts(); + opts.bare(true); + opts +} + +fn setup_config(config: &git2::Config) -> anyhow::Result<()> { + match config.open_level(git2::ConfigLevel::Local) { + Ok(mut local) => { + local.set_str("commit.gpgsign", "false")?; + local.set_str("user.name", "gitbutler-test")?; + local.set_str("user.email", "gitbutler-test@example.com")?; + Ok(()) + } + Err(err) => Err(err.into()), + } +} + +fn signature() -> git2::Signature<'static> { + git2::Signature::now("gitbutler-test", "gitbutler-test@example.com") + .expect("signature is valid") +} + +fn initial_commit(repo: &git2::Repository, message: &str) -> git2::Oid { + let tree_id = repo + .index() + .expect("index exists") + .write_tree() + .expect("tree can be written"); + let tree = repo.find_tree(tree_id).expect("tree exists"); + let sig = signature(); + repo.commit(Some("refs/heads/master"), &sig, &sig, message, &tree, &[]) + .expect("initial commit succeeds") +} + +pub fn test_repository() -> (git2::Repository, TempDir) { + let tmp = tempdir().expect("tempdir exists"); + let repo = git2::Repository::init_opts(&tmp, &init_opts()).expect("repository initializes"); + setup_config(&repo.config().expect("config exists")).expect("config is writable"); + initial_commit(&repo, "Initial commit"); + (repo, tmp) +} + +pub fn commit_all(repository: &git2::Repository) -> git2::Oid { + let mut index = repository.index().expect("index exists"); + index + .add_all(["."], git2::IndexAddOption::DEFAULT, None) + .expect("files can be added"); + index.write().expect("index can be written"); + let tree_id = index.write_tree().expect("tree can be written"); + let tree = repository.find_tree(tree_id).expect("tree exists"); + let head = repository.head().expect("head exists"); + let parent = head.peel_to_commit().expect("head commit exists"); + let sig = signature(); + repository + .commit( + Some(head.name().expect("head has name")), + &sig, + &sig, + "some commit", + &tree, + &[&parent], + ) + .expect("commit succeeds") +} + +pub struct RepoWithOrigin { + pub local_repo: git2::Repository, + _local_tmp: TempDir, + _remote_repo: git2::Repository, + _remote_tmp: TempDir, +} + +impl Default for RepoWithOrigin { + fn default() -> Self { + let (local_repo, local_tmp) = test_repository(); + + let remote_tmp = tempdir().expect("tempdir exists"); + let remote_repo = git2::Repository::init_opts(&remote_tmp, &init_opts_bare()) + .expect("bare repository initializes"); + setup_config(&remote_repo.config().expect("config exists")).expect("config is writable"); + + { + let mut remote = local_repo + .remote( + "origin", + remote_repo.path().to_str().expect("path is valid UTF-8"), + ) + .expect("remote can be created"); + remote + .push(&["refs/heads/master:refs/heads/master"], None) + .expect("push succeeds"); + } + + if local_repo + .find_reference("refs/remotes/origin/master") + .is_err() + { + let head = local_repo + .head() + .expect("head exists") + .target() + .expect("head target exists"); + local_repo + .reference( + "refs/remotes/origin/master", + head, + true, + "create remote tracking ref for tests", + ) + .expect("remote tracking ref can be created"); + } + + Self { + local_repo, + _local_tmp: local_tmp, + _remote_repo: remote_repo, + _remote_tmp: remote_tmp, + } + } +} + +pub fn setup_blackhole_store() { + keyring::set_default_credential_builder(Box::new(BlackholeBuilder)) +} + +struct BlackholeBuilder; + +struct BlackholeCredential; + +impl keyring::credential::CredentialApi for BlackholeCredential { + fn set_password(&self, _password: &str) -> keyring::Result<()> { + Ok(()) + } + + fn set_secret(&self, _password: &[u8]) -> keyring::Result<()> { + unreachable!("unused in tests") + } + + fn get_password(&self) -> keyring::Result { + Err(keyring::Error::NoEntry) + } + + fn get_secret(&self) -> keyring::Result> { + unreachable!("unused in tests") + } + + fn delete_credential(&self) -> keyring::Result<()> { + Ok(()) + } + + fn as_any(&self) -> &dyn Any { + self + } +} + +impl keyring::credential::CredentialBuilderApi for BlackholeBuilder { + fn build( + &self, + _target: Option<&str>, + _service: &str, + _user: &str, + ) -> keyring::Result> { + Ok(Box::new(BlackholeCredential)) + } + + fn as_any(&self) -> &dyn Any { + self + } +} diff --git a/crates/gitbutler-repo/tests/repo/support/testing_repository.rs b/crates/gitbutler-repo/tests/repo/support/testing_repository.rs new file mode 100644 index 00000000000..acacf4ecc60 --- /dev/null +++ b/crates/gitbutler-repo/tests/repo/support/testing_repository.rs @@ -0,0 +1,172 @@ +use std::{ + fs, + sync::atomic::{AtomicU64, Ordering}, +}; + +use but_oxidize::OidExt as _; +use gix::bstr::ByteSlice as _; +use tempfile::{TempDir, tempdir}; + +pub struct TestingRepository { + pub repository: git2::Repository, + pub tempdir: TempDir, +} + +static NEXT_COMMIT_ID: AtomicU64 = AtomicU64::new(1); + +impl TestingRepository { + pub fn open() -> Self { + let tempdir = tempdir().expect("tempdir exists"); + let repo = git2::Repository::init_opts(tempdir.path(), &super::init_opts()) + .expect("repository initializes"); + super::setup_config(&repo.config().expect("config exists")).expect("config is writable"); + + { + let empty_tree_id = repo + .treebuilder(None) + .expect("treebuilder exists") + .write() + .expect("tree writes"); + let tree = repo.find_tree(empty_tree_id).expect("tree exists"); + let sig = super::signature(); + repo.commit( + Some("refs/heads/master"), + &sig, + &sig, + "init to prevent load index failure", + &tree, + &[], + ) + .expect("initial commit succeeds"); + } + + Self { + tempdir, + repository: repo, + } + } + + pub fn gix_repository(&self) -> gix::Repository { + gix::open(self.repository.path()).expect("gix repo opens") + } + + pub fn open_with_initial_commit(files: &[(&str, &str)]) -> Self { + let tempdir = tempdir().expect("tempdir exists"); + let repository = git2::Repository::init_opts(tempdir.path(), &super::init_opts()) + .expect("repository initializes"); + super::setup_config(&repository.config().expect("config exists")) + .expect("config is writable"); + + let repo = Self { + repository, + tempdir, + }; + { + let commit = repo.commit_tree(None, files); + repo.repository + .branch("master", &commit, true) + .expect("branch can be written"); + } + repo + } + + pub fn commit_tree<'a>( + &'a self, + parent: Option<&git2::Commit<'_>>, + files: &[(&str, &str)], + ) -> git2::Commit<'a> { + let message = format!( + "test commit {}", + NEXT_COMMIT_ID.fetch_add(1, Ordering::Relaxed) + ); + self.commit_tree_inner(parent, &message, files) + } + + fn commit_tree_inner<'a>( + &'a self, + parent: Option<&git2::Commit<'_>>, + message: &str, + files: &[(&str, &str)], + ) -> git2::Commit<'a> { + for entry in fs::read_dir(self.tempdir.path()).expect("dir can be read") { + let entry = entry.expect("entry exists"); + if entry.file_name() != ".git" { + let path = entry.path(); + if path.is_dir() { + fs::remove_dir_all(path).expect("directory can be removed"); + } else { + fs::remove_file(path).expect("file can be removed"); + } + } + } + + for (file_name, contents) in files { + let path = self.tempdir.path().join(file_name); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).expect("parent directories can be created"); + } + fs::write(path, contents).expect("file contents can be written"); + } + + let mut index = self.repository.index().expect("index exists"); + index.read(true).expect("index can be read"); + index + .add_all(["*"], git2::IndexAddOption::DEFAULT, None) + .expect("files can be added"); + index.write().expect("index can be written"); + + let sig = super::signature(); + let tree_id = index.write_tree().expect("tree can be written"); + let tree = self.repository.find_tree(tree_id).expect("tree exists"); + let oid = self + .repository + .commit( + None, + &sig, + &sig, + message, + &tree, + parent.map(|c| vec![c]).unwrap_or_default().as_slice(), + ) + .expect("commit succeeds"); + + self.repository.find_commit(oid).expect("commit exists") + } +} + +pub fn assert_commit_tree_matches<'a>( + repository: &'a git2::Repository, + commit: &git2::Commit<'a>, + files: &[(&str, &[u8])], +) { + assert_tree_matches(repository, &commit.tree().expect("tree exists"), files); +} + +pub fn assert_tree_matches<'a>( + repository: &'a git2::Repository, + tree: &git2::Tree<'a>, + files: &[(&str, &[u8])], +) { + let gix_repository = gix::open(repository.path()).expect("gix repo opens"); + let tree = gix_repository + .find_tree(tree.id().to_gix()) + .expect("tree exists"); + + for (path, content) in files { + let tree_entry = tree + .lookup_entry_by_path(path) + .expect("lookup succeeds") + .unwrap_or_else(|| panic!("expected {path} in tree")); + let object = gix_repository + .find_object(tree_entry.id()) + .expect("object exists"); + assert_eq!( + object.data, + *content, + "{}: expect {} == {}", + path, + object.data.to_str_lossy(), + content.to_str_lossy() + ); + } +} diff --git a/crates/gitbutler-stack/Cargo.toml b/crates/gitbutler-stack/Cargo.toml index ed855f26d85..ca61be5f3de 100644 --- a/crates/gitbutler-stack/Cargo.toml +++ b/crates/gitbutler-stack/Cargo.toml @@ -5,7 +5,6 @@ edition.workspace = true authors.workspace = true publish = false rust-version.workspace = true -autotests = false [lib] doctest = false @@ -15,13 +14,11 @@ but-core.workspace = true but-rebase.workspace = true but-meta.workspace = true but-error.workspace = true -but-oxidize.workspace = true but-ctx.workspace = true gitbutler-repo.workspace = true gitbutler-reference.workspace = true -git2.workspace = true gix = { workspace = true, features = [] } itertools.workspace = true anyhow.workspace = true @@ -29,10 +26,6 @@ serde.workspace = true toml.workspace = true bstr.workspace = true -[[test]] -name = "stack" -path = "tests/mod.rs" - [dev-dependencies] but-testsupport.workspace = true gitbutler-repo-actions.workspace = true diff --git a/crates/gitbutler-stack/src/stack.rs b/crates/gitbutler-stack/src/stack.rs index 5bca64caa02..13a59d1a62b 100644 --- a/crates/gitbutler-stack/src/stack.rs +++ b/crates/gitbutler-stack/src/stack.rs @@ -9,7 +9,6 @@ use but_core::Reference; pub use but_core::ref_metadata::StackId; use but_ctx::Context; use but_meta::virtual_branches_legacy_types; -use but_oxidize::ObjectIdExt; use but_rebase::ReferenceSpec; use gitbutler_reference::{Refname, RemoteRefname, VirtualRefname, normalize_branch_name}; use gitbutler_repo::first_parent_commit_ids_until; @@ -564,7 +563,7 @@ impl Stack { let upstream_refname = RemoteRefname::from_str(&reference.remote_reference(remote_name.as_str()))?; Ok(PushDetails { - head: oid.to_git2(), + head: oid, remote_refname: upstream_refname, }) } @@ -695,7 +694,7 @@ pub struct PatchReferenceUpdate { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct PushDetails { /// The commit that is being pushed. - pub head: git2::Oid, + pub head: gix::ObjectId, /// A remote refname to push to. pub remote_refname: RemoteRefname, } diff --git a/crates/gitbutler-stack/tests/mod.rs b/crates/gitbutler-stack/tests/stack.rs similarity index 100% rename from crates/gitbutler-stack/tests/mod.rs rename to crates/gitbutler-stack/tests/stack.rs diff --git a/crates/gitbutler-tauri/src/projects.rs b/crates/gitbutler-tauri/src/projects.rs index 1e4351cb4b4..2c79257ad0a 100644 --- a/crates/gitbutler-tauri/src/projects.rs +++ b/crates/gitbutler-tauri/src/projects.rs @@ -54,11 +54,6 @@ pub fn set_project_active( return Ok(None); } }; - // --> WARNING <-- Be sure this runs BEFORE the database on `ctx` is used. - // Only capture this information here to prevent spawning too many errors because of this - // (the UI has many parallel calls in flight). - ctx.eagerly_populate_git2_repo_cache()?; - but_api::legacy::projects::prepare_project_for_activation(&mut ctx)?; let db_error = assure_database_valid(ctx.project_data_dir())?; diff --git a/crates/gitbutler-user/Cargo.toml b/crates/gitbutler-user/Cargo.toml index 54b7466eeaf..4fd42a79cfe 100644 --- a/crates/gitbutler-user/Cargo.toml +++ b/crates/gitbutler-user/Cargo.toml @@ -5,7 +5,6 @@ edition.workspace = true authors.workspace = true publish = false rust-version.workspace = true -autotests = false [lib] doctest = false @@ -19,10 +18,6 @@ anyhow.workspace = true serde.workspace = true serde_json = { workspace = true, features = ["arbitrary_precision"] } -[[test]] -name = "user" -path = "tests/mod.rs" - [dev-dependencies] serial_test = "3.2.0" tempfile.workspace = true diff --git a/crates/gitbutler-user/tests/mod.rs b/crates/gitbutler-user/tests/user/main.rs similarity index 100% rename from crates/gitbutler-user/tests/mod.rs rename to crates/gitbutler-user/tests/user/main.rs diff --git a/crates/gitbutler-user/tests/secret/credentials.rs b/crates/gitbutler-user/tests/user/secret/credentials.rs similarity index 100% rename from crates/gitbutler-user/tests/secret/credentials.rs rename to crates/gitbutler-user/tests/user/secret/credentials.rs diff --git a/crates/gitbutler-user/tests/secret/mod.rs b/crates/gitbutler-user/tests/user/secret/mod.rs similarity index 100% rename from crates/gitbutler-user/tests/secret/mod.rs rename to crates/gitbutler-user/tests/user/secret/mod.rs diff --git a/crates/gitbutler-user/tests/secret/users.rs b/crates/gitbutler-user/tests/user/secret/users.rs similarity index 100% rename from crates/gitbutler-user/tests/secret/users.rs rename to crates/gitbutler-user/tests/user/secret/users.rs diff --git a/crates/gitbutler-workspace/Cargo.toml b/crates/gitbutler-workspace/Cargo.toml index ae75cdac974..9d71a06f6c5 100644 --- a/crates/gitbutler-workspace/Cargo.toml +++ b/crates/gitbutler-workspace/Cargo.toml @@ -5,7 +5,6 @@ edition.workspace = true authors.workspace = true publish = false rust-version.workspace = true -autotests = false [lib] doctest = false diff --git a/crates/gitbutler-workspace/src/branch_trees.rs b/crates/gitbutler-workspace/src/branch_trees.rs index 53847f64ed7..350d67b1d70 100644 --- a/crates/gitbutler-workspace/src/branch_trees.rs +++ b/crates/gitbutler-workspace/src/branch_trees.rs @@ -5,7 +5,7 @@ use but_ctx::{ access::{RepoExclusive, RepoShared}, }; use but_oxidize::{ObjectIdExt, OidExt}; -use gitbutler_cherry_pick::RepositoryExt; +use gitbutler_cherry_pick::GixRepositoryExt as _; use gitbutler_stack::VirtualBranchesHandle; use crate::{workspace_base, workspace_base_from_heads}; @@ -14,29 +14,29 @@ use crate::{workspace_base, workspace_base_from_heads}; #[derive(Debug)] pub struct WorkspaceState { /// The heads of the stacks in the workspace. - heads: Vec, + heads: Vec, /// The base of the workspace. - base: git2::Oid, + base: gix::ObjectId, } impl WorkspaceState { pub fn create(ctx: &Context, perm: &RepoShared) -> Result { - let repo = &*ctx.git2_repo.get()?; + let repo = &*ctx.repo.get()?; let vb_state = VirtualBranchesHandle::new(ctx.project_data_dir()); let heads = vb_state .list_stacks_in_workspace()? .iter() - .map(|stack| -> Result { - let head = stack.head_oid(ctx)?.to_git2(); + .map(|stack| -> Result { + let head = stack.head_oid(ctx)?; let commit = repo.find_commit(head)?; let tree = repo.find_real_tree(&commit, Default::default())?; - Ok(tree.id()) + Ok(tree.detach()) }) .collect::>>()?; - let base = workspace_base(ctx, perm)?.to_git2(); - let base_tree_id = repo.find_commit(base)?.tree_id(); + let base = workspace_base(ctx, perm)?; + let base_tree_id = repo.find_commit(base)?.tree_id()?.detach(); Ok(WorkspaceState { heads, @@ -49,20 +49,20 @@ impl WorkspaceState { perm: &RepoShared, heads: &[gix::ObjectId], ) -> Result { - let repo = &*ctx.git2_repo.get()?; + let repo = &*ctx.repo.get()?; let base = workspace_base_from_heads(ctx, perm, heads)?; let heads = heads .iter() - .map(|head| -> Result { - let commit = repo.find_commit(head.to_git2())?; + .map(|head| -> Result { + let commit = repo.find_commit(*head)?; let tree = repo.find_real_tree(&commit, Default::default())?; - Ok(tree.id()) + Ok(tree.detach()) }) .collect::>>()?; - let base_tree_id = repo.find_commit(base.to_git2())?.tree_id(); + let base_tree_id = repo.find_commit(base)?.tree_id()?.detach(); Ok(WorkspaceState { heads, @@ -80,12 +80,12 @@ pub fn update_uncommitted_changes( perm: &mut RepoExclusive, ) -> Result<()> { let repo = &*ctx.repo.get()?; - let uncommitted_changes = (!ctx.settings.feature_flags.cv3) - .then(|| { - #[expect(deprecated)] - repo.create_wd_tree(0).map(|tree| tree.to_git2()) - }) - .transpose()?; + let uncommitted_changes = if ctx.settings.feature_flags.cv3 { + None + } else { + #[expect(deprecated)] + Some(repo.create_wd_tree(0)?) + }; update_uncommitted_changes_with_tree(ctx, old, new, uncommitted_changes, None, perm) } @@ -95,19 +95,20 @@ pub fn update_uncommitted_changes_with_tree( ctx: &Context, old: WorkspaceState, new: WorkspaceState, - old_uncommitted_changes: Option, + old_uncommitted_changes: Option, always_checkout: Option, _perm: &mut RepoExclusive, ) -> Result<()> { - let gix_repo = ctx.clone_repo_for_merging()?; - let repo = &*ctx.git2_repo.get()?; if let Some(worktree_id) = old_uncommitted_changes { + let gix_repo = ctx.clone_repo_for_merging()?; + #[expect(deprecated, reason = "checkout/index materialization boundary")] + let repo = &*ctx.git2_repo.get()?; let mut new_uncommitted_changes = - move_tree_between_workspaces(repo, &gix_repo, worktree_id, old, new)?; + move_tree_between_workspaces(repo, &gix_repo, worktree_id, &old, &new)?; // If the new tree and old tree are the same, then we don't need to do anything if !new_uncommitted_changes.has_conflicts() && !always_checkout.unwrap_or(false) { - let tree = new_uncommitted_changes.write_tree_to(repo)?; + let tree = new_uncommitted_changes.write_tree_to(repo)?.to_gix(); if tree == worktree_id { return Ok(()); } @@ -123,6 +124,7 @@ pub fn update_uncommitted_changes_with_tree( ), )?; } else { + let gix_repo = ctx.clone_repo_for_merging()?; let old_tree_id = merge_workspace(&gix_repo, &old)?; let new_tree_id = merge_workspace(&gix_repo, &new)?; but_core::worktree::safe_checkout( @@ -140,28 +142,28 @@ pub fn update_uncommitted_changes_with_tree( fn move_tree_between_workspaces( repo: &git2::Repository, gix_repo: &gix::Repository, - tree: git2::Oid, - old: WorkspaceState, - new: WorkspaceState, + tree: gix::ObjectId, + old: &WorkspaceState, + new: &WorkspaceState, ) -> Result { - let old_workspace = merge_workspace(gix_repo, &old)?.to_git2(); - let new_workspace = merge_workspace(gix_repo, &new)?.to_git2(); + let old_workspace = merge_workspace(gix_repo, old)?; + let new_workspace = merge_workspace(gix_repo, new)?; move_tree(repo, tree, old_workspace, new_workspace) } /// Cherry pick a tree from one base tree on to another, favoring the contents of the tree when conflicts occur -pub fn move_tree( +fn move_tree( repo: &git2::Repository, - tree: git2::Oid, - old_workspace: git2::Oid, - new_workspace: git2::Oid, + tree: gix::ObjectId, + old_workspace: gix::ObjectId, + new_workspace: gix::ObjectId, ) -> Result { // Read: Take the diff between old_workspace and tree, and apply it on top // of new_workspace let merge = repo.merge_trees( - &repo.find_tree(old_workspace)?, - &repo.find_tree(tree)?, - &repo.find_tree(new_workspace)?, + &repo.find_tree(old_workspace.to_git2())?, + &repo.find_tree(tree.to_git2())?, + &repo.find_tree(new_workspace.to_git2())?, None, )?; @@ -176,8 +178,8 @@ pub fn merge_workspace( repo: &gix::Repository, workspace: &WorkspaceState, ) -> Result { - let mut output = workspace.base.to_gix(); - let base = workspace.base.to_gix(); + let mut output = workspace.base; + let base = workspace.base; let (merge_options, conflict_kind) = repo.merge_options_fail_fast()?; @@ -185,7 +187,7 @@ pub fn merge_workspace( let mut merge = repo.merge_trees( base, output, - head.to_gix(), + *head, repo.default_merge_labels(), merge_options.clone(), )?; @@ -198,3 +200,14 @@ pub fn merge_workspace( Ok(output) } + +pub fn move_tree_has_conflicts( + ctx: &Context, + tree: gix::ObjectId, + old_workspace: gix::ObjectId, + new_workspace: gix::ObjectId, +) -> Result { + #[expect(deprecated, reason = "tree merge/index materialization boundary")] + let repo = &*ctx.git2_repo.get()?; + Ok(move_tree(repo, tree, old_workspace, new_workspace)?.has_conflicts()) +} diff --git a/etc/plans/git2-to-gix-migration.md b/etc/plans/git2-to-gix-migration.md index 03b967de9a1..b7dbe8f2af8 100644 --- a/etc/plans/git2-to-gix-migration.md +++ b/etc/plans/git2-to-gix-migration.md @@ -1,237 +1,47 @@ -# git2 to gix Migration Plan (Single File, Scoped) +# git2 to gix Migration Plan -## Summary +## Status -This plan orchestrates migration from `git2` to `gix` across the codebase where possible, while treating only these areas as hard boundaries: +Most of the broad migration is already done. This document only tracks the remaining scope needed to finish, plus the intentional `git2` boundaries that are still allowed. -- checkout execution / worktree materialization -- index/tree materialization that still requires `git2` (`Index::write_tree*`, `read_tree`, and immediately related adapters) -- `gitbutler-edit-mode` as a legacy checkout/edit-flow boundary crate until its remaining `git2` checkout/index handoff is isolated or replaced +Repository audit on 2026-04-02: -`create_wd_tree()` itself is already implemented in `gix` in `but-core`; remaining `git2` usage around it should be treated as legacy wrapper/caller cleanup, not as a true `git2` implementation boundary. +- `git2::` callsites in `crates/**/*.rs`: 146 +- files with `git2::` references: 24 +- `ctx.git2_repo` / `ctx.with_git2_repo` callsites: 23 -Config reading and config-setting are explicitly in-scope for migration in this plan. They should use the existing `git_config.rs` / `gix`-based config helpers and must not be treated as a valid reason to keep a `git2` boundary. +## Allowed Remaining Boundary -Transport strategy remains dual-backend for now, but direct `git2` transport/auth call-sites are in-scope for cleanup where `gix` or existing path-based helpers are sufficient. - -Tests and fixture helpers are also explicitly in-scope. This is not just cleanup: repository isolation is part of the migration goal, and `gix` can be opened with isolated options in a way that `git2` does not expose as an equivalent configurable mode for our test setup. - -This document was reconciled against the repository on 2026-03-21 using `rg` and `cargo metadata`. - -## Baseline and Scope - -Repository scan baseline at plan creation: - -- `git2::` callsites: 566 -- files with `git2::` references: 88 -- files currently in-scope under exclusions: 35 - -Current raw audit on 2026-03-21: - -- `git2::` callsites: 236 -- files with `git2::` references: 36 -- `ctx.git2_repo` / `ctx.with_git2_repo` dot-access matches: 77 -- all `git2_repo` / `with_git2_repo` identifier matches including field/setup definitions: 134 across 27 files -- crates with a normal `git2` dependency according to `cargo metadata`: 18 -- filtered in-scope audit: actionable residual `git2` usage remains concentrated in checkout/materialization boundaries, repo/transport adapters, legacy context threading, compatibility crates, and test support - -In-scope means all non-excluded `git2` usage should be migrated to `gix` or isolated behind explicit adapter boundaries. - -Out-of-scope means only the exact hard boundary remains intentionally on `git2`: +`git2` is still acceptable only where we do not yet have a practical `gix` replacement or where the code is deliberately acting as a compatibility adapter: - checkout execution and worktree materialization -- index/tree materialization from staged state (`write_tree/read_tree`, related) - -## Migration Goals - -1. Eliminate non-excluded direct `git2` usage from application/business logic. -2. Migrate public/internal data models from `git2` IDs/time to `gix` IDs/time where not blocked by exclusions. -3. Preserve JSON/TOML wire formats and behavior. -4. Reduce crate-level `git2` dependencies where no longer needed. -5. Primary success signal: remove `git2` as a non-dev dependency from every crate that does not need it for an explicitly excluded scope or a temporary compatibility boundary. -6. Secondary enforcement signal: once remaining `git2` test/runtime uses are narrowed to accepted boundaries, mark `ctx.git2_repo` as deprecated so new call-sites are explicitly discouraged. - -## Dependency Cleanup Priority - -High-value manifest targets for this effort are all `but-*` crates. - -Current `but-*` non-dev `git2` dependency audit on 2026-03-21: - -- `crates/but`: no direct normal `git2` dependency -- `crates/but-core`: still needed for the checkout/worktree hard boundary in `but-core::worktree::checkout::*` -- `crates/but-ctx`: still needed because `Context` exposes `git2_repo` / `with_git2_repo` and multiple callers still rely on that path -- `crates/but-napi`: still opens a `git2::Repository` and injects it into `Context` during project activation -- `crates/but-oxidize`: intentionally depends on `git2` as the conversion boundary crate -- `crates/but-serde`: still isolated behind the optional `legacy` feature -- `crates/but-workspace`: still has an optional `git2` dependency and legacy workspace flows still route through `git2` helpers - -Implication: - -- The highest-value cleanup work is to avoid introducing new non-dev `git2` dependencies in `but-*` crates. -- For `but-*` crates that still depend on `git2`, the preferred end state is either removal, or feature/boundary isolation with a clear excluded-scope justification. -- That end state is not yet reached under the narrowed boundary: `but-core` still owns the remaining hard boundary, while `but-workspace`, `but-ctx`, `but-napi`, and adjacent legacy crates still have additional actionable cleanup. - -## Public API/Type Direction - -Prefer these replacements in non-excluded areas: - -- `git2::Oid` -> `gix::ObjectId` -- `git2::Time` -> `gix::date::Time` -- `git2::Signature` use-sites -> `gix::actor::Signature` / `SignatureRef` at domain boundaries - -Compatibility requirements: - -- Keep existing serialized hex OID string representation. -- Keep existing key names and payload shape for API/frontend consumers. -- Keep persisted data backward-compatible (especially oplog metadata). - -## Workstreams +- index/tree materialization from staged state +- explicit transport/auth adapters that still rely on libgit2 +- narrow compatibility surfaces that exist only to bridge older code -### Workstream A: Foundational Type and Serde Alignment +Anything outside those areas should continue moving to `gix`. -Target modules: +## Remaining Work -- `crates/but-schemars/src/lib.rs` -- `crates/but-serde/src/lib.rs` -- `crates/but-ctx/src/lib.rs` -- `crates/but-workspace/src/ui/author.rs` -- `crates/gitbutler-project/src/lib.rs` -- `crates/gitbutler-repo/src/lib.rs` - -Tasks: - -1. Add/expand `gix` serde helpers and prefer them in migrated surfaces. -2. Reduce `git2` in shared utility type conversions not tied to excluded domains. -3. Keep explicit `git2`<->`gix` conversion only at necessary boundaries. - -Acceptance: - -1. Serialization roundtrips remain unchanged for IDs/time fields. -2. No new direct `git2` imports introduced in modern/non-excluded paths. - -Current reconciliation notes: - -- `crates/but-workspace/src/ui/author.rs` and `crates/gitbutler-project/src/project.rs` match the intended `gix`-first direction. -- `crates/but-serde/src/lib.rs` and `crates/but-schemars/src/lib.rs` still retain legacy `git2` helpers for compatibility; there is no further actionable migration work here without deleting public compatibility surfaces. -- `crates/but-ctx/src/lib.rs` still exposes `git2_repo` and `with_git2_repo`, and neither is deprecated in the current tree. -- `crates/gitbutler-project/src/lib.rs` still configures process-global `git2` options, and `crates/gitbutler-repo/src/lib.rs` still exposes legacy `git2` signature helpers as compatibility boundaries. - -### Workstream B: Config and Refname Migration - -Target modules: +The remaining work is concentrated in a small set of areas: -- `crates/but/src/command/alias.rs` -- `crates/but/src/command/config.rs` -- `crates/but-api/src/legacy/git.rs` -- `crates/gitbutler-repo/src/config.rs` -- `crates/gitbutler-reference/src/refname/mod.rs` -- `crates/gitbutler-reference/src/refname/local.rs` -- `crates/gitbutler-reference/src/refname/remote.rs` -- `crates/gitbutler-reference/src/refname/error.rs` +### Workspace and edit-mode boundary cleanup -Tasks: +These modules still sit on the main checkout/index boundary and should be kept narrow: -1. Replace `git2::Config` usage with `gix` config equivalents where practical. -2. Remove direct `TryFrom<&git2::Branch>` dependency from refname parsing path; use refname strings and `gix` refs where possible. -3. Preserve existing not-found and scope semantics in CLI behavior. - -Acceptance: - -1. Alias/config commands behave identically from user perspective. -2. Refname parse/format behavior remains backward-compatible. - -Current reconciliation notes: - -- `crates/but/src/command/alias.rs`, `crates/but/src/command/config.rs`, `crates/but-api/src/legacy/git.rs`, `crates/gitbutler-repo/src/config.rs`, and the `gitbutler-reference` refname modules no longer have direct `git2` usage. -- Wave 2 remains complete; no actionable migration tasks remain here. - -### Workstream C: Legacy Domain OID/Commit Surface Cleanup (Non-Excluded) - -Target modules: - -- `crates/but/src/command/commit/move.rs` -- `crates/but/src/command/legacy/branch/list.rs` -- `crates/but/src/command/legacy/branch/show.rs` -- `crates/but/src/command/legacy/pick.rs` -- `crates/but/src/command/legacy/rub/squash.rs` -- `crates/but/src/command/legacy/show.rs` -- `crates/but-api/src/legacy/virtual_branches.rs` -- `crates/gitbutler-branch-actions/src/actions.rs` -- `crates/gitbutler-branch-actions/src/author.rs` -- `crates/gitbutler-branch-actions/src/base.rs` -- `crates/gitbutler-branch-actions/src/branch.rs` -- `crates/gitbutler-branch-actions/src/hooks.rs` -- `crates/gitbutler-branch-actions/src/integration.rs` -- `crates/gitbutler-branch-actions/src/move_commits.rs` -- `crates/gitbutler-branch-actions/src/remote.rs` -- `crates/gitbutler-branch-actions/src/reorder.rs` -- `crates/gitbutler-branch-actions/src/squash.rs` -- `crates/gitbutler-branch-actions/src/stack.rs` -- `crates/gitbutler-branch-actions/src/undo_commit.rs` -- `crates/gitbutler-branch-actions/src/upstream_integration.rs` -- `crates/gitbutler-branch-actions/src/virtual.rs` -- `crates/gitbutler-cherry-pick/src/lib.rs` -- `crates/gitbutler-cherry-pick/src/repository_ext.rs` -- `crates/gitbutler-commit/src/commit_ext.rs` -- `crates/but-workspace/src/legacy/head.rs` -- `crates/but-workspace/src/legacy/integrated.rs` -- `crates/but-workspace/src/legacy/stacks.rs` +- `crates/gitbutler-workspace/src/branch_trees.rs` - `crates/gitbutler-edit-mode/src/lib.rs` -- `crates/gitbutler-edit-mode/src/commands.rs` -- `crates/gitbutler-stack/src/stack.rs` -- `crates/gitbutler-stack/src/stack_branch.rs` -- `crates/gitbutler-stack/src/state.rs` -- `crates/gitbutler-stack/src/target.rs` - -Tasks: - -1. Migrate function signatures and local structs from `git2` IDs to `gix` IDs where not crossing excluded paths. -2. Replace direct `git2` commit/tree access in non-excluded logic with `gix` equivalents. -3. For mixed files, isolate excluded operations behind narrow adapters and migrate surrounding logic. - -Acceptance: - -1. Non-excluded branch/cherry-pick/legacy-integrated flows compile with `gix`-first types. -2. Remaining `git2` in these modules is only for excluded-boundary interaction. - -Current reconciliation notes: - -- The files in this workstream that still have direct `git2::` usage today are `crates/gitbutler-branch-actions/src/integration.rs`, `crates/gitbutler-edit-mode/src/lib.rs`, `crates/gitbutler-stack/src/stack.rs`, `crates/gitbutler-workspace/src/branch_trees.rs`, `crates/gitbutler-cherry-pick/src/lib.rs`, `crates/gitbutler-cherry-pick/src/repository_ext.rs`, and `crates/gitbutler-commit/src/commit_ext.rs`. -- Additional branch/workspace flows still depend on the legacy context boundary via `ctx.git2_repo`, notably `crates/but-workspace/src/legacy/head.rs`, `crates/but-worktrees/src/integrate.rs`, `crates/but/src/command/legacy/resolve.rs`, `crates/gitbutler-branch-actions/src/base.rs`, `crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs`, `crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs`, `crates/gitbutler-branch-actions/src/integration.rs`, `crates/gitbutler-branch-actions/src/stack.rs`, `crates/gitbutler-edit-mode/src/lib.rs`, and `crates/gitbutler-workspace/src/branch_trees.rs`. -- `crates/gitbutler-branch-actions/src/author.rs`, `crates/gitbutler-branch-actions/src/hooks.rs`, `crates/gitbutler-branch-actions/src/remote.rs`, `crates/gitbutler-branch-actions/src/reorder.rs`, `crates/gitbutler-branch-actions/src/undo_commit.rs`, `crates/gitbutler-branch-actions/src/upstream_integration.rs`, and `crates/gitbutler-branch-actions/src/branch_upstream_integration.rs` no longer have direct `git2::` usage. -- Workstream C remains active. The next high-value reductions are `crates/gitbutler-edit-mode/src/lib.rs`, `crates/gitbutler-workspace/src/branch_trees.rs`, and the branch-action callers that still bridge into checkout/materialization logic. - -### Workstream D: Oplog Metadata and State Modernization (Non-Excluded) - -Target modules: - -- `crates/gitbutler-oplog/src/entry.rs` - `crates/gitbutler-oplog/src/oplog.rs` -- `crates/gitbutler-oplog/src/snapshot.rs` -- `crates/gitbutler-oplog/src/state.rs` -- `crates/gitbutler-oplog/tests/oplog/main.rs` - -Tasks: - -1. Migrate snapshot metadata/state structures from `git2` OID/time types to `gix` equivalents. -2. Preserve on-disk/read compatibility in `operations-log.toml` and snapshot message parsing. -3. Keep only the exact checkout/materialization and tree-creation boundary unchanged and boundary-adapted. +- boundary portions of `crates/gitbutler-branch-actions/src/integration.rs` -Acceptance: +Goal: -1. Existing snapshot metadata loads and writes without schema regression. -2. Oplog tests pass with migrated non-excluded types. +- keep `git2` usage isolated to the actual checkout/index handoff +- move surrounding read-side and domain logic to `gix` where possible -Current reconciliation notes: +### Repo and transport adapters -- Snapshot metadata/state surfaces in `entry.rs`, `snapshot.rs`, `state.rs`, and `tests/oplog/main.rs` are migrated to `gix`. -- `crates/gitbutler-oplog/src/oplog.rs` still uses `git2` in restore/diff/prepare helpers that cross the remaining hard boundary, and remains the main production target here. -- `crates/gitbutler-oplog/src/reflog.rs` test scaffolding is now `gix`-based. -- Workstream D remains active for `oplog.rs` boundary extraction; `reflog.rs` is no longer part of the residual test cleanup list. - -### Workstream G: Repo Adapter, Transport, and Legacy Boundary Reduction - -Target modules: +These files still appear to contain actionable non-boundary `git2` usage or backend leakage: - `crates/gitbutler-repo/src/repository_ext.rs` - `crates/gitbutler-repo/src/commands.rs` @@ -242,201 +52,81 @@ Target modules: - `crates/gitbutler-repo/src/remote.rs` - `crates/gitbutler-repo/src/staging.rs` - `crates/gitbutler-repo-actions/src/repository.rs` -- `crates/but-napi/src/lib.rs` - `crates/gitbutler-tauri/src/projects.rs` -- `crates/gitbutler-stack/src/stack.rs` - -Tasks: - -1. Replace direct `git2` repo/logging/remote helpers with `gix`-first equivalents where no hard boundary is involved. -2. Reduce transport/auth `git2` usage to explicit backend adapters instead of leaking `git2` types through business logic. -3. Shrink `ctx.git2_repo` callers to the exact remaining hard boundary and compatibility/public API surfaces. - -Acceptance: - -1. Read-side repo helpers, ref/remote inspection, and commit traversal no longer require `git2`. -2. Transport/auth code compiles with `gix`-first inputs except where a deliberate backend adapter still needs `git2`. -3. `gitbutler-tauri` and `but-napi` no longer open or thread `git2` repositories except for the accepted hard boundary. - -Current reconciliation notes: - -- `crates/gitbutler-repo/src/logging.rs` and `crates/gitbutler-project/src/project.rs` are no longer current migration targets; they do not have direct `git2::` usage in the reconciled tree. -- Remaining repo-adapter and transport cleanup is concentrated in `crates/gitbutler-repo/src/repository_ext.rs`, `crates/gitbutler-repo/src/commands.rs`, `crates/gitbutler-repo/src/rebase.rs`, `crates/gitbutler-repo/src/credentials.rs`, `crates/gitbutler-repo/src/hooks.rs`, `crates/gitbutler-repo/src/managed_hooks.rs`, `crates/gitbutler-repo/src/remote.rs`, `crates/gitbutler-repo/src/staging.rs`, and `crates/gitbutler-repo-actions/src/repository.rs`. -- Frontend/application entrypoints still thread `git2` through `Context` in `crates/gitbutler-tauri/src/projects.rs` and `crates/but-napi/src/lib.rs`. -- Transport/auth remains explicitly libgit2-backed in `crates/gitbutler-repo/src/credentials.rs` and `crates/gitbutler-repo-actions/src/repository.rs`. +- `crates/but-napi/src/lib.rs` -### Workstream E: Test Surface and Dependency Reduction +Goal: -This workstream exists for behavioral isolation as well as dependency cleanup. Tests should prefer `gix`/`but-*` helpers because isolated repository opens are a first-class requirement for deterministic test behavior, and that isolation cannot be configured equivalently through `git2` in our current setup. +- use `gix`-first APIs for repo reads and domain logic +- keep any remaining libgit2 use behind explicit transport/auth or hook adapters +- stop threading `git2` repositories through higher-level application code unless required by the accepted boundary -Target tests/modules: +### Compatibility surfaces still to shrink -- `crates/gitbutler-branch-actions/tests/branch-actions/squash.rs` -- `crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/mod.rs` -- `crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/move_commit_to_vbranch.rs` -- `crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/unapply_without_saving_virtual_branch.rs` -- `crates/gitbutler-edit-mode/tests/edit_mode.rs` -- `crates/gitbutler-stack/tests/mod.rs` -- `crates/gitbutler-repo/tests/create_wd_tree.rs` -- `crates/gitbutler-repo/tests/managed_hooks_tests.rs` -- `crates/gitbutler-project/tests/project/main.rs` -- `crates/but-testsupport/src/legacy.rs` -- `crates/but-testsupport/src/legacy/suite.rs` -- `crates/but-testsupport/src/legacy/test_project.rs` -- `crates/but-testsupport/src/legacy/testing_repository.rs` - -Tasks: - -1. Update tests to `gix` IDs where migrated paths changed. -2. Continue moving test helpers to `but-testsupport` and stop expanding `gitbutler-testsupport`; the end state is to replace `gitbutler-testsupport` with `but-testsupport` so `gitbutler-testsupport` can be deleted. -3. Rewrite direct test assertions/setup that still use `ctx.git2_repo` when an equivalent `gix`/helper path is available. -4. Keep only hard-boundary tests/fixtures on `git2`; migrate the rest as production code moves, especially where isolation-sensitive setup can move to `gix`-based helpers. -5. Remove `git2` from crate manifests only after last in-scope reference is gone. - -Acceptance: - -1. Workspace test suite compiles and passes relevant targets. -2. Cargo manifests show reduced `git2` dependencies in migrated crates. -3. Routine test authorship no longer requires `ctx.git2_repo` for non-excluded scenarios. -4. Isolation-sensitive tests and fixture helpers use `gix`/`but-*` paths by default instead of `git2` repository opens. - -Current reconciliation notes: - -- Actionable manifest cleanup is not complete yet. -- `gitbutler-branch` no longer depends on `git2` directly. -- `cargo metadata` still reports 18 crates with a normal `git2` dependency, so manifest cleanup remains blocked on production and test/helper cleanup. -- `crates/gitbutler-stack/tests/mod.rs` now bakes stack commit history via `gix` traversal instead of `ctx.git2_repo`. -- `crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/workspace_migration.rs` now inspects HEAD through `gix`. -- On 2026-03-21, the shared legacy helper surface (`Case`, `Suite`, `TestProject`, `testing_repository`, `secrets`, `paths`, `stack_details`, and related setup helpers) was moved into `crates/but-testsupport/src/legacy/*`, and the current test users in `gitbutler-operating-modes`, `gitbutler-project`, `gitbutler-repo`, and `gitbutler-branch-actions` were switched from `gitbutler-testsupport` to `but-testsupport` with the `legacy` feature. -- `crates/gitbutler-project/tests/project/main.rs` and `crates/gitbutler-oplog/src/reflog.rs` no longer contain direct `git2` usage. -- `crates/gitbutler-edit-mode/tests/edit_mode.rs`, `crates/gitbutler-repo/tests/create_wd_tree.rs`, `crates/gitbutler-repo/tests/managed_hooks_tests.rs`, and `crates/but-testsupport/src/legacy/*` still contain substantial direct `git2` usage. -- On 2026-03-21, the `gitbutler-testsupport` shim crate was deleted after the remaining users had moved to `but-testsupport`. -- `crates/gitbutler-branch-actions/tests/branch-actions/squash.rs` is currently the largest remaining `ctx.git2_repo` consumer in tests. -- Remaining direct `ctx.git2_repo` usage in tests should trend toward hard-boundary coverage and fixture/setup helpers only as reopened production migrations land. - -### Workstream F: Legacy Context Boundary Deprecation - -Target modules: +Some crates still intentionally expose `git2` compatibility helpers or legacy types: - `crates/but-ctx/src/lib.rs` -- all remaining callers of `ctx.git2_repo` - -Tasks: +- `crates/but-oxidize/src/lib.rs` +- `crates/but-serde/src/lib.rs` +- `crates/but-schemars/src/lib.rs` +- `crates/gitbutler-repo/src/lib.rs` +- `crates/gitbutler-cherry-pick/src/*` +- `crates/gitbutler-commit/src/commit_ext.rs` +- `crates/gitbutler-stack/src/stack.rs` -1. After Workstream E narrows test/runtime `git2` usage to intentional boundaries, add a deprecation annotation to `Context::git2_repo`. -2. Add targeted `#[expect(deprecated)]` only at accepted legacy/excluded boundary call-sites so the warning stays useful. -3. Document the allowed reasons to use `ctx.git2_repo`: the exact checkout/materialization and tree-creation hard boundary, plus intentional compatibility/public-boundary code only. +Goal: -Acceptance: +- avoid expanding these surfaces +- remove or reduce them only when callers have moved off them -1. Accessing `ctx.git2_repo` emits a deprecation warning by default. -2. Existing accepted call-sites compile cleanly via explicit local allowance. -3. New `git2` introductions through `ctx.git2_repo` become mechanically discouraged. +### Tests and legacy test support -Current reconciliation notes: +Test-only `git2` usage still exists and should continue shrinking unless it is exercising the accepted hard boundary: -- `crates/but-ctx/src/lib.rs` does not currently mark `Context::git2_repo` or `Context::with_git2_repo()` as deprecated. -- With 76 direct `ctx.git2_repo` / `with_git2_repo` call-sites still present across 24 files, landing the deprecation today would create too much noise. -- Workstream F is still pending. Finish the runtime/test narrowing first, then add the deprecation and local `#[allow(deprecated)]` or `#[expect(deprecated)]` annotations where justified. +- selected `crates/gitbutler-branch-actions/tests/*` +- `crates/gitbutler-edit-mode/tests/edit_mode.rs` +- selected `crates/gitbutler-repo/tests/*` +- `crates/but-testsupport/src/legacy/*` -## Ordered Execution Waves +Goal: -1. Wave 1: Foundational types/serde (`but-serde`, `but-ctx`, shared author/signature paths). -2. Wave 2: Config and refname path (`but` alias/config and `gitbutler-reference`). -3. Wave 3: Branch/cherry-pick legacy logic conversion (non-excluded functions only). -4. Wave 4: Oplog metadata/state migration. -5. Wave 5: Reopen mixed legacy modules, starting with `gitbutler-edit-mode`, workspace/materialization callers, and `gitbutler-oplog`. -6. Wave 6: Repo adapter and transport/auth cleanup (`gitbutler-repo*`, `gitbutler-project`, `gitbutler-tauri`, stack push helpers). -7. Wave 7: Tests and manifest cleanup. -8. Wave 8: Final `ctx.git2_repo` tightening after boundary narrowing is complete. +- prefer `gix` and `but-*` helpers for fixtures and assertions +- keep direct `git2` setup only where boundary coverage actually requires it -Each wave must finish with: +## Current Direction -- compile checks for touched crates -- migration audit command output updated -- explicit record of residual `git2` usage and why +The intended end state is: -Current execution status: +- `gix::ObjectId`, `gix` refs, config, and read-side repository access in normal application logic +- `Context::git2_repo` treated as a deprecated boundary escape hatch only +- residual `git2` usage limited to explicit hard-boundary or compatibility-adapter code -- Workstreams A and B are largely complete. -- Workstreams C, D, E, and G remain active under the narrowed hard boundary. -- Workstream F has not started yet. -- Residual `git2` usage is not yet limited to the hard boundary and compatibility/public-boundary crates. -- Open actionable items: repo adapter cleanup, transport/auth cleanup, workspace/oplog boundary extraction, and continued test/helper `git2` reduction inside `but-testsupport::legacy`. -- Recommended next wave: Wave 5, starting with `crates/gitbutler-workspace/src/branch_trees.rs`, `crates/gitbutler-oplog/src/oplog.rs`, and cleanup of the remaining `git2`-heavy helpers/tests that were just consolidated into `but-testsupport`. +## Completion Criteria -## Acceptance Criteria +This plan is complete when all of the following are true: -Global completion criteria: +1. Non-boundary application logic no longer depends directly on `git2`. +2. Remaining `git2` use is confined to the accepted boundary or explicit compatibility/adapter code. +3. `ctx.git2_repo` callers are limited to those accepted sites. +4. Test and fixture code uses `gix` by default unless boundary coverage requires `git2`. +5. Validation still passes for touched crates and workspace-level checks. -1. All `git2` callsites outside the exact checkout/materialization and tree-creation hard boundary are removed or boundary-isolated. -2. Residual `git2` use is only in the explicit hard boundary, compatibility/public-boundary crates, or deliberately isolated backend adapters. -3. No API/schema regressions for ID/time serialization. -4. Test and fixture setup no longer depends on `git2` where isolated repository access is required. -5. CI-level checks continue to pass. +## Verification -Recommended verification commands: +Recommended checks: ```bash cargo clippy --all-targets --workspace -``` - -Migration audits: - -```bash rg -n "git2::" crates -S --glob '*.rs' +rg -n "ctx\\.(git2_repo|with_git2_repo)" crates -S --glob '*.rs' ``` -Plus a filtered report that excludes only: - -- the exact checkout/worktree materialization boundary -- tree creation from worktree/index state -- explicit compatibility/public-boundary helper crates that are being retired separately - -The filtered report must trend to zero in-scope matches. - -## Current Residual git2 Inventory - -Remaining direct `git2` usage after reconciliation is split between hard-boundary code and still-actionable legacy adapters: - -- Hard-boundary and boundary-adjacent runtime code: `crates/but-core/src/worktree/checkout/*`, `crates/gitbutler-workspace/src/branch_trees.rs`, `crates/gitbutler-edit-mode/src/lib.rs` (treated as a boundary crate for its remaining checkout/index handoff), `crates/gitbutler-oplog/src/oplog.rs`, and the `git2` index/reset portions of `crates/gitbutler-branch-actions/src/integration.rs` -- Actionable repo/transport/frontend adapters: `crates/gitbutler-repo/src/repository_ext.rs`, `crates/gitbutler-repo/src/commands.rs`, `crates/gitbutler-repo/src/rebase.rs`, `crates/gitbutler-repo/src/credentials.rs`, `crates/gitbutler-repo/src/hooks.rs`, `crates/gitbutler-repo/src/remote.rs`, `crates/gitbutler-repo/src/staging.rs`, and `crates/gitbutler-repo-actions/src/repository.rs` -- Foundational/shared compatibility boundaries still to shrink once callers move: `crates/but-ctx/src/lib.rs`, `crates/but-oxidize/src/lib.rs`, `crates/but-serde/src/lib.rs`, `crates/but-schemars/src/lib.rs`, `crates/gitbutler-project/src/lib.rs`, `crates/gitbutler-repo/src/lib.rs`, `crates/gitbutler-cherry-pick/src/*`, `crates/gitbutler-commit/src/commit_ext.rs`, and the remaining `git2` type boundary in `crates/gitbutler-stack/src/stack.rs` -- Legacy test and fixture/setup code that still constructs `git2` values directly: selected `crates/gitbutler-branch-actions/tests/*`, `crates/gitbutler-edit-mode/tests/edit_mode.rs`, `crates/gitbutler-repo/tests/*`, and `crates/but-testsupport/src/legacy/*` -- Planned helper retirement: keep shrinking `git2` inside `but-testsupport::legacy` until the legacy feature itself can collapse away. - -Residual `git2` usage that is currently consistent with the narrowed hard boundary: - -- Checkout execution and worktree materialization -- Index tree materialization from staged state and its immediate adapters -- `ctx.git2_repo` call-sites that remain only to bridge into those exact boundary paths or compatibility/public-boundary code - -## Risks and Controls - -1. Mixed modules (in-scope and excluded code together) can cause churn. - - Control: isolate excluded behavior in adapters first, then migrate surrounding logic. -2. Serialization regressions on OID/time. - - Control: fixture-based roundtrip tests before/after each wave. -3. Legacy-heavy crates may require staged type aliases. - - Control: temporary compatibility aliases with strict TODO removal markers. - -## Explicit Assumptions - -1. The only strict hard boundary for this effort is checkout/worktree materialization and tree creation from worktree/index state. -2. Dual transport backend remains as-is, but transport/auth `git2` usage is still in-scope for cleanup and narrowing. -3. Single-file plan artifact is the source of truth for execution tracking. - -## Tracking Checklist - -Use this section as the running checklist during implementation. +## Tracking -- [x] Wave 1 complete -- [x] Wave 2 complete -- [ ] Wave 5 next -- [ ] Wave 6 pending -- [ ] Wave 7 pending -- [ ] Wave 8 pending -- [ ] In-scope `git2` audit at zero -- [x] Residual `git2` inventory documented -- [ ] `ctx.git2_repo` deprecation landed -- [ ] Residual `git2` usage is hard-boundary only +- [x] Broad migration completed +- [x] Config/refname migration completed +- [x] `Context::git2_repo` deprecated +- [ ] Workspace/edit-mode/oplog boundary reduced to the minimal handoff +- [ ] Repo and transport adapters narrowed further +- [ ] Test and legacy helper `git2` usage reduced to boundary coverage only +- [ ] In-scope `git2` audit at zero outside accepted boundaries