Remove SourceDistFilename from RegistrySourceDist (#3650)

## Summary

Uncertain about this, but we don't actually need the full
`SourceDistFilename`, only the name and version -- and we often have
that information already (as in the lockfile routines). So by flattening
the fields onto `RegistrySourceDist`, we can avoid re-parsing for
information we already have.
This commit is contained in:
Charlie Marsh 2024-05-20 09:25:23 -04:00 committed by GitHub
parent 1124df9bc5
commit 657eebd50b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 24 additions and 22 deletions

View file

@ -34,7 +34,7 @@ impl BuildableSource<'_> {
/// Return the [`Version`] of the source, if available. /// Return the [`Version`] of the source, if available.
pub fn version(&self) -> Option<&Version> { pub fn version(&self) -> Option<&Version> {
match self { match self {
Self::Dist(SourceDist::Registry(dist)) => Some(&dist.filename.version), Self::Dist(SourceDist::Registry(dist)) => Some(&dist.version),
Self::Dist(_) => None, Self::Dist(_) => None,
Self::Url(_) => None, Self::Url(_) => None,
} }

View file

@ -39,7 +39,7 @@ use std::str::FromStr;
use anyhow::Result; use anyhow::Result;
use url::Url; use url::Url;
use distribution_filename::{SourceDistFilename, WheelFilename}; use distribution_filename::WheelFilename;
use pep440_rs::Version; use pep440_rs::Version;
use pep508_rs::{Pep508Url, VerbatimUrl}; use pep508_rs::{Pep508Url, VerbatimUrl};
use uv_git::GitUrl; use uv_git::GitUrl;
@ -219,7 +219,8 @@ pub struct PathBuiltDist {
/// A source distribution that exists in a registry, like `PyPI`. /// A source distribution that exists in a registry, like `PyPI`.
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub struct RegistrySourceDist { pub struct RegistrySourceDist {
pub filename: SourceDistFilename, pub name: PackageName,
pub version: Version,
pub file: Box<File>, pub file: Box<File>,
pub index: IndexUrl, pub index: IndexUrl,
/// When an sdist is selected, it may be the case that there were /// When an sdist is selected, it may be the case that there were
@ -495,7 +496,7 @@ impl SourceDist {
pub fn version(&self) -> Option<&Version> { pub fn version(&self) -> Option<&Version> {
match self { match self {
Self::Registry(source_dist) => Some(&source_dist.filename.version), Self::Registry(source_dist) => Some(&source_dist.version),
Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None, Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None,
} }
} }
@ -551,7 +552,7 @@ impl Name for PathBuiltDist {
impl Name for RegistrySourceDist { impl Name for RegistrySourceDist {
fn name(&self) -> &PackageName { fn name(&self) -> &PackageName {
&self.filename.name &self.name
} }
} }
@ -636,7 +637,7 @@ impl DistributionMetadata for PathBuiltDist {
impl DistributionMetadata for RegistrySourceDist { impl DistributionMetadata for RegistrySourceDist {
fn version_or_url(&self) -> VersionOrUrlRef { fn version_or_url(&self) -> VersionOrUrlRef {
VersionOrUrlRef::Version(&self.filename.version) VersionOrUrlRef::Version(&self.version)
} }
} }

View file

@ -98,7 +98,7 @@ impl From<&ResolvedDist> for Requirement {
}, },
Dist::Source(SourceDist::Registry(sdist)) => RequirementSource::Registry { Dist::Source(SourceDist::Registry(sdist)) => RequirementSource::Registry {
specifier: pep440_rs::VersionSpecifiers::from( specifier: pep440_rs::VersionSpecifiers::from(
pep440_rs::VersionSpecifier::equals_version(sdist.filename.version.clone()), pep440_rs::VersionSpecifier::equals_version(sdist.version.clone()),
), ),
index: None, index: None,
}, },

View file

@ -61,7 +61,8 @@ impl ResolvedDistRef<'_> {
// has an sdist, so this always succeeds. // has an sdist, so this always succeeds.
let source = prioritized.source_dist().expect("a source distribution"); let source = prioritized.source_dist().expect("a source distribution");
assert_eq!( assert_eq!(
sdist.filename, source.filename, (&sdist.name, &sdist.version),
(&source.name, &source.version),
"expected chosen sdist to match prioritized sdist" "expected chosen sdist to match prioritized sdist"
); );
ResolvedDist::Installable(Dist::Source(SourceDist::Registry(source))) ResolvedDist::Installable(Dist::Source(SourceDist::Registry(source)))

View file

@ -93,8 +93,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
let cache_shard = self.build_context.cache().shard( let cache_shard = self.build_context.cache().shard(
CacheBucket::BuiltWheels, CacheBucket::BuiltWheels,
WheelCache::Index(&dist.index) WheelCache::Index(&dist.index)
.wheel_dir(dist.filename.name.as_ref()) .wheel_dir(dist.name.as_ref())
.join(dist.filename.version.to_string()), .join(dist.version.to_string()),
); );
let url = match &dist.file.url { let url = match &dist.file.url {
@ -250,8 +250,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
let cache_shard = self.build_context.cache().shard( let cache_shard = self.build_context.cache().shard(
CacheBucket::BuiltWheels, CacheBucket::BuiltWheels,
WheelCache::Index(&dist.index) WheelCache::Index(&dist.index)
.wheel_dir(dist.filename.name.as_ref()) .wheel_dir(dist.name.as_ref())
.join(dist.filename.version.to_string()), .join(dist.version.to_string()),
); );
let url = match &dist.file.url { let url = match &dist.file.url {

View file

@ -98,7 +98,8 @@ impl FlatIndex {
let compatibility = let compatibility =
Self::source_dist_compatibility(&filename, &file.hashes, hasher, no_build); Self::source_dist_compatibility(&filename, &file.hashes, hasher, no_build);
let dist = RegistrySourceDist { let dist = RegistrySourceDist {
filename: filename.clone(), name: filename.name.clone(),
version: filename.version.clone(),
file: Box::new(file), file: Box::new(file),
index, index,
wheels: vec![], wheels: vec![],

View file

@ -9,7 +9,7 @@ use std::str::FromStr;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use url::Url; use url::Url;
use distribution_filename::{SourceDistFilename, WheelFilename}; use distribution_filename::WheelFilename;
use distribution_types::{ use distribution_types::{
BuiltDist, DirectUrlBuiltDist, DirectUrlSourceDist, DirectorySourceDist, Dist, FileLocation, BuiltDist, DirectUrlBuiltDist, DirectUrlSourceDist, DirectorySourceDist, Dist, FileLocation,
GitSourceDist, IndexUrl, ParsedArchiveUrl, ParsedGitUrl, PathBuiltDist, PathSourceDist, GitSourceDist, IndexUrl, ParsedArchiveUrl, ParsedGitUrl, PathBuiltDist, PathSourceDist,
@ -339,13 +339,9 @@ impl Distribution {
Dist::Source(source_dist) Dist::Source(source_dist)
} }
SourceKind::Registry => { SourceKind::Registry => {
// TODO(charlie): Introduce an error type to the conversion functions.
let filename =
SourceDistFilename::parse(&sdist.url.filename().unwrap(), &self.id.name)
.unwrap();
let file = Box::new(distribution_types::File { let file = Box::new(distribution_types::File {
dist_info_metadata: false, dist_info_metadata: false,
filename: filename.to_string(), filename: sdist.url.filename().unwrap().to_string(),
hashes: vec![], hashes: vec![],
requires_python: None, requires_python: None,
size: sdist.size, size: sdist.size,
@ -355,7 +351,8 @@ impl Distribution {
}); });
let index = IndexUrl::Url(VerbatimUrl::from_url(self.id.source.url.clone())); let index = IndexUrl::Url(VerbatimUrl::from_url(self.id.source.url.clone()));
let reg_dist = RegistrySourceDist { let reg_dist = RegistrySourceDist {
filename, name: self.id.name.clone(),
version: self.id.version.clone(),
file, file,
index, index,
wheels: vec![], wheels: vec![],

View file

@ -1468,7 +1468,8 @@ impl<'a> From<ResolvedDistRef<'a>> for Request {
// has an sdist, so this always succeeds. // has an sdist, so this always succeeds.
let source = prioritized.source_dist().expect("a source distribution"); let source = prioritized.source_dist().expect("a source distribution");
assert_eq!( assert_eq!(
sdist.filename, source.filename, (&sdist.name, &sdist.version),
(&source.name, &source.version),
"expected chosen sdist to match prioritized sdist" "expected chosen sdist to match prioritized sdist"
); );
Request::Dist(Dist::Source(SourceDist::Registry(source))) Request::Dist(Dist::Source(SourceDist::Registry(source)))

View file

@ -413,7 +413,8 @@ impl VersionMapLazy {
upload_time, upload_time,
); );
let dist = RegistrySourceDist { let dist = RegistrySourceDist {
filename, name: filename.name.clone(),
version: filename.version.clone(),
file: Box::new(file), file: Box::new(file),
index: self.index.clone(), index: self.index.clone(),
wheels: vec![], wheels: vec![],