Deduplicate GitSha and GitOid types (#10802)

## Summary

I think this split is leftover from using `libgit2`. I kept `Oid` since
that seems to be the official terminology.
This commit is contained in:
Charlie Marsh 2025-01-21 09:15:11 -05:00 committed by GitHub
parent 54bb5a38a4
commit 5c7fba86e1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 66 additions and 104 deletions

View file

@ -40,7 +40,7 @@ use uv_distribution_types::{
}; };
use uv_extract::hash::Hasher; use uv_extract::hash::Hasher;
use uv_fs::{rename_with_retry, write_atomic, LockedFile}; use uv_fs::{rename_with_retry, write_atomic, LockedFile};
use uv_git::{GitHubRepository, GitSha}; use uv_git::{GitHubRepository, GitOid};
use uv_metadata::read_archive_metadata; use uv_metadata::read_archive_metadata;
use uv_normalize::PackageName; use uv_normalize::PackageName;
use uv_pep440::{release_specifiers_to_ranges, Version}; use uv_pep440::{release_specifiers_to_ranges, Version};
@ -1783,7 +1783,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
/// Attempts to fetch the `pyproject.toml` from the resolved commit using the GitHub API. /// Attempts to fetch the `pyproject.toml` from the resolved commit using the GitHub API.
async fn github_metadata( async fn github_metadata(
&self, &self,
commit: GitSha, commit: GitOid,
source: &BuildableSource<'_>, source: &BuildableSource<'_>,
resource: &GitSourceUrl<'_>, resource: &GitSourceUrl<'_>,
client: &ManagedClient<'_>, client: &ManagedClient<'_>,

View file

@ -18,8 +18,7 @@ use uv_fs::Simplified;
use uv_static::EnvVars; use uv_static::EnvVars;
use uv_version::version; use uv_version::version;
use crate::sha::GitOid; use crate::{GitHubRepository, GitOid};
use crate::{GitHubRepository, GitSha};
/// A file indicates that if present, `git reset` has been done and a repo /// A file indicates that if present, `git reset` has been done and a repo
/// checkout is ready to go. See [`GitCheckout::reset`] for why we need this. /// checkout is ready to go. See [`GitCheckout::reset`] for why we need this.
@ -32,6 +31,7 @@ pub enum GitError {
#[error(transparent)] #[error(transparent)]
Other(#[from] which::Error), Other(#[from] which::Error),
} }
/// A global cache of the result of `which git`. /// A global cache of the result of `which git`.
pub static GIT: LazyLock<Result<PathBuf, GitError>> = LazyLock::new(|| { pub static GIT: LazyLock<Result<PathBuf, GitError>> = LazyLock::new(|| {
which::which("git").map_err(|e| match e { which::which("git").map_err(|e| match e {
@ -123,10 +123,10 @@ impl GitReference {
} }
} }
/// Returns the precise [`GitSha`] of this reference, if it's a full commit. /// Returns the precise [`GitOid`] of this reference, if it's a full commit.
pub(crate) fn as_sha(&self) -> Option<GitSha> { pub(crate) fn as_sha(&self) -> Option<GitOid> {
if let Self::FullCommit(rev) = self { if let Self::FullCommit(rev) = self {
Some(GitSha::from_str(rev).expect("Full commit should be exactly 40 characters")) Some(GitOid::from_str(rev).expect("Full commit should be exactly 40 characters"))
} else { } else {
None None
} }

View file

@ -3,17 +3,17 @@ use url::Url;
pub use crate::credentials::{store_credentials_from_url, GIT_STORE}; pub use crate::credentials::{store_credentials_from_url, GIT_STORE};
pub use crate::git::{GitReference, GIT}; pub use crate::git::{GitReference, GIT};
pub use crate::github::GitHubRepository; pub use crate::github::GitHubRepository;
pub use crate::oid::{GitOid, OidParseError};
pub use crate::resolver::{ pub use crate::resolver::{
GitResolver, GitResolverError, RepositoryReference, ResolvedRepositoryReference, GitResolver, GitResolverError, RepositoryReference, ResolvedRepositoryReference,
}; };
pub use crate::sha::{GitSha, OidParseError};
pub use crate::source::{Fetch, GitSource, Reporter}; pub use crate::source::{Fetch, GitSource, Reporter};
mod credentials; mod credentials;
mod git; mod git;
mod github; mod github;
mod oid;
mod resolver; mod resolver;
mod sha;
mod source; mod source;
/// A URL reference to a Git repository. /// A URL reference to a Git repository.
@ -25,7 +25,7 @@ pub struct GitUrl {
/// The reference to the commit to use, which could be a branch, tag or revision. /// The reference to the commit to use, which could be a branch, tag or revision.
reference: GitReference, reference: GitReference,
/// The precise commit to use, if known. /// The precise commit to use, if known.
precise: Option<GitSha>, precise: Option<GitOid>,
} }
impl GitUrl { impl GitUrl {
@ -40,7 +40,7 @@ impl GitUrl {
} }
/// Create a new [`GitUrl`] from a repository URL and a precise commit. /// Create a new [`GitUrl`] from a repository URL and a precise commit.
pub fn from_commit(repository: Url, reference: GitReference, precise: GitSha) -> Self { pub fn from_commit(repository: Url, reference: GitReference, precise: GitOid) -> Self {
Self { Self {
repository, repository,
reference, reference,
@ -48,9 +48,9 @@ impl GitUrl {
} }
} }
/// Set the precise [`GitSha`] to use for this Git URL. /// Set the precise [`GitOid`] to use for this Git URL.
#[must_use] #[must_use]
pub fn with_precise(mut self, precise: GitSha) -> Self { pub fn with_precise(mut self, precise: GitOid) -> Self {
self.precise = Some(precise); self.precise = Some(precise);
self self
} }
@ -73,7 +73,7 @@ impl GitUrl {
} }
/// Return the precise commit, if known. /// Return the precise commit, if known.
pub fn precise(&self) -> Option<GitSha> { pub fn precise(&self) -> Option<GitOid> {
self.precise self.precise
} }
} }

View file

@ -3,81 +3,25 @@ use std::str::{self, FromStr};
use thiserror::Error; use thiserror::Error;
/// A complete Git SHA, i.e., a 40-character hexadecimal representation of a Git commit.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct GitSha(GitOid);
impl GitSha {
/// Return the Git SHA as a string.
pub fn as_str(&self) -> &str {
self.0.as_str()
}
/// Return a truncated representation, i.e., the first 16 characters of the SHA.
pub fn as_short_str(&self) -> &str {
&self.0.as_str()[..16]
}
}
impl From<GitSha> for GitOid {
fn from(value: GitSha) -> Self {
value.0
}
}
impl From<GitOid> for GitSha {
fn from(value: GitOid) -> Self {
Self(value)
}
}
impl Display for GitSha {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", self.0)
}
}
impl FromStr for GitSha {
type Err = OidParseError;
fn from_str(value: &str) -> Result<Self, Self::Err> {
Ok(Self(GitOid::from_str(value)?))
}
}
impl serde::Serialize for GitSha {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
self.0.as_str().serialize(serializer)
}
}
impl<'de> serde::Deserialize<'de> for GitSha {
fn deserialize<D>(deserializer: D) -> Result<GitSha, D::Error>
where
D: serde::Deserializer<'de>,
{
let value = String::deserialize(deserializer)?;
GitSha::from_str(&value).map_err(serde::de::Error::custom)
}
}
/// Unique identity of any Git object (commit, tree, blob, tag). /// Unique identity of any Git object (commit, tree, blob, tag).
/// ///
/// Note this type does not validate whether the input is a valid hash. /// Note this type does not validate whether the input is a valid hash.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub(crate) struct GitOid { pub struct GitOid {
len: usize, len: usize,
bytes: [u8; 40], bytes: [u8; 40],
} }
impl GitOid { impl GitOid {
/// Return the string representation of an object ID. /// Return the string representation of an object ID.
pub(crate) fn as_str(&self) -> &str { pub fn as_str(&self) -> &str {
str::from_utf8(&self.bytes[..self.len]).unwrap() str::from_utf8(&self.bytes[..self.len]).unwrap()
} }
/// Return a truncated representation, i.e., the first 16 characters of the SHA.
pub fn as_short_str(&self) -> &str {
&self.as_str()[..16]
}
} }
#[derive(Debug, Error, PartialEq)] #[derive(Debug, Error, PartialEq)]
@ -116,6 +60,25 @@ impl Display for GitOid {
} }
} }
impl serde::Serialize for GitOid {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
self.as_str().serialize(serializer)
}
}
impl<'de> serde::Deserialize<'de> for GitOid {
fn deserialize<D>(deserializer: D) -> Result<GitOid, D::Error>
where
D: serde::Deserializer<'de>,
{
let value = String::deserialize(deserializer)?;
GitOid::from_str(&value).map_err(serde::de::Error::custom)
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::str::FromStr; use std::str::FromStr;

View file

@ -5,7 +5,7 @@ use std::sync::Arc;
use tracing::debug; use tracing::debug;
use crate::{Fetch, GitHubRepository, GitReference, GitSha, GitSource, GitUrl, Reporter}; use crate::{Fetch, GitHubRepository, GitOid, GitReference, GitSource, GitUrl, Reporter};
use dashmap::mapref::one::Ref; use dashmap::mapref::one::Ref;
use dashmap::DashMap; use dashmap::DashMap;
use fs_err::tokio as fs; use fs_err::tokio as fs;
@ -30,28 +30,28 @@ pub enum GitResolverError {
/// A resolver for Git repositories. /// A resolver for Git repositories.
#[derive(Default, Clone)] #[derive(Default, Clone)]
pub struct GitResolver(Arc<DashMap<RepositoryReference, GitSha>>); pub struct GitResolver(Arc<DashMap<RepositoryReference, GitOid>>);
impl GitResolver { impl GitResolver {
/// Inserts a new [`GitSha`] for the given [`RepositoryReference`]. /// Inserts a new [`GitOid`] for the given [`RepositoryReference`].
pub fn insert(&self, reference: RepositoryReference, sha: GitSha) { pub fn insert(&self, reference: RepositoryReference, sha: GitOid) {
self.0.insert(reference, sha); self.0.insert(reference, sha);
} }
/// Returns the [`GitSha`] for the given [`RepositoryReference`], if it exists. /// Returns the [`GitOid`] for the given [`RepositoryReference`], if it exists.
fn get(&self, reference: &RepositoryReference) -> Option<Ref<RepositoryReference, GitSha>> { fn get(&self, reference: &RepositoryReference) -> Option<Ref<RepositoryReference, GitOid>> {
self.0.get(reference) self.0.get(reference)
} }
/// Resolve a Git URL to a specific commit without performing any Git operations. /// Resolve a Git URL to a specific commit without performing any Git operations.
/// ///
/// Returns a [`GitSha`] if the URL has already been resolved (i.e., is available in the cache), /// Returns a [`GitOid`] if the URL has already been resolved (i.e., is available in the cache),
/// or if it can be fetched via the GitHub API. Otherwise, returns `None`. /// or if it can be fetched via the GitHub API. Otherwise, returns `None`.
pub async fn github_fast_path( pub async fn github_fast_path(
&self, &self,
url: &GitUrl, url: &GitUrl,
client: ClientWithMiddleware, client: ClientWithMiddleware,
) -> Result<Option<GitSha>, GitResolverError> { ) -> Result<Option<GitOid>, GitResolverError> {
let reference = RepositoryReference::from(url); let reference = RepositoryReference::from(url);
// If we know the precise commit already, return it. // If we know the precise commit already, return it.
@ -92,7 +92,7 @@ impl GitResolver {
// Parse the response as a Git SHA. // Parse the response as a Git SHA.
let precise = response.text().await?; let precise = response.text().await?;
let precise = let precise =
GitSha::from_str(&precise).map_err(|err| GitResolverError::Git(err.into()))?; GitOid::from_str(&precise).map_err(|err| GitResolverError::Git(err.into()))?;
// Insert the resolved URL into the in-memory cache. This ensures that subsequent fetches // Insert the resolved URL into the in-memory cache. This ensures that subsequent fetches
// resolve to the same precise commit. // resolve to the same precise commit.
@ -207,7 +207,7 @@ pub struct ResolvedRepositoryReference {
/// tag, or revision). /// tag, or revision).
pub reference: RepositoryReference, pub reference: RepositoryReference,
/// The precise commit SHA of the reference. /// The precise commit SHA of the reference.
pub sha: GitSha, pub sha: GitOid,
} }
#[derive(Debug, Clone, PartialEq, Eq, Hash)] #[derive(Debug, Clone, PartialEq, Eq, Hash)]

View file

@ -14,8 +14,7 @@ use url::Url;
use uv_cache_key::{cache_digest, RepositoryUrl}; use uv_cache_key::{cache_digest, RepositoryUrl};
use crate::git::GitRemote; use crate::git::GitRemote;
use crate::sha::GitOid; use crate::{GitOid, GitUrl, GIT_STORE};
use crate::{GitSha, GitUrl, GIT_STORE};
/// A remote Git source that can be checked out locally. /// A remote Git source that can be checked out locally.
pub struct GitSource { pub struct GitSource {
@ -74,7 +73,7 @@ impl GitSource {
let (db, actual_rev, task) = match (self.git.precise, remote.db_at(&db_path).ok()) { let (db, actual_rev, task) = match (self.git.precise, remote.db_at(&db_path).ok()) {
// If we have a locked revision, and we have a preexisting database // If we have a locked revision, and we have a preexisting database
// which has that revision, then no update needs to happen. // which has that revision, then no update needs to happen.
(Some(rev), Some(db)) if db.contains(rev.into()) => { (Some(rev), Some(db)) if db.contains(rev) => {
debug!("Using existing Git source `{}`", self.git.repository); debug!("Using existing Git source `{}`", self.git.repository);
(db, rev, None) (db, rev, None)
} }
@ -99,13 +98,13 @@ impl GitSource {
&self.client, &self.client,
)?; )?;
(db, GitSha::from(actual_rev), task) (db, actual_rev, task)
} }
}; };
// Dont use the full hash, in order to contribute less to reaching the // Dont use the full hash, in order to contribute less to reaching the
// path length limit on Windows. // path length limit on Windows.
let short_id = db.to_short_id(actual_rev.into())?; let short_id = db.to_short_id(actual_rev)?;
// Check out `actual_rev` from the database to a scoped location on the // Check out `actual_rev` from the database to a scoped location on the
// filesystem. This will use hard links and such to ideally make the // filesystem. This will use hard links and such to ideally make the
@ -116,7 +115,7 @@ impl GitSource {
.join(&ident) .join(&ident)
.join(short_id.as_str()); .join(short_id.as_str());
db.copy_to(actual_rev.into(), &checkout_path)?; db.copy_to(actual_rev, &checkout_path)?;
// Report the checkout operation to the reporter. // Report the checkout operation to the reporter.
if let Some(task) = task { if let Some(task) = task {

View file

@ -5,7 +5,7 @@ use thiserror::Error;
use url::{ParseError, Url}; use url::{ParseError, Url};
use uv_distribution_filename::{DistExtension, ExtensionError}; use uv_distribution_filename::{DistExtension, ExtensionError};
use uv_git::{GitReference, GitSha, GitUrl, OidParseError}; use uv_git::{GitOid, GitReference, GitUrl, OidParseError};
use uv_pep508::{ use uv_pep508::{
looks_like_git_repository, Pep508Url, UnnamedRequirementUrl, VerbatimUrl, VerbatimUrlError, looks_like_git_repository, Pep508Url, UnnamedRequirementUrl, VerbatimUrl, VerbatimUrlError,
}; };
@ -23,7 +23,7 @@ pub enum ParsedUrlError {
#[error("Invalid path in file URL: `{0}`")] #[error("Invalid path in file URL: `{0}`")]
InvalidFileUrl(String), InvalidFileUrl(String),
#[error("Failed to parse Git reference from URL: `{0}`")] #[error("Failed to parse Git reference from URL: `{0}`")]
GitShaParse(String, #[source] OidParseError), GitOidParse(String, #[source] OidParseError),
#[error("Not a valid URL: `{0}`")] #[error("Not a valid URL: `{0}`")]
UrlParse(String, #[source] ParseError), UrlParse(String, #[source] ParseError),
#[error(transparent)] #[error(transparent)]
@ -247,7 +247,7 @@ impl ParsedGitUrl {
pub fn from_source( pub fn from_source(
repository: Url, repository: Url,
reference: GitReference, reference: GitReference,
precise: Option<GitSha>, precise: Option<GitOid>,
subdirectory: Option<PathBuf>, subdirectory: Option<PathBuf>,
) -> Self { ) -> Self {
let url = if let Some(precise) = precise { let url = if let Some(precise) = precise {
@ -275,7 +275,7 @@ impl TryFrom<Url> for ParsedGitUrl {
.unwrap_or(url_in.as_str()); .unwrap_or(url_in.as_str());
let url = Url::parse(url).map_err(|err| ParsedUrlError::UrlParse(url.to_string(), err))?; let url = Url::parse(url).map_err(|err| ParsedUrlError::UrlParse(url.to_string(), err))?;
let url = GitUrl::try_from(url) let url = GitUrl::try_from(url)
.map_err(|err| ParsedUrlError::GitShaParse(url_in.to_string(), err))?; .map_err(|err| ParsedUrlError::GitOidParse(url_in.to_string(), err))?;
Ok(Self { url, subdirectory }) Ok(Self { url, subdirectory })
} }
} }

View file

@ -8,7 +8,7 @@ use url::Url;
use uv_distribution_filename::DistExtension; use uv_distribution_filename::DistExtension;
use uv_fs::{relative_to, PortablePath, PortablePathBuf, CWD}; use uv_fs::{relative_to, PortablePath, PortablePathBuf, CWD};
use uv_git::{GitReference, GitSha, GitUrl}; use uv_git::{GitOid, GitReference, GitUrl};
use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pep440::VersionSpecifiers; use uv_pep440::VersionSpecifiers;
use uv_pep508::{ use uv_pep508::{
@ -406,7 +406,7 @@ pub enum RequirementSource {
/// Optionally, the revision, tag, or branch to use. /// Optionally, the revision, tag, or branch to use.
reference: GitReference, reference: GitReference,
/// The precise commit to use, if known. /// The precise commit to use, if known.
precise: Option<GitSha>, precise: Option<GitOid>,
/// The path to the source distribution if it is not in the repository root. /// The path to the source distribution if it is not in the repository root.
subdirectory: Option<PathBuf>, subdirectory: Option<PathBuf>,
/// The PEP 508 style url in the format /// The PEP 508 style url in the format
@ -823,7 +823,7 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
}; };
} }
let precise = repository.fragment().map(GitSha::from_str).transpose()?; let precise = repository.fragment().map(GitOid::from_str).transpose()?;
// Clear out any existing state. // Clear out any existing state.
repository.set_fragment(None); repository.set_fragment(None);

View file

@ -31,7 +31,7 @@ use uv_distribution_types::{
RemoteSource, ResolvedDist, StaticMetadata, ToUrlError, UrlString, RemoteSource, ResolvedDist, StaticMetadata, ToUrlError, UrlString,
}; };
use uv_fs::{relative_to, PortablePath, PortablePathBuf}; use uv_fs::{relative_to, PortablePath, PortablePathBuf};
use uv_git::{GitReference, GitSha, RepositoryReference, ResolvedRepositoryReference}; use uv_git::{GitOid, GitReference, RepositoryReference, ResolvedRepositoryReference};
use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pep440::Version; use uv_pep440::Version;
use uv_pep508::{split_scheme, MarkerEnvironment, MarkerTree, VerbatimUrl, VerbatimUrlError}; use uv_pep508::{split_scheme, MarkerEnvironment, MarkerTree, VerbatimUrl, VerbatimUrlError};
@ -3192,7 +3192,7 @@ struct DirectSource {
/// canonical ordering of package entries. /// canonical ordering of package entries.
#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] #[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)]
struct GitSource { struct GitSource {
precise: GitSha, precise: GitOid,
subdirectory: Option<PathBuf>, subdirectory: Option<PathBuf>,
kind: GitSourceKind, kind: GitSourceKind,
} }
@ -3219,7 +3219,7 @@ impl GitSource {
_ => continue, _ => continue,
}; };
} }
let precise = GitSha::from_str(url.fragment().ok_or(GitSourceError::MissingSha)?) let precise = GitOid::from_str(url.fragment().ok_or(GitSourceError::MissingSha)?)
.map_err(|_| GitSourceError::InvalidSha)?; .map_err(|_| GitSourceError::InvalidSha)?;
Ok(GitSource { Ok(GitSource {
@ -3587,7 +3587,7 @@ fn locked_git_url(git_dist: &GitSourceDist) -> Url {
.git .git
.precise() .precise()
.as_ref() .as_ref()
.map(GitSha::to_string) .map(GitOid::to_string)
.as_deref(), .as_deref(),
); );