Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Directory its own distribution kind #3519

Merged
merged 1 commit into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion crates/distribution-types/src/buildable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use url::Url;

use uv_normalize::PackageName;

use crate::{GitSourceDist, Name, PathSourceDist, SourceDist};
use crate::{DirectorySourceDist, GitSourceDist, Name, PathSourceDist, SourceDist};

/// A reference to a source that can be built into a built distribution.
///
Expand Down Expand Up @@ -62,6 +62,7 @@ pub enum SourceUrl<'a> {
Direct(DirectSourceUrl<'a>),
Git(GitSourceUrl<'a>),
Path(PathSourceUrl<'a>),
Directory(DirectorySourceUrl<'a>),
}

impl<'a> SourceUrl<'a> {
Expand All @@ -71,6 +72,7 @@ impl<'a> SourceUrl<'a> {
Self::Direct(dist) => dist.url,
Self::Git(dist) => dist.url,
Self::Path(dist) => dist.url,
Self::Directory(dist) => dist.url,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of wish the names were like... Directory and Archive? But Archive is a bit strange.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like Archive, pip uses that terminology too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Archive in this context referring to a specific file like a .tar.gz? If that's always the case, then I like the name Archive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it needs to be a .tar.gz or .zip or .tar.bz2 or a few other extensions that we support.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that it's a perfect name because (e.g.) when you provide Direct (i.e., a direct URL), that has to be a direct URL to... an archive. So they're both archives?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly Self::File could be the right name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's a good point. I suppose PathArchive would also work and is maybe a bit more descriptive. But I agree that File also works. And it's more succinct.

}
}
}
Expand All @@ -81,6 +83,7 @@ impl std::fmt::Display for SourceUrl<'_> {
Self::Direct(url) => write!(f, "{url}"),
Self::Git(url) => write!(f, "{url}"),
Self::Path(url) => write!(f, "{url}"),
Self::Directory(url) => write!(f, "{url}"),
}
}
}
Expand Down Expand Up @@ -133,3 +136,24 @@ impl<'a> From<&'a PathSourceDist> for PathSourceUrl<'a> {
}
}
}

#[derive(Debug, Clone)]
pub struct DirectorySourceUrl<'a> {
pub url: &'a Url,
pub path: Cow<'a, Path>,
}

impl std::fmt::Display for DirectorySourceUrl<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{url}", url = self.url)
}
}

impl<'a> From<&'a DirectorySourceDist> for DirectorySourceUrl<'a> {
fn from(dist: &'a DirectorySourceDist) -> Self {
Self {
url: &dist.url,
path: Cow::Borrowed(&dist.path),
}
}
}
7 changes: 7 additions & 0 deletions crates/distribution-types/src/cached.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ impl CachedDist {
editable: false,
}),
Dist::Source(SourceDist::Path(dist)) => Self::Url(CachedDirectUrlDist {
filename,
url: dist.url,
hashes,
path,
editable: false,
}),
Dist::Source(SourceDist::Directory(dist)) => Self::Url(CachedDirectUrlDist {
filename,
url: dist.url,
hashes,
Expand Down
87 changes: 79 additions & 8 deletions crates/distribution-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ pub enum SourceDist {
DirectUrl(DirectUrlSourceDist),
Git(GitSourceDist),
Path(PathSourceDist),
Directory(DirectorySourceDist),
}

/// A built distribution (wheel) that exists in a registry, like `PyPI`.
Expand Down Expand Up @@ -203,12 +204,20 @@ pub struct GitSourceDist {
pub url: VerbatimUrl,
}

/// A source distribution that exists in a local directory.
/// A source distribution that exists in a local archive (e.g., a `.tar.gz` file).
#[derive(Debug, Clone)]
pub struct PathSourceDist {
pub name: PackageName,
pub url: VerbatimUrl,
pub path: PathBuf,
}

/// A source distribution that exists in a local directory.
#[derive(Debug, Clone)]
pub struct DirectorySourceDist {
pub name: PackageName,
pub url: VerbatimUrl,
pub path: PathBuf,
pub editable: bool,
}

Expand Down Expand Up @@ -281,7 +290,15 @@ impl Dist {
Err(err) => return Err(err.into()),
};

if path
// Determine whether the path represents an archive or a directory.
if path.is_dir() {
Ok(Self::Source(SourceDist::Directory(DirectorySourceDist {
name,
url,
path,
editable,
})))
} else if path
.extension()
.is_some_and(|ext| ext.eq_ignore_ascii_case("whl"))
{
Expand All @@ -305,11 +322,14 @@ impl Dist {
path,
})))
} else {
if editable {
return Err(Error::EditableFile(url));
}

Ok(Self::Source(SourceDist::Path(PathSourceDist {
name,
url,
path,
editable,
})))
}
}
Expand Down Expand Up @@ -382,7 +402,7 @@ impl Dist {
/// Create a [`Dist`] for a local editable distribution.
pub fn from_editable(name: PackageName, editable: LocalEditable) -> Result<Self, Error> {
let LocalEditable { url, path, .. } = editable;
Ok(Self::Source(SourceDist::Path(PathSourceDist {
Ok(Self::Source(SourceDist::Directory(DirectorySourceDist {
name,
url,
path,
Expand Down Expand Up @@ -454,22 +474,22 @@ impl SourceDist {
pub fn index(&self) -> Option<&IndexUrl> {
match self {
Self::Registry(registry) => Some(&registry.index),
Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) => None,
Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None,
}
}

/// Returns the [`File`] instance, if this dist is from a registry with simple json api support
pub fn file(&self) -> Option<&File> {
match self {
Self::Registry(registry) => Some(&registry.file),
Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) => None,
Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None,
}
}

pub fn version(&self) -> Option<&Version> {
match self {
Self::Registry(source_dist) => Some(&source_dist.filename.version),
Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) => None,
Self::DirectUrl(_) | Self::Git(_) | Self::Path(_) | Self::Directory(_) => None,
}
}

Expand All @@ -487,7 +507,7 @@ impl SourceDist {
/// Return true if the distribution is editable.
pub fn is_editable(&self) -> bool {
match self {
Self::Path(PathSourceDist { editable, .. }) => *editable,
Self::Directory(DirectorySourceDist { editable, .. }) => *editable,
_ => false,
}
}
Expand All @@ -496,6 +516,7 @@ impl SourceDist {
pub fn as_path(&self) -> Option<&Path> {
match self {
Self::Path(dist) => Some(&dist.path),
Self::Directory(dist) => Some(&dist.path),
_ => None,
}
}
Expand Down Expand Up @@ -543,13 +564,20 @@ impl Name for PathSourceDist {
}
}

impl Name for DirectorySourceDist {
fn name(&self) -> &PackageName {
&self.name
}
}

impl Name for SourceDist {
fn name(&self) -> &PackageName {
match self {
Self::Registry(dist) => dist.name(),
Self::DirectUrl(dist) => dist.name(),
Self::Git(dist) => dist.name(),
Self::Path(dist) => dist.name(),
Self::Directory(dist) => dist.name(),
}
}
}
Expand Down Expand Up @@ -615,13 +643,20 @@ impl DistributionMetadata for PathSourceDist {
}
}

impl DistributionMetadata for DirectorySourceDist {
fn version_or_url(&self) -> VersionOrUrlRef {
VersionOrUrlRef::Url(&self.url)
}
}

impl DistributionMetadata for SourceDist {
fn version_or_url(&self) -> VersionOrUrlRef {
match self {
Self::Registry(dist) => dist.version_or_url(),
Self::DirectUrl(dist) => dist.version_or_url(),
Self::Git(dist) => dist.version_or_url(),
Self::Path(dist) => dist.version_or_url(),
Self::Directory(dist) => dist.version_or_url(),
}
}
}
Expand Down Expand Up @@ -760,13 +795,24 @@ impl RemoteSource for PathSourceDist {
}
}

impl RemoteSource for DirectorySourceDist {
fn filename(&self) -> Result<Cow<'_, str>, Error> {
self.url.filename()
}

fn size(&self) -> Option<u64> {
self.url.size()
}
}

impl RemoteSource for SourceDist {
fn filename(&self) -> Result<Cow<'_, str>, Error> {
match self {
Self::Registry(dist) => dist.filename(),
Self::DirectUrl(dist) => dist.filename(),
Self::Git(dist) => dist.filename(),
Self::Path(dist) => dist.filename(),
Self::Directory(dist) => dist.filename(),
}
}

Expand All @@ -776,6 +822,7 @@ impl RemoteSource for SourceDist {
Self::DirectUrl(dist) => dist.size(),
Self::Git(dist) => dist.size(),
Self::Path(dist) => dist.size(),
Self::Directory(dist) => dist.size(),
}
}
}
Expand Down Expand Up @@ -934,6 +981,16 @@ impl Identifier for PathSourceDist {
}
}

impl Identifier for DirectorySourceDist {
fn distribution_id(&self) -> DistributionId {
self.url.distribution_id()
}

fn resource_id(&self) -> ResourceId {
self.url.resource_id()
}
}

impl Identifier for GitSourceDist {
fn distribution_id(&self) -> DistributionId {
self.url.distribution_id()
Expand All @@ -951,6 +1008,7 @@ impl Identifier for SourceDist {
Self::DirectUrl(dist) => dist.distribution_id(),
Self::Git(dist) => dist.distribution_id(),
Self::Path(dist) => dist.distribution_id(),
Self::Directory(dist) => dist.distribution_id(),
}
}

Expand All @@ -960,6 +1018,7 @@ impl Identifier for SourceDist {
Self::DirectUrl(dist) => dist.resource_id(),
Self::Git(dist) => dist.resource_id(),
Self::Path(dist) => dist.resource_id(),
Self::Directory(dist) => dist.resource_id(),
}
}
}
Expand Down Expand Up @@ -1038,12 +1097,23 @@ impl Identifier for PathSourceUrl<'_> {
}
}

impl Identifier for DirectorySourceUrl<'_> {
fn distribution_id(&self) -> DistributionId {
self.url.distribution_id()
}

fn resource_id(&self) -> ResourceId {
self.url.resource_id()
}
}

impl Identifier for SourceUrl<'_> {
fn distribution_id(&self) -> DistributionId {
match self {
Self::Direct(url) => url.distribution_id(),
Self::Git(url) => url.distribution_id(),
Self::Path(url) => url.distribution_id(),
Self::Directory(url) => url.distribution_id(),
}
}

Expand All @@ -1052,6 +1122,7 @@ impl Identifier for SourceUrl<'_> {
Self::Direct(url) => url.resource_id(),
Self::Git(url) => url.resource_id(),
Self::Path(url) => url.resource_id(),
Self::Directory(url) => url.resource_id(),
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions crates/distribution-types/src/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,11 @@ impl From<&ResolvedDist> for Requirement {
url: sdist.url.clone(),
editable: None,
},
Dist::Source(SourceDist::Directory(sdist)) => RequirementSource::Path {
path: sdist.path.clone(),
url: sdist.url.clone(),
editable: None,
},
},
ResolvedDist::Installed(dist) => RequirementSource::Registry {
specifier: pep440_rs::VersionSpecifiers::from(
Expand Down
Loading
Loading