diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index b76550b1b..72d3c128f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1594,7 +1594,7 @@ command's integration tests check its output. | `redirect_unconfirmed` | `skipped` (`details.mode: "hosted"`) | hosted `scan` / `get` (v5.0, additive): the patch was granted but no lockfile entry pinning it could be rewritten. The status and exit code are unchanged for now, pending the open hosted exit-policy decision (#704); `--silent` hides the human line. | | `lockfile_unreadable` / `lockfile_unparseable` / `patched_ref_invalid` / `patched_ref_unattributable` | run-level `warnings[]` | vex (every form): lockfile-discovery diagnostics — see "Manifest-less VEX (lockfile discovery)". Never flip the exit on their own. | | `vex_npm_shrinkwrap_only` | run warning and `failed[].reason` | vex (every form, #899): the patch is wired only in a root `npm-shrinkwrap.json` with no `package-lock.json` twin, which npm >= 12 never reads; no statement until the twin exists. `list` / `rollback` / `remove` still manage the wiring. | -| `vendor_multiple_lockfiles` / `pypi_multiple_lockfiles` | `skipped` (warning) | vendor: a sibling lockfile of another package manager (for PyPI, also a root `requirements.txt` that pins the package beside the wired tool lock) will still install UNPATCHED bytes; names the wired winner + the ignored locks. Beside `Pipfile.lock`, `uv.lock` or a wired PEP 751 / script lock, an exact registry pin of the package in `requirements.txt` or an in-root `-r` include (`pipenv requirements` or `uv export` output, or the file `pip lock -r requirements.txt` read) is wired with the lock instead and never named here (#612, #1368, #1758): both files then refer to the committed wheel, the ledger entry records both, every revert restores both, a superseding patch re-wires both, and a re-run over a project whose lock is already wired (a vendor from before, or an export made since) wires the export too (`pypi_requirements_sibling_wired`). A pin the requirements wiring cannot rewrite (a range, extras, an include outside the root) stays a loser. Likewise, a `pylock.toml` / `pylock..toml` that `pipenv lock` writes beside `Pipfile.lock` under `[pipenv] use_pylock = true` is wired with it (#1368), with the same ledger, revert, supersede and re-run behavior (`pypi_pipenv_pylock_wired`); a pylock the wiring cannot rewrite in place (a symlink, a non-registry source) stays a loser. | +| `vendor_multiple_lockfiles` / `pypi_multiple_lockfiles` | `skipped` (warning) | vendor: a sibling lockfile of another package manager (for PyPI, also a root `requirements.txt` that pins the package beside the wired tool lock) will still install UNPATCHED bytes; names the wired winner + the ignored locks. Beside `Pipfile.lock`, `uv.lock` or a wired PEP 751 / script lock, an exact registry pin of the package in `requirements.txt` or an in-root `-r` include (`pipenv requirements` or `uv export` output, or the file `pip lock -r requirements.txt` read) is wired with the lock instead and never named here (#612, #1368, #1758): both files then refer to the committed wheel, the ledger entry records both, every revert restores both, a superseding patch re-wires both, and a re-run over a project whose lock is already wired (a vendor from before, or an export made since) wires the export too (`pypi_requirements_sibling_wired`). An exact pin of the package with extras (`six[foo]==1.16.0`), or a file's only line for it when that line is an unpinned extras requirement (`six[foo]`, which hosted mode also rewrites), cannot be expressed as a wheel path line. It refuses a fresh wiring of the lock `pypi_extras_unsupported`, as the requirements-only wiring does, rather than leave `pip install -r` unpatched; a hosted → vendored switch then keeps both files' hosted pins (#1794). A lock already wired re-runs in sync with the extras pin still named here. Any other pin the requirements wiring cannot rewrite (a range, an include outside the root) stays a loser. Likewise, a `pylock.toml` / `pylock..toml` that `pipenv lock` writes beside `Pipfile.lock` under `[pipenv] use_pylock = true` is wired with it (#1368), with the same ledger, revert, supersede and re-run behavior (`pypi_pipenv_pylock_wired`); a pylock the wiring cannot rewrite in place (a symlink, a non-registry source) stays a loser. | | `vendor_npm_shrinkwrap_only` | `skipped` (warning) | vendor / scan / get `--mode vendored` (npm, #899): the package was wired into `npm-shrinkwrap.json`, the only npm lock (also on an in-sync re-run). npm >= 12 never reads the shrinkwrap and installs the unpatched registry bytes; rename the lock to `package-lock.json` (or commit a copy under that name) and re-run. | | `vendor_npm_deno_lock` | `skipped` (warning) | vendor / scan / get `--mode vendored` (npm, every lockfile flavor, #406): the package was vendored (also on an in-sync re-run), but the project root's `deno.lock` locks the same `name@version`. `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads the rewired lock, so installs Deno runs keep the registry copy and `vex` omits the patch (`vex_deno_lock_copy`); npm / pnpm / yarn / bun installs are patched. To patch what Deno installs, run `deno install` and then `socket-patch scan --mode agent`. | | `vendor_workspace_member_skipped` | `skipped` (warning) | vendor / scan / get `--mode vendored` (npm, #688): a lock entry with the package's `name@version` is the project's own source (a workspace member or a `file:` directory), so it is left alone while the lock's registry copies are vendored; patch that source directly if it needs the fix. When it is the only instance, vendoring refuses instead. | diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index afac1baab..901ea8255 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -3339,6 +3339,46 @@ async fn pip_lock_rerun_wires_a_requirements_pin_added_later() { ); } +/// #1794: the extras refusal guards a FRESH lock wiring only. A pylock +/// already wired to the vendored wheel (by a run before the extras pin +/// was added, or by a release before the refusal) re-runs in sync instead +/// of failing every later `vendor`, and the extras pin stays a named +/// loser as before. +#[tokio::test] +async fn pip_lock_rerun_keeps_a_wired_lock_beside_an_extras_pin() { + let (_tmp, root) = project(); + stage_pip_lock(&root); + std::fs::write(root.join("requirements.txt"), "idna==3.7\n").unwrap(); + stage_manifest(&root); + let (code, env) = run_cli(&root, &["vendor"], &[]); + assert_eq!(code, 0, "{env:#}"); + let wired = std::fs::read_to_string(root.join("pylock.toml")).unwrap(); + assert!( + wired.contains(&format!(".socket/vendor/pypi/{UUID}/")), + "{wired}" + ); + + std::fs::write( + root.join("requirements.txt"), + "idna==3.7\nsix[foo]==1.16.0\n", + ) + .unwrap(); + let (code, env) = run_cli(&root, &["vendor"], &[]); + assert_eq!(code, 0, "an in-sync lock is not refused: {env:#}"); + assert!( + !env.to_string().contains("pypi_extras_unsupported"), + "{env:#}" + ); + assert_eq!( + std::fs::read_to_string(root.join("pylock.toml")).unwrap(), + wired + ); + assert_eq!( + std::fs::read_to_string(root.join("requirements.txt")).unwrap(), + "idna==3.7\nsix[foo]==1.16.0\n" + ); +} + /// #1758: hosted mode pins six in both `requirements.txt` and the /// `pip lock` pylock.toml. The hosted → vendored switch (plain `vendor`, /// the eject, and `scan --mode vendored`) restores both files to their @@ -3415,6 +3455,86 @@ async fn pip_lock_hosted_to_vendored_keeps_the_requirements_patched() { } } +/// #1794: a `requirements.txt` pin with extras (`six[foo]==1.16.0`) can't +/// be written as a vendored wheel path line, so the pylock beside it can't +/// be vendored alone: the switch's upstream restore had just un-hosted the +/// requirements pin, and `pip install -r requirements.txt` would then go +/// from patched to unpatched. The switch refuses the purl with the +/// requirements wiring's own code, and the restore is rolled back, so both +/// files keep their hosted pin byte for byte. Before the fix both files +/// were un-hosted, only pylock.toml was wired, and the run exited 0. +#[tokio::test] +async fn pip_lock_hosted_to_vendored_refuses_an_extras_pin() { + for takeover in [false, true] { + let server = MockServer::start().await; + let hosted_url = mount_hosted_api(&server, true).await; + mount_patch_view(&server).await; + let uri = server.uri(); + let switch: Vec<&str> = if takeover { + vec![ + "scan", + "--mode", + "vendored", + "--yes", + "--api-url", + &uri, + "--org", + ORG, + "--api-token", + "fake-token", + "--patch-server-url", + &uri, + "--vendor-url", + &uri, + ] + } else { + let mut args = vec!["vendor", "--patch-server-url", &uri, "--vendor-url", &uri]; + args.extend(api_args(&uri)); + args + }; + let (_tmp, root) = project(); + std::fs::write( + root.join("requirements.txt"), + "idna==3.7\nsix[foo]==1.16.0\n", + ) + .unwrap(); + std::fs::write(root.join("pylock.toml"), PIP_LOCK).unwrap(); + let files = ["requirements.txt", "pylock.toml"]; + let (code, env) = hosted_scan(&root, &server); + assert_eq!(code, 0, "hosted scan: {env:#}"); + let hosted: Vec = files + .iter() + .map(|file| std::fs::read_to_string(root.join(file)).unwrap()) + .collect(); + for (file, text) in files.iter().zip(&hosted) { + assert!( + text.contains(&hosted_url), + "hosted mode pins {file}:\n{text}\n{env:#}" + ); + } + + stage_manifest(&root); + prebuilt_common::mount_project(&server, &root).await; + let (code, env) = run_cli(&root, &switch, &[]); + assert_eq!(code, 1, "{switch:?}: the purl is refused: {env:#}"); + assert!( + env.to_string().contains("pypi_extras_unsupported"), + "{switch:?}: refused with the requirements wiring's code: {env:#}" + ); + for (file, before) in files.iter().zip(&hosted) { + let text = std::fs::read_to_string(root.join(file)).unwrap(); + assert_eq!( + &text, before, + "{switch:?}: {file} keeps its hosted pin byte for byte\n{env:#}" + ); + } + assert!( + !root.join(".socket/vendor/pypi").exists(), + "{switch:?}: nothing is vendored" + ); + } +} + /// The `pipenv requirements > requirements.txt` export of [`stage_pipenv`]'s /// lock: the file Docker / plain-pip installs read. const PIPENV_EXPORT: &str = "-i https://pypi.org/simple\nsix==1.16.0 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'\n"; diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index a033717ce..69cce4cf5 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -1117,12 +1117,13 @@ async fn pypi_prelude<'p>( warnings.extend(project.warnings.iter().cloned()); // #1368: a `uv export` requirements file is an install // path of its own; wire its pin with the lock. - let sibling = super::pypi_requirements::sibling_pin_wirable( + let sibling = super::pypi_requirements::fresh_sibling_pin( project_root, &canon_name, version, ) - .await; + .await + .map_err(|(code, detail)| refused(code, detail))?; if sibling && dry_run { warnings.push(sibling_dry_run_notice(flavor, &canon_name, version)); } @@ -1157,12 +1158,13 @@ async fn pypi_prelude<'p>( // #1758: the requirements.txt the pylock was locked // from is an install path of its own; wire its pin // with the lock. - let sibling = super::pypi_requirements::sibling_pin_wirable( + let sibling = super::pypi_requirements::fresh_sibling_pin( project_root, &canon_name, version, ) - .await; + .await + .map_err(|(code, detail)| refused(code, detail))?; if sibling && dry_run { warnings.push(sibling_dry_run_notice(flavor, &canon_name, version)); } @@ -1351,14 +1353,18 @@ async fn pypi_prelude<'p>( // requirements.txt`) is an install path of its own; wire its // pin with the lock, or re-wire the one the superseded entry // wired. - let sibling = + let sibling = if target == PipenvTarget::Fresh && superseded.is_none() { + super::pypi_requirements::fresh_sibling_pin(project_root, &canon_name, version) + .await + .map_err(|(code, detail)| refused(code, detail))? + } else { super::pypi_requirements::sibling_pin_wirable(project_root, &canon_name, version) .await - || superseded.as_ref().is_some_and(|prev| { - prev.wiring - .iter() - .any(|r| r.kind == super::pypi_requirements::RECORD_KIND) - }); + } || superseded.as_ref().is_some_and(|prev| { + prev.wiring + .iter() + .any(|r| r.kind == super::pypi_requirements::RECORD_KIND) + }); // #1368: a pylock `pipenv lock` wrote beside Pipfile.lock // (`use_pylock = true`) still pinning the registry release. let pylock_states = diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 25c68c50d..5c30658c5 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -73,6 +73,10 @@ struct PinScan { exact: Vec, /// Some occurrence carries extras. extras: bool, + /// Some occurrence with extras pins exactly this version (`==` or + /// `===`), or is the file's one unpinned line for the package: the pin + /// hosted mode rewrites, which a wheel path line can't (#1794). + exact_extras: bool, /// Some occurrence names the package ambiguously on its own. range: bool, /// Some occurrence exactly pins a DIFFERENT version on its own @@ -129,10 +133,18 @@ fn pin_refusal(scan: &PinScan, split_ok: bool) -> Option { fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { let mut exact: Vec = Vec::new(); let mut found_extras = false; + let mut exact_extras = false; let mut found_range = false; let mut other_branches = false; + // Hosted mode also rewrites a file's one line for the package when it + // is unpinned (`six[foo]`), so that extras line counts as exact too. + let mut named_lines = 0usize; + let mut unpinned_extras = false; for ll in logical_lines(content) { let code = strip_comment(&ll.text); + if requirement_name(code).is_some_and(|name| canonicalize_pypi_name(name) == canon_name) { + named_lines += 1; + } 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). @@ -147,6 +159,11 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { } if req.extras.is_some() { found_extras = true; + exact_extras |= req + .exact_version() + .is_some_and(|pinned| crate::utils::pep440::versions_equal(pinned, version)) + || req.arbitrary_version() == Some(version); + unpinned_extras |= req.specifier.is_empty() && req.direct_reference.is_none(); continue; } // pip resolves `==` under PEP 440 (`==1.16` installs 1.16.0). @@ -182,6 +199,7 @@ fn scan_pins(content: &str, canon_name: &str, version: &str) -> PinScan { PinScan { exact, extras: found_extras, + exact_extras: exact_extras || (unpinned_extras && named_lines == 1), range: found_range, other_branches, } @@ -361,9 +379,10 @@ fn wired_pin_in(content: &str, canon_name: &str, record_uuid: &str) -> Option<(S /// the registry in a shape the wiring rewrites in place, with no vendor line /// for the package yet (#612, #1368). Vendored Pipenv and uv wire such a pin /// together with their lock, as hosted mode rewrites both; anything else (no -/// pin, a range, extras, an include outside the root, an existing vendor -/// line, a symlinked file the rewrite would replace) is left to the -/// `pypi_multiple_lockfiles` warning. +/// pin, a range, an include outside the root, an existing vendor line, a +/// symlinked file the rewrite would replace) is left to the +/// `pypi_multiple_lockfiles` warning, except an exact pin with extras, which +/// refuses a fresh wiring ([`fresh_sibling_pin`]). pub(super) async fn sibling_pin_wirable(root: &Path, canon_name: &str, version: &str) -> bool { let Ok(files) = collect_requirements_files(root).await else { return false; @@ -386,6 +405,69 @@ pub(super) async fn sibling_pin_wirable(root: &Path, canon_name: &str, version: && refuse_symlinked(root, &rels).await.is_ok() } +/// The refusal for a requirements pin with extras, which a vendored wheel +/// path line cannot express. +fn extras_refusal(rel: &str, canon_name: &str) -> (&'static str, String) { + ( + "pypi_extras_unsupported", + format!( + "{rel}: the {canon_name} pin declares extras, which a vendored wheel path line \ + cannot express; remove the extras or use agent mode \ + (`scan --mode agent` + `socket-patch apply`) instead" + ), + ) +} + +/// [`sibling_pin_wirable`] for a lock about to be wired fresh, except +/// that a requirements set pinning exactly `canon_name==version` with +/// extras (`six[foo]==1.16.0`) refuses the package, as the +/// requirements-only wiring does (#1794). The wiring can't rewrite that pin, +/// and wiring the lock alone is no answer: after a hosted -> vendored +/// switch it is the pin the upstream restore just un-hosted, so `pip +/// install -r` would go from patched to unpatched. The refusal keeps the +/// hosted pin instead (the takeover rolls its restore back). A set that +/// already carries a vendor line for the package is left as it was. +pub(super) async fn fresh_sibling_pin( + root: &Path, + canon_name: &str, + version: &str, +) -> Result { + let files = match collect_requirements_files(root).await { + Ok(files) => files, + // A root file the planner refuses to read (say UTF-16, #1120) is + // still one pip installs from. + Err(_) => { + let root_file = root.join(crate::formats::governing_locks::PYPI_REQUIREMENTS); + let text = crate::utils::fs::read_regular_to_bytes(&root_file) + .await + .ok() + .and_then(|bytes| crate::utils::requirements::decode(&bytes)); + return match text { + Some(text) if scan_pins(&text, canon_name, version).exact_extras => { + Err(extras_refusal( + crate::formats::governing_locks::PYPI_REQUIREMENTS, + canon_name, + )) + } + _ => Ok(false), + }; + } + }; + if files + .iter() + .any(|f| vendored_uuid_for(&f.content, canon_name).is_some()) + { + return Ok(false); + } + if let Some(file) = files + .iter() + .find(|f| scan_pins(&f.content, canon_name, version).exact_extras) + { + return Err(extras_refusal(&file.rel, canon_name)); + } + Ok(sibling_pin_wirable(root, canon_name, version).await) +} + /// The `requirements_line` records of a mixed entry (a Pipenv or uv entry /// that also wired its exported requirements file, #612 / #1368). pub(super) const RECORD_KIND: &str = "requirements_line"; @@ -775,17 +857,7 @@ async fn plan_requirements( for (file, scan) in files.iter().zip(scans) { match pin_refusal(&scan, split_ok) { - Some(PinRefusal::Extras) => { - return Err(( - "pypi_extras_unsupported", - format!( - "{}: the {canon_name} pin declares extras, which a vendored wheel path \ - line cannot express; remove the extras or use agent mode \ - (`scan --mode agent` + `socket-patch apply`) instead", - file.rel - ), - )); - } + Some(PinRefusal::Extras) => return Err(extras_refusal(&file.rel, canon_name)), Some(PinRefusal::NotPinned) => { return Err(( "pypi_requirement_not_pinned", @@ -1809,6 +1881,79 @@ mod tests { tmp } + /// #1794: a requirements pin with extras of the release being vendored + /// (`==` or `===`, in the root or an include, UTF-8 or UTF-16) refuses + /// a fresh lock wiring with `pypi_extras_unsupported`, naming the file, + /// instead of reading as an unwirable loser, and so does the file's one + /// unpinned extras line (`six[foo]`), which hosted mode rewrites too. + /// Extras on another version or a range, two unpinned lines, a wirable pin, and a set that already carries a vendor + /// line for the package keep the old answer. + #[tokio::test] + async fn fresh_sibling_pin_refuses_an_exact_extras_pin() { + for (root, include, file) in [ + ("idna==3.7\nsix[foo]==1.16.0\n", None, "requirements.txt"), + ( + "Six[foo,bar] == 1.16 ; python_version >= \"3\"\n", + None, + "requirements.txt", + ), + ("six[foo]===1.16.0\n", None, "requirements.txt"), + ("-r base.txt\n", Some("six[foo]==1.16.0\n"), "base.txt"), + ("idna==3.7\nsix[foo]\n", None, "requirements.txt"), + ] { + let tmp = write_root(root).await; + if let Some(include) = include { + tokio::fs::write(tmp.path().join("base.txt"), include) + .await + .unwrap(); + } + let (code, detail) = fresh_sibling_pin(tmp.path(), "six", "1.16.0") + .await + .unwrap_err(); + assert_eq!(code, "pypi_extras_unsupported", "{root}: {detail}"); + assert!(detail.starts_with(&format!("{file}: ")), "{root}: {detail}"); + } + + let tmp = tempfile::tempdir().unwrap(); + let utf16: Vec = [0xFF, 0xFE] + .into_iter() + .chain( + "six[foo]==1.16.0\r\n" + .encode_utf16() + .flat_map(u16::to_le_bytes), + ) + .collect(); + tokio::fs::write(tmp.path().join("requirements.txt"), utf16) + .await + .unwrap(); + let (code, _) = fresh_sibling_pin(tmp.path(), "six", "1.16.0") + .await + .unwrap_err(); + assert_eq!(code, "pypi_extras_unsupported", "UTF-16"); + + for (root, wirable) in [ + ("six[foo]==1.15.0\n", false), + ("six[foo]>=1.15\n", false), + ("six[foo]\nsix[bar]\n", false), + ("idna[foo]==1.16.0\nsix==1.16.0\n", true), + ("VENDORED", false), + ] { + let vendored = + format!("./{REL_WHEEL} # socket-patch vendor: six==1.16.0\nsix[foo]==1.16.0\n"); + let root = if root == "VENDORED" { + vendored.as_str() + } else { + root + }; + let tmp = write_root(root).await; + assert_eq!( + fresh_sibling_pin(tmp.path(), "six", "1.16.0").await, + Ok(wirable), + "{root}" + ); + } + } + /// #721: pip installs from a UTF-16 requirements file (what Windows /// PowerShell 5.1's `pip freeze >` writes), so vendoring must refuse it /// by name, as the root file or as an include, never wire around it.