diff --git a/Cargo.lock b/Cargo.lock index e2b2fa420f3..f78e6e7ba4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1086,6 +1086,7 @@ dependencies = [ "anyhow", "but-core", "but-db", + "but-error", "but-graph", "but-meta", "but-path", @@ -1404,10 +1405,8 @@ dependencies = [ "anyhow", "but-api", "but-ctx", - "but-error", "but-path", "but-settings", - "git2", "gitbutler-watcher", "napi", "napi-build", @@ -1580,14 +1579,6 @@ dependencies = [ "ts-rs", ] -[[package]] -name = "but-status" -version = "0.0.0" -dependencies = [ - "anyhow", - "gix", -] - [[package]] name = "but-testing" version = "0.0.0" @@ -1628,15 +1619,31 @@ 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]] @@ -3917,7 +3924,6 @@ dependencies = [ "gitbutler-repo", "gitbutler-repo-actions", "gitbutler-stack", - "gitbutler-testsupport", "gitbutler-url", "gitbutler-workspace", "gix", @@ -4065,8 +4071,8 @@ dependencies = [ "but-fs", "but-schemars", "but-serde", + "but-testsupport", "but-workspace", - "gitbutler-testsupport", "gix", "schemars 1.2.1", "serde", @@ -4086,6 +4092,7 @@ dependencies = [ "but-meta", "but-oxidize", "but-serde", + "but-testsupport", "git2", "gitbutler-branch", "gitbutler-cherry-pick", @@ -4114,8 +4121,7 @@ dependencies = [ "but-project-handle", "but-schemars", "but-serde", - "git2", - "gitbutler-testsupport", + "but-testsupport", "gix", "insta", "resolve-path", @@ -4151,13 +4157,13 @@ dependencies = [ "but-gerrit", "but-oxidize", "but-settings", + "but-testsupport", "fuzzy-matcher", "git2", "git2-hooks", "gitbutler-cherry-pick", "gitbutler-project", "gitbutler-reference", - "gitbutler-testsupport", "gitbutler-url", "gitbutler-user", "gix", @@ -4249,7 +4255,6 @@ dependencies = [ "but-settings", "console-subscriber", "document-features", - "git2", "gitbutler-oplog", "gitbutler-project", "gitbutler-repo-actions", @@ -4287,35 +4292,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "gitbutler-testsupport" -version = "0.0.0" -dependencies = [ - "anyhow", - "but-core", - "but-ctx", - "but-fs", - "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", - "serde_json", - "tempfile", - "termtree", - "uuid", -] - [[package]] name = "gitbutler-url" version = "0.0.0" diff --git a/Cargo.toml b/Cargo.toml index 0fa3d4aefe7..738569fc192 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,6 @@ members = [ # Mostly supporting legacy code. # In big parts replaced by other crates. "crates/but-fs", # πŸ“„filesystem utilities πŸ‘‰we should be using `but-db` instead. - "crates/but-status", # πŸ“„ One function that will be put elsewhere soon to remove this crate. "crates/but-oxidize", # πŸ“„Utilities to help translate between `git2` and `gix`. πŸ‘‰Shouldn't be needed once `git2` is gone. ## @@ -161,7 +160,6 @@ but-meta = { path = "crates/but-meta" } but-rules = { path = "crates/but-rules" } but-action = { path = "crates/but-action" } but-bot = { path = "crates/but-bot" } -but-status = { path = "crates/but-status" } but-tools = { path = "crates/but-tools" } but-api = { path = "crates/but-api" } but-api-macros = { path = "crates/but-api-macros" } @@ -191,7 +189,6 @@ but-llm = { path = "crates/but-llm" } gitbutler-git = { path = "crates/gitbutler-git" } gitbutler-watcher = { path = "crates/gitbutler-watcher" } gitbutler-filemonitor = { path = "crates/gitbutler-filemonitor" } -gitbutler-testsupport = { path = "crates/gitbutler-testsupport" } gitbutler-branch-actions = { path = "crates/gitbutler-branch-actions" } gitbutler-oplog = { path = "crates/gitbutler-oplog" } gitbutler-repo = { path = "crates/gitbutler-repo" } diff --git a/crates/but-ctx/Cargo.toml b/crates/but-ctx/Cargo.toml index 43f11478cca..145a51e1ed9 100644 --- a/crates/but-ctx/Cargo.toml +++ b/crates/but-ctx/Cargo.toml @@ -30,6 +30,7 @@ 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/lib.rs b/crates/but-ctx/src/lib.rs index c9fe78ddc3f..52ec8c25075 100644 --- a/crates/but-ctx/src/lib.rs +++ b/crates/but-ctx/src/lib.rs @@ -13,6 +13,7 @@ use but_core::{ RepositoryExt, sync::{RepoExclusive, RepoExclusiveGuard, RepoShared, RepoSharedGuard}, }; +use but_error::Code; use but_settings::AppSettings; use tracing::instrument; @@ -460,12 +461,6 @@ impl Context { .with_repo(repo)) } - /// Use `git2_repo` instead of the default repository that would be opened on first query. - pub fn with_git2_repo(mut self, git2_repo: git2::Repository) -> Self { - self.git2_repo.assign(git2_repo); - self - } - /// Use `repo` instead of the default repository that would be opened on first query. pub fn with_repo(mut self, repo: gix::Repository) -> Self { self.repo.assign(repo); @@ -1052,6 +1047,34 @@ 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 avoids functions differently, + /// by opening 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/Cargo.toml b/crates/but-napi/Cargo.toml index 4cecba03794..50ada61fb50 100644 --- a/crates/but-napi/Cargo.toml +++ b/crates/but-napi/Cargo.toml @@ -32,10 +32,8 @@ napi-auto = ["but-api/napi"] anyhow.workspace = true but-api.workspace = true but-ctx.workspace = true -but-error.workspace = true but-path.workspace = true but-settings.workspace = true -git2.workspace = true gitbutler-watcher.workspace = true napi = { version = "3", features = ["serde-json", "async"] } napi-derive = "3" diff --git a/crates/but-napi/src/lib.rs b/crates/but-napi/src/lib.rs index f92396338ea..ca0b721935d 100644 --- a/crates/but-napi/src/lib.rs +++ b/crates/but-napi/src/lib.rs @@ -84,17 +84,8 @@ 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()?; - let repo = git2::Repository::open(&ctx.gitdir).map_err(|err| { - let code = err.code(); - let err = anyhow::Error::from(err); - if code == git2::ErrorCode::Owner { - err.context(but_error::Code::RepoOwnership) - } else { - err - } - })?; // Keep this before any database usage on `ctx`. - ctx = ctx.with_git2_repo(repo); + ctx.eagerly_populate_git2_repo_cache()?; but_api::legacy::projects::prepare_project_for_activation(&mut ctx)?; Ok(ctx) } diff --git a/crates/but-status/Cargo.toml b/crates/but-status/Cargo.toml deleted file mode 100644 index 70d6d8df5b0..00000000000 --- a/crates/but-status/Cargo.toml +++ /dev/null @@ -1,17 +0,0 @@ -[package] -name = "but-status" -version = "0.0.0" -edition.workspace = true -authors.workspace = true -publish = false -rust-version.workspace = true - -[lib] -doctest = false - -[dependencies] -gix = { workspace = true, features = ["status", "dirwalk", "attributes"] } -anyhow.workspace = true - -[lints] -workspace = true diff --git a/crates/but-status/src/lib.rs b/crates/but-status/src/lib.rs deleted file mode 100644 index d00b19e4406..00000000000 --- a/crates/but-status/src/lib.rs +++ /dev/null @@ -1,27 +0,0 @@ -/// Gets the status of a given repository. -pub fn get_status(repo: &gix::Repository) -> anyhow::Result> { - use gix::{dir::walk::EmissionMode, status::tree_index::TrackRenames}; - - let status_changes = repo - .status(gix::progress::Discard)? - .tree_index_track_renames(TrackRenames::Disabled) - .index_worktree_rewrites(None) - .index_worktree_submodules(gix::status::Submodule::Given { - ignore: gix::submodule::config::Ignore::Dirty, - check_dirty: true, - }) - .index_worktree_options_mut(|opts| { - if let Some(opts) = opts.dirwalk_options.as_mut() { - opts.set_emit_ignored(None) - .set_emit_pruned(false) - .set_emit_tracked(false) - .set_emit_untracked(EmissionMode::Matching) - .set_emit_collapsed(None); - } - }) - .into_iter(None)? - .filter_map(|change| change.ok()) - .collect::>(); - - Ok(status_changes) -} diff --git a/crates/but-testsupport/Cargo.toml b/crates/but-testsupport/Cargo.toml index 4e36f2acf66..692943f31ce 100644 --- a/crates/but-testsupport/Cargo.toml +++ b/crates/but-testsupport/Cargo.toml @@ -12,6 +12,29 @@ 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. @@ -37,6 +60,28 @@ 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/gitbutler-testsupport/src/fixtures/user/minimal.v1 b/crates/but-testsupport/src/fixtures/user/minimal.v1 similarity index 100% rename from crates/gitbutler-testsupport/src/fixtures/user/minimal.v1 rename to crates/but-testsupport/src/fixtures/user/minimal.v1 diff --git a/crates/gitbutler-testsupport/src/lib.rs b/crates/but-testsupport/src/legacy/mod.rs similarity index 53% rename from crates/gitbutler-testsupport/src/lib.rs rename to crates/but-testsupport/src/legacy/mod.rs index b3816f1334d..78aa47fbb7b 100644 --- a/crates/gitbutler-testsupport/src/lib.rs +++ b/crates/but-testsupport/src/legacy/mod.rs @@ -2,13 +2,10 @@ use but_ctx::Context; use but_oxidize::OidExt; use but_workspace::{legacy::StacksFilter, ui::StackDetails}; use gitbutler_stack::StackId; -use gix::bstr::BStr; +use gix::prelude::ObjectIdExt; pub const VAR_NO_CLEANUP: &str = "GITBUTLER_TESTS_NO_CLEANUP"; -/// Direct access to lower-level utilities for cases where this is enough. -pub use gix_testtools; - mod test_project; pub use test_project::TestProject; @@ -32,7 +29,7 @@ pub mod virtual_branches { use but_oxidize::OidExt; use gitbutler_stack::{Target, VirtualBranchesHandle}; - use crate::empty_bare_repository; + use super::empty_bare_repository; pub fn set_test_target(ctx: &Context) -> anyhow::Result<()> { let mut vb_state = VirtualBranchesHandle::new(ctx.project_data_dir()); @@ -71,62 +68,9 @@ pub fn init_opts_bare() -> git2::RepositoryInitOptions { opts } -/// Display a Git tree in the style of the `tree` CLI program, but include blob contents and usful Git metadata. -pub fn visualize_gix_tree(tree_id: gix::Id<'_>) -> termtree::Tree { - fn visualize_tree( - id: gix::Id<'_>, - name_and_mode: Option<(&BStr, gix::object::tree::EntryMode)>, - ) -> anyhow::Result> { - fn short_id(id: &gix::hash::oid) -> String { - id.to_hex_with_len(7).to_string() - } - let repo = id.repo; - let entry_name = - |id: &gix::hash::oid, name: Option<(&BStr, gix::object::tree::EntryMode)>| -> String { - match name { - None => short_id(id), - Some((name, mode)) => { - format!( - "{name}:{mode}{} {}", - short_id(id), - match repo.find_blob(id) { - Ok(blob) => format!("{:?}", blob.data.as_bstr()), - Err(_) => "".into(), - }, - mode = if mode.is_tree() { - "".into() - } else { - format!("{:o}:", mode.value()) - } - ) - } - } - }; - - let mut tree = termtree::Tree::new(entry_name(&id, name_and_mode)); - for entry in repo.find_tree(id)?.iter() { - let entry = entry?; - if entry.mode().is_tree() { - tree.push(visualize_tree( - entry.id(), - Some((entry.filename(), entry.mode())), - )?); - } else { - tree.push(entry_name( - entry.oid(), - Some((entry.filename(), entry.mode())), - )); - } - } - Ok(tree) - } - visualize_tree(tree_id.object().unwrap().peel_to_tree().unwrap().id(), None).unwrap() -} - -/// Visualize a git2 tree, otherwise just like [`visualize_gix_tree()`]. 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(); - visualize_gix_tree(tree_id.to_gix().attach(&repo)) + crate::visualize_tree(tree_id.to_gix().attach(&repo)) } pub fn stack_details(ctx: &Context) -> Vec<(StackId, StackDetails)> { @@ -155,10 +99,4 @@ pub fn stack_details(ctx: &Context) -> Vec<(StackId, StackDetails)> { details } -use gix::{bstr::ByteSlice, prelude::ObjectIdExt}; - -/// A secrets store to prevent secrets to be written into the systems own store. -/// -/// Note that this can't be used if secrets themselves are under test as it' doesn't act -/// like a real store, i.e. stored secrets can't be retrieved anymore. pub mod secrets; diff --git a/crates/gitbutler-testsupport/src/secrets.rs b/crates/but-testsupport/src/legacy/secrets.rs similarity index 80% rename from crates/gitbutler-testsupport/src/secrets.rs rename to crates/but-testsupport/src/legacy/secrets.rs index a5fca0c2fe3..0c0148c8e11 100644 --- a/crates/gitbutler-testsupport/src/secrets.rs +++ b/crates/but-testsupport/src/legacy/secrets.rs @@ -2,13 +2,10 @@ use std::any::Any; use keyring::Result; -/// Assure we have a mock secrets store so tests don't start writing secrets into the user's actual store, -/// as this will affect their GitButler instance. pub fn setup_blackhole_store() { keyring::set_default_credential_builder(Box::new(BlackholeBuilder)) } -/// A builder that is completely mocked, able to read no secret, but allowing to write any without error. struct BlackholeBuilder; struct BlackholeCredential; diff --git a/crates/gitbutler-testsupport/src/suite.rs b/crates/but-testsupport/src/legacy/suite.rs similarity index 96% rename from crates/gitbutler-testsupport/src/suite.rs rename to crates/but-testsupport/src/legacy/suite.rs index 22b268097c4..65027d52069 100644 --- a/crates/gitbutler-testsupport/src/suite.rs +++ b/crates/but-testsupport/src/legacy/suite.rs @@ -9,7 +9,7 @@ use but_settings::AppSettings; use gitbutler_repo::RepositoryExt; use tempfile::{TempDir, tempdir}; -use crate::{VAR_NO_CLEANUP, init_opts, init_opts_bare, test_project::setup_config}; +use super::{VAR_NO_CLEANUP, init_opts, init_opts_bare, test_project::setup_config}; pub struct Suite { pub local_app_data: Option, @@ -40,9 +40,9 @@ impl Suite { self.local_app_data.as_ref().unwrap().path() } pub fn sign_in(&self) -> gitbutler_user::User { - crate::secrets::setup_blackhole_store(); + crate::legacy::secrets::setup_blackhole_store(); let user: gitbutler_user::User = - serde_json::from_str(include_str!("fixtures/user/minimal.v1")) + 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 @@ -86,7 +86,6 @@ impl Suite { pub struct Case { pub project: gitbutler_project::Project, pub ctx: Context, - /// The directory containing the `ctx` pub project_tmp: Option, } diff --git a/crates/gitbutler-testsupport/src/test_project.rs b/crates/but-testsupport/src/legacy/test_project.rs similarity index 92% rename from crates/gitbutler-testsupport/src/test_project.rs rename to crates/but-testsupport/src/legacy/test_project.rs index 5f02c30b1a8..7e2a6a44900 100644 --- a/crates/gitbutler-testsupport/src/test_project.rs +++ b/crates/but-testsupport/src/legacy/test_project.rs @@ -5,7 +5,7 @@ use gitbutler_reference::{LocalRefname, Refname}; use gitbutler_repo::RepositoryExt; use tempfile::TempDir; -use crate::{VAR_NO_CLEANUP, init_opts}; +use super::{VAR_NO_CLEANUP, init_opts}; pub fn temp_dir() -> TempDir { tempfile::tempdir().unwrap() @@ -85,9 +85,6 @@ impl Default for TestProject { } impl TestProject { - /// Take the tmp directory holding the local repository and make sure it won't be deleted, - /// returning a path to it. - /// Best used inside a `dbg!(test_project.debug_local_repo())` pub fn debug_local_repo(&mut self) -> Option { self.local_tmp.take().map(|tmp| tmp.keep()) } @@ -107,10 +104,6 @@ impl TestProject { .unwrap(); } - /// ```text - /// git add -A - /// git reset --hard - /// ``` pub fn reset_hard(&self, oid: Option) { let mut index = self.local_repo.index().expect("failed to get index"); index @@ -131,7 +124,6 @@ impl TestProject { .unwrap(); } - /// fetch remote into local pub fn fetch(&self) { let mut remote = self.local_repo.find_remote("origin").unwrap(); remote @@ -143,7 +135,7 @@ impl TestProject { 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(), // todo + _ => "INVALID".parse().unwrap(), }; let branch = self .remote_repo @@ -215,12 +207,11 @@ impl TestProject { } } - /// works like if we'd open and merge a PR on github. does not update local. 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()?, // todo + _ => "INVALID".parse()?, }; let branch = self .remote_repo @@ -299,13 +290,6 @@ impl TestProject { head_commit.tree().unwrap() } }, - // Ok(branch) => branch.get().peel_to_tree().unwrap(), - // Err(err) if err.code() == git2::ErrorCode::NotFound => { - // self.local_repository - // .reference(&branch.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(); @@ -316,7 +300,6 @@ impl TestProject { .unwrap(); } - /// takes all changes in the working directory and commits them into local 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"); @@ -362,7 +345,6 @@ impl TestProject { .unwrap(); let repo = submodule.open().unwrap(); - // checkout submodule's master head repo.find_remote("origin") .unwrap() .fetch(&["+refs/heads/*:refs/heads/*"], None, None) @@ -372,8 +354,6 @@ impl TestProject { repo.checkout_tree(reference_head.tree().unwrap().as_object(), None) .unwrap(); - // be sure that `HEAD` points to the actual head - `git2` seems to initialize it - // with `init.defaultBranch`, causing failure otherwise. repo.set_head("refs/heads/master").unwrap(); submodule.add_finalize().unwrap(); } diff --git a/crates/gitbutler-testsupport/src/testing_repository.rs b/crates/but-testsupport/src/legacy/testing_repository.rs similarity index 93% rename from crates/gitbutler-testsupport/src/testing_repository.rs rename to crates/but-testsupport/src/legacy/testing_repository.rs index a25fc9fa9fe..91b5750f32d 100644 --- a/crates/gitbutler-testsupport/src/testing_repository.rs +++ b/crates/but-testsupport/src/legacy/testing_repository.rs @@ -7,7 +7,7 @@ use gix_testtools::bstr::ByteSlice as _; use tempfile::{TempDir, tempdir}; use uuid::Uuid; -use crate::init_opts; +use super::init_opts; pub struct TestingRepository { pub repository: git2::Repository, @@ -18,10 +18,6 @@ impl TestingRepository { pub fn open() -> Self { let tempdir = tempdir().unwrap(); let repo = git2::Repository::init_opts(tempdir.path(), &init_opts()).unwrap(); - // TODO(ST): remove this once `gix::Repository::index_or_load_from_tree_or_empty()` - // is available and used to get merge/diff resource caches. Also: name this - // `open_unborn()` to make it clear. - // For now we need a resemblance of an initialized repo. let signature = git2::Signature::now("Caleb", "caleb@gitbutler.com").unwrap(); let empty_tree_id = repo.treebuilder(None).unwrap().write().unwrap(); repo.commit( @@ -116,7 +112,6 @@ impl TestingRepository { files: &[(&str, &str)], change_id: Option<&str>, ) -> git2::Commit<'a> { - // Remove everything other than the .git folder for entry in fs::read_dir(self.tempdir.path()).unwrap() { let entry = entry.unwrap(); if entry.file_name() != ".git" { @@ -128,7 +123,6 @@ impl TestingRepository { } } } - // Write any files for (file_name, contents) in files { let path = self.tempdir.path().join(file_name); let parent = path.parent().unwrap(); @@ -140,9 +134,7 @@ impl TestingRepository { fs::write(path, contents).unwrap(); } - // Update the index let mut index = self.repository.index().unwrap(); - // Make sure we're not having weird cached state index.read(true).unwrap(); index .add_all(["*"], git2::IndexAddOption::DEFAULT, None) diff --git a/crates/but-testsupport/src/lib.rs b/crates/but-testsupport/src/lib.rs index 147ffa53a3b..6d691cebc98 100644 --- a/crates/but-testsupport/src/lib.rs +++ b/crates/but-testsupport/src/lib.rs @@ -15,6 +15,11 @@ use gix_testtools::{Creation, FixtureState, PostResult, tempfile}; mod in_memory_meta; pub use in_memory_meta::{InMemoryRefMetadata, InMemoryRefMetadataHandle, StackState}; +/// The contents are 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/src/command/legacy/setup.rs b/crates/but/src/command/legacy/setup.rs index 952e8103d50..389fd79a654 100644 --- a/crates/but/src/command/legacy/setup.rs +++ b/crates/but/src/command/legacy/setup.rs @@ -387,7 +387,7 @@ pub(crate) fn repo( // Install managed hooks to prevent accidental git commits if let Ok(repo) = ctx.repo.get() - && let Err(e) = gitbutler_repo::managed_hooks::install_managed_hooks_gix(&repo) + && let Err(e) = gitbutler_repo::managed_hooks::install_managed_hooks(&repo) && let Some(out) = out.for_human() { writeln!( diff --git a/crates/but/src/command/legacy/teardown.rs b/crates/but/src/command/legacy/teardown.rs index 9b9e901d054..3bd4d52238c 100644 --- a/crates/but/src/command/legacy/teardown.rs +++ b/crates/but/src/command/legacy/teardown.rs @@ -114,7 +114,7 @@ pub(crate) fn teardown(ctx: &mut Context, out: &mut OutputChannel) -> anyhow::Re // Uninstall managed hooks before checking out if let Ok(repo) = ctx.repo.get() - && let Err(e) = gitbutler_repo::managed_hooks::uninstall_managed_hooks_gix(&repo) + && let Err(e) = gitbutler_repo::managed_hooks::uninstall_managed_hooks(&repo) && let Some(out) = out.for_human() { writeln!( diff --git a/crates/gitbutler-branch-actions/Cargo.toml b/crates/gitbutler-branch-actions/Cargo.toml index e098b2bd9e4..b12a1301359 100644 --- a/crates/gitbutler-branch-actions/Cargo.toml +++ b/crates/gitbutler-branch-actions/Cargo.toml @@ -64,8 +64,7 @@ but-schemars = { workspace = true, optional = true } [dev-dependencies] pretty_assertions = "1.4" but-graph.workspace = true -but-testsupport.workspace = true -gitbutler-testsupport.workspace = true +but-testsupport = { workspace = true, features = ["legacy"] } gitbutler-workspace.workspace = true gix = { workspace = true, features = [] } glob = "0.3.3" diff --git a/crates/gitbutler-branch-actions/src/integration.rs b/crates/gitbutler-branch-actions/src/integration.rs index 222df8d501f..903d7ee6d7b 100644 --- a/crates/gitbutler-branch-actions/src/integration.rs +++ b/crates/gitbutler-branch-actions/src/integration.rs @@ -201,7 +201,7 @@ pub fn update_workspace_commit_with_vb_state( repo.set_head(&GITBUTLER_WORKSPACE_REFERENCE.clone().to_string())?; // Install managed hooks to prevent accidental git commits on workspace branch - if let Err(e) = gitbutler_repo::managed_hooks::install_managed_hooks_gix(&gix_repo) { + if let Err(e) = gitbutler_repo::managed_hooks::install_managed_hooks(&gix_repo) { tracing::warn!("Failed to install managed hooks: {}", e); } diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs b/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs index f4af23e4d84..c25a9d3f427 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/hooks.rs @@ -2,8 +2,8 @@ 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 gitbutler_testsupport::{Case, Suite}; #[test] fn post_commit_hook_rejection() -> anyhow::Result<()> { diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs b/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs index 2e80452c741..28ebb21cae4 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/reorder.rs @@ -3,10 +3,10 @@ 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 gitbutler_testsupport::testing_repository::assert_commit_tree_matches; use itertools::Itertools; use tempfile::TempDir; @@ -436,7 +436,7 @@ impl CommitHelpers for Vec<(gix::ObjectId, String, bool, u128)> { /// Commits from list_virtual_branches fn vb_commits(ctx: &Context) -> Vec> { - let details = gitbutler_testsupport::stack_details(ctx); + let details = but_testsupport::legacy::stack_details(ctx); let (_, my_stack) = details .iter() .find(|(_, d)| d.derived_name == "top-series") diff --git a/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs b/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs index da1b6be05e9..c84425d832e 100644 --- a/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs +++ b/crates/gitbutler-branch-actions/tests/branch-actions/squash.rs @@ -1,7 +1,6 @@ use anyhow::Result; use bstr::ByteSlice; use but_ctx::Context; -use but_oxidize::ObjectIdExt; use but_workspace::ui::Commit; use gitbutler_branch_actions::squash_commits; use gitbutler_stack::{StackBranch, VirtualBranchesHandle}; @@ -39,7 +38,7 @@ fn squash_without_affecting_stack() -> Result<()> { assert_eq!(branches.b2.patches[0].message, "commit 4"); assert_eq!(branches.b2.patches[1].message, "commit 2\ncommit 3"); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file2_3")?, "change3\n" ); @@ -79,11 +78,11 @@ fn squash_below() -> Result<()> { assert_eq!(branches.b2.patches[0].message, "commit 3"); assert_eq!(branches.b2.patches[1].message, "commit 2\ncommit 4"); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file2_3")?, "change2\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file4")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file4")?, "change4\n" ); @@ -124,11 +123,11 @@ fn squash_above() -> Result<()> { assert_eq!(branches.b2.patches[0].message, "commit 4"); assert_eq!(branches.b2.patches[1].message, "commit 3\ncommit 1"); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file2_3")?, "change3\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file1")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file1")?, "change1\n" ); assert_eq!(branches.b2.patches[2].message, "commit 2"); @@ -169,7 +168,7 @@ fn squash_upwards_works() -> Result<()> { assert_eq!(branches.b2.patches[0].message, "commit 4"); assert_eq!(branches.b2.patches[1].message, "commit 3\ncommit 2"); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file2_3")?, "change3\n" ); @@ -210,7 +209,7 @@ fn squash_down_with_overlap_ok() -> Result<()> { assert_eq!(branches.b2.patches[0].message, "commit 4"); assert_eq!(branches.b2.patches[1].message, "commit 2\ncommit 3"); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file2_3")?, "change3\n" ); @@ -244,11 +243,11 @@ fn squash_below_into_stack_head() -> Result<()> { assert_eq!(branches.b1.patches.len(), 1); assert_eq!(branches.b1.patches[0].message, "commit 1\ncommit 4"); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b1.patches[0].id, "file4")?, + blob_content(&*ctx.repo.get()?, branches.b1.patches[0].id, "file4")?, "change4\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b1.patches[0].id, "file1")?, + blob_content(&*ctx.repo.get()?, branches.b1.patches[0].id, "file1")?, "change1\n" ); @@ -295,15 +294,15 @@ fn squash_multiple() -> Result<()> { "commit 1\ncommit 4\ncommit 2" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b1.patches[0].id, "file4")?, + blob_content(&*ctx.repo.get()?, branches.b1.patches[0].id, "file4")?, "change4\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b1.patches[0].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b1.patches[0].id, "file2_3")?, "change2\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b1.patches[0].id, "file1")?, + blob_content(&*ctx.repo.get()?, branches.b1.patches[0].id, "file1")?, "change1\n" ); @@ -354,15 +353,15 @@ fn squash_multiple_from_heads() -> Result<()> { "commit 2\ncommit 5\ncommit 4" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file5")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file5")?, "change5\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file4")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file4")?, "change4\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file2_3")?, "change2\n" ); @@ -408,15 +407,15 @@ fn squash_multiple_above_and_below() -> Result<()> { "commit 3\ncommit 5\ncommit 1" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file2_3")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file2_3")?, "change3\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file5")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file5")?, "change5\n" ); assert_eq!( - blob_content(&*ctx.git2_repo.get()?, branches.b2.patches[1].id, "file1")?, + blob_content(&*ctx.repo.get()?, branches.b2.patches[1].id, "file1")?, "change1\n" ); assert_eq!(branches.b2.patches[2].message, "commit 2"); @@ -493,7 +492,7 @@ struct Branch { /// Stack branches from the API fn list_branches(ctx: &Context) -> Result { - let details = gitbutler_testsupport::stack_details(ctx); + let details = but_testsupport::legacy::stack_details(ctx); let (_, details) = details.first().unwrap(); let branches: Vec = details .branch_details @@ -513,10 +512,10 @@ fn list_branches(ctx: &Context) -> Result { }) } -fn blob_content(repo: &git2::Repository, commit_oid: gix::ObjectId, file: &str) -> Result { - let tree = repo.find_commit(commit_oid.to_git2())?.tree()?; - let entry = tree.get_name(file).unwrap(); - let blob = repo.find_blob(entry.id())?; - let blob_content: &str = blob.content().to_str()?; +fn blob_content(repo: &gix::Repository, commit_oid: gix::ObjectId, file: &str) -> Result { + let tree = repo.find_commit(commit_oid)?.tree()?; + let entry = tree.lookup_entry_by_path(file)?.unwrap(); + let blob = entry.object()?.into_blob(); + let blob_content: &str = blob.data.to_str()?; Ok(blob_content.to_string()) } 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 d70f95fff6a..7b41601a2ac 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,7 +1,6 @@ use but_core::{DiffSpec, HunkHeader}; -use but_oxidize::{ObjectIdExt, OidExt}; +use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; -use gitbutler_testsupport::stack_details; use super::{list_commit_files, *}; @@ -75,7 +74,7 @@ fn forcepush_allowed() -> anyhow::Result<()> { new_lines: 1, }], }]; - gitbutler_branch_actions::amend(ctx, stack_entry.id, commit_id.to_gix(), to_amend).unwrap(); + gitbutler_branch_actions::amend(ctx, stack_entry.id, commit_id, to_amend).unwrap(); let (_, b) = stack_details(ctx) .into_iter() @@ -83,7 +82,7 @@ fn forcepush_allowed() -> anyhow::Result<()> { .unwrap(); assert_eq!(b.branch_details[0].commits.len(), 1); assert_eq!( - list_commit_files(ctx, b.branch_details[0].commits[0].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].commits[0].id)?.len(), 2 ); } @@ -136,8 +135,7 @@ fn non_locked_hunk() -> anyhow::Result<()> { new_lines: 1, }], }]; - gitbutler_branch_actions::amend(ctx, stack_entry.id, commit_oid.to_gix(), to_amend) - .unwrap(); + gitbutler_branch_actions::amend(ctx, stack_entry.id, commit_oid, to_amend).unwrap(); let (_, b) = stack_details(ctx) .into_iter() @@ -145,7 +143,7 @@ fn non_locked_hunk() -> anyhow::Result<()> { .unwrap(); assert_eq!(b.branch_details[0].commits.len(), 1); assert_eq!( - list_commit_files(ctx, b.branch_details[0].commits[0].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].commits[0].id)?.len(), 2 ); } @@ -198,7 +196,7 @@ fn non_existing_ownership() { }], }]; assert_eq!( - gitbutler_branch_actions::amend(ctx, stack_entry.id, commit_oid.to_gix(), to_amend) + gitbutler_branch_actions::amend(ctx, stack_entry.id, commit_oid, to_amend) .unwrap_err() .to_string(), r#"Failed to amend with commit engine. Rejected specs: [(NoEffectiveChanges, DiffSpec { previous_path: None, path: "file2.txt", hunk_headers: [HunkHeader("-1,0", "+1,1")] })]"#, 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 e608d2ad093..88f9c2faf5f 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,10 +1,10 @@ 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; -use gitbutler_testsupport::stack_details; use super::*; 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 cc8970d2d55..6b4ef152b18 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,6 +1,6 @@ +use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use gitbutler_reference::LocalRefname; -use gitbutler_testsupport::stack_details; use super::*; #[test] 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 e96d9b1fbd3..b861bd6643a 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,4 +1,4 @@ -use gitbutler_testsupport::stack_details; +use but_testsupport::legacy::stack_details; use super::*; 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 d8fce34fb71..43b1db63aee 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,14 +2,13 @@ use std::{fs, path, path::PathBuf, str::FromStr}; use but_ctx::{Context, ProjectHandleOrLegacyProjectId, RepoOpenMode}; use but_error::Marker; -use but_oxidize::{ObjectIdExt, OidExt}; use but_settings::AppSettings; +use but_testsupport::legacy::{TestProject, VAR_NO_CLEANUP, paths}; 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_stack::StackId; -use gitbutler_testsupport::{TestProject, VAR_NO_CLEANUP, paths}; use tempfile::TempDir; struct Test { @@ -85,10 +84,9 @@ mod workspace_migration; pub fn list_commit_files( ctx: &Context, - commit_oid: git2::Oid, + commit_id: gix::ObjectId, ) -> anyhow::Result> { let repo = ctx.repo.get()?; - let commit_id = commit_oid.to_gix(); but_core::diff::CommitDetails::from_commit_id( gix::prelude::ObjectIdExt::attach(commit_id, &repo), false, @@ -100,7 +98,7 @@ pub fn create_commit( ctx: &mut Context, stack_id: StackId, message: &str, -) -> anyhow::Result { +) -> anyhow::Result { let mut guard = ctx.exclusive_worktree_access(); let repo = ctx.repo.get()?; @@ -149,6 +147,5 @@ pub fn create_commit( }); outcome? .new_commit - .map(|c| c.to_git2()) .ok_or(anyhow::anyhow!("No new commit created")) } 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 5752e552c99..058c03b19c8 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,8 +1,9 @@ +use std::str::FromStr; + use bstr::ByteSlice; -use but_oxidize::OidExt; +use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use gitbutler_stack::StackId; -use gitbutler_testsupport::stack_details; use super::Test; @@ -46,13 +47,8 @@ fn no_diffs() { .unwrap(); drop(guard); - gitbutler_branch_actions::move_commit( - ctx, - target_stack_entry.id, - commit_oid.to_gix(), - source_branch_id, - ) - .unwrap(); + gitbutler_branch_actions::move_commit(ctx, target_stack_entry.id, commit_oid, source_branch_id) + .unwrap(); let destination = stack_details(ctx) .into_iter() @@ -126,13 +122,8 @@ fn multiple_commits() { super::create_commit(ctx, target_stack_entry.id, "Add d").unwrap(); // Move the top commit from the source branch to the destination branch - gitbutler_branch_actions::move_commit( - ctx, - target_stack_entry.id, - commit_oid.to_gix(), - source_branch_id, - ) - .unwrap(); + gitbutler_branch_actions::move_commit(ctx, target_stack_entry.id, commit_oid, source_branch_id) + .unwrap(); let destination = stack_details(ctx) .into_iter() @@ -240,13 +231,8 @@ fn multiple_commits_with_diffs() { assert_eq!(destination.1.branch_details[0].clone().commits.len(), 1); // Move the top commit from the source branch to the destination branch - gitbutler_branch_actions::move_commit( - ctx, - target_stack_entry.id, - commit_oid.to_gix(), - source_branch_id, - ) - .unwrap(); + gitbutler_branch_actions::move_commit(ctx, target_stack_entry.id, commit_oid, source_branch_id) + .unwrap(); let source = stack_details(ctx) .into_iter() @@ -319,13 +305,8 @@ fn diffs_on_source_branch() { .unwrap(); drop(guard); - gitbutler_branch_actions::move_commit( - ctx, - target_stack_entry.id, - commit_oid.to_gix(), - source_branch_id, - ) - .unwrap(); + gitbutler_branch_actions::move_commit(ctx, target_stack_entry.id, commit_oid, source_branch_id) + .unwrap(); let source = stack_details(ctx) .into_iter() @@ -382,13 +363,8 @@ fn diffs_on_target_branch() { std::fs::write(repo.path().join("another file.txt"), "another content").unwrap(); - gitbutler_branch_actions::move_commit( - ctx, - target_stack_entry.id, - commit_oid.to_gix(), - source_branch_id, - ) - .unwrap(); + gitbutler_branch_actions::move_commit(ctx, target_stack_entry.id, commit_oid, source_branch_id) + .unwrap(); let source = stack_details(ctx) .into_iter() @@ -468,13 +444,8 @@ fn diffs_on_both_branches() { // State of the destination branch before the commit is moved assert_eq!(destination.1.branch_details[0].clone().commits.len(), 0); - gitbutler_branch_actions::move_commit( - ctx, - target_stack_entry.id, - commit_oid.to_gix(), - source_branch_id, - ) - .unwrap(); + gitbutler_branch_actions::move_commit(ctx, target_stack_entry.id, commit_oid, source_branch_id) + .unwrap(); let source = stack_details(ctx) .into_iter() @@ -535,7 +506,7 @@ fn locked_hunks_on_source_branch() { gitbutler_branch_actions::move_commit( ctx, target_stack_entry.id, - commit_oid.to_gix(), + commit_oid, source_branch_id ) .is_ok() @@ -585,7 +556,7 @@ fn no_commit() { gitbutler_branch_actions::move_commit( ctx, target_stack_entry.id, - git2::Oid::from_str(commit_id_hex).unwrap().to_gix(), + gix::ObjectId::from_str(commit_id_hex).unwrap(), source_branch_id, ) .unwrap_err() @@ -625,7 +596,7 @@ fn no_branch() { let id = StackId::generate(); assert_eq!( - gitbutler_branch_actions::move_commit(ctx, id, commit_oid.to_gix(), source_branch_id) + gitbutler_branch_actions::move_commit(ctx, id, commit_oid, source_branch_id) .unwrap_err() .to_string(), "Destination branch not found" 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 77fc23a173a..4dafe4b4952 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,12 +1,11 @@ use std::{io::Write, path::Path}; use but_core::{GitConfigSettings, RepositoryExt as _}; -use but_oxidize::ObjectIdExt; +use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; use gitbutler_oplog::OplogExt; use gitbutler_oplog::entry::{OperationKind, SnapshotDetails}; use gitbutler_stack::VirtualBranchesHandle; -use gitbutler_testsupport::stack_details; use itertools::Itertools; use super::*; @@ -186,11 +185,11 @@ fn basic_oplog() -> anyhow::Result<()> { assert_eq!(b.branch_details[0].clone().commits.len(), 3); assert_eq!( - list_commit_files(ctx, b.branch_details[0].clone().commits[0].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].clone().commits[0].id)?.len(), 1 ); assert_eq!( - list_commit_files(ctx, b.branch_details[0].clone().commits[1].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].clone().commits[1].id)?.len(), 3 ); @@ -248,8 +247,8 @@ fn basic_oplog() -> anyhow::Result<()> { std::fs::remove_file(file_path)?; // try to look up that object - let commit = repo.find_commit(commit2_id); - assert!(commit.is_err()); + let commit_missing = !ctx.repo.get()?.has_object(commit2_id); + assert!(commit_missing); { let mut guard = ctx.exclusive_worktree_access(); @@ -258,8 +257,8 @@ fn basic_oplog() -> anyhow::Result<()> { } // test missing commits are recreated - let commit = repo.find_commit(commit2_id); - assert!(commit.is_ok()); + let commit_restored = ctx.repo.get()?.has_object(commit2_id); + assert!(commit_restored); let file_path = repo.path().join("large.txt"); assert!(file_path.exists()); 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 0f695435e44..9b057ce7aee 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,6 @@ use but_core::DiffSpec; use but_hunk_assignment::HunkAssignmentRequest; -use gitbutler_testsupport::stack_details; +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 1c420ada4b3..d9d330d8f35 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,8 +37,8 @@ mod error { } mod go_back_to_workspace { + use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; - use gitbutler_testsupport::stack_details; use pretty_assertions::assert_eq; use super::*; 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 81c683594cc..c6a7a3f39e9 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,4 +1,4 @@ -use gitbutler_testsupport::stack_details; +use but_testsupport::legacy::stack_details; use super::*; 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 afeae183abe..e4cba3e8264 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,6 +1,5 @@ -use but_oxidize::ObjectIdExt; +use but_testsupport::legacy::stack_details; use gitbutler_branch::BranchCreateRequest; -use gitbutler_testsupport::stack_details; use super::*; @@ -39,7 +38,7 @@ fn undo_commit_simple() -> anyhow::Result<()> { fs::write(repo.path().join("file4.txt"), "content4").unwrap(); let _commit3_id = super::create_commit(ctx, stack_entry.id, "commit three").unwrap(); - gitbutler_branch_actions::undo_commit(ctx, stack_entry.id, commit2_id.to_gix()).unwrap(); + gitbutler_branch_actions::undo_commit(ctx, stack_entry.id, commit2_id).unwrap(); // should be two uncommitted files now (file2.txt and file3.txt) let changes = but_core::diff::ui::worktree_changes(&*ctx.repo.get()?)?.changes; @@ -50,11 +49,11 @@ fn undo_commit_simple() -> anyhow::Result<()> { .unwrap(); assert_eq!(b.branch_details[0].clone().commits.len(), 2); assert_eq!( - list_commit_files(ctx, b.branch_details[0].clone().commits[0].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].clone().commits[0].id)?.len(), 1 ); assert_eq!( - list_commit_files(ctx, b.branch_details[0].clone().commits[1].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].clone().commits[1].id)?.len(), 1 ); @@ -117,7 +116,7 @@ fn undo_commit_in_non_default_branch() -> anyhow::Result<()> { .unwrap(); drop(guard); - gitbutler_branch_actions::undo_commit(ctx, stack_entry.id, commit2_id.to_gix()).unwrap(); + gitbutler_branch_actions::undo_commit(ctx, stack_entry.id, commit2_id).unwrap(); // should be two uncommitted files now (file2.txt and file3.txt) let changes = but_core::diff::ui::worktree_changes(&*ctx.repo.get()?)?.changes; @@ -130,11 +129,11 @@ fn undo_commit_in_non_default_branch() -> anyhow::Result<()> { assert_eq!(b.branch_details[0].clone().commits.len(), 2); assert_eq!( - list_commit_files(ctx, b.branch_details[0].clone().commits[0].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].clone().commits[0].id)?.len(), 1 ); assert_eq!( - list_commit_files(ctx, b.branch_details[0].clone().commits[1].id.to_git2())?.len(), + list_commit_files(ctx, b.branch_details[0].clone().commits[1].id)?.len(), 1 ); 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 1dab20a1181..e51a1d63199 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,8 +1,7 @@ -use but_oxidize::OidExt as _; +use but_testsupport::legacy::stack_details; use but_workspace::ui::PushStatus; use gitbutler_branch::BranchCreateRequest; use gitbutler_commit::commit_ext::CommitExt; -use gitbutler_testsupport::stack_details; use super::*; @@ -44,14 +43,14 @@ fn head() { }; let before_change_id = { let repo = ctx.repo.get().unwrap(); - let commit_three = repo.find_commit(commit_three_oid.to_gix()).unwrap(); + let commit_three = repo.find_commit(commit_three_oid).unwrap(); commit_three.change_id() }; gitbutler_branch_actions::update_commit_message( ctx, stack_entry.id, - commit_three_oid.to_gix(), + commit_three_oid, "commit three updated", ) .unwrap(); @@ -71,7 +70,7 @@ fn head() { let commit = repo.find_commit(b.branch_details[0].commits[0].id).unwrap(); // make sure the SHA changed, but the change ID did not - assert_ne!(commit_three_oid.to_gix(), commit.id()); + assert_ne!(commit_three_oid, commit.id()); assert_eq!(before_change_id, commit.change_id()); assert_eq!( @@ -120,7 +119,7 @@ fn middle() { gitbutler_branch_actions::update_commit_message( ctx, stack_entry.id, - commit_two_oid.to_gix(), + commit_two_oid, "commit two updated", ) .unwrap(); @@ -195,7 +194,7 @@ fn forcepush_allowed() { gitbutler_branch_actions::update_commit_message( ctx, stack_entry.id, - commit_one_oid.to_gix(), + commit_one_oid, "commit one updated", ) .unwrap(); @@ -257,7 +256,7 @@ fn root() { gitbutler_branch_actions::update_commit_message( ctx, branch_id.id, - commit_one_oid.to_gix(), + commit_one_oid, "commit one updated", ) .unwrap(); @@ -306,14 +305,9 @@ fn empty() { }; assert_eq!( - gitbutler_branch_actions::update_commit_message( - ctx, - branch_id.id, - commit_one_oid.to_gix(), - "", - ) - .unwrap_err() - .to_string(), + gitbutler_branch_actions::update_commit_message(ctx, branch_id.id, commit_one_oid, "",) + .unwrap_err() + .to_string(), "commit message can not be empty" ); } diff --git a/crates/gitbutler-cli/src/main.rs b/crates/gitbutler-cli/src/main.rs index f9c2cf36ecb..5334ea8767a 100644 --- a/crates/gitbutler-cli/src/main.rs +++ b/crates/gitbutler-cli/src/main.rs @@ -12,7 +12,6 @@ mod command; fn main() -> Result<()> { let args: Args = clap::Parser::parse(); - gitbutler_project::configure_git2(); if args.trace { trace::init()?; diff --git a/crates/gitbutler-operating-modes/Cargo.toml b/crates/gitbutler-operating-modes/Cargo.toml index 9941c03b0c9..3f877489f20 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] -gitbutler-testsupport.workspace = true +but-testsupport = { workspace = true, features = ["legacy"] } [lints] workspace = true diff --git a/crates/gitbutler-operating-modes/tests/operating_modes.rs b/crates/gitbutler-operating-modes/tests/operating_modes.rs index 5e3f15f99de..fbd8e67de7d 100644 --- a/crates/gitbutler-operating-modes/tests/operating_modes.rs +++ b/crates/gitbutler-operating-modes/tests/operating_modes.rs @@ -64,8 +64,8 @@ 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 gitbutler_testsupport::{Case, Suite}; use crate::create_and_checkout_branch; @@ -140,8 +140,8 @@ 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 gitbutler_testsupport::{Case, Suite}; use crate::{create_and_checkout_branch, create_edit_mode_metadata}; @@ -221,8 +221,8 @@ mod operating_modes { } mod edit_mode { + use but_testsupport::legacy::{Case, Suite}; use gitbutler_operating_modes::{ensure_edit_mode, in_edit_mode}; - use gitbutler_testsupport::{Case, Suite}; use crate::{create_and_checkout_branch, create_edit_mode_metadata}; diff --git a/crates/gitbutler-oplog/Cargo.toml b/crates/gitbutler-oplog/Cargo.toml index 3e863d16e81..dba2e68e21a 100644 --- a/crates/gitbutler-oplog/Cargo.toml +++ b/crates/gitbutler-oplog/Cargo.toml @@ -32,6 +32,7 @@ gix = { workspace = true, features = ["dirwalk", "credentials", "parallel"] } toml.workspace = true [dev-dependencies] +but-testsupport.workspace = true pretty_assertions = "1.4" tempfile.workspace = true diff --git a/crates/gitbutler-oplog/src/reflog.rs b/crates/gitbutler-oplog/src/reflog.rs index a165e00e28a..7cf32df22a9 100644 --- a/crates/gitbutler-oplog/src/reflog.rs +++ b/crates/gitbutler-oplog/src/reflog.rs @@ -163,9 +163,10 @@ fn serialize_line(line: gix::refs::file::log::LineRef<'_>) -> String { #[cfg(test)] mod set_target_ref { - use std::path::PathBuf; + use std::{path::Path, str::FromStr}; - use but_oxidize::OidExt; + use anyhow::{Context as _, bail}; + use but_testsupport::git_at_dir; use gix::refs::file::log::LineRef; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -181,7 +182,7 @@ mod set_target_ref { let worktree_dir = dir.path(); let git_dir = worktree_dir.join(".git"); - let oplog = git2::Oid::from_str("0123456789abcdef0123456789abcdef0123456")?; + let oplog = gix::ObjectId::from_str("0123456789abcdef0123456789abcdef01234567")?; set_reference_to_oplog(&git_dir, reflog_commits(commit_id, oplog)).expect("success"); let log_file_path = worktree_dir.join(".git/logs/refs/heads/gitbutler/target"); @@ -203,7 +204,7 @@ mod set_target_ref { let worktree_dir = dir.path(); let git_dir = worktree_dir.join(".git"); - let oplog = git2::Oid::from_str("0123456789abcdef0123456789abcdef0123456")?; + let oplog = gix::ObjectId::from_str("0123456789abcdef0123456789abcdef01234567")?; set_reference_to_oplog(&git_dir, reflog_commits(commit_id, oplog)).expect("success"); let log_file_path = worktree_dir.join(".git/logs/refs/heads/gitbutler/target"); @@ -222,7 +223,7 @@ mod set_target_ref { let worktree_dir = dir.path(); let git_dir = worktree_dir.join(".git"); - let oplog = git2::Oid::from_str("0123456789abcdef0123456789abcdef0123456")?; + let oplog = gix::ObjectId::from_str("0123456789abcdef0123456789abcdef01234567")?; set_reference_to_oplog(&git_dir, reflog_commits(commit_id, oplog)).expect("success"); let loose_ref_path = worktree_dir.join(".git/refs/heads/gitbutler/target"); @@ -242,7 +243,7 @@ mod set_target_ref { let worktree_dir = dir.path(); let git_dir = worktree_dir.join(".git"); - let oplog = git2::Oid::from_str("0123456789abcdef0123456789abcdef0123456")?; + let oplog = gix::ObjectId::from_str("0123456789abcdef0123456789abcdef01234567")?; set_reference_to_oplog(&git_dir, reflog_commits(commit_id, oplog)).expect("success"); let log_file_path = worktree_dir.join(".git/logs/refs/heads/gitbutler/target"); @@ -271,7 +272,7 @@ mod set_target_ref { // Set ref for the first time let oplog_hex = "0123456789abcdef0123456789abcdef01234567"; - let oplog = git2::Oid::from_str(oplog_hex)?; + let oplog: gix::ObjectId = oplog_hex.parse()?; set_reference_to_oplog(&git_dir, reflog_commits(commit_id, oplog)).expect("success"); assert!(log_file_path.exists()); let contents = std::fs::read_to_string(&log_file_path)?; @@ -305,7 +306,7 @@ mod set_target_ref { // Update the oplog head only let another_oplog_hex = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; - let another_oplog = git2::Oid::from_str(another_oplog_hex)?; + let another_oplog: gix::ObjectId = another_oplog_hex.parse()?; set_reference_to_oplog(&git_dir, reflog_commits(commit_id, another_oplog)) .expect("success"); @@ -335,7 +336,7 @@ mod set_target_ref { // Update the target head only let new_target_hex = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - let new_target = git2::Oid::from_str(new_target_hex)?; + let new_target: gix::ObjectId = new_target_hex.parse()?; set_reference_to_oplog(&git_dir, reflog_commits(new_target, another_oplog)) .expect("success"); @@ -383,33 +384,51 @@ mod set_target_ref { ); } - fn reflog_commits(target: git2::Oid, oplog: git2::Oid) -> ReflogCommits { + fn reflog_commits(target: gix::ObjectId, oplog: gix::ObjectId) -> ReflogCommits { ReflogCommits { - target: target.to_gix(), - oplog: Some(oplog.to_gix()), + target, + oplog: Some(oplog), last_pushed_base: None, } } - fn setup_repo() -> anyhow::Result<(tempfile::TempDir, git2::Oid)> { + fn setup_repo() -> anyhow::Result<(tempfile::TempDir, gix::ObjectId)> { let dir = tempdir()?; - let repo = git2::Repository::init(dir.path())?; let file_path = dir.path().join("foo.txt"); - std::fs::write(file_path, "test")?; - let mut index = repo.index()?; - index.add_path(&PathBuf::from("foo.txt"))?; - let oid = index.write_tree()?; - let name = "Your name"; - let email = "your.email@example.com"; - let signature = git2::Signature::now(name, email)?; - let commit_id = repo.commit( - Some("HEAD"), - &signature, - &signature, - "initial commit", - &repo.find_tree(oid)?, - &[], + git(dir.path(), &["init"])?; + git(dir.path(), &["config", "user.name", "Your name"])?; + git( + dir.path(), + &["config", "user.email", "your.email@example.com"], )?; + git(dir.path(), &["config", "commit.gpgsign", "false"])?; + std::fs::write(file_path, "test")?; + git(dir.path(), &["add", "foo.txt"])?; + git(dir.path(), &["commit", "-m", "initial commit"])?; + let repo = gix::open(dir.path())?; + let commit_id = repo.head_id()?.detach(); Ok((dir, commit_id)) } + + fn git(worktree_dir: &Path, args: &[&str]) -> anyhow::Result<()> { + let status = git_at_dir(worktree_dir) + .args(args) + .status() + .with_context(|| { + format!( + "failed to run git in {} {}", + worktree_dir.display(), + args.join(" ") + ) + })?; + if status.success() { + Ok(()) + } else { + bail!( + "git in {} {} failed with status {status}", + worktree_dir.display(), + args.join(" ") + ); + } + } } diff --git a/crates/gitbutler-project/Cargo.toml b/crates/gitbutler-project/Cargo.toml index d59719e81ca..022c8c5df15 100644 --- a/crates/gitbutler-project/Cargo.toml +++ b/crates/gitbutler-project/Cargo.toml @@ -25,7 +25,7 @@ but-schemars = { workspace = true, optional = true } anyhow.workspace = true serde.workspace = true serde_json = { workspace = true, features = ["arbitrary_precision"] } -git2.workspace = true +# git2.workspace = true gix = { workspace = true, features = [ "dirwalk", "credentials", @@ -38,7 +38,7 @@ resolve-path = "0.1.0" schemars = { workspace = true, optional = true } [dev-dependencies] -gitbutler-testsupport.workspace = true +but-testsupport = { workspace = true, features = ["legacy"] } tempfile.workspace = true insta.workspace = true diff --git a/crates/gitbutler-project/src/lib.rs b/crates/gitbutler-project/src/lib.rs index ef793192fec..ddfccd850fd 100644 --- a/crates/gitbutler-project/src/lib.rs +++ b/crates/gitbutler-project/src/lib.rs @@ -13,17 +13,6 @@ use project::ApiProject; pub use project::{AddProjectOutcome, AuthKey, CodePushState, FetchResult, Project}; pub use storage::UpdateRequest; -/// A utility to be used from applications to optimize `git2` configuration. -/// See comments for details. -pub fn configure_git2() { - // Do not re-hash each decoded objects for quite a significant performance gain. - // This delegates object validation to `git fsck`, which seems fair. - git2::opts::strict_hash_verification(false); - // Thus far, no broken object was created, and if that would be the case, tests should catch it. - // These settings are only changed from `main` of applications. - git2::opts::strict_object_creation(false); -} - /// The maximum size of files to automatically start tracking, i.e. untracked files we pick up for tree-creation. /// **Inactive for now** while it's hard to tell if it's safe *not* to pick up everything. pub const AUTO_TRACK_LIMIT_BYTES: u64 = 0; diff --git a/crates/gitbutler-project/src/project.rs b/crates/gitbutler-project/src/project.rs index d8dceda6485..28696558474 100644 --- a/crates/gitbutler-project/src/project.rs +++ b/crates/gitbutler-project/src/project.rs @@ -192,10 +192,10 @@ impl Project { } /// Testing -// TODO: remove once `gitbutler-testsupport` isn't needed anymore, and `gitbutler-repo` +// TODO: remove once the remaining legacy testsupport constructor isn't needed anymore, and `gitbutler-repo` impl Project { /// A special constructor needed as `worktree_dir` isn't accessible anymore. - pub fn new_for_gitbutler_testsupport(title: String, worktree_dir: PathBuf) -> Self { + pub fn new_for_but_testsupport(title: String, worktree_dir: PathBuf) -> Self { let project_id = ProjectHandleOrLegacyProjectId::ProjectHandle( ProjectHandle::from_path(&worktree_dir) .expect("testsupport projects require a valid path for ProjectHandle"), diff --git a/crates/gitbutler-project/tests/add_cwd_as_project.rs b/crates/gitbutler-project/tests/add_cwd_as_project.rs index c6b31107c85..e9b4c3ec50c 100644 --- a/crates/gitbutler-project/tests/add_cwd_as_project.rs +++ b/crates/gitbutler-project/tests/add_cwd_as_project.rs @@ -1,9 +1,9 @@ -use gitbutler_testsupport::paths; +use but_testsupport::legacy::paths; #[test] fn current_directory_dot() -> anyhow::Result<()> { let tmp = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::TestProject::default(); let repo_path = repo.path(); // Change to the repository directory and add "." as the path diff --git a/crates/gitbutler-project/tests/project/main.rs b/crates/gitbutler-project/tests/project/main.rs index f41dc5bd5c6..70e78b9a78c 100644 --- a/crates/gitbutler-project/tests/project/main.rs +++ b/crates/gitbutler-project/tests/project/main.rs @@ -1,26 +1,59 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use anyhow::{Context as _, Result, bail}; use but_core::RepositoryExt; -use gitbutler_testsupport::{self, paths}; +use but_testsupport::{git_at_dir, legacy::paths}; use tempfile::TempDir; pub fn new() -> TempDir { paths::data_dir() } -fn storage_key() -> String { - but_project_handle::storage_path_config_key().to_owned() -} - fn repo_path_at(name: &str) -> PathBuf { - gitbutler_testsupport::gix_testtools::scripted_fixture_read_only("various-repositories.sh") + but_testsupport::gix_testtools::scripted_fixture_read_only("various-repositories.sh") .unwrap() .join(name) } fn writable_fixture() -> TempDir { - gitbutler_testsupport::gix_testtools::scripted_fixture_writable("various-repositories.sh") - .unwrap() + but_testsupport::gix_testtools::scripted_fixture_writable("various-repositories.sh").unwrap() +} + +fn repo_git_dir(path: &Path) -> Result { + let repo = gix::open_opts(path, gix::open::Options::isolated())?; + Ok(repo.git_dir().canonicalize()?) +} + +fn git(args: &[&str]) -> Result<()> { + let cwd = std::env::current_dir()?; + let status = git_at_dir(cwd) + .args(args) + .status() + .with_context(|| format!("failed to run git {}", args.join(" ")))?; + if status.success() { + Ok(()) + } else { + bail!("git {} failed with status {status}", args.join(" ")); + } +} + +fn set_storage_path_config( + repo_path: &Path, + value: impl AsRef, +) -> anyhow::Result { + let key = but_project_handle::storage_path_config_key(); + let status = git_at_dir(repo_path) + .args(["config", "--local", key]) + .arg(value) + .status() + .with_context(|| format!("failed to configure {key} in {}", repo_path.display()))?; + if !status.success() { + bail!( + "git config --local {key} failed in {} with status {status}", + repo_path.display() + ); + } + but_testsupport::open_repo(repo_path) } mod add { @@ -29,7 +62,7 @@ mod add { #[test] fn success() -> anyhow::Result<()> { let tmp = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(tmp.path(), path) .unwrap() @@ -44,15 +77,14 @@ mod add { #[test] fn creates_configured_storage_dir() -> anyhow::Result<()> { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); - let key = storage_key(); - git2::Repository::open(repo.path())? - .config()? - .set_str(&key, "gitbutler-custom")?; + let repo = but_testsupport::legacy::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 project = gitbutler_project::add_at_app_data_dir(data_dir.path(), repo.path())?.unwrap_project(); let gb_dir = project.open_isolated_repo()?.gitbutler_storage_path()?; + assert_eq!(gb_dir, expected_gb_dir); assert!(gb_dir.exists()); Ok(()) } @@ -60,15 +92,14 @@ mod add { #[test] fn get_recreates_configured_storage_dir() -> anyhow::Result<()> { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); - let key = storage_key(); - git2::Repository::open(repo.path())? - .config()? - .set_str(&key, "gitbutler-custom")?; + let repo = but_testsupport::legacy::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 project = gitbutler_project::add_at_app_data_dir(data_dir.path(), repo.path())?.unwrap_project(); let gb_dir = project.open_isolated_repo()?.gitbutler_storage_path()?; + assert_eq!(gb_dir, expected_gb_dir); std::fs::remove_dir_all(&gb_dir)?; assert!(!gb_dir.exists(), "sanity check"); @@ -133,10 +164,10 @@ mod add { #[test] fn best_effort_adds_parent_repo_from_nested_directory() -> anyhow::Result<()> { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::TestProject::default(); let nested_dir = repo.path().join("nested/inside"); let expected_worktree_dir = repo.path().canonicalize()?; - let expected_git_dir = git2::Repository::open(repo.path())?.path().canonicalize()?; + let expected_git_dir = repo_git_dir(repo.path())?; std::fs::create_dir_all(&nested_dir)?; let outcome = @@ -158,7 +189,7 @@ mod add { #[test] fn best_effort_finds_existing_project_from_file_path() -> anyhow::Result<()> { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::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"); @@ -226,7 +257,7 @@ 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 = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::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()) @@ -255,7 +286,7 @@ mod add { #[test] fn twice() { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::TestProject::default(); let path = repo.path(); gitbutler_project::add_at_app_data_dir(data_dir.path(), path).unwrap(); @@ -269,8 +300,7 @@ mod add { let tmp = tempfile::tempdir().unwrap(); let repo_dir = tmp.path().join("bare"); - let repo = git2::Repository::init_bare(&repo_dir).unwrap(); - create_initial_commit(&repo); + git(&["init", "--bare", repo_dir.to_str().unwrap()]).unwrap(); let outcome = gitbutler_project::add_at_app_data_dir(data_dir.path(), repo_dir.as_path()) @@ -285,31 +315,17 @@ mod add { let main_worktree_dir = tmp.path().join("main"); let worktree_dir = tmp.path().join("worktree"); - let repo = git2::Repository::init(main_worktree_dir).unwrap(); - create_initial_commit(&repo); - - let worktree = repo.worktree("feature", &worktree_dir, None).unwrap(); + let cwd = main_worktree_dir.to_str().unwrap(); + git(&["init", cwd]).unwrap(); + git(&["-C", cwd, "config", "user.name", "test"]).unwrap(); + git(&["-C", cwd, "config", "user.email", "test@email.com"]).unwrap(); + git(&["-C", cwd, "commit", "--allow-empty", "-m", "initial commit"]).unwrap(); + let worktree_dir = worktree_dir.to_str().unwrap(); + git(&["-C", cwd, "worktree", "add", "-b", "feature", worktree_dir]).unwrap(); let outcome = - gitbutler_project::add_at_app_data_dir(data_dir.path(), worktree.path()).unwrap(); + gitbutler_project::add_at_app_data_dir(data_dir.path(), worktree_dir).unwrap(); assert!(matches!(outcome, AddProjectOutcome::NonMainWorktree)); } - - fn create_initial_commit(repo: &git2::Repository) -> git2::Oid { - let signature = git2::Signature::now("test", "test@email.com").unwrap(); - - let mut index = repo.index().unwrap(); - let oid = index.write_tree().unwrap(); - - repo.commit( - Some("HEAD"), - &signature, - &signature, - "initial commit", - &repo.find_tree(oid).unwrap(), - &[], - ) - .unwrap() - } } } @@ -318,7 +334,7 @@ mod delete { #[test] fn success() { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path) .unwrap() @@ -363,7 +379,7 @@ mod delete { #[test] fn deletes_gitbutler_references() -> anyhow::Result<()> { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path)?.unwrap_project(); @@ -422,7 +438,7 @@ mod delete { 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 = gitbutler_testsupport::TestProject::default(); + let repo = but_testsupport::legacy::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path)?.unwrap_project(); @@ -464,11 +480,7 @@ mod delete { #[test] fn removes_configured_storage_dir() -> anyhow::Result<()> { let data_dir = paths::data_dir(); - let repo = gitbutler_testsupport::TestProject::default(); - let key = storage_key(); - git2::Repository::open(repo.path())? - .config()? - .set_str(&key, "gitbutler-custom")?; + let repo = but_testsupport::legacy::TestProject::default(); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path)?.unwrap_project(); @@ -483,12 +495,13 @@ 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 = gitbutler_testsupport::TestProject::default(); - let key = storage_key(); - let git_dir = git2::Repository::open(repo.path())?.path().to_path_buf(); - git2::Repository::open(repo.path())? - .config()? - .set_str(&key, ".")?; + let repo = but_testsupport::legacy::TestProject::default(); + let git_dir = repo_git_dir(repo.path())?; + let repo_after_config = set_storage_path_config(repo.path(), ".")?; + assert!( + repo_after_config.gitbutler_storage_path().is_err(), + "sanity check: '.' must be rejected as storage path" + ); let path = repo.path(); let project = gitbutler_project::add_at_app_data_dir(data_dir.path(), path)?.unwrap_project(); diff --git a/crates/gitbutler-repo/Cargo.toml b/crates/gitbutler-repo/Cargo.toml index 79a1cf2f79c..268435ba1a9 100644 --- a/crates/gitbutler-repo/Cargo.toml +++ b/crates/gitbutler-repo/Cargo.toml @@ -45,7 +45,7 @@ name = "repo" path = "tests/mod.rs" [dev-dependencies] -gitbutler-testsupport.workspace = true +but-testsupport = { workspace = true, features = ["legacy"] } gitbutler-user.workspace = true but-settings.workspace = true serde_json = { workspace = true, features = ["arbitrary_precision"] } diff --git a/crates/gitbutler-repo/src/hooks.rs b/crates/gitbutler-repo/src/hooks.rs index 457d9057876..9860aeb646d 100644 --- a/crates/gitbutler-repo/src/hooks.rs +++ b/crates/gitbutler-repo/src/hooks.rs @@ -11,7 +11,7 @@ use but_oxidize::ObjectIdExt; use git2_hooks::{self, HookResult as H}; use serde::Serialize; -use crate::{managed_hooks::get_hooks_dir_gix, staging}; +use crate::{managed_hooks::get_hooks_dir, staging}; #[derive(Serialize, PartialEq, Debug, Clone)] pub struct MessageData { @@ -145,7 +145,7 @@ pub fn pre_push( remote_tracking_branch: &gitbutler_reference::RemoteRefname, run_husky_hooks: bool, ) -> Result { - let hooks_dir = get_hooks_dir_gix(repo); + let hooks_dir = get_hooks_dir(repo); let hooks_path = hooks_dir.join("pre-push"); let husky_path = run_husky_hooks.then(|| { repo.workdir() diff --git a/crates/gitbutler-repo/src/managed_hooks.rs b/crates/gitbutler-repo/src/managed_hooks.rs index 854b15736fc..0ab5080f538 100644 --- a/crates/gitbutler-repo/src/managed_hooks.rs +++ b/crates/gitbutler-repo/src/managed_hooks.rs @@ -191,19 +191,8 @@ fn hooks_dir_from_git_dir_and_config_path_for_run_dir( hooks_path.unwrap_or_else(|| git_dir.join("hooks")) } -/// Get the hooks directory, respecting core.hooksPath configuration -fn get_hooks_dir(repo: &git2::Repository) -> PathBuf { - hooks_dir_from_git_dir_and_config_path_for_run_dir( - repo.path(), - repo.workdir().unwrap_or(repo.path()), - repo.config() - .and_then(|config| config.get_path("core.hooksPath")) - .ok(), - ) -} - /// Get the hooks directory for a `gix` repository, respecting `core.hooksPath`. -pub(crate) fn get_hooks_dir_gix(repo: &gix::Repository) -> PathBuf { +pub(crate) fn get_hooks_dir(repo: &gix::Repository) -> PathBuf { hooks_dir_from_git_dir_and_config_path_for_run_dir( repo.git_dir(), repo.workdir().unwrap_or(repo.git_dir()), @@ -293,17 +282,11 @@ fn uninstall_hook(hooks_dir: &Path, hook_type: ManagedHookType) -> Result Result { +pub fn install_managed_hooks(repo: &gix::Repository) -> Result { let hooks_dir = get_hooks_dir(repo); install_managed_hooks_at(&hooks_dir) } -/// Install all GitButler managed hooks for a `gix` repository. -pub fn install_managed_hooks_gix(repo: &gix::Repository) -> Result { - let hooks_dir = get_hooks_dir_gix(repo); - install_managed_hooks_at(&hooks_dir) -} - fn install_managed_hooks_at(hooks_dir: &Path) -> Result { let mut warnings = Vec::new(); let mut already_configured_count = 0; @@ -343,17 +326,11 @@ fn install_managed_hooks_at(hooks_dir: &Path) -> Result /// Uninstall all GitButler managed hooks and restore user's originals /// /// Called during teardown -pub fn uninstall_managed_hooks(repo: &git2::Repository) -> Result { +pub fn uninstall_managed_hooks(repo: &gix::Repository) -> Result { let hooks_dir = get_hooks_dir(repo); uninstall_managed_hooks_at(&hooks_dir) } -/// Uninstall all GitButler managed hooks for a `gix` repository. -pub fn uninstall_managed_hooks_gix(repo: &gix::Repository) -> Result { - let hooks_dir = get_hooks_dir_gix(repo); - uninstall_managed_hooks_at(&hooks_dir) -} - fn uninstall_managed_hooks_at(hooks_dir: &Path) -> Result { let mut warnings = Vec::new(); diff --git a/crates/gitbutler-repo/tests/create_wd_tree.rs b/crates/gitbutler-repo/tests/create_wd_tree.rs index bebf8b97574..206100f5d7d 100644 --- a/crates/gitbutler-repo/tests/create_wd_tree.rs +++ b/crates/gitbutler-repo/tests/create_wd_tree.rs @@ -4,15 +4,19 @@ use std::{ }; use but_core::RepositoryExt as _; -use but_oxidize::ObjectIdExt as _; -use gitbutler_repo::RepositoryExt as _; -use gitbutler_testsupport::{ - gix_testtools::scripted_fixture_read_only, testing_repository::TestingRepository, - visualize_git2_tree, +use but_testsupport::{ + gix_testtools::scripted_fixture_read_only, legacy::testing_repository::TestingRepository, + open_repo, visualize_tree, }; +use gitbutler_repo::RepositoryExt as _; +use gix::prelude::ObjectIdExt as _; const MAX_SIZE: u64 = 20; +fn tree_entry_count(tree: gix::Id<'_>) -> anyhow::Result { + Ok(tree.object()?.peel_to_tree()?.iter().count()) +} + /// These tests exercise the truth table that we use to update the HEAD /// tree to match the worktree. /// @@ -29,8 +33,6 @@ const MAX_SIZE: u64 = 20; /// | modify | modify | upsert | #[cfg(test)] mod head_upsert_truthtable { - use gitbutler_testsupport::visualize_git2_tree; - use super::*; // | add | delete | no-action | @@ -46,9 +48,10 @@ mod head_upsert_truthtable { std::fs::remove_file(test.tempdir.path().join("file1.txt"))?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - assert_eq!(tree.len(), 0, "Tree should end up empty"); + assert_eq!(tree_entry_count(tree)?, 0, "Tree should end up empty"); Ok(()) } @@ -65,9 +68,10 @@ mod head_upsert_truthtable { std::fs::remove_file(test.tempdir.path().join("file1.txt"))?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - assert_eq!(tree.len(), 0, "Tree should end up empty"); + assert_eq!(tree_entry_count(tree)?, 0, "Tree should end up empty"); Ok(()) } @@ -78,9 +82,10 @@ mod head_upsert_truthtable { std::fs::remove_file(test.tempdir.path().join("file1.txt"))?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - assert_eq!(tree.len(), 0, "Tree should end up empty"); + assert_eq!(tree_entry_count(tree)?, 0, "Tree should end up empty"); Ok(()) } @@ -93,10 +98,11 @@ mod head_upsert_truthtable { index.remove_all(["*"], None)?; index.write()?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // We should ignore whatever happens to the index - the current worktree state matters. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 7cd1c45 └── file1.txt:100644:dd954e7 "content1" "#); @@ -114,10 +120,11 @@ mod head_upsert_truthtable { std::fs::write(test.tempdir.path().join("file1.txt"), "content2")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // Tree should match whatever is written on disk - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" f87e9ef └── file1.txt:100644:db00fd6 "content2" "#); @@ -135,9 +142,10 @@ mod head_upsert_truthtable { index.add_path(Path::new("file1.txt"))?; index.write()?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" f87e9ef └── file1.txt:100644:db00fd6 "content2" "#); @@ -151,9 +159,10 @@ mod head_upsert_truthtable { std::fs::write(test.tempdir.path().join("file1.txt"), "content2")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" f87e9ef └── file1.txt:100644:db00fd6 "content2" "#); @@ -173,9 +182,10 @@ mod head_upsert_truthtable { std::fs::write(test.tempdir.path().join("file1.txt"), "content2")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" f87e9ef └── file1.txt:100644:db00fd6 "content2" "#); @@ -198,9 +208,10 @@ mod head_upsert_truthtable { // this change won't be seen. std::fs::write(file_path, "content3")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" d377861 └── file1.txt:100644:a2b3229 "content3" "#); @@ -219,9 +230,10 @@ mod head_upsert_truthtable { index.add_path(Path::new("file1.txt"))?; index.write()?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" f87e9ef └── file1.txt:100644:db00fd6 "content2" "#); @@ -236,9 +248,10 @@ fn lists_uncommited_changes() -> anyhow::Result<()> { std::fs::write(test.tempdir.path().join("file1.txt"), "content1")?; std::fs::write(test.tempdir.path().join("file2.txt"), "content2")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 1ae8c21 β”œβ”€β”€ file1.txt:100644:dd954e7 "content1" └── file2.txt:100644:db00fd6 "content2" @@ -254,14 +267,15 @@ fn does_not_include_staged_but_deleted_files() -> anyhow::Result<()> { std::fs::write(test.tempdir.path().join("file2.txt"), "content2")?; std::fs::write(test.tempdir.path().join("file3.txt"), "content2")?; - let mut index: git2::Index = test.repository.index()?; + let mut index = test.repository.index()?; index.add_path(Path::new("file3.txt"))?; index.write()?; std::fs::remove_file(test.tempdir.path().join("file3.txt"))?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 1ae8c21 β”œβ”€β”€ file1.txt:100644:dd954e7 "content1" └── file2.txt:100644:db00fd6 "content2" @@ -290,11 +304,12 @@ fn should_be_empty_after_checking_out_empty_tree() -> anyhow::Result<()> { assert!(!test.tempdir.path().join("file1.txt").exists()); assert!(!test.tempdir.path().join("file2.txt").exists()); - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // `create_wd_tree` uses the head commit as the base, and then performs // modifications to the tree to match the working tree. - assert_eq!(tree.len(), 0); + assert_eq!(tree_entry_count(tree)?, 0); Ok(()) } @@ -306,7 +321,7 @@ fn should_track_deleted_files() -> anyhow::Result<()> { ]); // Make sure the index is empty, perhaps the user did this action - let mut index: git2::Index = test.repository.index()?; + let mut index = test.repository.index()?; index.remove_all(["*"], None)?; index.write()?; @@ -315,9 +330,10 @@ fn should_track_deleted_files() -> anyhow::Result<()> { assert!(!test.tempdir.path().join("file1.txt").exists()); assert!(test.tempdir.path().join("file2.txt").exists()); - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 295a2e4 └── file2.txt:100644:db00fd6 "content2" "#); @@ -336,7 +352,8 @@ fn should_not_change_index() -> anyhow::Result<()> { let index_tree = test.repository.find_tree(index_tree)?; assert_eq!(index_tree.len(), 0); - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; let mut index = test.repository.index()?; let index_tree = index.write_tree()?; @@ -344,7 +361,7 @@ fn should_not_change_index() -> anyhow::Result<()> { assert_eq!(index_tree.len(), 0); // Tree should match whatever is written on disk - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 7cd1c45 └── file1.txt:100644:dd954e7 "content1" "#); @@ -364,9 +381,10 @@ fn tree_behavior() -> anyhow::Result<()> { std::fs::create_dir(test.tempdir.path().join("dir3"))?; std::fs::write(test.tempdir.path().join("dir3/file1.txt"), "new2")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" c8aa4f7 β”œβ”€β”€ dir1:dce0d03 β”‚ └── file1.txt:100644:e4a8953 "new1" @@ -389,10 +407,11 @@ fn executable_blobs() -> anyhow::Result<()> { file.set_permissions(Permissions::from_mode(0o755))?; file.write_all(b"content1")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // The executable bit is also present in the tree. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 4cb9de9 └── file1.txt:100755:dd954e7 "content1" "#); @@ -406,10 +425,11 @@ fn links() -> anyhow::Result<()> { std::os::unix::fs::symlink("target", test.tempdir.path().join("link1.txt"))?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // Links are also present in the tree. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 0aefe10 β”œβ”€β”€ link1.txt:120000:1de5659 "target" └── target:100644:620ffd0 "helloworld" @@ -428,8 +448,9 @@ fn tracked_file_becomes_directory_in_worktree() -> anyhow::Result<()> { std::fs::create_dir(&worktree_path)?; std::fs::write(worktree_path.join("file"), "content in directory")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; + insta::assert_snapshot!(visualize_tree(tree), @r#" 8b80519 └── soon-directory:df6d699 └── file:100644:dadf628 "content in directory" @@ -447,8 +468,9 @@ fn tracked_directory_becomes_file_in_worktree() -> anyhow::Result<()> { std::fs::remove_dir_all(&worktree_path)?; std::fs::write(worktree_path, "content")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; + insta::assert_snapshot!(visualize_tree(tree), @r#" 637be29 └── soon-file:100644:6b584e8 "content" "#); @@ -468,9 +490,10 @@ fn non_files_are_ignored() -> anyhow::Result<()> { .success() ); - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; assert_eq!( - tree.len(), + tree_entry_count(tree)?, 0, "It completely ignores non-files, it doesn't see them, just like Git" ); @@ -491,9 +514,10 @@ fn tracked_file_swapped_with_non_file() -> anyhow::Result<()> { .success() ); - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; assert_eq!( - tree.len(), + tree_entry_count(tree)?, 0, "It completely ignores non-files, it doesn't see them, just like Git, even when previously tracked" ); @@ -510,9 +534,10 @@ fn ignored_files() -> anyhow::Result<()> { let ignored_path = test.tempdir.path().join("I-am.ignored"); std::fs::write(&ignored_path, "")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // ignored files aren't picked up. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 38b94c0 β”œβ”€β”€ .gitignore:100644:669be81 "*.ignored" └── tracked:100644:6b584e8 "content" @@ -527,9 +552,10 @@ fn can_autotrack_empty_files() -> anyhow::Result<()> { let ignored_path = test.tempdir.path().join("soon-empty"); std::fs::write(&ignored_path, "")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // ignored files aren't picked up. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 4fe2781 └── soon-empty:100644:e69de29 "" "#); @@ -542,7 +568,7 @@ fn intent_to_add_is_picked_up_just_like_untracked() -> anyhow::Result<()> { let tree = create_wd_tree(&repo, MAX_SIZE)?; // We pick up what's in the worktree, independently of the intent-to-add flag. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &repo), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" d6a22f9 └── to-be-added:100644:6b584e8 "content" "#); @@ -556,7 +582,7 @@ fn submodule_in_index_is_picked_up() -> anyhow::Result<()> { let tree = create_wd_tree(&repo, MAX_SIZE)?; // Everything that is not contending with the worktree that is already in the index // is picked up, even if it involves submodules. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &repo), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" de956ee β”œβ”€β”€ .gitmodules:100644:db28142 "[submodule \"sm\"]\n\tpath = sm\n\turl = ../module\n" └── sm:160000:2e70126 @@ -571,7 +597,7 @@ fn submodule_change() -> anyhow::Result<()> { let tree = create_wd_tree(&repo, MAX_SIZE)?; // Changes to submodule heads are also picked up. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &repo), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 8b0adff β”œβ”€β”€ .gitmodules:100644:db28142 "[submodule \"sm\"]\n\tpath = sm\n\turl = ../module\n" └── sm:160000:e8a2d3a @@ -586,10 +612,11 @@ fn big_files_check_is_disabled_with_zero() -> anyhow::Result<()> { std::fs::write(test.tempdir.path().join("empty"), "")?; std::fs::write(test.tempdir.path().join("with-content"), "content")?; - let tree = create_wd_tree(&test.repository, 0)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, 0)?; // Everything goes with 0 - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" f6e159b β”œβ”€β”€ empty:100644:e69de29 "" └── with-content:100644:6b584e8 "content" @@ -605,10 +632,11 @@ fn big_files_are_ignored_based_on_threshold_in_working_tree() -> anyhow::Result< let big_file_path = test.tempdir.path().join("soon-too-big"); std::fs::write(&big_file_path, "a massive file above the threshold")?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // It does not pickup the big worktree change. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" 26ea3c5 └── soon-too-big:100644:7d72316 "still small enough" "#); @@ -628,29 +656,32 @@ fn big_files_are_fine_when_in_the_index() -> anyhow::Result<()> { index.add_path("soon-too-big".as_ref())?; index.write()?; - let tree = create_wd_tree(&test.repository, MAX_SIZE)?; + let repo = gix_repo(&test)?; + let tree = create_wd_tree(&repo, MAX_SIZE)?; // It keeps files that were already added. - insta::assert_snapshot!(visualize_git2_tree(tree.id(), &test.repository), @r#" + insta::assert_snapshot!(visualize_tree(tree), @r#" bbd82c6 └── soon-too-big:100644:1799e5a "a massive file above the threshold" "#); Ok(()) } -fn repo(name: &str) -> anyhow::Result { +fn gix_repo(test: &TestingRepository) -> anyhow::Result { + open_repo(test.repository.path()) +} + +fn repo(name: &str) -> anyhow::Result { let worktree_dir = scripted_fixture_read_only("make_create_wd_tree_repos.sh") .map_err(anyhow::Error::from_boxed)? .join(name); - Ok(git2::Repository::open(worktree_dir)?) + open_repo(&worktree_dir) } fn create_wd_tree<'repo>( - repo: &'repo git2::Repository, + repo: &'repo gix::Repository, untracked_limit_in_bytes: u64, -) -> anyhow::Result> { +) -> anyhow::Result> { #[expect(deprecated)] - let tree_id = gix::open_opts(repo.path(), gix::open::Options::isolated())? - .create_wd_tree(untracked_limit_in_bytes)?; - Ok(repo.find_tree(tree_id.to_git2())?) + Ok(repo.create_wd_tree(untracked_limit_in_bytes)?.attach(repo)) } diff --git a/crates/gitbutler-repo/tests/credentials.rs b/crates/gitbutler-repo/tests/credentials.rs index 085fb96882a..9791981f37a 100644 --- a/crates/gitbutler-repo/tests/credentials.rs +++ b/crates/gitbutler-repo/tests/credentials.rs @@ -2,9 +2,9 @@ use std::{path::PathBuf, str}; 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_testsupport::test_repository; use gitbutler_user as users; #[derive(Default)] @@ -16,7 +16,7 @@ struct TestCase<'a> { impl TestCase<'_> { fn run(&self) -> Vec<(String, Vec)> { - gitbutler_testsupport::secrets::setup_blackhole_store(); + but_testsupport::legacy::secrets::setup_blackhole_store(); let user: users::User = serde_json::from_str(if self.with_github_login { include_str!("../tests/fixtures/users/with-github.v1") } else { diff --git a/crates/gitbutler-repo/tests/hooks.rs b/crates/gitbutler-repo/tests/hooks.rs index 4c73284b71c..17e1c74e217 100644 --- a/crates/gitbutler-repo/tests/hooks.rs +++ b/crates/gitbutler-repo/tests/hooks.rs @@ -2,18 +2,13 @@ use std::fs; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; -use but_oxidize::OidExt as _; +use but_testsupport::{legacy::TestProject, open_repo}; use gitbutler_repo::hooks::{HookResult, pre_push}; -use gitbutler_testsupport::TestProject; - -fn gix_repo(repo: &git2::Repository) -> anyhow::Result { - Ok(gix::open_opts(repo.path(), gix::open::Options::isolated())?) -} #[test] fn pre_push_hook_not_configured() -> anyhow::Result<()> { let test_project = TestProject::default(); - let repo = gix_repo(&test_project.local_repo)?; + let repo = open_repo(test_project.local_repo.path())?; let result = pre_push( &repo, @@ -32,8 +27,7 @@ fn pre_push_hook_not_configured() -> anyhow::Result<()> { fn pre_push_hook_success() -> anyhow::Result<()> { let test_project = TestProject::default(); - let repo = &test_project.local_repo; - let gix_repo = gix_repo(repo)?; + let repo = open_repo(test_project.local_repo.path())?; let hooks_dir = repo.path().join("hooks"); fs::create_dir_all(&hooks_dir)?; let hook_path = hooks_dir.join("pre-push"); @@ -44,10 +38,10 @@ fn pre_push_hook_success() -> anyhow::Result<()> { fs::set_permissions(&hook_path, fs::Permissions::from_mode(0o755))?; let result = pre_push( - &gix_repo, + &repo, "origin", "https://github.com/test/repo.git", - repo.head()?.target().expect("not detached").to_gix(), + repo.head_id()?.detach(), &gitbutler_reference::RemoteRefname::new("origin", "master"), true, )?; @@ -69,8 +63,7 @@ fn pre_push_hook_success() -> anyhow::Result<()> { fn pre_push_hook_failure() -> anyhow::Result<()> { let test_project = TestProject::default(); - let repo = &test_project.local_repo; - let gix_repo = gix_repo(repo)?; + let repo = open_repo(test_project.local_repo.path())?; let hooks_dir = repo.path().join("hooks"); fs::create_dir_all(&hooks_dir)?; let hook_path = hooks_dir.join("pre-push"); @@ -84,10 +77,10 @@ fn pre_push_hook_failure() -> anyhow::Result<()> { fs::set_permissions(&hook_path, fs::Permissions::from_mode(0o755))?; let result = pre_push( - &gix_repo, + &repo, "origin", "https://github.com/test/repo.git", - repo.head()?.target().expect("not detached").to_gix(), + repo.head_id()?.detach(), &gitbutler_reference::RemoteRefname::new("origin", "master"), true, ); @@ -107,8 +100,8 @@ fn pre_push_hook_failure() -> anyhow::Result<()> { fn pre_push_ignores_husky_core_hooks_path_when_disabled() -> anyhow::Result<()> { let test_project = TestProject::default(); - let repo = &test_project.local_repo; - let workdir = repo.workdir().expect("non-bare"); + let mut repo = open_repo(test_project.local_repo.path())?; + let workdir = repo.workdir().expect("non-bare").to_path_buf(); let hooks_dir = workdir.join(".husky").join("_"); fs::create_dir_all(&hooks_dir)?; let hook_path = hooks_dir.join("pre-push"); @@ -118,15 +111,14 @@ fn pre_push_ignores_husky_core_hooks_path_when_disabled() -> anyhow::Result<()> #[cfg(unix)] fs::set_permissions(&hook_path, fs::Permissions::from_mode(0o755))?; - repo.config()? - .set_str("core.hooksPath", hooks_dir.to_string_lossy().as_ref())?; - let gix_repo = gix_repo(repo)?; + repo.config_snapshot_mut() + .set_raw_value(&"core.hooksPath", gix::path::into_bstr(&hooks_dir).as_ref())?; let result = pre_push( - &gix_repo, + &repo, "origin", "https://github.com/test/repo.git", - repo.head()?.target().expect("not detached").to_gix(), + repo.head_id()?.detach(), &gitbutler_reference::RemoteRefname::new("origin", "master"), false, )?; @@ -134,10 +126,10 @@ fn pre_push_ignores_husky_core_hooks_path_when_disabled() -> anyhow::Result<()> assert!(!workdir.join("husky-pre-push-ran").exists()); let result = pre_push( - &gix_repo, + &repo, "origin", "https://github.com/test/repo.git", - repo.head()?.target().expect("not detached").to_gix(), + repo.head_id()?.detach(), &gitbutler_reference::RemoteRefname::new("origin", "master"), true, )?; @@ -150,8 +142,8 @@ fn pre_push_ignores_husky_core_hooks_path_when_disabled() -> anyhow::Result<()> fn pre_push_resolves_relative_core_hooks_path_against_workdir() -> anyhow::Result<()> { let test_project = TestProject::default(); - let repo = &test_project.local_repo; - let workdir = repo.workdir().expect("non-bare"); + let mut repo = open_repo(test_project.local_repo.path())?; + let workdir = repo.workdir().expect("non-bare").to_path_buf(); let relative_hooks = format!( "relative-hooks-{}", workdir @@ -168,14 +160,14 @@ fn pre_push_resolves_relative_core_hooks_path_against_workdir() -> anyhow::Resul #[cfg(unix)] fs::set_permissions(&hook_path, fs::Permissions::from_mode(0o755))?; - repo.config()?.set_str("core.hooksPath", &relative_hooks)?; - let gix_repo = gix_repo(repo)?; + repo.config_snapshot_mut() + .set_raw_value(&"core.hooksPath", relative_hooks.as_str())?; let result = pre_push( - &gix_repo, + &repo, "origin", "https://github.com/test/repo.git", - repo.head()?.target().expect("not detached").to_gix(), + repo.head_id()?.detach(), &gitbutler_reference::RemoteRefname::new("origin", "master"), true, )?; diff --git a/crates/gitbutler-repo/tests/managed_hooks_tests.rs b/crates/gitbutler-repo/tests/managed_hooks_tests.rs index 8d501cd3cdb..28537f7cbe3 100644 --- a/crates/gitbutler-repo/tests/managed_hooks_tests.rs +++ b/crates/gitbutler-repo/tests/managed_hooks_tests.rs @@ -11,25 +11,24 @@ use std::os::unix::fs::PermissionsExt; use anyhow::Result; use gitbutler_repo::managed_hooks::{ - HookInstallationResult, install_managed_hooks, install_managed_hooks_gix, - uninstall_managed_hooks, uninstall_managed_hooks_gix, + HookInstallationResult, install_managed_hooks, uninstall_managed_hooks, }; use tempfile::TempDir; /// Helper to create a test git repository -fn create_test_repo() -> Result<(TempDir, git2::Repository)> { +fn create_test_repo() -> Result<(TempDir, gix::Repository)> { let temp_dir = TempDir::new()?; - let repo = git2::Repository::init(temp_dir.path())?; + let repo = gix::init(temp_dir.path())?; Ok((temp_dir, repo)) } -fn open_gix_repo(repo: &git2::Repository) -> Result { - Ok(gix::open_opts(repo.path(), gix::open::Options::isolated())?) +fn hooks_dir(repo: &gix::Repository) -> std::path::PathBuf { + repo.git_dir().join("hooks") } /// Helper to create a user hook file with content -fn create_user_hook(repo: &git2::Repository, hook_name: &str, content: &str) -> Result<()> { - let hooks_dir = repo.path().join("hooks"); +fn create_user_hook(repo: &gix::Repository, hook_name: &str, content: &str) -> Result<()> { + let hooks_dir = hooks_dir(repo); fs::create_dir_all(&hooks_dir)?; let hook_path = hooks_dir.join(hook_name); fs::write(&hook_path, content)?; @@ -41,13 +40,13 @@ fn create_user_hook(repo: &git2::Repository, hook_name: &str, content: &str) -> } /// Helper to check if a file exists -fn hook_exists(repo: &git2::Repository, hook_name: &str) -> bool { - repo.path().join("hooks").join(hook_name).exists() +fn hook_exists(repo: &gix::Repository, hook_name: &str) -> bool { + hooks_dir(repo).join(hook_name).exists() } /// Helper to read hook content -fn read_hook(repo: &git2::Repository, hook_name: &str) -> Result { - let path = repo.path().join("hooks").join(hook_name); +fn read_hook(repo: &gix::Repository, hook_name: &str) -> Result { + let path = hooks_dir(repo).join(hook_name); Ok(fs::read_to_string(path)?) } @@ -57,8 +56,8 @@ fn hook_exists_at(hooks_dir: &std::path::Path, hook_name: &str) -> bool { /// Helper to check if hook is executable on Unix #[cfg(unix)] -fn is_executable(repo: &git2::Repository, hook_name: &str) -> bool { - let path = repo.path().join("hooks").join(hook_name); +fn is_executable(repo: &gix::Repository, hook_name: &str) -> bool { + let path = hooks_dir(repo).join(hook_name); if let Ok(metadata) = fs::metadata(&path) { let permissions = metadata.permissions(); permissions.mode() & 0o111 != 0 @@ -72,7 +71,7 @@ fn test_install_hooks_creates_hooks_directory() -> Result<()> { let (_temp, repo) = create_test_repo()?; // Remove hooks directory if it exists - let hooks_dir = repo.path().join("hooks"); + let hooks_dir = hooks_dir(&repo); if hooks_dir.exists() { fs::remove_dir_all(&hooks_dir)?; } @@ -392,7 +391,7 @@ fn test_hook_manually_modified_after_install() -> Result<()> { // User manually modifies the hook let modified_hook = "#!/bin/sh\n# User modified this\necho 'modified'\n"; - let hook_path = repo.path().join("hooks").join("pre-commit"); + let hook_path = hooks_dir(&repo).join("pre-commit"); fs::write(&hook_path, modified_hook)?; // Uninstall should not remove the modified hook (no signature) @@ -407,15 +406,17 @@ fn test_hook_manually_modified_after_install() -> Result<()> { #[test] fn test_respects_absolute_core_hooks_path() -> Result<()> { - let (_temp, repo) = create_test_repo()?; + let (_temp, mut repo) = create_test_repo()?; // Create a custom hooks directory let custom_hooks = _temp.path().join("custom-hooks"); fs::create_dir_all(&custom_hooks)?; // Configure core.hooksPath - let mut config = repo.config()?; - config.set_str("core.hooksPath", custom_hooks.to_str().unwrap())?; + repo.config_snapshot_mut().set_raw_value( + &"core.hooksPath", + gix::path::into_bstr(&custom_hooks).as_ref(), + )?; // Install hooks install_managed_hooks(&repo)?; @@ -432,7 +433,7 @@ fn test_respects_absolute_core_hooks_path() -> Result<()> { // Should NOT be in default .git/hooks assert!( - !repo.path().join("hooks").join("pre-commit").exists(), + !hooks_dir(&repo).join("pre-commit").exists(), "Hook should not be in default location" ); Ok(()) @@ -440,7 +441,7 @@ fn test_respects_absolute_core_hooks_path() -> Result<()> { #[test] fn test_respects_relative_core_hooks_path() -> Result<()> { - let (_temp, repo) = create_test_repo()?; + let (_temp, mut repo) = create_test_repo()?; let relative_hooks = format!( "custom-hooks-{}", @@ -452,8 +453,8 @@ fn test_respects_relative_core_hooks_path() -> Result<()> { ); let expected_hooks_dir = _temp.path().join(&relative_hooks); - let mut config = repo.config()?; - config.set_str("core.hooksPath", &relative_hooks)?; + repo.config_snapshot_mut() + .set_raw_value(&"core.hooksPath", relative_hooks.as_str())?; install_managed_hooks(&repo)?; @@ -466,18 +467,18 @@ fn test_respects_relative_core_hooks_path() -> Result<()> { "Hook should be resolved relative to the worktree root" ); assert!( - !repo.path().join("hooks").join("pre-commit").exists(), + !hooks_dir(&repo).join("pre-commit").exists(), "Hook should not be installed in the default location" ); Ok(()) } #[test] -fn test_respects_relative_core_hooks_path_gix() -> Result<()> { - let (_temp, repo) = create_test_repo()?; +fn test_uninstall_respects_relative_core_hooks_path() -> Result<()> { + let (_temp, mut repo) = create_test_repo()?; let relative_hooks = format!( - "custom-hooks-gix-{}", + "custom-hooks-uninstall-{}", _temp .path() .file_name() @@ -486,11 +487,9 @@ fn test_respects_relative_core_hooks_path_gix() -> Result<()> { ); let expected_hooks_dir = _temp.path().join(&relative_hooks); - let mut config = repo.config()?; - config.set_str("core.hooksPath", &relative_hooks)?; - - let gix_repo = open_gix_repo(&repo)?; - install_managed_hooks_gix(&gix_repo)?; + repo.config_snapshot_mut() + .set_raw_value(&"core.hooksPath", relative_hooks.as_str())?; + install_managed_hooks(&repo)?; assert!( hook_exists_at(&expected_hooks_dir, "pre-commit"), @@ -501,7 +500,7 @@ fn test_respects_relative_core_hooks_path_gix() -> Result<()> { "Hook should be resolved relative to the worktree root" ); - uninstall_managed_hooks_gix(&gix_repo)?; + uninstall_managed_hooks(&repo)?; assert!( !hook_exists_at(&expected_hooks_dir, "pre-commit"), @@ -588,7 +587,7 @@ fn test_empty_hooks_directory() -> Result<()> { let (_temp, repo) = create_test_repo()?; // Ensure hooks directory exists but is empty - let hooks_dir = repo.path().join("hooks"); + let hooks_dir = hooks_dir(&repo); fs::create_dir_all(&hooks_dir)?; // Should install cleanly diff --git a/crates/gitbutler-repo/tests/merge_base_octopussy.rs b/crates/gitbutler-repo/tests/merge_base_octopussy.rs index b5d338a1281..4f1bab12e10 100644 --- a/crates/gitbutler-repo/tests/merge_base_octopussy.rs +++ b/crates/gitbutler-repo/tests/merge_base_octopussy.rs @@ -1,5 +1,5 @@ +use but_testsupport::legacy::testing_repository::TestingRepository; use gitbutler_repo::RepositoryExt as _; -use gitbutler_testsupport::testing_repository::TestingRepository; use itertools::Itertools; #[test] diff --git a/crates/gitbutler-repo/tests/read_file_from_workspace_security.rs b/crates/gitbutler-repo/tests/read_file_from_workspace_security.rs index a2b720b5a1f..33bf2d17b9f 100644 --- a/crates/gitbutler-repo/tests/read_file_from_workspace_security.rs +++ b/crates/gitbutler-repo/tests/read_file_from_workspace_security.rs @@ -2,13 +2,13 @@ use std::{fs, path::Path}; 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; -use gitbutler_testsupport::{commit_all, test_repository}; -fn context_for_repo(repo: &git2::Repository) -> Context { +fn context_for_repo(workdir: &Path) -> Context { let project = projects::Project::new_for_gitbutler_repo( - repo.workdir().expect("workdir exists").to_path_buf(), + workdir.to_path_buf(), projects::AuthKey::default(), ); Context::new_from_legacy_project_and_settings_with_repo_open_mode( @@ -25,7 +25,7 @@ fn allows_read_inside_worktree_with_relative_path() { let workdir = repo.workdir().expect("workdir exists"); fs::write(workdir.join("file.txt"), "hello from workspace").expect("write file"); - let ctx = context_for_repo(&repo); + let ctx = context_for_repo(workdir); let info = ctx .read_file_from_workspace("file.txt".as_ref()) .expect("read file in workspace"); @@ -51,7 +51,7 @@ fn rejects_dotdot_traversal() { .to_string_lossy() ); - let ctx = context_for_repo(&repo); + let ctx = context_for_repo(workdir); let err = ctx .read_file_from_workspace(Path::new(&traversal)) .expect_err("traversal must be rejected"); @@ -74,7 +74,7 @@ fn rejects_symlink_escape() { fs::write(&outside_path, "outside via symlink").expect("write outside file"); gix::fs::symlink::create(&outside_path, &workdir.join("link.txt")).expect("create symlink"); - let ctx = context_for_repo(&repo); + let ctx = context_for_repo(workdir); let err = ctx .read_file_from_workspace(Path::new("link.txt")) .expect_err("symlink escape must be rejected"); @@ -93,7 +93,7 @@ fn reads_deleted_file_from_index() { commit_all(&repo); fs::remove_file(workdir.join("deleted.txt")).expect("delete file from workspace"); - let ctx = context_for_repo(&repo); + let ctx = context_for_repo(workdir); let info = ctx .read_file_from_workspace(Path::new("deleted.txt")) .expect("deleted tracked file should still be readable from index fallback"); @@ -110,7 +110,7 @@ fn reads_deleted_file_from_head_commit() { fs::remove_file(workdir.join("deleted.txt")).expect("delete file from workspace"); fs::remove_file(repo.path().join("index")).expect("delete index file"); - let ctx = context_for_repo(&repo); + let ctx = context_for_repo(workdir); let info = ctx .read_file_from_workspace(Path::new("deleted.txt")) .expect("deleted tracked file should still be readable from head fallback"); @@ -125,7 +125,7 @@ fn keeps_absolute_inside_worktree_behavior() { let abs_path = workdir.join("absolute.txt"); fs::write(&abs_path, "absolute read").expect("write file"); - let ctx = context_for_repo(&repo); + let ctx = context_for_repo(workdir); let info = ctx .read_file_from_workspace(&abs_path) .expect("absolute in-worktree path should be readable"); diff --git a/crates/gitbutler-repo/tests/rebase.rs b/crates/gitbutler-repo/tests/rebase.rs index c6975576994..8b0844f8484 100644 --- a/crates/gitbutler-repo/tests/rebase.rs +++ b/crates/gitbutler-repo/tests/rebase.rs @@ -1,8 +1,8 @@ mod gitbutler_merge_commits { - use gitbutler_repo::rebase::gitbutler_merge_commits; - use gitbutler_testsupport::testing_repository::{ + use but_testsupport::legacy::testing_repository::{ TestingRepository, assert_commit_tree_matches, }; + use gitbutler_repo::rebase::gitbutler_merge_commits; #[test] fn unconflicting_merge() { diff --git a/crates/gitbutler-stack/tests/mod.rs b/crates/gitbutler-stack/tests/mod.rs index 0040840ec11..c0826d20033 100644 --- a/crates/gitbutler-stack/tests/mod.rs +++ b/crates/gitbutler-stack/tests/mod.rs @@ -8,7 +8,6 @@ use but_core::{ use but_ctx::Context; use but_db::DbHandle; use but_meta::virtual_branches_legacy_types as legacy_types; -use but_oxidize::{ObjectIdExt, OidExt}; use but_testsupport::{gix_testtools, open_repo}; use filetime::{FileTime, set_file_mtime}; use gitbutler_reference::RemoteRefname; @@ -28,11 +27,7 @@ fn init() { fn add_series_success() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let mut test_ctx = test_ctx(&ctx)?; - let reference = StackBranch::new( - test_ctx.commits[1].id().to_gix(), - "asdf".into(), - &*ctx.repo.get()?, - )?; + let reference = StackBranch::new(test_ctx.commits[1].id(), "asdf".into(), &*ctx.repo.get()?)?; let result = test_ctx.stack.add_series(&ctx, reference, None); assert!(result.is_ok()); assert_eq!(test_ctx.stack.heads.len(), 2); @@ -91,7 +86,7 @@ fn add_multiple_series() -> Result<()> { let default_head = test_ctx.stack.heads[0].clone(); let head_4 = StackBranch::new( - test_ctx.commits.last().unwrap().id().to_gix(), + test_ctx.commits.last().unwrap().id(), "head_4".into(), &*ctx.repo.get()?, )?; @@ -102,7 +97,7 @@ fn add_multiple_series() -> Result<()> { assert_eq!(head_names(&test_ctx), vec!["virtual", "head_4"]); let head_2 = StackBranch::new( - test_ctx.commits.last().unwrap().id().to_gix(), + test_ctx.commits.last().unwrap().id(), "head_2".into(), &*ctx.repo.get()?, )?; @@ -111,7 +106,7 @@ fn add_multiple_series() -> Result<()> { assert_eq!(head_names(&test_ctx), vec!["head_2", "virtual", "head_4"]); let head_1 = StackBranch::new( - test_ctx.commits.first().unwrap().id().to_gix(), + test_ctx.commits.first().unwrap().id(), "head_1".into(), &*ctx.repo.get()?, )?; @@ -137,7 +132,7 @@ fn add_series_invalid_name_fails() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let test_ctx = test_ctx(&ctx)?; let result = StackBranch::new( - test_ctx.commits[0].id().to_gix(), + test_ctx.commits[0].id(), "name with spaces".into(), &*ctx.repo.get()?, ); @@ -152,11 +147,7 @@ fn add_series_invalid_name_fails() -> Result<()> { fn add_series_duplicate_name_fails() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let mut test_ctx = test_ctx(&ctx)?; - let reference = StackBranch::new( - test_ctx.commits[1].id().to_gix(), - "asdf".into(), - &*ctx.repo.get()?, - )?; + let reference = StackBranch::new(test_ctx.commits[1].id(), "asdf".into(), &*ctx.repo.get()?)?; let result = test_ctx.stack.add_series(&ctx, reference.clone(), None); assert!(result.is_ok()); let result = test_ctx.stack.add_series(&ctx, reference, None); @@ -172,7 +163,7 @@ fn add_series_matching_git_ref_is_ok() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let mut test_ctx = test_ctx(&ctx)?; let reference = StackBranch::new( - test_ctx.commits[0].parent(0)?.to_gix(), + test_ctx.commits[0].parent(0)?, "existing-branch".into(), &*ctx.repo.get()?, )?; @@ -186,7 +177,7 @@ fn add_series_including_refs_head_fails() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let mut test_ctx = test_ctx(&ctx)?; let reference = StackBranch::new( - test_ctx.commits[0].id().to_gix(), + test_ctx.commits[0].id(), "refs/heads/my-branch".into(), &*ctx.repo.get()?, )?; @@ -238,7 +229,7 @@ fn add_series_target_commit_not_in_stack() -> Result<()> { let mut test_ctx = test_ctx(&ctx)?; let other_commit_id = test_ctx.other_commits.last().unwrap().id(); let reference = StackBranch::new( - other_commit_id.to_gix(), // does not exist + other_commit_id, // does not exist "my-branch".into(), &*ctx.repo.get()?, )?; @@ -285,7 +276,7 @@ fn remove_branch_with_multiple_last_heads() -> Result<()> { let default_head = test_ctx.stack.heads[0].clone(); let repo = &*ctx.repo.get()?; let to_stay = StackBranch::new( - test_ctx.commits.last().unwrap().id().to_gix(), + test_ctx.commits.last().unwrap().id(), "to_stay".into(), repo, )?; @@ -297,7 +288,7 @@ fn remove_branch_with_multiple_last_heads() -> Result<()> { assert!(result.is_ok()); assert_eq!(head_names(&test_ctx), vec!["to_stay"]); assert_eq!( - test_ctx.commits.last().unwrap().id().to_gix(), + test_ctx.commits.last().unwrap().id(), test_ctx.stack.heads[0].head_oid(repo)?, ); // it references the newest commit Ok(()) @@ -314,7 +305,7 @@ fn remove_branch_no_orphan_commits() -> Result<()> { let repo = &*ctx.repo.get()?; let to_stay = StackBranch::new( - test_ctx.commits.first().unwrap().id().to_gix(), + test_ctx.commits.first().unwrap().id(), "to_stay".into(), repo, )?; // references the oldest commit @@ -326,7 +317,7 @@ fn remove_branch_no_orphan_commits() -> Result<()> { assert!(result.is_ok()); assert_eq!(head_names(&test_ctx), vec!["to_stay"]); assert_eq!( - test_ctx.commits.last().unwrap().id().to_gix(), + test_ctx.commits.last().unwrap().id(), test_ctx.stack.heads[0].head_oid(repo)? ); // it was updated to reference the newest commit Ok(()) @@ -482,11 +473,7 @@ fn list_series_default_head() -> Result<()> { branches[0] .commit_ids(&repo, &ctx, &test_ctx.stack)? .local_commits, - test_ctx - .commits - .iter() - .map(|c| c.id().to_gix()) - .collect_vec() + test_ctx.commits.iter().map(|c| c.id()).collect_vec() ); Ok(()) } @@ -496,7 +483,7 @@ fn list_series_two_heads_same_commit() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let mut test_ctx = test_ctx(&ctx)?; let head_before = StackBranch::new( - test_ctx.commits.last().unwrap().id().to_gix(), + test_ctx.commits.last().unwrap().id(), "head_before".into(), &*ctx.repo.get()?, )?; @@ -514,11 +501,7 @@ fn list_series_two_heads_same_commit() -> Result<()> { branches[0] .commit_ids(&repo, &ctx, &test_ctx.stack)? .local_commits, - test_ctx - .commits - .iter() - .map(|c| c.id().to_gix()) - .collect_vec() + test_ctx.commits.iter().map(|c| c.id()).collect_vec() ); assert_eq!(branches[0].name(), "head_before"); assert_eq!( @@ -536,7 +519,7 @@ fn list_series_two_heads_different_commit() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let mut test_ctx = test_ctx(&ctx)?; let head_before = StackBranch::new( - test_ctx.commits.first().unwrap().id().to_gix(), + test_ctx.commits.first().unwrap().id(), "head_before".into(), &*ctx.repo.get()?, )?; @@ -547,11 +530,7 @@ fn list_series_two_heads_different_commit() -> Result<()> { let branches = test_ctx.stack.branches(); // the number of series matches the number of heads assert_eq!(branches.len(), test_ctx.stack.heads.len()); - let mut expected_patches = test_ctx - .commits - .iter() - .map(|c| c.id().to_gix()) - .collect_vec(); + let mut expected_patches = test_ctx.commits.iter().map(|c| c.id()).collect_vec(); let repo = ctx.repo.get()?; assert_eq!( branches[0] @@ -580,7 +559,7 @@ fn set_stack_head_commit_invalid() -> Result<()> { let repo = ctx.repo.get()?; let result = test_ctx .stack - .set_stack_head(&mut vb_state, &repo, git2::Oid::zero().to_gix()); + .set_stack_head(&mut vb_state, &repo, repo.object_hash().null()); assert!(result.is_err()); Ok(()) } @@ -594,16 +573,16 @@ fn set_stack_head() -> Result<()> { let repo = ctx.repo.get()?; let result = test_ctx .stack - .set_stack_head(&mut vb_state, &repo, commit.id().to_gix()); + .set_stack_head(&mut vb_state, &repo, commit.id()); assert!(result.is_ok()); let branches = test_ctx.stack.branches(); assert_eq!( - commit.id().to_gix(), + commit.id(), branches.first().unwrap().head_oid(&*ctx.repo.get()?)? ); assert_eq!( test_ctx.stack.head_oid(&ctx)?, - test_ctx.other_commits.last().unwrap().id().to_gix() + test_ctx.other_commits.last().unwrap().id() ); Ok(()) } @@ -633,7 +612,7 @@ fn archive_heads_success() -> Result<()> { test_ctx.stack.heads.insert( 0, StackBranch::new( - test_ctx.other_commits.first().unwrap().id.to_gix(), + test_ctx.other_commits.first().unwrap().id, "foo".to_string(), &*ctx.repo.get()?, )?, @@ -723,26 +702,23 @@ fn add_head_with_archived_bottom_head() -> Result<()> { let (ctx, _temp_dir) = command_ctx("multiple-commits")?; let test_ctx = test_ctx(&ctx)?; let mut head_1_archived = StackBranch::new( - test_ctx.commits[0].id.to_gix(), + test_ctx.commits[0].id, "kv-branch-3".to_string(), &*ctx.repo.get()?, )?; head_1_archived.archived = true; let head_2 = StackBranch::new( - test_ctx.commits[1].id.to_gix(), + test_ctx.commits[1].id, "more-on-top".to_string(), &*ctx.repo.get()?, )?; let existing_heads = vec![head_1_archived.clone(), head_2.clone()]; let new_head = StackBranch::new( - test_ctx.commits[1].id.to_gix(), // same as head_2 + test_ctx.commits[1].id, // same as head_2 "abcd".to_string(), &*ctx.repo.get()?, )?; - let patches: Vec = vec![ - test_ctx.commits[0].id.to_gix(), - test_ctx.commits[1].id.to_gix(), - ]; + let patches = vec![test_ctx.commits[0].id, test_ctx.commits[1].id]; let updated_heads = gitbutler_stack::add_head( existing_heads, @@ -865,23 +841,23 @@ struct TestContext { } struct BakedCommit { - id: git2::Oid, - parents: Vec, + id: gix::ObjectId, + parents: Vec, } fn bake_commit(repo: &gix::Repository, id: gix::ObjectId) -> Result { let commit = repo.find_commit(id)?; Ok(BakedCommit { - id: id.to_git2(), - parents: commit.parent_ids().map(|parent| parent.to_git2()).collect(), + id, + parents: commit.parent_ids().map(Into::into).collect(), }) } impl BakedCommit { - pub fn id(&self) -> git2::Oid { + pub fn id(&self) -> gix::ObjectId { self.id } - pub fn parent(&self, index: usize) -> anyhow::Result { + pub fn parent(&self, index: usize) -> anyhow::Result { Ok(self.parents[index]) } } @@ -1172,14 +1148,14 @@ fn legacy_with_target(branch: &str, sha: &str) -> Result Result { - let oid = git2::Oid::from_str(sha)?; - if oid.is_zero() { + let oid = gix::ObjectId::from_hex(sha.as_bytes())?; + if oid.is_null() { bail!("test target oid must not be zero"); } Ok(gitbutler_stack::Target { branch: RemoteRefname::new("origin", branch), remote_url: "https://example.invalid/repo".into(), - sha: oid.to_gix(), + sha: oid, push_remote_name: Some("origin".into()), }) } diff --git a/crates/gitbutler-tauri/Cargo.toml b/crates/gitbutler-tauri/Cargo.toml index 748a587fda6..e39cd35eb0b 100644 --- a/crates/gitbutler-tauri/Cargo.toml +++ b/crates/gitbutler-tauri/Cargo.toml @@ -71,7 +71,6 @@ serde_json = { workspace = true, features = ["arbitrary_precision"] } parking_lot.workspace = true log = "^0.4" -git2.workspace = true # The features here optimize for performance. tokio = { workspace = true, features = ["rt-multi-thread", "parking_lot"] } tracing.workspace = true diff --git a/crates/gitbutler-tauri/src/main.rs b/crates/gitbutler-tauri/src/main.rs index b35bd1e9154..f32aff964c6 100644 --- a/crates/gitbutler-tauri/src/main.rs +++ b/crates/gitbutler-tauri/src/main.rs @@ -63,7 +63,6 @@ fn main() -> anyhow::Result<()> { let performance_logging = std::env::var_os("GITBUTLER_PERFORMANCE_LOG").is_some(); let tauri_debug_logging = std::env::var_os("GITBUTLER_TAURI_DEBUG_LOG").is_some(); - gitbutler_project::configure_git2(); let mut tauri_context = generate_context!(); but_secret::secret::set_application_namespace(&tauri_context.config().identifier); diff --git a/crates/gitbutler-tauri/src/projects.rs b/crates/gitbutler-tauri/src/projects.rs index 702829ac0d8..1e4351cb4b4 100644 --- a/crates/gitbutler-tauri/src/projects.rs +++ b/crates/gitbutler-tauri/src/projects.rs @@ -54,20 +54,10 @@ pub fn set_project_active( return Ok(None); } }; - let repo = git2::Repository::open(&ctx.gitdir) - // Only capture this information here to prevent spawning too many errors because of this - // (the UI has many parallel calls in flight). - .map_err(|err| { - let code = err.code(); - let err = anyhow::Error::from(err); - if code == git2::ErrorCode::Owner { - err.context(but_error::Code::RepoOwnership) - } else { - err - } - })?; // --> WARNING <-- Be sure this runs BEFORE the database on `ctx` is used. - ctx = ctx.with_git2_repo(repo); + // 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)?; diff --git a/crates/gitbutler-testsupport/Cargo.toml b/crates/gitbutler-testsupport/Cargo.toml deleted file mode 100644 index 080f091d818..00000000000 --- a/crates/gitbutler-testsupport/Cargo.toml +++ /dev/null @@ -1,41 +0,0 @@ -[package] -name = "gitbutler-testsupport" -version = "0.0.0" -edition.workspace = true -publish = false -rust-version.workspace = true - -[lib] -path = "src/lib.rs" -doctest = false -test = false - -[dependencies] -but-core = { workspace = true, features = ["legacy"] } -but-settings.workspace = true -but-oxidize.workspace = true -but-workspace.workspace = true -but-fs = {workspace = true, features = ["legacy"]} -but-ctx.workspace = true -but-project-handle.workspace = true - -gitbutler-branch-actions = { path = "../gitbutler-branch-actions" } -gitbutler-repo = { path = "../gitbutler-repo" } -gitbutler-project.workspace = true -gitbutler-user.workspace = true -gitbutler-reference.workspace = true -gitbutler-url.workspace = true -gitbutler-stack.workspace = true - -anyhow.workspace = true -git2.workspace = true -tempfile.workspace = true -keyring.workspace = true -serde_json.workspace = true -gix-testtools.workspace = true -gix.workspace = true -termtree = "0.5.1" -uuid.workspace = true - -[lints] -workspace = true diff --git a/etc/plans/consistent-data.md b/etc/plans/consistent-data.md index 67ce330a933..b0eb5623128 100644 --- a/etc/plans/consistent-data.md +++ b/etc/plans/consistent-data.md @@ -153,10 +153,6 @@ not as independent stateful-handle flows. - [ ] `crates/gitbutler-operating-modes/src/lib.rs` -#### gitbutler-testsupport (1) - -- [ ] `crates/gitbutler-testsupport/src/lib.rs` - #### gitbutler-edit-mode (2) - [x] `crates/gitbutler-edit-mode/src/lib.rs` @@ -216,7 +212,7 @@ Sync the TOML file _on drop_ only, knowing well that this may write data that is - `crates/gitbutler-branch-actions/src/upstream_integration.rs` - `crates/gitbutler-branch-actions/tests/branch-actions/virtual_branches/mod.rs` - `crates/gitbutler-cli/src/command/vbranch.rs` - - `crates/gitbutler-testsupport/src/lib.rs` + - `crates/but-testsupport/src/legacy.rs` - [ ] Not started ### 4. Modernize workspace metadata schema diff --git a/etc/plans/git2-to-gix-migration.md b/etc/plans/git2-to-gix-migration.md index 92d8b24ae26..03b967de9a1 100644 --- a/etc/plans/git2-to-gix-migration.md +++ b/etc/plans/git2-to-gix-migration.md @@ -8,7 +8,7 @@ This plan orchestrates migration from `git2` to `gix` across the codebase where - 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 -`create_wd_tree()` itself is already implemented in `gix` via `but-status`; remaining `git2` usage around it should be treated as legacy wrapper/caller cleanup, not as a true `git2` implementation boundary. +`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. 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. @@ -28,10 +28,10 @@ Repository scan baseline at plan creation: Current raw audit on 2026-03-21: -- `git2::` callsites: 314 -- files with `git2::` references: 43 -- direct `ctx.git2_repo` / `with_git2_repo` call-sites: 76 across 24 files -- all `git2_repo` / `with_git2_repo` identifier matches including field/setup definitions: 157 across 31 files +- `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 @@ -226,8 +226,8 @@ Current reconciliation notes: - 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` still contains `git2`-based test scaffolding under `#[cfg(test)]`. -- Workstream D remains active for `oplog.rs` boundary extraction, with `reflog.rs` cleanup as a follow-on test task. +- `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 @@ -280,9 +280,10 @@ Target tests/modules: - `crates/gitbutler-repo/tests/create_wd_tree.rs` - `crates/gitbutler-repo/tests/managed_hooks_tests.rs` - `crates/gitbutler-project/tests/project/main.rs` -- `crates/gitbutler-testsupport/src/lib.rs` -- `crates/gitbutler-testsupport/src/suite.rs` -- `crates/gitbutler-testsupport/src/test_project.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: @@ -306,8 +307,10 @@ Current reconciliation notes: - `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`. -- `crates/gitbutler-edit-mode/tests/edit_mode.rs`, `crates/gitbutler-repo/tests/create_wd_tree.rs`, `crates/gitbutler-repo/tests/managed_hooks_tests.rs`, `crates/gitbutler-project/tests/project/main.rs`, and `crates/gitbutler-testsupport/src/*` still contain substantial direct `git2` usage. -- `gitbutler-testsupport` should be treated as a migration target, not a keeper crate: move coverage helpers to `but-testsupport`, then delete `gitbutler-testsupport`. +- 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. @@ -359,8 +362,8 @@ Current execution status: - 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 follow-on test/helper migration including `gitbutler-testsupport` replacement. -- Recommended next wave: Wave 5, starting with `crates/gitbutler-workspace/src/branch_trees.rs`, `crates/gitbutler-oplog/src/oplog.rs`, and `gitbutler-testsupport` helper migration into `but-testsupport`. +- 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`. ## Acceptance Criteria @@ -397,10 +400,10 @@ The filtered report must trend to zero in-scope matches. 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/managed_hooks.rs`, `crates/gitbutler-repo/src/remote.rs`, `crates/gitbutler-repo/src/staging.rs`, `crates/gitbutler-repo-actions/src/repository.rs`, `crates/gitbutler-tauri/src/projects.rs`, and `crates/but-napi/src/lib.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/*`, `crates/gitbutler-project/tests/project/main.rs`, and `crates/gitbutler-testsupport/src/*` -- Planned helper consolidation: migrate remaining reusable fixture/test utilities from `gitbutler-testsupport` into `but-testsupport`, then remove `gitbutler-testsupport` from the tree. +- 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: