Skip to content

Commit

Permalink
Lift requirement that .egg-info filenames must include version (#6179)
Browse files Browse the repository at this point in the history 
## Summary

PR #4533 introduced (almost) spec compliant parsing of `.egg-info`
filenames, but added the overly strict requirement that the distribution
version must be present. This causes various `uv pip` operations to fail
in environments where there are `.egg-info` files without a version
component, so loosen this check by making the version component optional
and reading the version from the egg metadata when it is not present.

As an example of the issue, running `uv pip list` on my system currently
results in
```
error: Failed to read metadata from: `/usr/lib/python3.12/site-packages/PySide6.egg-info`
  Caused by: The `.egg-info` filename "PySide6.egg-info" is missing a version
```
whereas regular `pip list` succeeds:
```
$ pip list | rg -S pyside
PySide6                   6.7.2
```

## Test Plan

This has been tested by altering the `.egg-info` filename tests as
needed and ensuring the full test suite passes locally.
  • Loading branch information
@severen
severen committed Aug 18, 2024
1 parent 53159b5 commit f8bda46
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 38 deletions.
45 changes: 25 additions & 20 deletions crates/distribution-filename/src/egg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ pub enum EggInfoFilenameError {
InvalidExtension(String),
#[error("The `.egg-info` filename \"{0}\" is missing a package name")]
MissingPackageName(String),
#[error("The `.egg-info` filename \"{0}\" is missing a version")]
MissingVersion(String),
#[error("The `.egg-info` filename \"{0}\" has an invalid package name")]
InvalidPackageName(String, InvalidNameError),
#[error("The `.egg-info` filename \"{0}\" has an invalid version: {1}")]
Expand All @@ -31,11 +29,11 @@ pub enum EggInfoFilenameError {
#[derive(Debug, Clone)]
pub struct EggInfoFilename {
pub name: PackageName,
pub version: Version,
pub version: Option<Version>,
}

impl EggInfoFilename {
/// Parse an `.egg-info` filename, requiring at least a name and version.
/// Parse an `.egg-info` filename, requiring at least a name.
pub fn parse(stem: &str) -> Result<Self, EggInfoFilenameError> {
// pip uses the following regex:
// ```python
Expand All @@ -56,13 +54,16 @@ impl EggInfoFilename {
let name = parts
.next()
.ok_or_else(|| EggInfoFilenameError::MissingPackageName(format!("{stem}.egg-info")))?;
let version = parts
.next()
.ok_or_else(|| EggInfoFilenameError::MissingVersion(format!("{stem}.egg-info")))?;
let name = PackageName::from_str(name)
.map_err(|e| EggInfoFilenameError::InvalidPackageName(format!("{stem}.egg-info"), e))?;
let version = Version::from_str(version)
.map_err(|e| EggInfoFilenameError::InvalidVersion(format!("{stem}.egg-info"), e))?;
let version = parts
.next()
.map(|s| {
Version::from_str(s).map_err(|e| {
EggInfoFilenameError::InvalidVersion(format!("{stem}.egg-info"), e)
})
})
.transpose()?;
Ok(Self { name, version })
}
}
Expand All @@ -87,26 +88,30 @@ mod tests {
let filename = "zstandard-0.22.0-py3.12-darwin.egg-info";
let parsed = EggInfoFilename::from_str(filename).unwrap();
assert_eq!(parsed.name.as_ref(), "zstandard");
assert_eq!(parsed.version.to_string(), "0.22.0");
assert_eq!(
parsed.version.map(|v| v.to_string()),
Some("0.22.0".to_string())
);

let filename = "zstandard-0.22.0-py3.12.egg-info";
let parsed = EggInfoFilename::from_str(filename).unwrap();
assert_eq!(parsed.name.as_ref(), "zstandard");
assert_eq!(parsed.version.to_string(), "0.22.0");
assert_eq!(
parsed.version.map(|v| v.to_string()),
Some("0.22.0".to_string())
);

let filename = "zstandard-0.22.0.egg-info";
let parsed = EggInfoFilename::from_str(filename).unwrap();
assert_eq!(parsed.name.as_ref(), "zstandard");
assert_eq!(parsed.version.to_string(), "0.22.0");
}

#[test]
fn egg_info_filename_missing_version() {
let filename = "zstandard.egg-info";
let err = EggInfoFilename::from_str(filename).unwrap_err();
assert_eq!(
err.to_string(),
"The `.egg-info` filename \"zstandard.egg-info\" is missing a version"
parsed.version.map(|v| v.to_string()),
Some("0.22.0".to_string())
);

let filename = "zstandard.egg-info";
let parsed = EggInfoFilename::from_str(filename).unwrap();
assert_eq!(parsed.name.as_ref(), "zstandard");
assert!(parsed.version.is_none());
}
}
68 changes: 50 additions & 18 deletions crates/distribution-types/src/installed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,42 @@ impl InstalledDist {
};
let file_name = EggInfoFilename::parse(file_stem)?;

if let Some(version) = file_name.version {
if metadata.is_dir() {
return Ok(Some(Self::EggInfoDirectory(InstalledEggInfoDirectory {
name: file_name.name,
version,
path: path.to_path_buf(),
})));
}

if metadata.is_file() {
return Ok(Some(Self::EggInfoFile(InstalledEggInfoFile {
name: file_name.name,
version,
path: path.to_path_buf(),
})));
}
};

if metadata.is_dir() {
let Some(egg_metadata) = read_metadata(&path.join("PKG-INFO")) else {
return Ok(None);
};
return Ok(Some(Self::EggInfoDirectory(InstalledEggInfoDirectory {
name: file_name.name,
version: file_name.version,
version: Version::from_str(&egg_metadata.version)?,
path: path.to_path_buf(),
})));
}

if metadata.is_file() {
return Ok(Some(Self::EggInfoFile(InstalledEggInfoFile {
let Some(egg_metadata) = read_metadata(path) else {
return Ok(None);
};
return Ok(Some(Self::EggInfoDirectory(InstalledEggInfoDirectory {
name: file_name.name,
version: file_name.version,
version: Version::from_str(&egg_metadata.version)?,
path: path.to_path_buf(),
})));
}
Expand Down Expand Up @@ -189,24 +213,13 @@ impl InstalledDist {
.map_err(|()| anyhow!("Invalid `.egg-link` target: {}", target.user_display()))?;

// Mildly unfortunate that we must read metadata to get the version.
let content = match fs::read(egg_info.join("PKG-INFO")) {
Ok(content) => content,
Err(err) => {
warn!("Failed to read metadata for {path:?}: {err}");
return Ok(None);
}
};
let metadata = match pypi_types::Metadata10::parse_pkg_info(&content) {
Ok(metadata) => metadata,
Err(err) => {
warn!("Failed to parse metadata for {path:?}: {err}");
return Ok(None);
}
let Some(egg_metadata) = read_metadata(&egg_info.join("PKG-INFO")) else {
return Ok(None);
};

return Ok(Some(Self::LegacyEditable(InstalledLegacyEditable {
name: metadata.name,
version: Version::from_str(&metadata.version)?,
name: egg_metadata.name,
version: Version::from_str(&egg_metadata.version)?,
egg_link: path.to_path_buf(),
target,
target_url: url,
Expand Down Expand Up @@ -411,3 +424,22 @@ impl InstalledMetadata for InstalledDist {
}
}
}

fn read_metadata(path: &Path) -> Option<pypi_types::Metadata10> {
let content = match fs::read(path) {
Ok(content) => content,
Err(err) => {
warn!("Failed to read metadata for {path:?}: {err}");
return None;
}
};
let metadata = match pypi_types::Metadata10::parse_pkg_info(&content) {
Ok(metadata) => metadata,
Err(err) => {
warn!("Failed to parse metadata for {path:?}: {err}");
return None;
}
};

Some(metadata)
}

0 comments on commit f8bda46

Please sign in to comment.