Fix log crashing in subdirectories (#2301)

* Fix log crashing in subdirectories
Replace `gix::open` by `gix::discover`. `gix::open` errors when the
passed directory is not a git repository.
* Add test for AsyncLog in sub-directory
* Respect env variables when discovering repo

---------

Co-authored-by: extrawurst <[email protected]>

Author: cruessler

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<gix::open::Error>),
+	#[error("gix::discover error: {0}")]
+	GixDiscover(#[from] Box<gix::discover::Error>),
 
 	///
 	#[error("gix::reference::find::existing error: {0}")]
@@ -139,8 +139,8 @@ impl<T> From<crossbeam_channel::SendError<T>> for Error {
 	}
 }
 
-impl From<gix::open::Error> for Error {
-	fn from(error: gix::open::Error) -> Self {
-		Self::GixOpen(Box::new(error))
+impl From<gix::discover::Error> for Error {
+	fn from(error: gix::discover::Error) -> Self {
+		Self::GixDiscover(Box::new(error))
 	}
 }

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::open(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)?;
 
@@ -321,3 +323,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());
+	}
+}

asyncgit/src/sync/logwalker.rs

@@ -113,7 +113,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
@@ -270,7 +270,10 @@ 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::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)));

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,