Skip to content

Commit

Permalink
Avoid deserialization error for paths above the root (#10789)
Browse files Browse the repository at this point in the history 
## Summary

Closes #10777

## Test Plan

Copied over this lockfile:

```toml
version = 1
requires-python = ">=3.12"
resolution-markers = [
    "sys_platform == 'win32'",
    "sys_platform != 'win32'",
]

[[package]]
name = "pyasn1"
version = "0.6.1"
source = { registry = "https://pypi.org/simple" }
sdist = { url = "https://files.pythonhosted.org/packages/ba/e9/01f1a64245b89f039897cb0130016d79f77d52669aae6ee7b159a6c4c018/pyasn1-0.6.1.tar.gz", hash = "sha256:6f580d2bdd84365380830acf45550f2511469f673cb4a5ae3857a3170128b034", size = 145322 }
wheels = [
    { url = "https://files.pythonhosted.org/packages/c8/f1/d6a797abb14f6283c0ddff96bbdd46937f64122b8c925cab503dd37f8214/pyasn1-0.6.1-py3-none-any.whl", hash = "sha256:0d632f46f2ba09143da3a8afe9e33fb6f92fa2320ab7e886e2d0f7672af84629", size = 83135 },
]

[[package]]
name = "pyasn1-modules"
version = "0.4.1"
source = { registry = "https://pypi.org/simple" }
dependencies = [
    { name = "pyasn1" },
]
sdist = { url = "https://files.pythonhosted.org/packages/1d/67/6afbf0d507f73c32d21084a79946bfcfca5fbc62a72057e9c23797a737c9/pyasn1_modules-0.4.1.tar.gz", hash = "sha256:c28e2dbf9c06ad61c71a075c7e0f9fd0f1b0bb2d2ad4377f240d33ac2ab60a7c", size = 310028 }
wheels = [
    { url = "https://files.pythonhosted.org/packages/77/89/bc88a6711935ba795a679ea6ebee07e128050d6382eaa35a0a47c8032bdc/pyasn1_modules-0.4.1-py3-none-any.whl", hash = "sha256:49bfa96b45a292b711e986f222502c1c9a5e1f4e568fc30e2574a6c7d07838fd", size = 181537 },
]

[[package]]
name = "python-ldap"
version = "3.4.4"
source = { registry = "https://pypi.org/simple" }
resolution-markers = [
    "sys_platform != 'win32'",
]
dependencies = [
    { name = "pyasn1", marker = "sys_platform != 'win32'" },
    { name = "pyasn1-modules", marker = "sys_platform != 'win32'" },
]
sdist = { url = "https://files.pythonhosted.org/packages/fd/8b/1eeb4025dc1d3ac2f16678f38dec9ebdde6271c74955b72db5ce7a4dbfbd/python-ldap-3.4.4.tar.gz", hash = "sha256:7edb0accec4e037797705f3a05cbf36a9fde50d08c8f67f2aef99a2628fab828", size = 377889 }

[[package]]
name = "python-ldap"
version = "3.4.4"
source = { path = "../../../uti/Python/python_ldap-3.4.4-cp312-cp312-win_amd64.whl" }
resolution-markers = [
    "sys_platform == 'win32'",
]
dependencies = [
    { name = "pyasn1", marker = "sys_platform == 'win32'" },
    { name = "pyasn1-modules", marker = "sys_platform == 'win32'" },
]
wheels = [
    { filename = "python_ldap-3.4.4-cp312-cp312-win_amd64.whl", hash = "sha256:94d2ca2b3ced81c9d89aa5c79d4965d03053e1ffdcfae73e9fac85d25b692e85" },
]

[package.metadata]
requires-dist = [
    { name = "pyasn1", specifier = ">=0.3.7" },
    { name = "pyasn1-modules", specifier = ">=0.1.5" },
]

[[package]]
name = "uv-test"
version = "1.0"
source = { virtual = "." }
dependencies = [
    { name = "python-ldap", version = "3.4.4", source = { registry = "https://pypi.org/simple" }, marker = "sys_platform != 'win32'" },
    { name = "python-ldap", version = "3.4.4", source = { path = "../../../uti/Python/python_ldap-3.4.4-cp312-cp312-win_amd64.whl" }, marker = "sys_platform == 'win32'" },
]

[package.metadata]
requires-dist = [
    { name = "python-ldap", marker = "sys_platform != 'win32'" },
    { name = "python-ldap", marker = "sys_platform == 'win32'", path = "../../../../../../../../../../../../uti/Python/python_ldap-3.4.4-cp312-cp312-win_amd64.whl" },
]
```

Verified that `cargo run sync --frozen` installs `python-ldap` from
PyPI, without erroring.
  • Loading branch information
@charliermarsh
charliermarsh authored Jan 20, 2025
1 parent 6b4c8a9 commit 07e1e85
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 12 deletions.
28 changes: 28 additions & 0 deletions crates/uv-pep508/src/verbatim_url.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,34 @@ impl VerbatimUrl {
Ok(Self { url, given: None })
}

/// Parse a URL from a normalized path.
///
/// Like [`VerbatimUrl::from_absolute_path`], but skips the normalization step.
pub fn from_normalized_path(path: impl AsRef<Path>) -> Result<Self, VerbatimUrlError> {
let path = path.as_ref();

// Error if the path is relative.
let path = if path.is_absolute() {
path
} else {
return Err(VerbatimUrlError::WorkingDirectory(path.to_path_buf()));
};

// Extract the fragment, if it exists.
let (path, fragment) = split_fragment(path);

// Convert to a URL.
let mut url = Url::from_file_path(path.clone())
.unwrap_or_else(|()| panic!("path is absolute: {}", path.display()));

// Set the fragment, if it exists.
if let Some(fragment) = fragment {
url.set_fragment(Some(fragment));
}

Ok(Self { url, given: None })
}

/// Set the verbatim representation of the URL.
#[must_use]
pub fn with_given(self, given: impl AsRef<str>) -> Self {
Expand Down
18 changes: 13 additions & 5 deletions crates/uv-pypi-types/src/requirement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -870,10 +870,12 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
}
// TODO(charlie): The use of `CWD` here is incorrect. These should be resolved relative
// to the workspace root, but we don't have access to it here. When comparing these
// sources in the lockfile, we replace the URL anyway.
// sources in the lockfile, we replace the URL anyway. Ideally, we'd either remove the
// URL field or make it optional.
RequirementSourceWire::Path { path } => {
let path = PathBuf::from(path);
let url = VerbatimUrl::from_path(&path, &*CWD)?;
let url =
VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf(CWD.join(&path)))?;
Ok(Self::Path {
ext: DistExtension::from_path(path.as_path())
.map_err(|err| ParsedUrlError::MissingExtensionPath(path.clone(), err))?,
Expand All @@ -883,7 +885,9 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
}
RequirementSourceWire::Directory { directory } => {
let directory = PathBuf::from(directory);
let url = VerbatimUrl::from_path(&directory, &*CWD)?;
let url = VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf(
CWD.join(&directory),
))?;
Ok(Self::Directory {
install_path: directory,
editable: false,
Expand All @@ -893,7 +897,9 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
}
RequirementSourceWire::Editable { editable } => {
let editable = PathBuf::from(editable);
let url = VerbatimUrl::from_path(&editable, &*CWD)?;
let url = VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf(
CWD.join(&editable),
))?;
Ok(Self::Directory {
install_path: editable,
editable: true,
Expand All @@ -903,7 +909,9 @@ impl TryFrom<RequirementSourceWire> for RequirementSource {
}
RequirementSourceWire::Virtual { r#virtual } => {
let r#virtual = PathBuf::from(r#virtual);
let url = VerbatimUrl::from_path(&r#virtual, &*CWD)?;
let url = VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf(
CWD.join(&r#virtual),
))?;
Ok(Self::Directory {
install_path: r#virtual,
editable: false,
Expand Down
15 changes: 8 additions & 7 deletions crates/uv-resolver/src/lock/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2489,12 +2489,13 @@ impl Package {
}
}

/// Attempts to construct a `VerbatimUrl` from the given `Path`.
/// Attempts to construct a `VerbatimUrl` from the given normalized `Path`.
fn verbatim_url(path: &Path, id: &PackageId) -> Result<VerbatimUrl, LockError> {
let url = VerbatimUrl::from_absolute_path(path).map_err(|err| LockErrorKind::VerbatimUrl {
id: id.clone(),
err,
})?;
let url =
VerbatimUrl::from_normalized_path(path).map_err(|err| LockErrorKind::VerbatimUrl {
id: id.clone(),
err,
})?;
Ok(url)
}

Expand Down Expand Up @@ -4146,7 +4147,7 @@ fn normalize_requirement(requirement: Requirement, root: &Path) -> Result<Requir
url: _,
} => {
let install_path = uv_fs::normalize_path_buf(root.join(&install_path));
let url = VerbatimUrl::from_absolute_path(&install_path)
let url = VerbatimUrl::from_normalized_path(&install_path)
.map_err(LockErrorKind::RequirementVerbatimUrl)?;

Ok(Requirement {
Expand All @@ -4169,7 +4170,7 @@ fn normalize_requirement(requirement: Requirement, root: &Path) -> Result<Requir
url: _,
} => {
let install_path = uv_fs::normalize_path_buf(root.join(&install_path));
let url = VerbatimUrl::from_absolute_path(&install_path)
let url = VerbatimUrl::from_normalized_path(&install_path)
.map_err(LockErrorKind::RequirementVerbatimUrl)?;

Ok(Requirement {
Expand Down

0 comments on commit 07e1e85

Please sign in to comment.