diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 8e533b2c2..fefc14363 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1566,7 +1566,7 @@ command's integration tests check its output. | `hosted_reverted` | `removed` | remove (v5.0): a hosted lockfile pin was restored to its upstream registry entry as part of removing the patch (`verified` on dry-run). Beside a manifest entry it bypasses `summary.removed` like `vendor_reverted`. v5.0: carries `details.mode: "hosted"`. | | `hosted_revert_failed` | top-level error | remove (v5.0): a matched hosted pin could not be restored to its upstream registry entry (`--offline`, a registry that does not answer, `bun.lockb`, a lock shape the restore refuses — see "Hosted unwind coverage"), or writing the restored files failed; the message names the `git checkout -- ` remedy. The manifest was not modified, exit 1. Rollback's counterpart is a `failed` `hosted_restore_refused` event (exit 1). v4's `hosted_revert_unsupported` is no longer emitted (every ecosystem has a restore). | | `cargo_cache_patch_kept` | `scan --prune`/`--sync` top-level `warnings[]` (human: `GC: …`) | a Cargo manifest entry the crawl no longer reports (the lock bumped or dropped the crate) was NOT pruned because its copy in the shared `$CARGO_HOME/registry/src` cache still carries the agent-mode patch; the record holds the only blobs that can restore it. The detail names the purl and `socket-patch rollback ` (with `--global` when a `cargo vendor` dir hides the registry cache from the project crawl), which restores the copy and drops the entry. Never affects the exit. | -| `reinstall_required` | rollback `warnings[]` | rollback (v5.0): vendored/hosted wiring was unwound, but installed trees keep their patched bytes until the next package-manager install — the stale-install advisory. When the run also emits a Bun advisory (`vendor_bun_reinstall_required` / `redirect_bun_reinstall_required`) the detail and the human note add " (Bun: a plain `bun install` keeps them; run `bun install --force`)". When it emits a PyPI advisory (`vendor_pypi_reinstall_required` / `redirect_pypi_reinstall_required`) they add " (PDM, uv and Pipenv keep a same-version install through a plain sync: run the reinstall the pypi_reinstall_required warning names)". | +| `reinstall_required` | rollback `warnings[]` | rollback (v5.0): vendored/hosted wiring was unwound, but installed trees keep their patched bytes until the next package-manager install — the stale-install advisory. When the run also emits a Bun advisory (`vendor_bun_reinstall_required` / `redirect_bun_reinstall_required`) the detail and the human note add " (Bun: a plain `bun install` keeps them; run `bun install --force`)". When it emits a PyPI advisory (`vendor_pypi_reinstall_required` / `redirect_pypi_reinstall_required`) they add " (pip, PDM, uv and Pipenv keep a same-version install through a plain install or sync: run the reinstall the pypi_reinstall_required warning names)". | | `hosted_state_not_preservable` | rollback / remove `warnings[]` | rollback `--preserve-state` (v5.0): hosted pins were restored to upstream anyway — the lockfile pins are hosted mode's only record, so there is no local state to preserve; re-run `scan --mode hosted` to re-wire. (`remove --preserve-state` reports the same code in its `warnings[]` — manifest-backed and hosted-only alike — and prints it as a `Note:` on stderr in human mode.) | | `out_of_scope_copies_restored` | rollback `warnings[]` | path-scoped rollback (v5.0): a selected patch had installed copies outside the given patterns; ALL copies were restored (patches are per-package). Informational — never flips the exit. | | `patched_in_other_scope` | rollback `warnings[]` | the manifest records no scope, so before a rollback drops a record it restored, found already original, or found not installed in its own scope, it checks the other scope (the project at `--cwd` for a `-g` / `--global-prefix` run, the default global install otherwise). A copy there that still holds the patch's afterHash bytes, or has a patched file that exists but cannot be hashed, keeps the record (with its variant group) and its blobs. This warning names the purl, the copy and the rollback of just that purl that restores it (`socket-patch rollback ` or `socket-patch rollback -g`, plus `--cwd` when the run used one). Never flips the exit (#450). | @@ -1737,8 +1737,8 @@ command's integration tests check its output. | `redirect_vlt_artifact_unverifiable` | `warnings[]` (warning), `skipped` (hosted event) | scan/get `--mode hosted` (vlt): before any takeover or rewrite (dry runs included), each granted artifact with a default-registry instance in `vlt-lock.json` (or, for a purl a `flavor: "vlt"` vendored entry claims, its vendored node, probed before the takeover reverts it) is fetched once as vlt fetches it (`accept-encoding: gzip;q=1.0, identity;q=0.5`, no `Authorization`, up to 10 redirects) and must return 200 with no content encoding (or `identity`) and the granted sha512. On failure (`content-encoding `, `sha512 mismatch`, `http `, `fetch error `, `offline`) the dep is withheld from every rewriter when vlt drives or it is vlt-vendored (which also keeps it vendored), and from the vlt rewrite only otherwise (detail "…; vlt-lock.json was not changed for {purl}"; only the sibling lock this run rewrote can confirm it). A lock already pinned by an earlier run is left pinned, and neither confirmed nor attested. Projects without `vlt-lock.json` make no such request. The detail quotes the artifact URL (and any fetch error that echoes it) with its grant-token path level, the one just before the patch uuid, spelled ``; host, uuid and leaf stay. The in-memory hosted engine (`hosted-bundle`, the Node addon) has no network for this fetch, so it judges every in-scope artifact as `--offline` does (withheld, never pinned; the vendored takeover it refuses anyway). Exit 0. | | `redirect_vlt_reinstall_required` | `warnings[]` (advisory); rollback/remove `warnings[]` (+ human stderr) | vlt: `vlt-lock.json` pins (or, after rollback/remove, no longer pins) Socket-patched packages, and vlt never refreshes an installed copy. The heal removes `node_modules/.vlt-lock.json` and each stale `node_modules/.vlt/` of a Socket-owned node (never a link's target, never outside the project, never a copy it cannot judge) unless `--no-vlt-install-cleanup` or `--dry-run`. It never removes an optional node's copy (lock flags 1 or 3, or flags it cannot read): `vlt install` does not put a removed optional dependency back (its link dangles) unless the same install also reinstalls a non-optional node, so such a copy is left stale and the detail says to run `vlt ci` (or delete `node_modules` and run `vlt install`); vlt 0.0.0-30 … 1.0.4 install no optional dependency from the lock of a project that declares only optional dependencies, so there both commands remove the installed copy and the detail says to upgrade vlt to 1.0.5 or later first. The detail says whether copies were removed, left stale by a skipped cleanup, could not be checked, or none were stale, and adds how many optional copies were kept whenever there are any. The kept optional copies are named by what they are: `unpatched copies of optional dependencies` after `scan`/`get`, `patched copies of optional dependencies` after `rollback`/`remove`, and `installed copies of the vendored optional dependencies` after a hosted → vendored takeover (the copy the hosted pin left installed, which may still be the registry bytes). Stale or unchecked copies are not attested by the run's `--vex`, nor is a confirmed vlt pin the heal did not check (a URL on a host other than patch.socket.dev and the configured `--patch-server-url`/`--api-url`). A hidden lock that cannot be removed keeps every store entry. Invalidation failures only warn. | | `redirect_bun_reinstall_required` | rollback/remove `warnings[]` (+ human stderr), `vendor --revert` `warnings[]` | a wet hosted unwind that restored `bun.lock` / `bun.lockb` pins to the registry record while their installed copy may be kept (the `vendor_bun_reinstall_required` rule: Bun's hoisted linker keeps it through a plain `bun install`). One run-level warning naming every such `name@version` and `bun install --force` (or deleting `node_modules`). `vendor --revert` leaves it out when the vendored revert already advised `vendor_bun_reinstall_required` for every package it re-hosted. | -| `vendor_pypi_reinstall_required` | `skipped` (advisory; human: `Warning: …`); rollback/remove `warnings[]`; `scan --prune` top-level `warnings[]` (human: `GC: …`) | a PyPI vendored revert (`vendor --revert`, rollback / remove of a vendored entry, the manifest reconcile, `--preserve-state` and `--dry-run` included) of a PDM, uv or Pipenv entry (#477). These tools reinstall a same-version package only when the LOCKED candidate is a URL or file that differs from the installed one, so once the lock is back on the registry release a plain `pdm sync` / `pdm install`, `uv sync` or `pipenv sync` reports nothing to do and keeps the vendored build. The detail names the purl and the reinstall that restores the upstream bytes: `pdm sync --reinstall` (or recreating the virtualenv / `__pypackages__`), `uv sync --reinstall-package `, or Pipenv's `pipenv run pip uninstall -y && pipenv sync` (the categories from `Pipfile.lock`; `pipenv --rm && pipenv sync` for a clean virtualenv). Poetry, requirements.txt, Hatch and PEP 751 entries do not get it. | -| `redirect_pypi_reinstall_required` | rollback/remove `warnings[]` (+ human stderr), `vendor --revert` `warnings[]` | a hosted unwind (wet or `--dry-run`) that restored PyPI pins in `pdm.lock`, `uv.lock` or `Pipfile.lock` to their registry entry: the `vendor_pypi_reinstall_required` rule for hosted pins (the patched build's `direct_url.json` still names the patch server). One run-level warning naming each purl and its reinstall. | +| `vendor_pypi_reinstall_required` | `skipped` (advisory; human: `Warning: …`); rollback/remove `warnings[]`; `scan --prune` top-level `warnings[]` (human: `GC: …`) | a PyPI vendored revert (`vendor --revert`, rollback / remove of a vendored entry, the manifest reconcile, `--preserve-state` and `--dry-run` included) of a requirements.txt (pip), PDM, uv or Pipenv entry (#477, #1953). These tools reinstall a same-version package only when the LOCKED candidate is a URL or file that differs from the installed one, so once the lock is back on the registry release a plain `pdm sync` / `pdm install`, `uv sync` or `pipenv sync` reports nothing to do and keeps the vendored build; once a requirements line is back to `name==version`, `pip install -r` (and `uv pip install -r`) reports it already satisfied and keeps it too. The detail names the purl and the reinstall that restores the upstream bytes: `pip uninstall -y ` then `pip install -r ` for each requirements file that wired the entry (or the `uv pip uninstall` / `uv pip install -r` pair), which keeps those files' version ranges, index and hash options, `pdm sync --reinstall` (or recreating the virtualenv / `__pypackages__`), `uv sync --reinstall-package `, or Pipenv's `pipenv run pip uninstall -y && pipenv sync` (the categories from `Pipfile.lock`; `pipenv --rm && pipenv sync` for a clean virtualenv). Poetry, Hatch and PEP 751 entries do not get it. | +| `redirect_pypi_reinstall_required` | rollback/remove `warnings[]` (+ human stderr), `vendor --revert` `warnings[]` | a hosted unwind (wet or `--dry-run`) that restored PyPI pins in `pdm.lock`, `uv.lock`, `Pipfile.lock` or a pip requirements file (`*.txt`) to their registry entry: the `vendor_pypi_reinstall_required` rule for hosted pins (the patched build's `direct_url.json` still names the patch server). One run-level warning naming each purl and its reinstall; a pin wired by both a lock and a requirements file (a `uv export` beside `uv.lock`) gets the lock tool's reinstall. | | `redirect_withdrawn_check_failed` | `warnings[]` (warning; human: `Warning: …`) | scan `--mode hosted`: the reference lookup for the uuids the lockfiles pin failed, so the run could not tell whether any was withdrawn. Every pin is kept; status and exit unchanged. See "Withdrawn pins". | | `redirect_withdrawn_restore_failed` | `warnings[]` (warning; human: `Warning: …` / `Error: …`) | scan `--mode hosted`: a pin whose patch is `withdrawn` / `not_found` could not be restored to its upstream registry entry (the restore refused it, writing the restored files failed, or the apply lock was held). The pin is kept; the detail names the remedy (`git checkout -- `, or re-run / `socket-patch rollback `). Status and exit unchanged. | | `patch_withdrawn` | `rolledBack` / `verified` (hosted event) | scan `--mode hosted`: the lockfile pinned a patch the server reports `withdrawn` or `not_found`; the pin was restored (dry run: would be) to its upstream registry entry. `reason` names the uuid and status. See "Withdrawn pins". | diff --git a/crates/socket-patch-cli/src/commands/hosted_unwind.rs b/crates/socket-patch-cli/src/commands/hosted_unwind.rs index e0d8bd04a..529e61881 100644 --- a/crates/socket-patch-cli/src/commands/hosted_unwind.rs +++ b/crates/socket-patch-cli/src/commands/hosted_unwind.rs @@ -144,16 +144,17 @@ pub(crate) async fn run_hosted_leg(common: &GlobalArgs, pins: &[HostedPin]) -> H } } } - // PDM, uv and Pipenv keep a same-version install through a plain sync - // (#477): name the reinstall that restores the upstream bytes. + // pip, PDM, uv and Pipenv keep a same-version install through a plain + // install or sync (#477, #1953): name the reinstall that restores the + // upstream bytes. if outcome.flush_error.is_none() { - use crate::commands::pypi_reinstall::{advisory, Tool, HOSTED_CODE}; - let unwound: Vec<(String, Tool)> = outcome + use crate::commands::pypi_reinstall::{advisory, Tool, Unwound, HOSTED_CODE}; + let unwound: Vec = outcome .restored() .filter(|pin| pin.purl.starts_with("pkg:pypi/")) .filter_map(|pin| { - let tool = pin.files.iter().find_map(|f| Tool::of_file(f))?; - Some((pin.purl.clone(), tool)) + let tool = Tool::of_files(&pin.files)?; + Some(Unwound::new(&pin.purl, tool, &pin.files)) }) .collect(); if let Some(detail) = advisory(&common.cwd, &unwound).await { diff --git a/crates/socket-patch-cli/src/commands/pypi_reinstall.rs b/crates/socket-patch-cli/src/commands/pypi_reinstall.rs index b2505c9a5..0043125c0 100644 --- a/crates/socket-patch-cli/src/commands/pypi_reinstall.rs +++ b/crates/socket-patch-cli/src/commands/pypi_reinstall.rs @@ -9,9 +9,13 @@ //! installs the patch, and why the reverse (URL / file → registry) never //! does: `pdm sync`, `uv sync` and `pipenv sync` all report nothing to do //! and keep the patched build, whose `direct_url.json` still names the -//! patch server or the deleted `.socket/vendor/` wheel. So the generic -//! "until the next package-manager install" note is wrong for them, and -//! the advisory names the reinstall that does restore the upstream bytes. +//! patch server or the deleted `.socket/vendor/` wheel. Plain pip does the +//! same for a requirements file (#1953): once the line is back to +//! `name==version`, `pip install -r` reports `Requirement already +//! satisfied` and keeps the patched wheel, and `pip freeze` keeps emitting +//! its URL. So the generic "until the next package-manager install" note +//! is wrong for them, and the advisory names the reinstall that does +//! restore the upstream bytes. use socket_patch_core::utils::purl::strip_purl_qualifiers; @@ -22,7 +26,7 @@ pub(crate) const HOSTED_CODE: &str = "redirect_pypi_reinstall_required"; /// Appended to rollback's generic reinstall note next to either advisory. pub(crate) const NOTE_QUALIFIER: &str = - " (PDM, uv and Pipenv keep a same-version install through a plain sync: \ + " (pip, PDM, uv and Pipenv keep a same-version install through a plain install or sync: \ run the reinstall the pypi_reinstall_required warning names)"; /// True when `codes` carry a PyPI reinstall advisory. @@ -33,6 +37,9 @@ pub(crate) fn advised<'a>(mut codes: impl Iterator) -> bool { /// The Python tool whose sync keeps a same-version install. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)] pub(crate) enum Tool { + /// A requirements file installed with `pip install -r` (or + /// `uv pip install -r`). + Pip, Pdm, Pipenv, Uv, @@ -42,6 +49,7 @@ impl Tool { /// The tool of a vendored PyPI ledger entry's flavor. pub(crate) fn of_flavor(flavor: &str) -> Option { match flavor { + "requirements" => Some(Tool::Pip), "pdm" => Some(Tool::Pdm), "pipenv" => Some(Tool::Pipenv), "uv" => Some(Tool::Uv), @@ -50,17 +58,73 @@ impl Tool { } /// The tool of a hosted pin's wiring file (`pdm.lock`, `Pipfile.lock`, - /// `uv.lock`, in any directory). + /// `uv.lock`, in any directory, or a file the hosted restore reads as a + /// pip requirements file). pub(crate) fn of_file(rel: &str) -> Option { match rel.rsplit(['/', '\\']).next().unwrap_or(rel) { "pdm.lock" => Some(Tool::Pdm), "Pipfile.lock" => Some(Tool::Pipenv), "uv.lock" => Some(Tool::Uv), - _ => None, + _ => socket_patch_core::patch::redirect::upstream::is_requirements_file(rel) + .then_some(Tool::Pip), + } + } + + /// The tool of a hosted pin wired by `files`. A lock tool wins over a + /// requirements file next to it (a `uv export` requirements.txt beside + /// `uv.lock`): the project installs through the lock, whose sync is + /// what keeps the patched build. + pub(crate) fn of_files>(files: &[S]) -> Option { + files + .iter() + .filter_map(|f| Tool::of_file(f.as_ref())) + .min_by_key(|tool| *tool == Tool::Pip) + } +} + +/// One unwound PyPI package: its purl, the tool whose plain sync or +/// install keeps the patched build, and (pip only) the project-relative +/// requirements files that wired it, which the remedy reinstalls from. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct Unwound { + pub(crate) purl: String, + pub(crate) tool: Tool, + pub(crate) requirements: Vec, +} + +impl Unwound { + /// `purl` under `tool`, keeping the requirements files among `files`. + pub(crate) fn new>(purl: &str, tool: Tool, files: &[S]) -> Self { + let mut requirements: Vec = files + .iter() + .map(AsRef::as_ref) + .filter(|f| Tool::of_file(f) == Some(Tool::Pip)) + .map(str::to_string) + .collect(); + requirements.sort(); + requirements.dedup(); + Unwound { + purl: purl.to_string(), + tool, + requirements, } } } +/// `path` as one POSIX shell word: as-is when it holds only safe +/// characters, else single-quoted. +fn shell_word(path: &str) -> String { + if !path.is_empty() + && path + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b"._-/+=:@,".contains(&b)) + { + path.to_string() + } else { + format!("'{}'", path.replace('\'', "'\\''")) + } +} + /// The distribution name of a `pkg:pypi/@` purl. fn pypi_name(purl: &str) -> Option<&str> { strip_purl_qualifiers(purl) @@ -71,9 +135,46 @@ fn pypi_name(purl: &str) -> Option<&str> { /// The reinstall that brings back the upstream release of `name` under /// `tool`. `pipfile_lock` is the project's parsed `Pipfile.lock` (it picks -/// the `pipenv sync` categories), when readable. -fn remedy(tool: Tool, name: &str, pipfile_lock: Option<&serde_json::Value>) -> String { +/// the `pipenv sync` categories), when readable. `requirements` are the +/// project-relative requirements files that wired a pip package. +fn remedy( + tool: Tool, + name: &str, + requirements: &[String], + pipfile_lock: Option<&serde_json::Value>, +) -> String { match tool { + // Uninstall, then reinstall from the requirements files that + // wired it: unlike a pinned `--force-reinstall`, that keeps the + // files' own version range, index and `--hash` options. + Tool::Pip => { + let (from, args) = if requirements.is_empty() { + ( + "the requirements file".to_string(), + "-r ".to_string(), + ) + } else { + ( + format!( + "the requirements file{} that pinned it", + if requirements.len() == 1 { "" } else { "s" } + ), + requirements + .iter() + .map(|f| format!("-r {}", shell_word(f))) + .collect::>() + .join(" "), + ) + }; + format!( + "pip keeps the installed {name} because the patched build still satisfies the \ + restored requirement (`pip install -r` reports `Requirement already \ + satisfied`, and `pip freeze` still names the patched build): in each \ + environment that installed it, run `pip uninstall -y {name}` and then \ + reinstall from {from} from the project root (`pip install {args}`; under \ + `uv pip`: `uv pip uninstall {name} && uv pip install {args}`)" + ) + } Tool::Pdm => format!( "PDM keeps the installed {name} because the restored lock pins the same version \ (`pdm sync` / `pdm install` report nothing to do): run `pdm sync --reinstall` (or \ @@ -91,15 +192,20 @@ fn remedy(tool: Tool, name: &str, pipfile_lock: Option<&serde_json::Value>) -> S } } -/// The advisory detail for `unwound` (`(purl, tool)` pairs), or `None` when -/// none of them is a PyPI package under PDM, uv or Pipenv. +/// The advisory detail for `unwound`, or `None` when none of them is a +/// PyPI package under pip, PDM, uv or Pipenv. pub(crate) async fn advisory( project_root: &std::path::Path, - unwound: &[(String, Tool)], + unwound: &[Unwound], ) -> Option { let mut parts: Vec<(String, String)> = Vec::new(); let mut pipfile_lock: Option> = None; - for (purl, tool) in unwound { + for Unwound { + purl, + tool, + requirements, + } in unwound + { let Some(name) = pypi_name(purl) else { continue; }; @@ -122,14 +228,14 @@ pub(crate) async fn advisory( if parts.iter().any(|(p, _)| p == &base) { continue; } - parts.push((base, remedy(*tool, name, lock))); + parts.push((base, remedy(*tool, name, requirements, lock))); } if parts.is_empty() { return None; } Some(format!( - "the restored lock pins the same version as the patched build still installed, and \ - a plain sync keeps it: {}", + "the restored lock or requirement pins the same version as the patched build still \ + installed, and a plain sync or install keeps it: {}", parts .iter() .map(|(purl, remedy)| format!("{purl}: {remedy}")) @@ -139,18 +245,20 @@ pub(crate) async fn advisory( } /// Push the vendored revert's advisory onto `warnings` when the reverted -/// ledger entry `key` (lockfile flavor `flavor`) is a PyPI package under -/// PDM, uv or Pipenv. The caller decides the revert unwired the entry. +/// ledger entry `key` (lockfile flavor `flavor`, wiring files `files`) is +/// a PyPI package under pip (a requirements file), PDM, uv or Pipenv. The +/// caller decides the revert unwired the entry. pub(crate) async fn push_vendor_advisory( project_root: &std::path::Path, key: &str, flavor: Option<&str>, + files: &[String], warnings: &mut Vec, ) { let Some(tool) = flavor.and_then(Tool::of_flavor) else { return; }; - if let Some(detail) = advisory(project_root, &[(key.to_string(), tool)]).await { + if let Some(detail) = advisory(project_root, &[Unwound::new(key, tool, files)]).await { warnings.push(socket_patch_core::vendor::VendorWarning::new( VENDOR_CODE, detail, @@ -172,19 +280,111 @@ mod tests { assert_eq!(Tool::of_file("pdm.lock"), Some(Tool::Pdm)); assert_eq!(Tool::of_file("app/Pipfile.lock"), Some(Tool::Pipenv)); assert_eq!(Tool::of_file("uv.lock"), Some(Tool::Uv)); - assert_eq!(Tool::of_file("requirements.txt"), None); + assert_eq!(Tool::of_file("pyproject.toml"), None); + } + + /// #1953: plain pip keeps a same-version install through + /// `pip install -r` (`Requirement already satisfied`), so a vendored + /// `requirements` entry and a hosted pin in any requirements file get + /// the pip remedy. + #[test] + fn requirements_wiring_is_pip() { + assert_eq!(Tool::of_flavor("requirements"), Some(Tool::Pip)); + assert_eq!(Tool::of_file("requirements.txt"), Some(Tool::Pip)); + assert_eq!(Tool::of_file("reqs/dev.txt"), Some(Tool::Pip)); + // Only what the hosted restore itself reads as requirements. + assert_eq!(Tool::of_file("requirements.in"), None); + assert_eq!(Tool::of_file("pylock.toml"), None); + assert_eq!(Tool::of_flavor("python-lock"), None); + // A `uv export` requirements.txt beside uv.lock: uv's sync is the + // install that keeps the patched build. + assert_eq!( + Tool::of_files(&["requirements.txt", "uv.lock"]), + Some(Tool::Uv) + ); + assert_eq!(Tool::of_files(&["requirements.txt"]), Some(Tool::Pip)); + assert_eq!(Tool::of_files::<&str>(&[]), None); + } + + #[tokio::test] + async fn advisory_names_pips_uninstall_and_reinstall() { + let tmp = tempfile::tempdir().unwrap(); + let detail = advisory( + tmp.path(), + &[Unwound::new( + "pkg:pypi/six@1.16.0?artifact_id=x", + Tool::Pip, + &["requirements.txt"], + )], + ) + .await + .expect("an advisory"); + assert!(detail.contains("pkg:pypi/six@1.16.0: pip"), "{detail}"); + assert!(detail.contains("`pip uninstall -y six`"), "{detail}"); + assert!( + detail.contains("`pip install -r requirements.txt`"), + "{detail}" + ); + assert!( + detail.contains("`uv pip uninstall six && uv pip install -r requirements.txt`"), + "{detail}" + ); + assert!(!detail.contains("artifact_id"), "{detail}"); + } + + /// Bugbot on #1954: the remedy reinstalls from the files that wired + /// the pin, not a guessed `requirements.txt`, and drops lock files. + #[tokio::test] + async fn pip_remedy_names_the_wiring_requirements_files() { + let tmp = tempfile::tempdir().unwrap(); + let detail = advisory( + tmp.path(), + &[Unwound::new( + "pkg:pypi/six@1.16.0", + Tool::Pip, + &[ + "reqs/dev.txt", + "my reqs/base.txt", + "uv.lock", + "reqs/dev.txt", + ], + )], + ) + .await + .expect("an advisory"); + assert!( + detail.contains("`pip install -r 'my reqs/base.txt' -r reqs/dev.txt`"), + "{detail}" + ); + assert!(!detail.contains("requirements.txt"), "{detail}"); + assert!(!detail.contains("uv.lock"), "{detail}"); + // No recorded wiring (a `repair`-reconstructed entry): a + // placeholder, never a guessed file name. + let none: &[&str] = &[]; + let detail = advisory( + tmp.path(), + &[Unwound::new("pkg:pypi/six@1.16.0", Tool::Pip, none)], + ) + .await + .expect("an advisory"); + assert!( + detail.contains("`pip install -r `"), + "{detail}" + ); + assert_eq!(shell_word("a'b c"), "'a'\\''b c'"); } #[tokio::test] async fn advisory_names_each_tools_reinstall() { let tmp = tempfile::tempdir().unwrap(); + let none: &[&str] = &[]; let detail = advisory( tmp.path(), &[ - ("pkg:pypi/urllib3@1.26.18".into(), Tool::Pdm), - ("pkg:pypi/six@1.16.0?artifact_id=x".into(), Tool::Pipenv), - ("pkg:pypi/idna@3.7".into(), Tool::Uv), - ("pkg:npm/left-pad@1.3.0".into(), Tool::Uv), + Unwound::new("pkg:pypi/urllib3@1.26.18", Tool::Pdm, none), + Unwound::new("pkg:pypi/six@1.16.0?artifact_id=x", Tool::Pipenv, none), + Unwound::new("pkg:pypi/idna@3.7", Tool::Uv, none), + Unwound::new("pkg:npm/left-pad@1.3.0", Tool::Uv, none), ], ) .await diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 3fd9d1816..3299a891c 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -6511,7 +6511,7 @@ mod tests { fn reinstall_note_defers_to_the_pypi_advisory() { let note = format_reinstall_note(1, false, false, true); assert!( - note.contains("(PDM, uv and Pipenv keep a same-version install") + note.contains("(pip, PDM, uv and Pipenv keep a same-version install through a plain install or sync") && note.contains("pypi_reinstall_required"), "{note}" ); diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 1dc4e921d..1b58eca17 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -4870,7 +4870,7 @@ pub(crate) struct VendorGcSummary { /// The wet reverts' reinstall advisories (`code`, `detail`): Bun's /// `vendor_bun_reinstall_required` (#764) and vlt's /// `vendor_vlt_reinstall_required`, and, for a manifest-dropped PyPI - /// entry under PDM, uv or Pipenv, `vendor_pypi_reinstall_required` + /// entry under pip, PDM, uv or Pipenv, `vendor_pypi_reinstall_required` /// (#477). Every other reverting command surfaces these, and the GC /// must not drop them. pub advisories: Vec<(&'static str, String)>, @@ -4999,13 +4999,18 @@ pub(crate) async fn run_vendor_gc( let mut outcome = dispatch_revert_one(&entry, &common.cwd, false).await; // The patch left the manifest while the package stays locked: // the restored lock pins the version the vendored build is - // installed as, which PDM, uv and Pipenv keep (#477), and a + // installed as, which pip, PDM, uv and Pipenv keep (#477), and a // patched gem archive in Bundler's cache installs again (#1594). if outcome.success && !outcome.kept_artifact { crate::commands::pypi_reinstall::push_vendor_advisory( &common.cwd, &purl, entry.flavor.as_deref(), + &entry + .wiring + .iter() + .map(|w| w.file.clone()) + .collect::>(), &mut outcome.warnings, ) .await; diff --git a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs index a00d1e93c..8e10d1384 100644 --- a/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs +++ b/crates/socket-patch-cli/src/commands/vendored_backend/mod.rs @@ -145,6 +145,11 @@ impl<'a> VendoredBackend<'a> { for key in keys { // Captured before a clean revert drops the entry. let flavor = state.entries.get(key).and_then(|e| e.flavor.clone()); + let wiring_files: Vec = state + .entries + .get(key) + .map(|e| e.wiring.iter().map(|w| w.file.clone()).collect()) + .unwrap_or_default(); let gem = state .entries .get(key) @@ -155,7 +160,7 @@ impl<'a> VendoredBackend<'a> { VendorRevertStep::Failed(_) | VendorRevertStep::LedgerWriteFailed(_) ); // The wiring is back on the registry while the installed tree - // may still hold the vendored build, which PDM, uv and Pipenv + // may still hold the vendored build, which pip, PDM, uv and Pipenv // keep through a plain sync (#477): name the reinstall. let unwired = matches!( result.step, @@ -169,6 +174,7 @@ impl<'a> VendoredBackend<'a> { &self.common.cwd, key, flavor.as_deref(), + &wiring_files, &mut result.warnings, ) .await; diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index 901ea8255..325183b88 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -3738,14 +3738,21 @@ async fn mount_pypi_json(server: &MockServer) { /// restored lock still pins (they reinstall a same-version package only /// when the LOCKED candidate is a differing URL or file), so after an /// unwind a plain `pdm sync` / `uv sync` / `pipenv sync` keeps the patched -/// build. Every unwind — hosted and vendored `rollback` and `remove`, and +/// build. #1953: so does `pip install -r requirements.txt`. Every unwind — hosted and vendored `rollback` and `remove`, and /// `vendor --revert` — must name the reinstall that restores the upstream /// bytes, and rollback's generic note must defer to it. #[tokio::test] async fn pypi_unwinds_name_the_reinstall_a_plain_sync_skips() { type Stage = fn(&Path) -> &'static [&'static str]; - let tools: [(&str, Stage, Stage, &str); 3] = [ + let tools: [(&str, Stage, Stage, &str); 4] = [ ("pdm", stage_pdm, stage_pdm, "`pdm sync --reinstall`"), + // #1953: `pip install -r` keeps a same-version install too. + ( + "pip", + stage_requirements, + stage_requirements, + "`pip install -r requirements.txt`", + ), ( "pipenv", stage_pipenv, diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs b/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs index 5b49adf6d..1b1e54f55 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/mod.rs @@ -810,6 +810,12 @@ enum Format { Unsupported, } +/// Whether the hosted restore reads the root-relative wiring file `rel` as +/// a pip requirements file (the format a plain `pip install -r` installs). +pub fn is_requirements_file(rel: &str) -> bool { + format_of(rel) == Format::Requirements +} + fn format_of(rel: &str) -> Format { let leaf = rel.rsplit('/').next().unwrap_or(rel); match leaf {