From b51dd0f7cd48b8ff3b93acf626a8ede1b2511033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Tue, 23 Jul 2024 14:58:10 +0200 Subject: [PATCH 1/3] Fix log crashing in subdirectories Replace `gix::open` by `gix::discover`. `gix::open` errors when the passed directory is not a git repository. --- asyncgit/src/error.rs | 10 +++++----- asyncgit/src/revlog.rs | 2 +- asyncgit/src/sync/logwalker.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index bc17a5caa3..37fba9fa91 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -94,8 +94,8 @@ pub enum Error { Sign(#[from] crate::sync::sign::SignError), /// - #[error("gix::open error: {0}")] - GixOpen(#[from] Box), + #[error("gix::discover error: {0}")] + GixDiscover(#[from] Box), /// #[error("gix::reference::find::existing error: {0}")] @@ -139,8 +139,8 @@ impl From> for Error { } } -impl From for Error { - fn from(error: gix::open::Error) -> Self { - Self::GixOpen(Box::new(error)) +impl From for Error { + fn from(error: gix::discover::Error) -> Self { + Self::GixDiscover(Box::new(error)) } } diff --git a/asyncgit/src/revlog.rs b/asyncgit/src/revlog.rs index 6465cf3265..e5531b0344 100644 --- a/asyncgit/src/revlog.rs +++ b/asyncgit/src/revlog.rs @@ -276,7 +276,7 @@ impl AsyncLog { let mut entries = vec![CommitId::default(); LIMIT_COUNT]; entries.resize(0, CommitId::default()); - let mut repo = gix::open(repo_path.gitpath())?; + let mut repo = gix::discover(repo_path.gitpath())?; let mut walker = LogWalkerWithoutFilter::new(&mut repo, LIMIT_COUNT)?; diff --git a/asyncgit/src/sync/logwalker.rs b/asyncgit/src/sync/logwalker.rs index 99fe2f93ff..46ca72991d 100644 --- a/asyncgit/src/sync/logwalker.rs +++ b/asyncgit/src/sync/logwalker.rs @@ -114,7 +114,7 @@ impl<'a> LogWalker<'a> { /// /// `SharedCommitFilterFn` requires access to a `git2::repo::Repository` because, under the hood, /// it calls into functions that work with a `git2::repo::Repository`. It seems unwise to open a -/// repo both through `gix::open` and `Repository::open_ext` at the same time, so there is a +/// repo both through `gix::discover` and `Repository::open_ext` at the same time, so there is a /// separate struct that works with `gix::Repository` only. /// /// A more long-term option is to refactor filtering to work with a `gix::Repository` and to remove @@ -271,7 +271,7 @@ mod tests { stage_add_file(repo_path, file_path).unwrap(); let oid2 = commit(repo_path, "commit2").unwrap(); - let mut repo = gix::open(repo_path.gitpath()).unwrap(); + let mut repo = gix::discover(repo_path.gitpath()).unwrap(); let mut walk = LogWalkerWithoutFilter::new(&mut repo, 100)?; let mut items = Vec::new(); assert!(matches!(walk.read(&mut items), Ok(2))); From a10ed21abe225de1d27d2176c8d1d04f7e5b12ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Mon, 5 Aug 2024 13:47:34 +0200 Subject: [PATCH 2/3] Add test for AsyncLog in sub-directory --- asyncgit/src/revlog.rs | 46 ++++++++++++++++++++++++++++++++++++++++ asyncgit/src/sync/mod.rs | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/revlog.rs b/asyncgit/src/revlog.rs index e5531b0344..0f04324e56 100644 --- a/asyncgit/src/revlog.rs +++ b/asyncgit/src/revlog.rs @@ -321,3 +321,49 @@ impl AsyncLog { .expect("error sending"); } } + +#[cfg(test)] +mod tests { + use std::sync::atomic::AtomicBool; + use std::sync::{Arc, Mutex}; + use std::time::Duration; + + use crossbeam_channel::unbounded; + + use crate::sync::tests::{debug_cmd_print, repo_init}; + use crate::sync::RepoPath; + use crate::AsyncLog; + + use super::AsyncLogResult; + + #[test] + fn test_smoke_in_subdir() { + let (_td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path: RepoPath = + root.as_os_str().to_str().unwrap().into(); + + let (tx_git, _rx_git) = unbounded(); + + debug_cmd_print(&repo_path, "mkdir subdir"); + + let subdir = repo.path().parent().unwrap().join("subdir"); + let subdir_path: RepoPath = + subdir.as_os_str().to_str().unwrap().into(); + + let arc_current = Arc::new(Mutex::new(AsyncLogResult { + commits: Vec::new(), + duration: Duration::default(), + })); + let arc_background = Arc::new(AtomicBool::new(false)); + + let result = AsyncLog::fetch_helper_without_filter( + &subdir_path, + &arc_current, + &arc_background, + &tx_git, + ); + + assert!(result.is_ok()); + } +} diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 3a27c4a883..f95e8b0537 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -110,7 +110,7 @@ pub use utils::{ pub use git2::ResetType; #[cfg(test)] -mod tests { +pub mod tests { use super::{ commit, repository::repo, From 6f20bd84d8c8cedb7ae7de5b4c1ff4a5daf8ec0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Mon, 5 Aug 2024 16:20:31 +0200 Subject: [PATCH 3/3] Respect env variables when discovering repo --- asyncgit/src/revlog.rs | 4 +++- asyncgit/src/sync/logwalker.rs | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/asyncgit/src/revlog.rs b/asyncgit/src/revlog.rs index a2c28eabd4..0a2a2d3680 100644 --- a/asyncgit/src/revlog.rs +++ b/asyncgit/src/revlog.rs @@ -276,7 +276,9 @@ impl AsyncLog { let mut entries = vec![CommitId::default(); LIMIT_COUNT]; entries.resize(0, CommitId::default()); - let mut repo = gix::discover(repo_path.gitpath())?; + let mut repo: gix::Repository = + gix::ThreadSafeRepository::discover_with_environment_overrides(repo_path.gitpath()) + .map(Into::into)?; let mut walker = LogWalkerWithoutFilter::new(&mut repo, LIMIT_COUNT)?; diff --git a/asyncgit/src/sync/logwalker.rs b/asyncgit/src/sync/logwalker.rs index 46ca72991d..3c7f8705c5 100644 --- a/asyncgit/src/sync/logwalker.rs +++ b/asyncgit/src/sync/logwalker.rs @@ -271,7 +271,10 @@ mod tests { stage_add_file(repo_path, file_path).unwrap(); let oid2 = commit(repo_path, "commit2").unwrap(); - let mut repo = gix::discover(repo_path.gitpath()).unwrap(); + let mut repo: gix::Repository = + gix::ThreadSafeRepository::discover_with_environment_overrides(repo_path.gitpath()) + .map(Into::into) + .unwrap(); let mut walk = LogWalkerWithoutFilter::new(&mut repo, 100)?; let mut items = Vec::new(); assert!(matches!(walk.read(&mut items), Ok(2)));