Avoid enforcing URL correctness for installed distributions (#1793)

## Summary

Allows the corresponding `pypi_types` struct to use any URL, since other
installers can put those into the environment, and Poetry seems to write
invalid URLs.

If we see a distribution with an invalid URL, we just treat it as a
registry distribution, which isn't ideal, but is better than (1)
erroring, and (2) changing `Url` to `String` everywhere internally. (I'm
torn on this second option.)

Closes https://github.com/astral-sh/uv/issues/1744.

## Test Plan

- Added `flask = { git = "git@github.com:pallets/flask.git", rev =
"b90a4f1f4a370e92054b9cc9db0efcb864f87ebe" }` to
`scripts/editable-installs/poetry_editable/pyproject.toml`.
- Ran `poetry install`.
- Ran `cargo pip freeze`. Verified that it errored on `main`, but passed
here.
- Ran `cargo run pip install "flask==3.0.0"`. Verified that it
uninstalled the existing Flask, and installed a new version from the
registry.
This commit is contained in:
Charlie Marsh 2024-02-21 09:06:31 -05:00 committed by GitHub
parent 0f520d8716
commit a2a1b2fb0f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 38 additions and 23 deletions

1
Cargo.lock generated
View File

@ -906,6 +906,7 @@ dependencies = [
"serde_json",
"sha2",
"thiserror",
"tracing",
"url",
"urlencoding",
"uv-fs",

View File

@ -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 }

View File

@ -132,7 +132,7 @@ impl TryFrom<&LocalFileUrl> for pypi_types::DirectUrl {
fn try_from(value: &LocalFileUrl) -> Result<Self, Self::Error> {
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<Self, Self::Error> {
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<Self, Self::Error> {
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),

View File

@ -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,

View File

@ -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<PathBuf>,
@ -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<PathBuf>,
@ -83,36 +83,38 @@ impl std::fmt::Display for VcsKind {
}
}
impl From<DirectUrl> for Url {
fn from(value: DirectUrl) -> Self {
impl TryFrom<&DirectUrl> for Url {
type Error = url::ParseError;
fn try_from(value: &DirectUrl) -> Result<Self, Self::Error> {
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)
}
}
}