Avoid canonicalizing paths in `requirements-txt` (#1019)

## Summary

When you specify an editable that doesn't exist, it should error, but
not in the parser -- the error should be downstream.
This commit is contained in:
Charlie Marsh 2024-01-19 16:28:04 -05:00 committed by GitHub
parent d55e34c310
commit 459c2abc81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 58 additions and 33 deletions

View File

@ -44,7 +44,7 @@ use pep440_rs::{Version, VersionSpecifier, VersionSpecifiers};
#[cfg(feature = "pyo3")]
use puffin_normalize::InvalidNameError;
use puffin_normalize::{ExtraName, PackageName};
pub use verbatim_url::{VerbatimUrl, VerbatimUrlError};
pub use verbatim_url::VerbatimUrl;
mod marker;
mod verbatim_url;

View File

@ -1,7 +1,7 @@
use std::borrow::Cow;
use std::fmt::Debug;
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::path::{Component, Path, PathBuf};
use once_cell::sync::Lazy;
use regex::Regex;
@ -37,11 +37,7 @@ impl VerbatimUrl {
}
/// Parse a URL from a path.
pub fn from_path(
path: impl AsRef<str>,
working_dir: impl AsRef<Path>,
given: String,
) -> Result<Self, VerbatimUrlError> {
pub fn from_path(path: impl AsRef<str>, working_dir: impl AsRef<Path>, given: String) -> Self {
// Expand any environment variables.
let path = PathBuf::from(expand_env_vars(path.as_ref(), false).as_ref());
@ -52,17 +48,16 @@ impl VerbatimUrl {
working_dir.as_ref().join(path)
};
// Canonicalize the path.
let path =
fs_err::canonicalize(path).map_err(|err| VerbatimUrlError::Path(given.clone(), err))?;
// Normalize the path.
let path = normalize_path(&path);
// Convert to a URL.
let url = Url::from_file_path(path).expect("path is absolute");
Ok(Self {
Self {
url,
given: Some(given),
})
}
}
/// Return the original string as given by the user, if available.
@ -114,10 +109,6 @@ impl Deref for VerbatimUrl {
/// An error that can occur when parsing a [`VerbatimUrl`].
#[derive(thiserror::Error, Debug)]
pub enum VerbatimUrlError {
/// Failed to canonicalize a path.
#[error("{0}")]
Path(String, #[source] std::io::Error),
/// Failed to parse a URL.
#[error("{0}")]
Url(String, #[source] url::ParseError),
@ -164,3 +155,33 @@ fn expand_env_vars(s: &str, escape: bool) -> Cow<'_, str> {
})
})
}
/// Normalize a path, removing things like `.` and `..`.
///
/// Source: <https://github.com/rust-lang/cargo/blob/b48c41aedbd69ee3990d62a0e2006edbb506a480/crates/cargo-util/src/paths.rs#L76C1-L109C2>
fn normalize_path(path: &Path) -> PathBuf {
let mut components = path.components().peekable();
let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().copied() {
components.next();
PathBuf::from(c.as_os_str())
} else {
PathBuf::new()
};
for component in components {
match component {
Component::Prefix(..) => unreachable!(),
Component::RootDir => {
ret.push(component.as_os_str());
}
Component::CurDir => {}
Component::ParentDir => {
ret.pop();
}
Component::Normal(c) => {
ret.push(c);
}
}
}
ret
}

View File

@ -3467,9 +3467,7 @@ fn missing_editable_requirement() -> Result<()> {
let requirements_in = temp_dir.child("requirements.in");
requirements_in.write_str("-e ../tmp/django-3.2.8.tar.gz")?;
let workspace_dir = temp_dir.path().canonicalize()?;
let filters = iter::once((workspace_dir.to_str().unwrap(), "[WORKSPACE_DIR]"))
let filters: Vec<_> = iter::once((r"(file:/)?/.*/", "file://[TEMP_DIR]/"))
.chain(INSTA_FILTERS.to_vec())
.collect::<Vec<_>>();
@ -3491,8 +3489,10 @@ fn missing_editable_requirement() -> Result<()> {
----- stdout -----
----- stderr -----
error: Invalid editable path in requirements.in: ../tmp/django-3.2.8.tar.gz
Caused by: failed to canonicalize path `[WORKSPACE_DIR]/../tmp/django-3.2.8.tar.gz`
error: Failed to build editables
Caused by: Failed to build editable: file://[TEMP_DIR]/django-3.2.8.tar.gz
Caused by: Failed to build: file://[TEMP_DIR]/django-3.2.8.tar.gz
Caused by: failed to query metadata of file `file://[TEMP_DIR]/django-3.2.8.tar.gz`
Caused by: No such file or directory (os error 2)
"###);
});

View File

@ -41,12 +41,13 @@ use std::path::{Path, PathBuf};
use std::str::FromStr;
use fs_err as fs;
use once_cell::sync::Lazy;
use serde::{Deserialize, Serialize};
use tracing::warn;
use unscanny::{Pattern, Scanner};
use url::Url;
use pep508_rs::{Pep508Error, Requirement, VerbatimUrl, VerbatimUrlError};
use pep508_rs::{Pep508Error, Requirement, VerbatimUrl};
/// We emit one of those for each requirements.txt entry
enum RequirementsTxtStatement {
@ -88,8 +89,10 @@ impl FromStr for EditableRequirement {
type Err = RequirementsTxtParserError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
static CWD: Lazy<PathBuf> = Lazy::new(|| std::env::current_dir().unwrap());
let editable_requirement = ParsedEditableRequirement::from(s.to_string());
editable_requirement.with_working_dir(".")
editable_requirement.with_working_dir(&*CWD)
}
}
@ -153,11 +156,9 @@ impl ParsedEditableRequirement {
if let Some(path) = path.strip_prefix("//") {
// Ex) `file:///home/ferris/project/scripts/...`
VerbatimUrl::from_path(path, working_dir, given.clone())
.map_err(RequirementsTxtParserError::InvalidEditablePath)?
} else {
// Ex) `file:../editable/`
VerbatimUrl::from_path(path, working_dir, given.clone())
.map_err(RequirementsTxtParserError::InvalidEditablePath)?
}
} else {
// Ex) `https://...`
@ -168,13 +169,12 @@ impl ParsedEditableRequirement {
} else {
// Ex) `../editable/`
VerbatimUrl::from_path(&given, working_dir, given.clone())
.map_err(RequirementsTxtParserError::InvalidEditablePath)?
};
// Create a `PathBuf`.
let path = url
.to_file_path()
.expect("file:// URLs should be valid paths");
.map_err(|()| RequirementsTxtParserError::InvalidEditablePath(given.clone()))?;
Ok(EditableRequirement { url, path })
}
@ -538,7 +538,7 @@ pub struct RequirementsTxtFileError {
#[derive(Debug)]
pub enum RequirementsTxtParserError {
IO(io::Error),
InvalidEditablePath(VerbatimUrlError),
InvalidEditablePath(String),
UnsupportedUrl(String),
Parser {
message: String,
@ -560,8 +560,8 @@ impl Display for RequirementsTxtParserError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
RequirementsTxtParserError::IO(err) => err.fmt(f),
RequirementsTxtParserError::InvalidEditablePath(err) => {
write!(f, "Invalid editable path: {err}")
RequirementsTxtParserError::InvalidEditablePath(given) => {
write!(f, "Invalid editable path: {given}")
}
RequirementsTxtParserError::UnsupportedUrl(url) => {
write!(f, "Unsupported URL (expected a `file://` scheme): {url}")
@ -586,7 +586,7 @@ impl std::error::Error for RequirementsTxtParserError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match &self {
RequirementsTxtParserError::IO(err) => err.source(),
RequirementsTxtParserError::InvalidEditablePath(err) => err.source(),
RequirementsTxtParserError::InvalidEditablePath(_) => None,
RequirementsTxtParserError::UnsupportedUrl(_) => None,
RequirementsTxtParserError::Pep508 { source, .. } => Some(source),
RequirementsTxtParserError::Subfile { source, .. } => Some(source.as_ref()),
@ -599,8 +599,12 @@ impl Display for RequirementsTxtFileError {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match &self.error {
RequirementsTxtParserError::IO(err) => err.fmt(f),
RequirementsTxtParserError::InvalidEditablePath(err) => {
write!(f, "Invalid editable path in {}: {err}", self.file.display())
RequirementsTxtParserError::InvalidEditablePath(given) => {
write!(
f,
"Invalid editable path in {}: {given}",
self.file.display()
)
}
RequirementsTxtParserError::UnsupportedUrl(url) => {
write!(