diff --git a/CHANGELOG.md b/CHANGELOG.md index d674df25be..5c7c6acd5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - compilation broken on freebsd ([#461](https://github.com/extrawurst/gitui/issues/461)) +- don’t fail if `user.name` is not set [[@cruessler](https://github.com/cruessler)] ([#79](https://github.com/extrawurst/gitui/issues/79)) ([#228](https://github.com/extrawurst/gitui/issues/228)) ## [0.11.0] - 2020-12-20 diff --git a/asyncgit/src/lib.rs b/asyncgit/src/lib.rs index 3ca6d2a393..8fc89937e3 100644 --- a/asyncgit/src/lib.rs +++ b/asyncgit/src/lib.rs @@ -1,7 +1,7 @@ //! asyncgit -#![forbid(unsafe_code)] #![forbid(missing_docs)] +#![deny(unsafe_code)] #![deny(unused_imports)] #![deny(clippy::all)] #![deny(clippy::unwrap_used)] diff --git a/asyncgit/src/sync/commit.rs b/asyncgit/src/sync/commit.rs index de96c8bc19..56457c725c 100644 --- a/asyncgit/src/sync/commit.rs +++ b/asyncgit/src/sync/commit.rs @@ -36,17 +36,24 @@ pub fn amend( fn signature_allow_undefined_name( repo: &Repository, ) -> std::result::Result, git2::Error> { - match repo.signature() { - Err(e) if e.code() == ErrorCode::NotFound => { + let signature = repo.signature(); + + if let Err(ref e) = signature { + if e.code() == ErrorCode::NotFound { let config = repo.config()?; - Signature::now( - config.get_str("user.name").unwrap_or("unknown"), - config.get_str("user.email")?, - ) - } - v => v, + if let (Err(_), Ok(email_entry)) = ( + config.get_entry("user.name"), + config.get_entry("user.email"), + ) { + if let Some(email) = email_entry.value() { + return Signature::now("unknown", email); + } + }; + } } + + signature } /// this does not run any git hooks @@ -245,4 +252,86 @@ mod tests { Ok(()) } + + /// Beware: this test has to be run with a `$HOME/.gitconfig` that has + /// `user.email` not set. Otherwise, git falls back to the value of + /// `user.email` in `$HOME/.gitconfig` and this test fails. + /// + /// As of February 2021, `repo_init_empty` sets all git config locations + /// to an empty temporary directory, so this constraint is met. + #[test] + fn test_empty_email() -> Result<()> { + let file_path = Path::new("foo"); + let (_td, repo) = repo_init_empty().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path = root.as_os_str().to_str().unwrap(); + + File::create(&root.join(file_path))? + .write_all(b"test\nfoo")?; + + stage_add_file(repo_path, file_path)?; + + repo.config()?.remove("user.email")?; + + let error = commit(repo_path, "commit msg"); + + assert!(matches!(error, Err(_))); + + repo.config()?.set_str("user.email", "email")?; + + let success = commit(repo_path, "commit msg"); + + assert!(matches!(success, Ok(_))); + assert_eq!(count_commits(&repo, 10), 1); + + let details = + get_commit_details(repo_path, success.unwrap()).unwrap(); + + assert_eq!(details.author.name, "name"); + assert_eq!(details.author.email, "email"); + + Ok(()) + } + + /// See comment to `test_empty_email`. + #[test] + fn test_empty_name() -> Result<()> { + let file_path = Path::new("foo"); + let (_td, repo) = repo_init_empty().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path = root.as_os_str().to_str().unwrap(); + + File::create(&root.join(file_path))? + .write_all(b"test\nfoo")?; + + stage_add_file(repo_path, file_path)?; + + repo.config()?.remove("user.name")?; + + let mut success = commit(repo_path, "commit msg"); + + assert!(matches!(success, Ok(_))); + assert_eq!(count_commits(&repo, 10), 1); + + let mut details = + get_commit_details(repo_path, success.unwrap()).unwrap(); + + assert_eq!(details.author.name, "unknown"); + assert_eq!(details.author.email, "email"); + + repo.config()?.set_str("user.name", "name")?; + + success = commit(repo_path, "commit msg"); + + assert!(matches!(success, Ok(_))); + assert_eq!(count_commits(&repo, 10), 2); + + details = + get_commit_details(repo_path, success.unwrap()).unwrap(); + + assert_eq!(details.author.name, "name"); + assert_eq!(details.author.email, "email"); + + Ok(()) + } } diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 618039b86a..c6f5d1934c 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -60,8 +60,32 @@ mod tests { use std::process::Command; use tempfile::TempDir; + /// Calling `set_search_path` with an empty directory makes sure that there + /// is no git config interfering with our tests (for example user-local + /// `.gitconfig`). + #[allow(unsafe_code)] + fn sandbox_config_files() { + use git2::{opts::set_search_path, ConfigLevel}; + use std::sync::Once; + + static INIT: Once = Once::new(); + + // Adapted from https://github.com/rust-lang/cargo/pull/9035 + INIT.call_once(|| unsafe { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path(); + + set_search_path(ConfigLevel::System, &path).unwrap(); + set_search_path(ConfigLevel::Global, &path).unwrap(); + set_search_path(ConfigLevel::XDG, &path).unwrap(); + set_search_path(ConfigLevel::ProgramData, &path).unwrap(); + }); + } + /// pub fn repo_init_empty() -> Result<(TempDir, Repository)> { + sandbox_config_files(); + let td = TempDir::new()?; let repo = Repository::init(td.path())?; { @@ -74,6 +98,8 @@ mod tests { /// pub fn repo_init() -> Result<(TempDir, Repository)> { + sandbox_config_files(); + let td = TempDir::new()?; let repo = Repository::init(td.path())?; {