Skip to content

Fix Pipenv stale-install remedy built twice (#1878) #1883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into from
Nov 2, 2026
Merged

Fix Pipenv stale-install remedy built twice (#1878) #1883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
32f7b99
Start fix for #1878
claude Oct 30, 2026
1e057b9
Word the Pipenv stale-install remedy once
claude Oct 30, 2026
d35ba44
Keep hosted wording and guard remedy names
claude Oct 30, 2026
e7151f5
Refuse option-like names in Pipenv remedy
claude Oct 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 63 additions & 77 deletions crates/socket-patch-cli/src/commands/scan/hosted/python.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, BTreeSet};

use socket_patch_core::crawlers::PythonCrawler;
use socket_patch_core::manifest::schema::PatchRecord;
use socket_patch_core::utils::purl::strip_purl_qualifiers;
use socket_patch_core::utils::purl::{purl_parts, strip_purl_qualifiers};
use socket_patch_core::vex::verify::judge_installed_record;

use super::StaleInstallOutcome;
Expand Down Expand Up @@ -145,52 +145,22 @@ pub(super) async fn stale_install_warnings(
let remedy = if let Some(env) = hatch_env {
socket_patch_core::crawlers::hatch_env::stale_install_remedy(&env.name)
} else if pipenv_purls.contains(&purl) {
let name = strip_purl_qualifiers(&purl)
.strip_prefix("pkg:pypi/")
.and_then(|rest| rest.split('@').next())
.unwrap_or("<package>")
.to_string();
if inherited.contains(&site) {
let remedy =
socket_patch_core::vendor::pypi_pipenv::base_site_stale_install_remedy(
pipenv_lock.as_ref(),
&name,
&site.display().to_string(),
);
format!(
"{remedy}; then `socket-patch vex --product <purl>` re-verifies the \
installed files."
)
} else if let Some(base) =
socket_patch_core::crawlers::python_crawler::inherited_same_release(
&site, &name, &version,
)
.await
{
let remedy =
socket_patch_core::vendor::pypi_pipenv::shadowing_stale_install_remedy(
pipenv_lock.as_ref(),
&name,
&site.display().to_string(),
&base.display().to_string(),
);
format!(
"{remedy}; then `socket-patch vex --product <purl>` re-verifies the \
installed files."
)
} else {
let remedy = socket_patch_core::vendor::pypi_pipenv::stale_install_remedy(
pipenv_lock.as_ref(),
&name,
);
format!(
"Pipenv does not reinstall a release that is already present (`pipenv \
install`, `pipenv install --deploy` and `pipenv sync` all keep those \
bytes), so the rewritten Pipfile.lock only protects fresh installs. \
{remedy}; then `socket-patch vex --product <purl>` re-verifies the \
installed files."
)
}
let name = purl_parts(&purl)
.map(|(_, name, _)| name)
.or_else(|| {
let rest = strip_purl_qualifiers(&purl).strip_prefix("pkg:pypi/")?;
Some(rest.split('@').next().unwrap_or(rest).to_string())
})
.unwrap_or_default();
socket_patch_core::vendor::pypi_pipenv::stale_install_detail(
pipenv_lock.as_ref(),
&name,
&version,
std::slice::from_ref(&site),
&inherited,
"rewritten",
)
.await
} else {
"Reinstall from the rewritten lock in this interpreter and verify with \
`socket-patch vex`. Poetry before 1.4 may keep same-version packages: \
Expand Down Expand Up @@ -361,12 +331,13 @@ mod tests {
assert_eq!(out.warnings[0]["code"], "redirect_pypi_stale_install");
}

/// #790: the Pipenv remedy names the lock category that pins the
/// package. Plain `pipenv sync` installs only `default`, so for a
/// `[dev-packages]` entry it uninstalled the package and left it
/// uninstalled.
/// #1878: hosted words the Pipenv remedy through core's shared
/// `stale_install_detail`, as vendored does: the lock categories pick
/// the `pipenv sync` arguments (core's tests cover each category), and
/// the re-verify step names a runnable command, with no `<purl>` or
/// `<package>` placeholder left in.
#[tokio::test]
async fn pipenv_remedy_resyncs_the_category_that_pins_the_package() {
async fn pipenv_remedy_is_the_shared_core_wording() {
let tmp = tempfile::tempdir().unwrap();
let site = tmp.path().join("site-packages");
std::fs::create_dir_all(site.join("six-1.16.0.dist-info")).unwrap();
Expand All @@ -380,41 +351,56 @@ mod tests {
let confirmed = vec![(purl.to_string(), "six-uuid".to_string())];
let ledger = BTreeMap::from([("k".into(), record("six-uuid", "six.py", b"patched"))]);
let pipenv = BTreeSet::from(["six-uuid".to_string()]);
let detail = |lock: &str| {
let lock = lock.to_string();
let (common, confirmed, ledger, pipenv) = (&common, &confirmed, &ledger, &pipenv);
async move {
let out =
stale_install_warnings(common, confirmed, pipenv, Some(&lock), ledger).await;
assert_eq!(out.warnings.len(), 1);
out.warnings[0]["detail"].as_str().unwrap().to_string()
}
};

let develop = detail(
r#"{"_meta": {"pipfile-spec": 6}, "default": {}, "develop": {"six": {"file": "x"}}}"#,
let text =
r#"{"_meta": {"pipfile-spec": 6}, "default": {}, "develop": {"six": {"file": "x"}}}"#;
let out = stale_install_warnings(&common, &confirmed, &pipenv, Some(text), &ledger).await;
assert_eq!(out.warnings.len(), 1);
let detail = out.warnings[0]["detail"].as_str().unwrap();
let lock: serde_json::Value = serde_json::from_str(text).unwrap();
let shared = socket_patch_core::vendor::pypi_pipenv::stale_install_detail(
Some(&lock),
"six",
"1.16.0",
std::slice::from_ref(&site),
&Default::default(),
"rewritten",
)
.await;
assert_eq!(
detail,
format!(
"{purl} was switched to a hosted patch, but installed files in {} still differ \
from the patched hashes. {shared} The installed files were left unchanged.",
site.display()
)
);
assert!(
develop.contains("`pipenv run pip uninstall -y six && pipenv sync --dev`"),
"{develop}"
detail.contains("`pipenv run pip uninstall -y six && pipenv sync --dev`"),
"{detail}"
);
assert!(
develop.contains("`pipenv --rm && pipenv sync --dev`"),
"{develop}"
detail.contains("so the rewritten Pipfile.lock only protects fresh installs"),
"{detail}"
);
assert!(
detail.contains("Then `socket-patch vex` re-verifies the installed files."),
"{detail}"
);
assert!(!detail.contains("<purl>"), "{detail}");
assert!(!detail.contains("<package>"), "{detail}");

let docs = detail(
r#"{"_meta": {"pipfile-spec": 6}, "default": {"requests": {}}, "docs": {"six": {}}}"#,
)
.await;
// #790: a named category reaches the hosted remedy too.
let docs =
r#"{"_meta": {"pipfile-spec": 6}, "default": {"requests": {}}, "docs": {"six": {}}}"#;
let out = stale_install_warnings(&common, &confirmed, &pipenv, Some(docs), &ledger).await;
let detail = out.warnings[0]["detail"].as_str().unwrap();
assert!(
docs.contains("-y six && pipenv sync --categories \"docs\"`"),
"{docs}"
detail.contains("-y six && pipenv sync --categories \"docs\"`"),
"{detail}"
);
assert!(
docs.contains("`pipenv --rm && pipenv sync --categories \"packages docs\"`"),
"{docs}"
detail.contains("`pipenv --rm && pipenv sync --categories \"packages docs\"`"),
"{detail}"
);
}

Expand Down
64 changes: 12 additions & 52 deletions crates/socket-patch-core/src/vendor/pypi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,61 +802,21 @@ async fn pipenv_stale_install_warning(
let (name, version) = parse_pypi_purl(strip_purl_qualifiers(purl))
.map(|(name, version)| (name.to_string(), version.to_string()))
.unwrap_or_else(|| (purl.to_string(), String::new()));
let join = |dirs: &[&std::path::PathBuf]| {
dirs.iter()
.map(|d| d.display().to_string())
.collect::<Vec<_>>()
.join(", ")
};
let listed = join(&stale_dirs.iter().collect::<Vec<_>>());
let listed = super::pypi_pipenv::join_dirs(&stale_dirs);
// A base interpreter's copy a `--site-packages` venv inherits survives
// both the uninstall-and-sync and the recreate-and-sync remedies (#1450).
let inherited = crate::crawlers::python_crawler::inherited_base_site_dirs(&sites).await;
let (base, own): (Vec<_>, Vec<_>) = stale_dirs.iter().partition(|d| inherited.contains(*d));
// A venv copy that hides the same release in its base interpreter: the
// uninstall-and-sync remedy would only expose that base copy.
let mut shadowing = Vec::new();
let mut plain = Vec::new();
for dir in own {
match crate::crawlers::python_crawler::inherited_same_release(dir, &name, &version).await {
Some(base_dir) => shadowing.push((dir, base_dir)),
None => plain.push(dir),
}
}
let own = plain;
let mut detail =
format!("{purl}: the UNPATCHED upstream release is still installed in {listed}.");
let several = [!own.is_empty(), !shadowing.is_empty(), !base.is_empty()]
.iter()
.filter(|kind| **kind)
.count()
> 1;
if !own.is_empty() {
let remedy = super::pypi_pipenv::stale_install_remedy(Some(lock), &name);
let lead = if !several {
String::new()
} else {
format!("For {}: ", join(&own))
};
detail.push_str(&format!(
" {lead}Pipenv does not reinstall a release that is already present (`pipenv install`, `pipenv install --deploy` and `pipenv sync` all keep those bytes), so the wired Pipfile.lock only protects fresh installs. {remedy}."
));
}
for (dir, base_dir) in &shadowing {
let remedy = super::pypi_pipenv::shadowing_stale_install_remedy(
Some(lock),
&name,
&dir.display().to_string(),
&base_dir.display().to_string(),
);
detail.push_str(&format!(" {remedy}."));
}
if !base.is_empty() {
let remedy =
super::pypi_pipenv::base_site_stale_install_remedy(Some(lock), &name, &join(&base));
detail.push_str(&format!(" {remedy}."));
}
detail.push_str(" Then `socket-patch vex` re-verifies the installed files.");
let remedy = super::pypi_pipenv::stale_install_detail(
Some(lock),
&name,
&version,
&stale_dirs,
&inherited,
"wired",
)
.await;
let detail =
format!("{purl}: the UNPATCHED upstream release is still installed in {listed}. {remedy}");
Some(VendorWarning::new("pypi_pipenv_stale_install", detail))
}

Expand Down
Loading
Loading