From e8d20832e258f49ddd4908f8833754b9eaee233b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 20 Feb 2024 23:12:55 -0500 Subject: [PATCH] Avoid enforcing URL correctness for installed distributions --- Cargo.lock | 1 + crates/distribution-types/Cargo.toml | 1 + crates/distribution-types/src/direct_url.rs | 6 ++--- crates/distribution-types/src/installed.rs | 25 ++++++++++++------ crates/pypi-types/src/direct_url.rs | 28 +++++++++++---------- 5 files changed, 38 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 663866504027e..6e4e16a50891b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -906,6 +906,7 @@ dependencies = [ "serde_json", "sha2", "thiserror", + "tracing", "url", "urlencoding", "uv-fs", diff --git a/crates/distribution-types/Cargo.toml b/crates/distribution-types/Cargo.toml index 1e60c796c997f..00c1274402fb1 100644 --- a/crates/distribution-types/Cargo.toml +++ b/crates/distribution-types/Cargo.toml @@ -34,5 +34,6 @@ serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sha2 = { workspace = true } thiserror = { workspace = true } +tracing = { workspace = true } url = { workspace = true } urlencoding = { workspace = true } diff --git a/crates/distribution-types/src/direct_url.rs b/crates/distribution-types/src/direct_url.rs index 87bba177c2f34..c05144b639319 100644 --- a/crates/distribution-types/src/direct_url.rs +++ b/crates/distribution-types/src/direct_url.rs @@ -132,7 +132,7 @@ impl TryFrom<&LocalFileUrl> for pypi_types::DirectUrl { fn try_from(value: &LocalFileUrl) -> Result { Ok(pypi_types::DirectUrl::LocalDirectory { - url: value.url.clone(), + url: value.url.to_string(), dir_info: pypi_types::DirInfo { editable: value.editable.then_some(true), }, @@ -145,7 +145,7 @@ impl TryFrom<&DirectArchiveUrl> for pypi_types::DirectUrl { fn try_from(value: &DirectArchiveUrl) -> Result { Ok(pypi_types::DirectUrl::ArchiveUrl { - url: value.url.clone(), + url: value.url.to_string(), archive_info: pypi_types::ArchiveInfo { hash: None, hashes: None, @@ -160,7 +160,7 @@ impl TryFrom<&DirectGitUrl> for pypi_types::DirectUrl { fn try_from(value: &DirectGitUrl) -> Result { Ok(pypi_types::DirectUrl::VcsUrl { - url: value.url.repository().clone(), + url: value.url.repository().to_string(), vcs_info: pypi_types::VcsInfo { vcs: pypi_types::VcsKind::Git, commit_id: value.url.precise().as_ref().map(ToString::to_string), diff --git a/crates/distribution-types/src/installed.rs b/crates/distribution-types/src/installed.rs index 56ac0aaacf171..271e26d9757dc 100644 --- a/crates/distribution-types/src/installed.rs +++ b/crates/distribution-types/src/installed.rs @@ -3,6 +3,7 @@ use std::str::FromStr; use anyhow::{anyhow, Context, Result}; use fs_err as fs; +use tracing::warn; use url::Url; use pep440_rs::Version; @@ -55,13 +56,23 @@ impl InstalledDist { let name = PackageName::from_str(name)?; let version = Version::from_str(version).map_err(|err| anyhow!(err))?; return if let Some(direct_url) = Self::direct_url(path)? { - Ok(Some(Self::Url(InstalledDirectUrlDist { - name, - version, - editable: matches!(&direct_url, pypi_types::DirectUrl::LocalDirectory { dir_info, .. } if dir_info.editable == Some(true)), - url: Url::from(direct_url), - path: path.to_path_buf(), - }))) + match Url::try_from(&direct_url) { + Ok(url) => Ok(Some(Self::Url(InstalledDirectUrlDist { + name, + version, + editable: matches!(&direct_url, pypi_types::DirectUrl::LocalDirectory { dir_info, .. } if dir_info.editable == Some(true)), + url, + path: path.to_path_buf(), + }))), + Err(err) => { + warn!("Failed to parse direct URL: {err}"); + Ok(Some(Self::Registry(InstalledRegistryDist { + name, + version, + path: path.to_path_buf(), + }))) + } + } } else { Ok(Some(Self::Registry(InstalledRegistryDist { name, diff --git a/crates/pypi-types/src/direct_url.rs b/crates/pypi-types/src/direct_url.rs index b99dd7275e19c..4bf13773dc125 100644 --- a/crates/pypi-types/src/direct_url.rs +++ b/crates/pypi-types/src/direct_url.rs @@ -14,13 +14,13 @@ pub enum DirectUrl { /// ```json /// {"url": "file:///home/user/project", "dir_info": {}} /// ``` - LocalDirectory { url: Url, dir_info: DirInfo }, + LocalDirectory { url: String, dir_info: DirInfo }, /// The direct URL is a path to an archive. For example: /// ```json /// {"archive_info": {"hash": "sha256=75909db2664838d015e3d9139004ee16711748a52c8f336b52882266540215d8", "hashes": {"sha256": "75909db2664838d015e3d9139004ee16711748a52c8f336b52882266540215d8"}}, "url": "https://files.pythonhosted.org/packages/b8/8b/31273bf66016be6ad22bb7345c37ff350276cfd46e389a0c2ac5da9d9073/wheel-0.41.2-py3-none-any.whl"} /// ``` ArchiveUrl { - url: Url, + url: String, archive_info: ArchiveInfo, #[serde(skip_serializing_if = "Option::is_none")] subdirectory: Option, @@ -30,7 +30,7 @@ pub enum DirectUrl { /// {"url": "https://github.com/pallets/flask.git", "vcs_info": {"commit_id": "8d9519df093864ff90ca446d4af2dc8facd3c542", "vcs": "git"}} /// ``` VcsUrl { - url: Url, + url: String, vcs_info: VcsInfo, #[serde(skip_serializing_if = "Option::is_none")] subdirectory: Option, @@ -83,36 +83,38 @@ impl std::fmt::Display for VcsKind { } } -impl From for Url { - fn from(value: DirectUrl) -> Self { +impl TryFrom<&DirectUrl> for Url { + type Error = url::ParseError; + + fn try_from(value: &DirectUrl) -> Result { match value { - DirectUrl::LocalDirectory { url, .. } => url, + DirectUrl::LocalDirectory { url, .. } => Url::parse(&url), DirectUrl::ArchiveUrl { - mut url, + url, subdirectory, archive_info: _, } => { + let mut url = Url::parse(&url)?; if let Some(subdirectory) = subdirectory { url.set_fragment(Some(&format!("subdirectory={}", subdirectory.display()))); } - url + Ok(url) } DirectUrl::VcsUrl { url, vcs_info, subdirectory, } => { - let mut url = - Url::parse(&format!("{}+{}", vcs_info.vcs, url)).expect("VCS URL is invalid"); - if let Some(commit_id) = vcs_info.commit_id { + let mut url = Url::parse(&format!("{}+{}", vcs_info.vcs, url))?; + if let Some(commit_id) = &vcs_info.commit_id { url.set_path(&format!("{}@{commit_id}", url.path())); - } else if let Some(requested_revision) = vcs_info.requested_revision { + } else if let Some(requested_revision) = &vcs_info.requested_revision { url.set_path(&format!("{}@{requested_revision}", url.path())); } if let Some(subdirectory) = subdirectory { url.set_fragment(Some(&format!("subdirectory={}", subdirectory.display()))); } - url + Ok(url) } } }