From 76e911083fbb789e1cd3f84c194759517625182c Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 28 Feb 2022 20:11:22 +0800 Subject: [PATCH] verify that Id::prefix() makes use of the git configuration (#301) --- git-repository/src/id.rs | 27 ++++++++++++++++----- git-repository/src/repository/config.rs | 2 +- git-repository/tests/easy/ext/reference.rs | 2 +- git-repository/tests/easy/{oid.rs => id.rs} | 19 +++++++++++++-- git-repository/tests/easy/mod.rs | 2 +- git-repository/tests/repo.rs | 7 ++++-- 6 files changed, 46 insertions(+), 13 deletions(-) rename git-repository/tests/easy/{oid.rs => id.rs} (66%) diff --git a/git-repository/src/id.rs b/git-repository/src/id.rs index 625c3bafaf4..2ca48a936cf 100644 --- a/git-repository/src/id.rs +++ b/git-repository/src/id.rs @@ -1,4 +1,5 @@ //! +use std::convert::TryInto; use std::ops::Deref; use git_hash::{oid, ObjectId}; @@ -28,11 +29,22 @@ impl<'repo> Id<'repo> { /// Turn this object id into a shortened id with a length in hex as configured by `core.abbrev`. pub fn prefix(&self) -> Result { - // let hex_len = self.handle.config.get_int("core.abbrev")?; + let hex_len = self.repo.config_int("core.abbrev", 8); + let hex_len = hex_len.try_into().map_err(|_| prefix::Error::ConfigValue { + actual: hex_len, + max_range: self.inner.kind().len_in_hex(), + err: None, + })?; + let prefix = + git_odb::find::PotentialPrefix::new(self.inner, hex_len).map_err(|err| prefix::Error::ConfigValue { + actual: hex_len as i64, + max_range: self.inner.kind().len_in_hex(), + err: Some(err), + })?; Ok(self .repo .objects - .disambiguate_prefix(git_odb::find::PotentialPrefix::new(self.inner, 7)?) + .disambiguate_prefix(prefix) .map_err(crate::object::find::existing::OdbError::Find)? .ok_or(crate::object::find::existing::OdbError::NotFound { oid: self.inner })?) } @@ -46,10 +58,13 @@ mod prefix { pub enum Error { #[error(transparent)] FindExisting(#[from] crate::object::find::existing::OdbError), - #[error(transparent)] - Config(#[from] crate::config::open::Error), - #[error(transparent)] - Prefix(#[from] git_hash::prefix::Error), + #[error("core.abbrev length was {}, but needs to be between 4 and {}", .actual, .max_range)] + ConfigValue { + #[source] + err: Option, + actual: i64, + max_range: usize, + }, } } diff --git a/git-repository/src/repository/config.rs b/git-repository/src/repository/config.rs index c342aa977ff..5d986a45ec7 100644 --- a/git-repository/src/repository/config.rs +++ b/git-repository/src/repository/config.rs @@ -3,7 +3,7 @@ impl crate::Repository { /// Return the integer value at `key` (like `core.abbrev`) or use the given `default` value if it isn't present. // TODO: figure out how to identify sub-sections, or how to design such an API. This is really just a first test. // TODO: tests - pub fn config_int(&self, key: &str, default: i64) -> i64 { + pub(crate) fn config_int(&self, key: &str, default: i64) -> i64 { let (section, key) = key.split_once('.').expect("valid section.key format"); self.config .value::(section, None, key) diff --git a/git-repository/tests/easy/ext/reference.rs b/git-repository/tests/easy/ext/reference.rs index a7d6dc29ae4..10a60f18e5e 100644 --- a/git-repository/tests/easy/ext/reference.rs +++ b/git-repository/tests/easy/ext/reference.rs @@ -3,7 +3,7 @@ mod set_namespace { use git_repository::refs::transaction::PreviousValue; fn easy_repo_rw() -> crate::Result<(git::Repository, tempfile::TempDir)> { - crate::repo_rw("make_references_repo.sh").map(|(r, d)| (r.to_thread_local(), d)) + crate::repo_rw("make_references_repo.sh") } #[test] diff --git a/git-repository/tests/easy/oid.rs b/git-repository/tests/easy/id.rs similarity index 66% rename from git-repository/tests/easy/oid.rs rename to git-repository/tests/easy/id.rs index e2917283f1d..5621a6c13bb 100644 --- a/git-repository/tests/easy/oid.rs +++ b/git-repository/tests/easy/id.rs @@ -4,11 +4,26 @@ use std::cmp::Ordering; #[test] fn prefix() -> crate::Result { - let repo = crate::repo("make_repo_with_fork_and_dates.sh")?.to_thread_local(); + let (repo, worktree_dir) = crate::repo_rw("make_repo_with_fork_and_dates.sh")?; let id = hex_to_id("288e509293165cb5630d08f4185bdf2445bf6170").attach(&repo); let prefix = id.prefix()?; assert_eq!(prefix.cmp_oid(&id), Ordering::Equal); - assert_eq!(prefix.hex_len(), 7, "preconfigured via core.abbrev"); + assert_eq!(prefix.hex_len(), 8, "preconfigured via core.abbrev default value"); + + assert!( + std::process::Command::new("git") + .current_dir(worktree_dir.path()) + .args(["config", "--int", "core.abbrev", "5"]) + .status()? + .success(), + "set core abbrev value successfully" + ); + + let repo = git_repository::open(worktree_dir.path()).unwrap(); + let id = id.detach().attach(&repo); + let prefix = id.prefix()?; + assert_eq!(prefix.cmp_oid(&id), Ordering::Equal); + assert_eq!(prefix.hex_len(), 5, "preconfigured via core.abbrev in the repo file"); Ok(()) } diff --git a/git-repository/tests/easy/mod.rs b/git-repository/tests/easy/mod.rs index 6bade1de3f5..0fa32321ac8 100644 --- a/git-repository/tests/easy/mod.rs +++ b/git-repository/tests/easy/mod.rs @@ -1,4 +1,4 @@ mod ext; +mod id; mod object; -mod oid; mod reference; diff --git a/git-repository/tests/repo.rs b/git-repository/tests/repo.rs index d933650a292..4fd11d8b354 100644 --- a/git-repository/tests/repo.rs +++ b/git-repository/tests/repo.rs @@ -8,9 +8,12 @@ fn repo(name: &str) -> crate::Result { Ok(ThreadSafeRepository::open(repo_path)?) } -fn repo_rw(name: &str) -> crate::Result<(ThreadSafeRepository, tempfile::TempDir)> { +fn repo_rw(name: &str) -> crate::Result<(git_repository::Repository, tempfile::TempDir)> { let repo_path = git_testtools::scripted_fixture_repo_writable(name)?; - Ok((ThreadSafeRepository::discover(repo_path.path())?, repo_path)) + Ok(( + ThreadSafeRepository::discover(repo_path.path())?.to_thread_local(), + repo_path, + )) } fn easy_repo_rw(name: &str) -> crate::Result<(Repository, tempfile::TempDir)> {