Treat missing package name error as an unsupported requirement (#1025)

## Summary

Based on user feedback. Calling it a "parse error" is misleading, since
this is really something we don't support, but that users can work
around.
This commit is contained in:
Charlie Marsh 2024-01-21 19:53:10 -05:00 committed by GitHub
parent 4026710189
commit 540442b8de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 90 additions and 18 deletions

View file

@ -71,6 +71,9 @@ pub enum Pep508ErrorSource {
/// A URL parsing error. /// A URL parsing error.
#[error(transparent)] #[error(transparent)]
UrlError(#[from] verbatim_url::VerbatimUrlError), UrlError(#[from] verbatim_url::VerbatimUrlError),
/// The version requirement is not supported.
#[error("{0}")]
UnsupportedRequirement(String),
} }
impl Display for Pep508Error { impl Display for Pep508Error {
@ -842,11 +845,9 @@ fn parse(cursor: &mut Cursor) -> Result<Requirement, Pep508Error> {
// a package name. pip supports this in `requirements.txt`, but it doesn't adhere to // a package name. pip supports this in `requirements.txt`, but it doesn't adhere to
// the PEP 508 grammar. // the PEP 508 grammar.
let mut clone = cursor.clone().at(start); let mut clone = cursor.clone().at(start);
return if let Ok(url) = parse_url(&mut clone) { return if parse_url(&mut clone).is_ok() {
Err(Pep508Error { Err(Pep508Error {
message: Pep508ErrorSource::String(format!( message: Pep508ErrorSource::UnsupportedRequirement("URL requirement must be preceded by a package name. Add the name of the package before the URL (e.g., `package_name @ https://...`).".to_string()),
"URL requirement is missing a package name; expected: `package_name @ {url}`",
)),
start, start,
len: clone.pos() - start, len: clone.pos() - start,
input: clone.to_string(), input: clone.to_string(),
@ -1305,7 +1306,7 @@ mod tests {
assert_err( assert_err(
r#"git+https://github.com/pallets/flask.git"#, r#"git+https://github.com/pallets/flask.git"#,
indoc! {" indoc! {"
URL requirement is missing a package name; expected: `package_name @ git+https://github.com/pallets/flask.git` URL requirement must be preceded by a package name. Add the name of the package before the URL (e.g., `package_name @ https://...`).
git+https://github.com/pallets/flask.git git+https://github.com/pallets/flask.git
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^" ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
}, },

View file

@ -5,18 +5,20 @@ use std::path::{Component, Path, PathBuf};
use once_cell::sync::Lazy; use once_cell::sync::Lazy;
use regex::Regex; use regex::Regex;
use serde::{Deserialize, Serialize};
use url::Url; use url::Url;
/// A wrapper around [`Url`] that preserves the original string. /// A wrapper around [`Url`] that preserves the original string.
#[derive(Debug, Clone, Eq, derivative::Derivative)] #[derive(Debug, Clone, Eq, derivative::Derivative)]
#[derivative(PartialEq, Hash)] #[derivative(PartialEq, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct VerbatimUrl { pub struct VerbatimUrl {
/// The parsed URL. /// The parsed URL.
#[serde( #[cfg_attr(
serialize_with = "Url::serialize_internal", feature = "serde",
deserialize_with = "Url::deserialize_internal" serde(
serialize_with = "Url::serialize_internal",
deserialize_with = "Url::deserialize_internal"
)
)] )]
url: Url, url: Url,
/// The URL as it was provided by the user. /// The URL as it was provided by the user.

View file

@ -3495,3 +3495,41 @@ fn missing_editable_requirement() -> Result<()> {
Ok(()) Ok(())
} }
/// Attempt to resolve a URL requirement without a package name.
#[test]
fn missing_package_name() -> Result<()> {
let temp_dir = TempDir::new()?;
let cache_dir = TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
let requirements_in = temp_dir.child("requirements.in");
requirements_in.write_str("https://files.pythonhosted.org/packages/36/42/015c23096649b908c809c69388a805a571a3bea44362fe87e33fc3afa01f/flask-3.0.0-py3-none-any.whl")?;
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip")
.arg("compile")
.arg("requirements.in")
.arg("--cache-dir")
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("VIRTUAL_ENV", venv.as_os_str())
.current_dir(&temp_dir), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: Unsupported requirement in requirements.in position 0 to 135
Caused by: URL requirement must be preceded by a package name. Add the name of the package before the URL (e.g., `package_name @ https://...`).
https://files.pythonhosted.org/packages/36/42/015c23096649b908c809c69388a805a571a3bea44362fe87e33fc3afa01f/flask-3.0.0-py3-none-any.whl
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"###);
});
Ok(())
}

View file

@ -47,7 +47,7 @@ use tracing::warn;
use unscanny::{Pattern, Scanner}; use unscanny::{Pattern, Scanner};
use url::Url; use url::Url;
use pep508_rs::{Pep508Error, Requirement, VerbatimUrl}; use pep508_rs::{Pep508Error, Pep508ErrorSource, Requirement, VerbatimUrl};
/// We emit one of those for each requirements.txt entry /// We emit one of those for each requirements.txt entry
enum RequirementsTxtStatement { enum RequirementsTxtStatement {
@ -468,13 +468,26 @@ fn parse_requirement_and_hashes(
break (end, false); break (end, false);
} }
}; };
let requirement = Requirement::from_str(&content[start..end]).map_err(|err| { let requirement =
RequirementsTxtParserError::Pep508 { Requirement::from_str(&content[start..end]).map_err(|err| match err.message {
source: err, Pep508ErrorSource::String(_) => RequirementsTxtParserError::Pep508 {
start, source: err,
end, start,
} end,
})?; },
Pep508ErrorSource::UrlError(_) => RequirementsTxtParserError::Pep508 {
source: err,
start,
end,
},
Pep508ErrorSource::UnsupportedRequirement(_) => {
RequirementsTxtParserError::UnsupportedRequirement {
source: err,
start,
end,
}
}
})?;
let hashes = if has_hashes { let hashes = if has_hashes {
let hashes = parse_hashes(s)?; let hashes = parse_hashes(s)?;
eat_trailing_line(s)?; eat_trailing_line(s)?;
@ -547,6 +560,11 @@ pub enum RequirementsTxtParserError {
message: String, message: String,
location: usize, location: usize,
}, },
UnsupportedRequirement {
source: Pep508Error,
start: usize,
end: usize,
},
Pep508 { Pep508 {
source: Pep508Error, source: Pep508Error,
start: usize, start: usize,
@ -572,6 +590,9 @@ impl Display for RequirementsTxtParserError {
RequirementsTxtParserError::Parser { message, location } => { RequirementsTxtParserError::Parser { message, location } => {
write!(f, "{message} at position {location}") write!(f, "{message} at position {location}")
} }
RequirementsTxtParserError::UnsupportedRequirement { start, end, .. } => {
write!(f, "Unsupported requirement in position {start} to {end}")
}
RequirementsTxtParserError::Pep508 { start, end, .. } => { RequirementsTxtParserError::Pep508 { start, end, .. } => {
write!(f, "Couldn't parse requirement in position {start} to {end}") write!(f, "Couldn't parse requirement in position {start} to {end}")
} }
@ -591,6 +612,7 @@ impl std::error::Error for RequirementsTxtParserError {
RequirementsTxtParserError::IO(err) => err.source(), RequirementsTxtParserError::IO(err) => err.source(),
RequirementsTxtParserError::InvalidEditablePath(_) => None, RequirementsTxtParserError::InvalidEditablePath(_) => None,
RequirementsTxtParserError::UnsupportedUrl(_) => None, RequirementsTxtParserError::UnsupportedUrl(_) => None,
RequirementsTxtParserError::UnsupportedRequirement { source, .. } => Some(source),
RequirementsTxtParserError::Pep508 { source, .. } => Some(source), RequirementsTxtParserError::Pep508 { source, .. } => Some(source),
RequirementsTxtParserError::Subfile { source, .. } => Some(source.as_ref()), RequirementsTxtParserError::Subfile { source, .. } => Some(source.as_ref()),
RequirementsTxtParserError::Parser { .. } => None, RequirementsTxtParserError::Parser { .. } => None,
@ -626,6 +648,15 @@ impl Display for RequirementsTxtFileError {
location location
) )
} }
RequirementsTxtParserError::UnsupportedRequirement { start, end, .. } => {
write!(
f,
"Unsupported requirement in {} position {} to {}",
self.file.display(),
start,
end,
)
}
RequirementsTxtParserError::Pep508 { start, end, .. } => { RequirementsTxtParserError::Pep508 { start, end, .. } => {
write!( write!(
f, f,