diff --git a/crates/socket-patch-core/src/patch/redirect/requirements.rs b/crates/socket-patch-core/src/patch/redirect/requirements.rs index 507a8134d..dd1469677 100644 --- a/crates/socket-patch-core/src/patch/redirect/requirements.rs +++ b/crates/socket-patch-core/src/patch/redirect/requirements.rs @@ -1,12 +1,12 @@ use std::collections::{BTreeMap, BTreeSet}; -use regex::Regex; use serde_json::Value; use super::{DepOverride, FileEdit, RewriteResult, RewriteWarning}; use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::formats::text::{split_bom, strip_bom}; use crate::utils::purl::percent_decode_purl_component; +use crate::utils::requirements::{parse_requirement, requirement_name, Requirement}; use crate::vendor::state::{VendorEntry, WiringAction}; pub(super) struct LogicalRequirement { @@ -152,28 +152,22 @@ fn archive_version(location: &str, name: &str) -> Option { (canonicalize_pypi_name(distribution) == name).then(|| version.to_string()) } -fn requirement_version(specifier: &str, name_re: &Regex, name: &str) -> RequirementVersion { - let Some(captures) = name_re.captures(specifier.trim()) else { +/// Classify the pin of one requirement, read through the shared +/// requirements grammar (#1365), with the hosted policy on top: `===` is +/// arbitrary equality, a direct reference is socket-patch's own artifact +/// or the user's source, and anything that is not one exact clause is +/// ambiguous. +fn requirement_version(requirement: &Requirement<'_>, name: &str) -> RequirementVersion { + // The rewrite carries long `--` options over, but a short one (`-C`, + // pip's `--config-settings`) would be dropped: refuse, as before. + if requirement + .options + .split_whitespace() + .any(|token| token.starts_with('-') && !token.starts_with("--")) + { return RequirementVersion::Ambiguous; - }; - let end = captures.get(2).or_else(|| captures.get(1)).unwrap().end(); - let tail = &specifier.trim()[end..]; - let tail = requirement_tokens(tail) - .into_iter() - .take_while(|token| !token.starts_with("--")) - .collect::>() - .join(" "); - let tail = tail.trim(); - let tail = tail - .strip_prefix('(') - .and_then(|value| value.strip_suffix(')')) - .unwrap_or(tail) - .trim(); - if tail.is_empty() { - return RequirementVersion::Unpinned; } - if let Some(location) = tail.strip_prefix('@') { - let location = location.trim(); + if let Some(location) = requirement.direct_reference { let Some(version) = archive_version(location, name) else { return RequirementVersion::Ambiguous; }; @@ -185,26 +179,27 @@ fn requirement_version(specifier: &str, name_re: &Regex, name: &str) -> Requirem } return RequirementVersion::Exact(version); } - let (arbitrary, version) = match tail.strip_prefix("===") { - Some(version) => (true, Some(version)), - None => (false, tail.strip_prefix("==")), - }; - if let Some(version) = version.map(str::trim) { - if !version.is_empty() - && !version - .chars() - .any(|character| character.is_whitespace() || ",*<>=~".contains(character)) - { - return if arbitrary { - RequirementVersion::Arbitrary(version.to_string()) - } else { - RequirementVersion::Exact(version.to_string()) - }; - } + if requirement.specifier.is_empty() { + return RequirementVersion::Unpinned; + } + if let Some(version) = requirement.exact_version() { + return RequirementVersion::Exact(version.to_string()); + } + if let Some(version) = requirement.arbitrary_version() { + return RequirementVersion::Arbitrary(version.to_string()); } RequirementVersion::Ambiguous } +/// The requirement on one logical line, comment removed. An unterminated +/// continuation's trailing `\\` is dropped so the line still names its +/// package (it is refused as unterminated, never rewritten). +fn line_requirement(text: &str) -> Option> { + let text = text.trim(); + let code = unquoted_index(text, '#', true).map_or(text, |index| &text[..index]); + parse_requirement(code.trim_end().trim_end_matches('\\')) +} + pub(super) fn rewrite( files: &BTreeMap, overrides: &[DepOverride], @@ -213,17 +208,15 @@ pub(super) fn rewrite( let Some(content) = files.get("requirements.txt") else { return; }; - let name_re = Regex::new( - r"^([A-Za-z0-9][A-Za-z0-9._-]*)(\s*\[[^\]\r\n]*\])?\s*(?:[=<>~!]=?|@|;|\(|\s|$)", - ) - .expect("static requirements-name regex is valid"); let mut requirements = logical_requirements(content); let mut row_counts = BTreeMap::::new(); + // Every line that names a package counts, parseable or not, so a + // stray second line keeps an unpinned entry ambiguous (fail closed). for requirement in &requirements { - if let Some(captures) = name_re.captures(requirement.text.trim()) { - *row_counts - .entry(canonicalize_pypi_name(&captures[1])) - .or_default() += 1; + let text = requirement.text.trim(); + let code = unquoted_index(text, '#', true).map_or(text, |index| &text[..index]); + if let Some(name) = requirement_name(code) { + *row_counts.entry(canonicalize_pypi_name(name)).or_default() += 1; } } let mut override_versions = BTreeMap::>::new(); @@ -250,12 +243,15 @@ pub(super) fn rewrite( let mut matched = false; for requirement in &mut requirements { let text = requirement.text.trim(); - let Some(captures) = name_re.captures(text) else { + let Some(parsed) = line_requirement(text) else { continue; }; - if canonicalize_pypi_name(&captures[1]) != target { + if canonicalize_pypi_name(parsed.name) != target { continue; } + let extras = parsed + .extras + .map_or(String::new(), |extras| format!("[{extras}]")); if requirement.unterminated { matched = true; result.warnings.push(RewriteWarning { @@ -274,7 +270,7 @@ pub(super) fn rewrite( .map_or((cleaned.as_str(), ""), |index| { (&cleaned[..index], &cleaned[index..]) }); - match requirement_version(specifier, &name_re, &target) { + match requirement_version(&parsed, &target) { // pip resolves `==` under PEP 440 (`==1.16` installs 1.16.0). RequirementVersion::Exact(version) if !crate::utils::pep440::versions_equal(&version, &dep.version) => @@ -326,9 +322,6 @@ pub(super) fn rewrite( .skip_while(|token| !token.starts_with("--")) .collect::>() .join(" "); - let extras = captures - .get(2) - .map_or("", |capture| capture.as_str().trim()); let (bom, prefix_body) = split_bom(&requirement.original); let indent = &prefix_body [..prefix_body.len() - prefix_body.trim_start_matches([' ', '\t']).len()]; @@ -649,6 +642,76 @@ mod tests { ); } + /// #1365: the hosted rewriter reads a requirements line with the same + /// grammar as the lock inventory and the vendored planner. It keeps its + /// own policy on top: `===` is rewritten as arbitrary equality, and a + /// pin with a second clause is ambiguous. + #[test] + fn hosted_reads_pins_with_the_shared_requirements_grammar() { + for (line, rewritten) in [ + ("requests==2.28.1", true), + ("requests (==2.28.1)", true), + ("requests (== 2.28.1)", true), + ("requests ( == 2.28.1 ) ; python_version >= \"3.8\"", true), + ("requests===2.28.1", true), + ("requests==0!2.28.1", true), + ("requests==v2.28.1", true), + ] { + let result = rewrite_registry_redirect(&input(&format!("{line}\n")), &[patch()]); + assert!(result.warnings.is_empty(), "{line}: {:?}", result.warnings); + assert_eq!( + result.files["requirements.txt"].starts_with(&format!("requests @ {URL}")), + rewritten, + "{line}" + ); + } + let result = rewrite_registry_redirect(&input("requests==2.28.1,!=2.0\n"), &[patch()]); + assert!(result.files.is_empty()); + assert_eq!( + result.warnings[0].code, + "redirect_requirements_version_ambiguous" + ); + // A short option the rewrite would drop keeps the line ambiguous. + for source in [ + "requests==2.28.1 --hash=sha256:aa -C x=y\n", + "requests==2.28.1 --hash sha256:aa -Cx=y\n", + ] { + let result = rewrite_registry_redirect(&input(source), &[patch()]); + assert!(result.files.is_empty(), "{source}"); + assert_eq!( + result.warnings[0].code, "redirect_requirements_version_ambiguous", + "{source}" + ); + } + // An unterminated continuation still names its package. + for source in ["requests \\", "requests==2.28.1 \\"] { + let result = rewrite_registry_redirect(&input(source), &[patch()]); + assert!(result.files.is_empty(), "{source}"); + assert_eq!( + result.warnings[0].code, "redirect_requirements_continuation", + "{source}" + ); + } + // A second, unparseable line naming the package keeps an unpinned + // entry ambiguous. + let result = rewrite_registry_redirect( + &input("requests\nrequests @ https://h/x.whl junk\n"), + &[patch()], + ); + assert!(result.files.is_empty()); + assert_eq!( + result.warnings[0].code, + "redirect_requirements_version_ambiguous" + ); + // Not a requirement line at all: the name runs into a non-specifier. + let result = rewrite_registry_redirect(&input("requests/x==2.28.1\n"), &[patch()]); + assert!(result.files.is_empty()); + assert_eq!( + result.warnings[0].code, + "redirect_requirements_entry_not_found" + ); + } + #[test] fn markers_after_hashes_and_hash_text_in_quoted_markers_are_preserved() { let source = "requests==2.28.1 --hash=sha256:OLD ; platform_version != \"text --hash=keep # retained\"\n"; diff --git a/crates/socket-patch-core/src/utils/pep440.rs b/crates/socket-patch-core/src/utils/pep440.rs index a184827f5..641e602c9 100644 --- a/crates/socket-patch-core/src/utils/pep440.rs +++ b/crates/socket-patch-core/src/utils/pep440.rs @@ -110,6 +110,11 @@ pub(crate) fn versions_equal(a: &str, b: &str) -> bool { } } +/// Whether `text` is a valid PEP 440 version. +pub(crate) fn is_valid(text: &str) -> bool { + parse(text).is_some() +} + /// Whether a whitespace-free specifier is an exact `==X` pin of `version` /// under PEP 440. Wildcards (`==1.*`), arbitrary equality (`===`) and every /// other operator are not. @@ -120,16 +125,6 @@ pub(crate) fn is_exact_pin_of(specifier: &str, version: &str) -> bool { .is_some_and(|pinned| versions_equal(pinned, version)) } -/// Whether `specifier` (whitespace already stripped) is an exact `==` pin -/// of SOME valid release — the shape [`is_exact_pin_of`] accepts, for any -/// version. Arbitrary equality (`===`) and wildcards are not exact pins. -pub(crate) fn is_exact_pin(specifier: &str) -> bool { - specifier - .strip_prefix("==") - .filter(|pinned| !pinned.starts_with('=') && !pinned.contains('*')) - .is_some_and(|pinned| parse(pinned).is_some()) -} - /// The other spellings a registry may have published the pure release /// `version` (digits and dots only) under, as PEP 440 equality reads them: /// leading zeros dropped and the release padded with `.0` from its @@ -250,11 +245,8 @@ mod tests { assert!(!is_exact_pin_of("~=1.16.0", "1.16.0")); assert!(!is_exact_pin_of("==1.15", "1.16.0")); assert!(!is_exact_pin_of("1.16.0", "1.16.0")); - assert!(is_exact_pin("==1.17.0")); - assert!(is_exact_pin("==1.16")); - assert!(!is_exact_pin("===1.17.0")); - assert!(!is_exact_pin("==1.17.*")); - assert!(!is_exact_pin(">=1.17")); - assert!(!is_exact_pin("==not a version")); + assert!(is_valid("1.17.0")); + assert!(is_valid("0!1.16")); + assert!(!is_valid("not a version")); } } diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index d1a585819..7809d367e 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -381,48 +381,172 @@ pub(crate) fn shlex_split(text: &str) -> Option> { Some(words) } +/// One requirements.txt requirement, read from a logical line's code part +/// (comment already stripped) the way pip reads it. The ONE requirements +/// grammar: the lock inventory, lockfile discovery, the hosted rewriter and +/// the vendored planner all read pins through it (#1365), and each keeps +/// its own policy on top. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Requirement<'a> { + /// The PEP 508 name as spelled. + pub(crate) name: &'a str, + /// The text between `[` and `]`, as spelled. + pub(crate) extras: Option<&'a str>, + /// The version specifier, trimmed, with the legacy `( … )` wrapper + /// removed (`six (==1.0)` is `==1.0`). Empty for an unpinned name or + /// a direct reference. + pub(crate) specifier: &'a str, + /// The `name @ ` location. + pub(crate) direct_reference: Option<&'a str>, + /// The environment marker after `;`, trimmed; `None` when absent or + /// empty. + pub(crate) marker: Option<&'a str>, + /// The per-requirement options (`--hash=…`) after the requirement. + pub(crate) options: &'a str, +} + +impl<'a> Requirement<'a> { + /// The version of an exact `==X` pin, as spelled: one clause, no + /// wildcard (`==1.*`), not arbitrary equality (`===`). Callers compare + /// it under PEP 440, which also reads epochs (`0!1.0`) and a `v` + /// prefix. + pub(crate) fn exact_version(&self) -> Option<&'a str> { + let version = self.specifier.strip_prefix("==")?; + bare_version(version).filter(|version| !version.contains('*')) + } + + /// The version of an arbitrary-equality `===X` pin (a string match), + /// as spelled. + pub(crate) fn arbitrary_version(&self) -> Option<&'a str> { + bare_version(self.specifier.strip_prefix("===")?) + } +} + +/// The one version token after a specifier's operator: `None` when it is +/// empty or a second operator or clause follows. +fn bare_version(text: &str) -> Option<&str> { + let version = text.trim_start(); + (!version.is_empty() + && !version.contains(|c: char| c.is_whitespace() || "(),;<>~=".contains(c))) + .then_some(version) +} + +/// The PEP 508 name a requirement line starts with, as spelled: it must +/// start alphanumeric. `None` for option lines and path or URL lines that +/// start with `.`, `/` or `-`. Readers that must fail closed treat a line +/// with a name but no valid [`parse_requirement`] as naming the package. +pub(crate) fn requirement_name(code: &str) -> Option<&str> { + let code = code.trim_start(); + if !code.starts_with(|c: char| c.is_ascii_alphanumeric()) { + return None; + } + let end = code + .find(|c: char| !(c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))) + .unwrap_or(code.len()); + Some(&code[..end]) +} + +/// Parse one logical line's code part (comment already stripped) as a +/// requirement. `None` for blank lines, option lines (`-r`, `-e`, +/// `--index-url`, …), path and URL lines (no leading name), and a name +/// that runs into something that is not a specifier (`six/x`). +pub(crate) fn parse_requirement(code: &str) -> Option> { + let code = code.trim(); + if code.is_empty() || code.starts_with('-') { + return None; + } + // Per-requirement options start at the first whitespace-preceded `--` + // outside a quoted marker string. + let mut quote = None; + let options_at = code + .char_indices() + .find(|&(i, c)| { + match quote { + Some(open) if c == open => quote = None, + Some(_) => {} + None if c == '\'' || c == '"' => quote = Some(c), + None => { + return code[i..].starts_with("--") && code[..i].ends_with(char::is_whitespace) + } + } + false + }) + .map_or(code.len(), |(i, _)| i); + let (spec, options) = (code[..options_at].trim_end(), code[options_at..].trim()); + let name = requirement_name(spec)?; + let mut rest = spec[name.len()..].trim_start(); + let mut extras = None; + if let Some(open) = rest.strip_prefix('[') { + let close = open.find(']')?; + extras = Some(&open[..close]); + rest = open[close + 1..].trim_start(); + } + fn marker_of(text: &str) -> Option<&str> { + Some(text.trim()).filter(|marker| !marker.is_empty()) + } + if let Some(location) = rest.strip_prefix('@') { + // A url may itself contain `;`: the reference is one token, and a + // marker after it needs whitespace before its `;` (PEP 508). + let location = location.trim_start(); + let end = location.find(char::is_whitespace).unwrap_or(location.len()); + let (reference, tail) = location.split_at(end); + if reference.is_empty() { + return None; + } + let tail = tail.trim_start(); + let marker = match tail.strip_prefix(';') { + Some(marker) => marker_of(marker), + None if tail.is_empty() => None, + None => return None, + }; + return Some(Requirement { + name, + extras, + specifier: "", + direct_reference: Some(reference), + marker, + options, + }); + } + // Names and specifiers cannot contain `;`: the marker is everything + // after the first one. + let (specifier, marker) = match rest.split_once(';') { + Some((specifier, marker)) => (specifier.trim_end(), marker_of(marker)), + None => (rest, None), + }; + if !(specifier.is_empty() || specifier.starts_with(['=', '<', '>', '~', '!', '('])) { + return None; + } + let specifier = specifier + .strip_prefix('(') + .and_then(|inner| inner.strip_suffix(')')) + .map_or(specifier, str::trim); + Some(Requirement { + name, + extras, + specifier, + direct_reference: None, + marker, + options, + }) +} + /// The `(name as spelled, version)` of an exact `name[extras]==X` registry /// requirement (a logical line's code part; an optional `; marker` and /// options may follow), `None` for anything else — ranges, `===`, wildcards /// (`==1.*`), a version not starting with a digit. Spelled as pip reads it: /// whitespace may surround the extras and the `==` (`six == 1.0`, /// `six[x] ==1.0`), and the legacy parenthesised form `six (==1.0)` is the -/// same pin. The ONE exact-pin rule the lock inventory and lockfile -/// discovery read requirements with. +/// same pin. Read through [`parse_requirement`], the grammar every +/// requirements reader shares. pub(crate) fn exact_pin(code: &str) -> Option<(&str, &str)> { - let spec = code.split(';').next()?.trim_start(); - let name_end = spec - .find(|c: char| !(c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-'))) - .unwrap_or(spec.len()); - let (name, mut rest) = spec.split_at(name_end); - rest = rest.trim_start(); - if rest.starts_with('[') { - rest = rest[rest.find(']')? + 1..].trim_start(); - } - let parenthesised = rest.starts_with('('); - if parenthesised { - rest = rest[1..].trim_start(); - } - rest = rest.strip_prefix("==")?.trim_start(); - let version_end = rest - .find(|c: char| c.is_whitespace() || matches!(c, ')' | ',')) - .unwrap_or(rest.len()); - let (version, mut rest) = rest.split_at(version_end); - rest = rest.trim_start(); - if parenthesised { - rest = rest.strip_prefix(')')?.trim_start(); - } - // Only options (`--hash=…`) may follow the specifier; anything else - // (`,<2`, a stray `)`, a second token) is not one exact pin. - if name.is_empty() - || !(rest.is_empty() || rest.starts_with("--")) - || version.starts_with('=') - || version.contains('*') - || !version.starts_with(|c: char| c.is_ascii_digit()) - { - return None; - } - Some((name, version)) + let requirement = parse_requirement(code)?; + let version = requirement.exact_version()?; + // The inventory emits the version into a purl, so it takes only a + // plain release spelling. + version + .starts_with(|c: char| c.is_ascii_digit()) + .then_some((requirement.name, version)) } /// The `(name as spelled, reference)` of a PEP 508 direct reference @@ -435,10 +559,8 @@ pub(crate) fn exact_pin(code: &str) -> Option<(&str, &str)> { /// the name cannot contain an `@`, so the first one is always the separator /// and a url's own `user@host` stays inside the reference. pub(crate) fn direct_reference(code: &str) -> Option<(&str, &str)> { - let (name, rest) = code.split(';').next()?.split_once('@')?; - let name = name.split('[').next()?.trim(); - let reference = rest.split_whitespace().next()?; - (!name.is_empty() && !reference.is_empty()).then_some((name, reference)) + let requirement = parse_requirement(code)?; + Some((requirement.name, requirement.direct_reference?)) } /// The `(name, version)` of a `socket-patch vendor: ==[ (transitive)]` diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 7ec9241a2..33fea6eac 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -27,8 +27,8 @@ use crate::crawlers::python_crawler::canonicalize_pypi_name; use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_string}; use crate::utils::line_endings::terminator; use crate::utils::requirements::{ - expand_env_vars, hash_options, logical_lines, requires_hashes, shlex_split, split_comment, - strip_comment, vendor_tag, + expand_env_vars, hash_options, logical_lines, parse_requirement, requirement_name, + requires_hashes, shlex_split, split_comment, strip_comment, vendor_tag, }; use super::common::refuse_symlinked; @@ -81,25 +81,48 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> (Vec, b let mut found_range = false; let mut other_branches = false; for ll in logical_lines(content) { - let Some(req) = parse_requirement_line(&ll.text) else { + let code = strip_comment(&ll.text); + let Some(req) = parse_requirement(code) else { + // A line that names the package but is no requirement pip + // reads cleanly is never a pin to rewrite (fail closed). + if requirement_name(code).is_some_and(|name| canonicalize_pypi_name(name) == canon_name) + { + found_range = true; + } continue; }; - if canonicalize_pypi_name(&req.name) != canon_name { + if canonicalize_pypi_name(req.name) != canon_name { continue; } if req.extras.is_some() { found_extras = true; continue; } - let spec_no_ws: String = req - .specifier - .chars() - .filter(|c| !c.is_whitespace()) - .collect(); // pip resolves `==` under PEP 440 (`==1.16` installs 1.16.0). - if crate::utils::pep440::is_exact_pin_of(&spec_no_ws, version) { - exact.push((ll.start, ll.physical.len(), req.marker, req.hashed)); - } else if req.marker.is_some() && crate::utils::pep440::is_exact_pin(&spec_no_ws) { + // Arbitrary equality (`===`) stays a refusal here by policy: the + // vendored rewrite replaces only an exact registry pin. + let pinned = req.exact_version(); + let hashed = req.options.contains("--hash"); + // The rewritten wheel line carries only `--hash`: a pin with any + // other per-requirement option (`--config-settings`, its short + // `-C`, …; a `--hash` value never starts with `-`) would lose + // it, so it is refused rather than rewritten. + let other_options = req + .options + .split_whitespace() + .any(|token| token.starts_with('-') && !token.starts_with("--hash")); + if other_options { + found_range = true; + continue; + } + if pinned.is_some_and(|pinned| crate::utils::pep440::versions_equal(pinned, version)) { + exact.push(( + ll.start, + ll.physical.len(), + req.marker.map(str::to_string), + hashed, + )); + } else if req.marker.is_some() && pinned.is_some_and(crate::utils::pep440::is_valid) { other_branches = true; } else { found_range = true; @@ -115,8 +138,8 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> (Vec, b /// spec, extras or marker): a file that pins it is an install source of it. pub(super) fn names_package(content: &str, canon_name: &str) -> bool { logical_lines(content).into_iter().any(|ll| { - parse_requirement_line(&ll.text) - .is_some_and(|req| canonicalize_pypi_name(&req.name) == canon_name) + requirement_name(strip_comment(&ll.text)) + .is_some_and(|name| canonicalize_pypi_name(name) == canon_name) }) } @@ -1195,61 +1218,6 @@ fn include_target_with(text: &str, env: impl Fn(&str) -> Option) -> Opti // The logical-line lexer lives in `utils::requirements` (shared with the // lockfile inventory and lockfile discovery). -struct ParsedRequirement { - name: String, - extras: Option, - specifier: String, - marker: Option, - hashed: bool, -} - -/// Parse one logical line as a requirement; `None` for blank lines, option -/// lines (`-r`, `--index-url`, …) and path/URL lines (no leading name). -fn parse_requirement_line(text: &str) -> Option { - let code = strip_comment(text).trim(); - if code.is_empty() || code.starts_with('-') { - return None; - } - // Per-line `--hash` options come after the requirement (and marker). - let (req_part, hashed) = match code.find(" --hash") { - Some(i) => (code[..i].trim_end(), true), - None => (code, false), - }; - // The environment marker is everything after the first `;` (specifiers - // and names cannot contain one), carried VERBATIM for the rewrite. - let (req_part, marker) = match req_part.find(';') { - Some(i) => ( - req_part[..i].trim_end(), - Some(req_part[i + 1..].trim().to_string()).filter(|m| !m.is_empty()), - ), - None => (req_part, None), - }; - // PEP 508 name: must start alphanumeric. - let name_end = req_part - .char_indices() - .find(|(_, c)| !(c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.'))) - .map(|(i, _)| i) - .unwrap_or(req_part.len()); - if name_end == 0 || !req_part.starts_with(|c: char| c.is_ascii_alphanumeric()) { - return None; - } - let name = req_part[..name_end].to_string(); - let mut rest = req_part[name_end..].trim_start(); - let mut extras = None; - if let Some(stripped) = rest.strip_prefix('[') { - let close = stripped.find(']')?; - extras = Some(stripped[..close].trim().to_string()); - rest = stripped[close + 1..].trim_start(); - } - Some(ParsedRequirement { - name, - extras, - specifier: rest.trim().to_string(), - marker, - hashed, - }) -} - #[cfg(test)] mod tests { use super::*; @@ -1449,6 +1417,66 @@ mod tests { } } + /// #1365: the vendored planner reads a requirements line with the same + /// grammar as the lock inventory (`exact_pin`), so a pin scan offers is + /// one vendor wires. pip reads the legacy parenthesised `six (==1.16.0)` + /// as the exact pin `==1.16.0`. Arbitrary equality stays a refusal + /// here by policy, although the inventory and hosted mode differ on it. + #[test] + fn find_pin_agrees_with_the_inventory_exact_pin() { + for (line, inventory, vendored) in [ + ("six==1.16.0", true, true), + ("six (==1.16.0)", true, true), + ("six (== 1.16.0)", true, true), + ("six===1.16.0", false, false), + ("six==1.16.0,!=1.15", false, false), + ] { + assert_eq!( + crate::utils::requirements::exact_pin(line) == Some(("six", "1.16.0")), + inventory, + "inventory: {line}" + ); + let found = find_pin(&format!("{line}\n"), "six", "1.16.0"); + assert_eq!( + matches!(found, PinSearch::Exact { .. }), + vendored, + "vendored: {line} -> {found:?}" + ); + if !vendored { + assert_eq!(found, PinSearch::Range, "{line}"); + } + } + // PEP 440 reads an epoch and a `v` prefix; pip installs 1.16.0. + for line in ["six==0!1.16.0\n", "six==v1.16.0\n"] { + assert!( + matches!(find_pin(line, "six", "1.16.0"), PinSearch::Exact { .. }), + "{line}" + ); + } + // A pin whose other options the rewrite would drop is refused. + for line in [ + "six==1.16.0 --config-settings x=y --hash=sha256:ab\n", + "six==1.16.0 --hash=sha256:ab -C x=y\n", + "six==1.16.0 --hash sha256:ab -Cx=y\n", + ] { + assert_eq!(find_pin(line, "six", "1.16.0"), PinSearch::Range, "{line}"); + } + assert!(matches!( + find_pin( + "six==1.16.0 --hash sha256:ab --hash=sha256:cd\n", + "six", + "1.16.0" + ), + PinSearch::Exact { hashed: true, .. } + )); + // A line that names the package but is no clean requirement is + // never treated as absent (fail closed). + for line in ["six @ ./six-1.16.0.whl extra\n", "six/x==1.16.0\n"] { + assert_eq!(find_pin(line, "six", "1.16.0"), PinSearch::Range, "{line}"); + assert!(names_package(line, "six"), "{line}"); + } + } + #[test] fn find_pin_classifies_every_shape() { // Clean pin, with marker + hash flags captured. @@ -1597,6 +1625,28 @@ mod tests { ); } + /// #1365: the legacy parenthesised pin is the exact pin pip reads, so + /// vendor wires it (it used to refuse with a false "not pinned"), and + /// the revert restores the line as spelled. + #[tokio::test] + async fn wires_and_reverts_a_parenthesised_pin() { + for pin in ["six (==1.16.0)", "six (== 1.16.0)", "six ( ==1.16 )"] { + let original = format!("requests==2.31.0\n{pin}\n"); + let tmp = write_root(&original).await; + let wiring = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + .await + .unwrap_or_else(|err| panic!("{pin}: {err:?}")); + assert_eq!( + read_root(tmp.path()).await, + format!("requests==2.31.0\n{}\n", expected_unhashed_line()), + "{pin}" + ); + let outcome = revert_requirements(&entry_for(wiring), tmp.path(), false).await; + assert!(outcome.success, "{pin}"); + assert_eq!(read_root(tmp.path()).await, original, "{pin}"); + } + } + #[tokio::test] async fn rewrites_hash_pinned_continuation_and_preserves_crlf() { // A hash-pinned requirement spanning two physical lines, CRLF file. @@ -2842,10 +2892,10 @@ mod tests { /// pin during a later scan. #[test] fn parse_requirement_line_ignores_path_and_nonname_lines() { - assert!(parse_requirement_line(&expected_line()).is_none()); - assert!(parse_requirement_line("./local/wheel.whl --hash=sha256:abc").is_none()); - assert!(parse_requirement_line("=six==1.0").is_none()); - assert!(parse_requirement_line("[section]").is_none()); + assert!(parse_requirement(strip_comment(&expected_line())).is_none()); + assert!(parse_requirement(strip_comment("./local/wheel.whl --hash=sha256:abc")).is_none()); + assert!(parse_requirement(strip_comment("=six==1.0")).is_none()); + assert!(parse_requirement(strip_comment("[section]")).is_none()); // A stray path line neither matches nor shifts the classification of // the real pin below it. assert_eq!(