From 15596cfdeedba77dda6f467986bd06070a3889ad Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 15:24:49 +0000 Subject: [PATCH 1/2] Start fix for #1028 Assisted-by: Claude Code:claude-opus-5-5 From 2fa402155cf0be972742ced94a99df92c8dae8c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 15:32:37 +0000 Subject: [PATCH 2/2] Follow -r after other options in requirements A requirements line like `--pre -r dev.txt` or `-i https://mirror/simple -r base.txt` is an include pip follows, but socket-patch only looked at the first word of the line. Lock-only scans then never discovered the pins in that include and reported "No patches", and the vendored planner, in-use probe and lock inventory were blind to it too. Read the option part of the line the way pip's optparse does: walk every option word, consume the values of value-taking options (so `-i -r x` is an index URL, not an include), accept unique long-option prefixes, and follow the first `-r`. Requirement and `-e` lines stay non-includes. Fixes #1028 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/scan_requirements_lock_only.rs | 31 ++- .../src/vendor/pypi_requirements.rs | 192 ++++++++++++++++-- 2 files changed, 206 insertions(+), 17 deletions(-) diff --git a/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs index 5feeb532b..4617afef4 100644 --- a/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs +++ b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs @@ -8,7 +8,9 @@ //! * #412: pins reached through in-root `-r` includes; //! * #994: include targets pip unquotes (`-r "dev reqs.txt"`, //! `--requirement="dev.txt"`, `-r dev\ reqs.txt`) or expands -//! (`-r ${REQDIR}/dev.txt`). +//! (`-r ${REQDIR}/dev.txt`); +//! * #1028: a `-r` that follows other options on the line +//! (`--pre -r dev.txt`, `-i URL -r dev.txt`). //! //! Driven through the built binary against a mock patch API; the //! assertion is what discovery sends to the batch endpoint and the @@ -207,3 +209,30 @@ async fn lock_only_scan_discovers_env_var_include_target() { ) .await; } + +/// #1028: pip runs optparse over every option word of a line, so a `-r` +/// that follows another option (`--pre`, `-i URL`, `-c FILE`) is still +/// an include pip follows. +#[tokio::test] +async fn lock_only_scan_discovers_include_after_other_options() { + let cases: &[&str] = &[ + "--pre -r dev.txt\n", + "-i https://pypi.org/simple -r dev.txt\n", + "--index-url=https://pypi.org/simple -r dev.txt\n", + "--prefer-binary -r dev.txt\n", + "-c c.txt -r dev.txt\n", + "--prefer-binary --requirement=dev.txt\n", + ]; + for root in cases { + eprintln!("case {root:?}"); + assert_lock_only_discovers( + &[ + ("requirements.txt", root), + ("dev.txt", "sp-fixture-optfirst==1.0.0\n"), + ("c.txt", "\n"), + ], + &["pkg:pypi/sp-fixture-optfirst@1.0.0"], + ) + .await; + } +} diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 7733f6154..d056bb37a 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -1000,31 +1000,121 @@ fn include_target(text: &str) -> Option { include_target_with(text, |name| std::env::var(name).ok()) } +/// The long options pip's requirements-file parser knows that take a +/// value (`SUPPORTED_OPTIONS` plus the per-requirement +/// `SUPPORTED_OPTIONS_REQ`, and `--install-option` from older pips). +const REQ_FILE_VALUE_OPTIONS: &[&str] = &[ + "--index-url", + "--extra-index-url", + "--constraint", + "--requirement", + "--editable", + "--find-links", + "--no-binary", + "--only-binary", + "--trusted-host", + "--use-feature", + "--global-option", + "--install-option", + "--hash", + "--config-settings", +]; + +/// The flag-only long options of pip's requirements-file parser. Only +/// used to resolve optparse's unique-prefix abbreviations +/// (`--requirem` is `--requirement`, `--require` is ambiguous). +const REQ_FILE_FLAG_OPTIONS: &[&str] = + &["--no-index", "--prefer-binary", "--require-hashes", "--pre"]; + +/// Resolve a long option name the way optparse's `_match_abbrev` does: an +/// exact name, else the one known option it is a unique prefix of. +/// `None` for an unknown or ambiguous name. +fn resolve_long_option(name: &str) -> Option<&'static str> { + let all = REQ_FILE_VALUE_OPTIONS + .iter() + .chain(REQ_FILE_FLAG_OPTIONS) + .copied(); + if let Some(exact) = all.clone().find(|o| *o == name) { + return Some(exact); + } + let mut matches = all.filter(|o| o.starts_with(name)); + let first = matches.next()?; + matches.next().is_none().then_some(first) +} + /// [`include_target`] with the environment lookup injected (tests). +/// +/// pip splits a line into its requirement part (the leading words that +/// don't start with `-`) and its options, and runs optparse over every +/// option word (#1028). A line with a requirement or an `-e` is a +/// requirement, never an include; otherwise the first `-r` value is the +/// file pip follows (`opts.requirements[0]`). An option's value is +/// consumed even when it looks like `-r`, as optparse does. fn include_target_with(text: &str, env: impl Fn(&str) -> Option) -> Option { let code = expand_env_vars(strip_comment(text), env); + // pip's break_args_options: a leading word without `-` is a + // requirement, whose per-requirement options never recurse. + if !code.trim_start().starts_with('-') { + return None; + } // An unbalanced quote is pip's "Could not split options" error: there // is no file to follow. let mut words = shlex_split(&code)?.into_iter(); - let first = words.next()?; - let target = if let Some(rest) = first.strip_prefix("--requirement=") { - // `--requirement= dev.txt` (a space after the `=`) is read as - // the next word, as before the shlex split. - if rest.is_empty() { - words.next() - } else { - Some(rest.to_string()) + let mut target: Option = None; + while let Some(word) = words.next() { + if word == "--" { + break; } - } else { - match first.as_str() { - "-r" | "--requirement" => words.next(), + if let Some(long) = word.strip_prefix("--") { + let (name, attached) = match long.split_once('=') { + Some((name, value)) => (name, Some(value.to_string())), + None => (long, None), + }; + // An unknown option is pip's parse error; read past it as a + // flag rather than drop the rest of the line. + let Some(option) = resolve_long_option(&format!("--{name}")) else { + continue; + }; + if !REQ_FILE_VALUE_OPTIONS.contains(&option) { + continue; + } + let value = match attached { + // `--requirement= dev.txt` (a space after the `=`) is + // read as the next word, as before the shlex split. + Some(v) if v.is_empty() && option == "--requirement" => words.next(), + Some(v) => Some(v), + None => words.next(), + }; + match option { + "--editable" => return None, + "--requirement" if target.is_none() => target = value, + _ => {} + } + } else if let Some(short) = word.strip_prefix('-') { + let mut chars = short.chars(); + let Some(flag) = chars.next() else { + continue; + }; + if !matches!(flag, 'i' | 'c' | 'r' | 'e' | 'f') { + continue; + } // pip's optparse also accepts the attached short form - // (`-rdev.txt`, `-r"dev reqs.txt"`). No other - // requirements-file option starts with `-r`. - t if t.starts_with("-r") && !t.starts_with("--") => Some(t[2..].to_string()), - _ => None, + // (`-rdev.txt`, `-r"dev reqs.txt"`). + let rest = chars.as_str(); + let value = if rest.is_empty() { + words.next() + } else { + Some(rest.to_string()) + }; + match flag { + 'e' => return None, + 'r' if target.is_none() => target = value, + _ => {} + } } - }; + // A bare word among the options is an optparse positional that + // pip ignores. + } target.filter(|t| !t.is_empty()) } @@ -2542,6 +2632,76 @@ mod tests { } } + /// #1028: pip parses every option word of a line, so a `-r` after + /// other options is an include, an option's value is never read as + /// `-r`, and the first of several `-r`s wins (`opts.requirements[0]`). + /// A requirement line (`six==1.16.0 ...`) or an editable is never an + /// include. + #[test] + fn include_target_scans_every_option_word() { + let env = |_: &str| None; + let cases: &[(&str, Option<&str>)] = &[ + ("--pre -r dev.txt", Some("dev.txt")), + ("-i https://pypi.org/simple -r dev.txt", Some("dev.txt")), + ("-ihttps://pypi.org/simple -r dev.txt", Some("dev.txt")), + ("--index-url https://x/simple -r dev.txt", Some("dev.txt")), + ("--index-url=https://x/simple -r dev.txt", Some("dev.txt")), + ( + "--extra-index-url https://x/simple -r dev.txt", + Some("dev.txt"), + ), + ("--prefer-binary -r dev.txt", Some("dev.txt")), + ("-c c.txt -r dev.txt", Some("dev.txt")), + ("--constraint c.txt --requirement dev.txt", Some("dev.txt")), + ("--prefer-binary --requirement=dev.txt", Some("dev.txt")), + ("-f ./wheels -rdev.txt", Some("dev.txt")), + ("--no-binary :all: -r dev.txt", Some("dev.txt")), + ("--trusted-host h -r \"dev reqs.txt\"", Some("dev reqs.txt")), + ("-r dev.txt -r other.txt", Some("dev.txt")), + ("-r dev.txt --pre", Some("dev.txt")), + // optparse's unique-prefix long options. + ("--requirem dev.txt", Some("dev.txt")), + ("--pre --requirem=dev.txt", Some("dev.txt")), + // An option value that looks like `-r` is the value. + ("-i -r dev.txt", None), + ("--index-url -r", None), + ("-c -r", None), + // Only constraints: not a requirements include. + ("-c c.txt", None), + // A requirement line's per-requirement options never recurse. + ("six==1.16.0 -r dev.txt", None), + // An editable makes the line a requirement. + ("-e ./pkg -r dev.txt", None), + ("-r dev.txt -e ./pkg", None), + // `--` ends the options. + ("--pre -- -r dev.txt", None), + ("--pre", None), + ]; + for (line, want) in cases { + assert_eq!(include_target_with(line, env).as_deref(), *want, "{line:?}"); + } + } + + /// #1028: the planner follows an include that comes after another + /// option and wires the pin there instead of appending at the root. + #[tokio::test] + async fn include_after_other_option_is_followed() { + let tmp = write_root("--pre -r dev.txt\n").await; + tokio::fs::write(tmp.path().join("dev.txt"), "six==1.16.0\n") + .await + .unwrap(); + let wiring = wire_requirements(tmp.path(), "six", "1.16.0", REL_WHEEL, SHA) + .await + .unwrap(); + assert_eq!(wiring.len(), 1); + assert_eq!(wiring[0].file, "dev.txt"); + assert_eq!( + read_root(tmp.path()).await, + "--pre -r dev.txt\n", + "root untouched — no duplicate appended" + ); + } + /// #994: the planner follows a quoted include with a space in its name /// and wires the pin there instead of appending a duplicate at the root. #[tokio::test]