diff --git a/crates/socket-patch-cli/tests/e2e_vex_build/pipenv.rs b/crates/socket-patch-cli/tests/e2e_vex_build/pipenv.rs index 5c1404998..b04ac2cd3 100644 --- a/crates/socket-patch-cli/tests/e2e_vex_build/pipenv.rs +++ b/crates/socket-patch-cli/tests/e2e_vex_build/pipenv.rs @@ -523,3 +523,123 @@ fn pipenv_pylock_projects_wire_what_pipenv_installs() { } assert!(failures.is_empty(), "{}", failures.join("\n\n")); } + +const SPLIT_PIPFILE: &str = "[[source]]\nurl = \"https://pypi.org/simple\"\nverify_ssl = true\nname = \"pypi\"\n\n[packages]\n\n[dev-packages]\n\n[docs]\nsix = \"==1.15.0\"\n\n[tests]\nsix = \"==1.16.0\"\n"; + +/// #1616: Pipenv 2023+ resolves each category on its own, so `[docs]` can +/// lock six 1.15.0 while `[tests]` locks the patched 1.16.0. Both modes +/// rewire only `tests.six`, leave `docs.six` as Pipenv wrote it, and a +/// fresh `pipenv sync --categories tests` installs the PATCHED six. +fn split_categories_flow(pipenv: &Pipenv, mode: Mode, root: &Path) { + let v = pipenv.version.as_str(); + let what = format!("pipenv {v} split categories {}", mode.label()); + let proj = root.join("split").join(mode.label()).join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + std::fs::write(proj.join("Pipfile"), SPLIT_PIPFILE).unwrap(); + let python = venv_bin(&pipenv.venv, "python"); + pipenv.project_venv(&proj); + let out = pipenv.run(&proj, &["lock", "--python", python.to_str().unwrap()]); + assert_ok(&out, &format!("{what}: pipenv lock")); + let out = pipenv.run(&proj, &["sync", "--categories", "tests"]); + assert_ok(&out, &format!("{what}: pipenv sync --categories tests")); + let pristine_lock: Value = serde_json::from_slice(&read(&proj.join("Pipfile.lock"))).unwrap(); + assert_eq!(pristine_lock["docs"]["six"]["version"], "==1.15.0"); + assert_eq!(pristine_lock["tests"]["six"]["version"], "==1.16.0"); + let py = venv_bin(&proj.join(".venv"), "python"); + let (_, pristine, _) = + six_oracle(&py, &proj).unwrap_or_else(|| panic!("{what}: six not importable")); + let patched = [pristine.as_slice(), PATCH_SUFFIX].concat(); + let api = RealApi::start(mode.uuid(), &pristine, &patched); + if mode == Mode::Hosted { + std::fs::remove_dir_all(proj.join(".venv")).unwrap(); + } + + let (code, env, stderr) = socket_scan(&proj, &api, mode.scan_flags(), &pipenv.envs); + assert_eq!(code, Some(0), "{what}: scan: {env}\n{stderr}"); + let lock: Value = serde_json::from_slice(&read(&proj.join("Pipfile.lock"))).unwrap(); + let wired = lock["tests"]["six"].to_string(); + let ok = lock["docs"]["six"] == pristine_lock["docs"]["six"] + && match mode { + Mode::Hosted => wired.contains(&api.artifact_url()), + Mode::Vendored => wired.contains(&format!(".socket/vendor/pypi/{}/", mode.uuid())), + }; + record( + "pipenv", + v, + &format!("split-categories/{}", mode.label()), + "wire", + if ok { "pass" } else { "FAIL" }, + ); + assert!( + ok, + "{what}: only tests.six is rewired: {lock}\n{env}\n{stderr}" + ); + + let fresh = root.join("split").join(mode.label()).join("fresh"); + copy_tree(&proj, &fresh, &[".venv"]); + pipenv.project_venv(&fresh); + let out = pipenv.run( + &fresh, + &[ + "sync", + "--categories", + "tests", + "--python", + python.to_str().unwrap(), + ], + ); + assert_ok( + &out, + &format!("{what}: fresh `pipenv sync --categories tests`"), + ); + let (_, bytes, is_patched) = six_oracle(&venv_bin(&fresh.join(".venv"), "python"), &fresh) + .unwrap_or_else(|| panic!("{what}: six not importable in the fresh checkout")); + let ok = is_patched && bytes == patched; + record( + "pipenv", + v, + &format!("split-categories/{}", mode.label()), + "install-patched", + if ok { "pass" } else { "FAIL" }, + ); + assert!(ok, "{what}: the fresh install must carry the PATCHED six"); +} + +#[test] +#[ignore = "real Pipenv releases + PyPI (network). Run with --ignored."] +fn pipenv_split_category_versions_wire_the_patched_category() { + let Some(uv) = find_uv() else { + return skip_or_fail(REQUIRED, "uv is not installed"); + }; + let mut failures = Vec::new(); + // Per-category resolution (and so a version split) is Pipenv 2023+. + for version in versions(VERSIONS_VAR, VERSIONS) + .into_iter() + .filter(|v| year(v) >= 2023) + { + let scratch = tempfile::tempdir().unwrap(); + let pipenv = match Pipenv::bootstrap(&uv, &version, scratch.path()) { + Ok(p) => p, + Err(e) => { + record("pipenv", &version, "split-categories", "bootstrap", "SKIP"); + skip_or_fail(REQUIRED, &format!("pipenv {version} bootstrap: {e}")); + continue; + } + }; + for mode in [Mode::Hosted, Mode::Vendored] { + let root = scratch.path().join("run"); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + split_categories_flow(&pipenv, mode, &root) + })); + if let Err(e) = result { + let msg = e + .downcast_ref::() + .cloned() + .or_else(|| e.downcast_ref::<&str>().map(|s| s.to_string())) + .unwrap_or_default(); + failures.push(format!("pipenv {version} split {}: {msg}", mode.label())); + } + } + } + assert!(failures.is_empty(), "{}", failures.join("\n\n")); +} diff --git a/crates/socket-patch-core/src/formats/pipenv.rs b/crates/socket-patch-core/src/formats/pipenv.rs index 8974b4f5e..2d9262806 100644 --- a/crates/socket-patch-core/src/formats/pipenv.rs +++ b/crates/socket-patch-core/src/formats/pipenv.rs @@ -10,6 +10,8 @@ use std::ops::Range; use serde::Serialize; use serde_json::Value; +use crate::utils::pep440; + /// One `"name": value` member of a JSON object in the lock text. pub(crate) struct Property { pub(crate) name: String, @@ -226,11 +228,63 @@ pub(crate) fn reserialized_around_reference(live: &Value, ours: &Value) -> bool .all(|key| live.get(key) == ours.get(key)) } +/// Whether a Pipfile.lock entry is a plain registry pin of a release other +/// than `version`: an exact `==` and no `file`/`path`/VCS/editable +/// source. Pipenv 2023+ resolves each category on its own, so one category +/// can lock another release of a patched package (#1616); such an entry is +/// not the patch's target and is left as Pipenv wrote it, while a wildcard, +/// a range or a missing pin is still a conflict for the caller. +pub(crate) fn pins_another_release(entry: &Value, version: &str) -> bool { + const SOURCES: [&str; 7] = ["file", "path", "git", "hg", "svn", "bzr", "editable"]; + let Some(object) = entry.as_object() else { + return false; + }; + if SOURCES.iter().any(|key| object.contains_key(*key)) { + return false; + } + object + .get("version") + .and_then(Value::as_str) + .and_then(|pin| pin.strip_prefix("==")) + // PEP 440-valid and unequal: `==1.16` is still the patched 1.16.0 + // (a conflict for the caller), never a release to leave alone. + .is_some_and(|pinned| pep440::is_valid(pinned) && !pep440::versions_equal(pinned, version)) +} + #[cfg(test)] mod tests { use super::*; use serde_json::json; + #[test] + fn pins_another_release_accepts_only_exact_registry_pins() { + for (entry, expected) in [ + (json!({"version":"==1.15.0","hashes":[]}), true), + ( + json!({"version":"==1.15.0","index":"pypi","markers":"x"}), + true, + ), + (json!({"version":"==1.16.0"}), false), + (json!({"version":"==1.16"}), false), + (json!({"version":"==v1.16.0"}), false), + (json!({"version":"*"}), false), + (json!({"version":"==1.*"}), false), + (json!({"version":">=1.15"}), false), + (json!({"version":"===1.15.0"}), false), + (json!({"version":"=="}), false), + (json!({}), false), + ( + json!({"version":"==1.15.0","file":"https://x/six.whl"}), + false, + ), + (json!({"version":"==1.15.0","git":"https://x/six"}), false), + (json!({"version":"==1.15.0","editable":true}), false), + (json!("==1.15.0"), false), + ] { + assert_eq!(pins_another_release(&entry, "1.16.0"), expected, "{entry}"); + } + } + /// #815: a re-rendered entry takes the lock's majority line ending. #[test] fn formatted_entry_takes_the_majority_line_ending() { diff --git a/crates/socket-patch-core/src/patch/redirect/pipenv.rs b/crates/socket-patch-core/src/patch/redirect/pipenv.rs index fda598633..755b4e74f 100644 --- a/crates/socket-patch-core/src/patch/redirect/pipenv.rs +++ b/crates/socket-patch-core/src/patch/redirect/pipenv.rs @@ -5,7 +5,7 @@ use serde_json::{json, Value}; use super::{DepOverride, FileEdit, RewriteResult, RewriteWarning}; use crate::crawlers::python_crawler::canonicalize_pypi_name; pub(super) use crate::formats::pipenv::reserialized_around_reference; -use crate::formats::pipenv::{entries, format_entry}; +use crate::formats::pipenv::{entries, format_entry, pins_another_release}; use crate::vendor::lock_inventory::pypi::replaceable_hosted_pin; /// Whether any pypi override names a package this `Pipfile.lock` pins — the @@ -157,7 +157,13 @@ fn plan( "file" }; let mut changes = Vec::new(); + let mut targeted = false; for (section, entry) in targets { + // Another category's own release (#1616): not this patch's target. + if pins_another_release(&entry.value, &dep.version) { + continue; + } + targeted = true; let object = entry .value .as_object() @@ -235,6 +241,12 @@ fn plan( }; changes.push((entry.range, replacement, edit)); } + if !targeted { + return Err(PlanError::Conflict(format!( + "Pipenv version for {} does not match {}: no Pipfile.lock category pins it", + dep.name, dep.version + ))); + } let mut rewritten = text.to_owned(); let mut edits = Vec::new(); for (range, replacement, edit) in changes.into_iter().rev() { @@ -287,7 +299,6 @@ mod tests { fn refusal_is_atomic_across_categories() { let dep = dependency("urllib3", "1.26.18", "patch-one"); for bad in [ - json!({"version":"==2.0"}), json!({"version":"*"}), json!({"file":"https://example.org/fork.whl"}), json!({"version":"==1.26.18","git":"https://example.org/fork"}), @@ -325,6 +336,55 @@ mod tests { } } + /// Pipenv 2023+ resolves each category on its own, so one category can + /// pin another release of the patched package (#1616): only the + /// categories pinning the patched release are redirected, the other one + /// stays as Pipenv wrote it, and the sibling rewriters are not vetoed. + #[test] + fn category_pinning_another_release_is_left_alone() { + let dep = dependency("urllib3", "1.26.18", "patch-one"); + let mut value: Value = serde_json::from_str(&lock()).unwrap(); + value["docs"] = json!({"urllib3":{"version":"==1.26.17","hashes":["older"]}}); + value.sort_all_objects(); + let original = format_entry(&value, "{", 0).unwrap() + "\n"; + let files = BTreeMap::from([ + ( + "Pipfile".to_string(), + "[packages]\nurllib3 = \"*\"\n".to_string(), + ), + ("Pipfile.lock".to_string(), original.clone()), + ]); + let mut result = RewriteResult::default(); + rewrite(&files, std::slice::from_ref(&dep), None, &mut result); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + assert!(result.refused_pipenv_uuids.is_empty()); + assert!(result.confirmed_pipenv_uuids.contains("patch-one")); + let keys: Vec<_> = result.edits.iter().filter_map(|e| e.key.clone()).collect(); + assert_eq!(keys.len(), 2); + assert!(keys.iter().all(|key| !key.contains("docs")), "{keys:?}"); + let text = &result.files["Pipfile.lock"]; + let after: Value = serde_json::from_str(text).unwrap(); + assert_eq!(after["docs"], value["docs"]); + for section in ["default", "tests"] { + assert!(after[section]["urllib3"]["file"] + .as_str() + .is_some_and(|url| url.contains("patch.socket.dev"))); + } + // Re-planning the redirected lock is a no-op. + assert_eq!(plan(text, &dep, None).unwrap(), (text.clone(), Vec::new())); + + // An unpinned or non-exact version is still not a release to skip. + for spec in ["*", ">=1.26", "==1.26.*"] { + let mut bad = value.clone(); + bad["docs"]["urllib3"]["version"] = json!(spec); + let text = serde_json::to_string(&bad).unwrap(); + assert!( + matches!(plan(&text, &dep, None), Err(PlanError::Conflict(_))), + "{spec}" + ); + } + } + /// Only conflicts veto the sibling rewriters: a /// Pipfile.lock without the package, an old pipfile-spec, an unparseable /// lock or a digest-less patch is SKIPPED with `redirect_pipenv_skipped` diff --git a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs index f3a3ec3d1..fd53b1685 100644 --- a/crates/socket-patch-core/src/vendor/pypi_pipenv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_pipenv.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use serde_json::{Map, Value}; use crate::crawlers::python_crawler::canonicalize_pypi_name; -use crate::formats::pipenv::{reserialized_around_reference, splice_entry}; +use crate::formats::pipenv::{pins_another_release, reserialized_around_reference, splice_entry}; use crate::utils::fs::{atomic_write_bytes_preserving_mode, read_regular_to_string}; use crate::vendor::lock_inventory::pypi::{hosted_pypi_reference, parse_pipfile_lock}; @@ -194,7 +194,14 @@ pub(super) fn check_target_guards_superseding( )); } let mut all_in_sync = true; + let mut targeted = false; for (section, key, entry) in &entries { + // Another category's own release (#1616): not this patch's target, + // left as Pipenv wrote it. + if pins_another_release(entry, version) { + continue; + } + targeted = true; let Some(obj) = entry.as_object() else { return Err(( "pypi_pipenv_lock_parse_failed", @@ -297,6 +304,16 @@ pub(super) fn check_target_guards_superseding( } all_in_sync = false; } + if !targeted { + let (section, key, _) = &entries[0]; + return Err(( + "pypi_pipenv_version_mismatch", + format!( + "{LOCK_FILE} {section}.{key} does not pin {version}, and no other category \ + does" + ), + )); + } Ok(if all_in_sync { PipenvTarget::InSync } else { @@ -425,6 +442,10 @@ pub(super) async fn wire_pipenv_superseding( .collect(); for key in keys { let old = map.get(&key).cloned().unwrap_or(Value::Null); + // Left alone by the guards: another category's own release. + if pins_another_release(&old, version) { + continue; + } // The file-ref entry shape: file + OUR hash; markers preserved // verbatim; index/version dropped (transitive entries never had // an index key). @@ -2190,6 +2211,60 @@ mod tests { assert_eq!(read_lock(tmp.path()).await, before); } + /// Pipenv 2023+ resolves each category on its own, so one category can + /// pin another release of the patched package (#1616). Only the + /// categories pinning the patched release are wired; the other-release + /// entry stays as Pipenv wrote it, and a lock where no category pins the + /// patched release is still refused. + #[tokio::test] + async fn category_pinning_another_release_is_left_alone() { + let mut lock: Value = serde_json::from_str(LOCK_DIRECT_REGISTRY).unwrap(); + let six = lock["default"]["six"].take(); + let mut older = six.clone(); + older["version"] = Value::String("==1.15.0".into()); + older["hashes"] = serde_json::json!(["sha256:older"]); + lock["default"] = serde_json::json!({}); + lock["docs"] = serde_json::json!({ "six": older.clone() }); + lock["tests"] = serde_json::json!({ "six": six }); + let before = pipenv_dumps(&lock); + let tmp = write_lock(&before).await; + let project = load_pipenv_project(tmp.path()).await.unwrap(); + assert_eq!( + check_target_guards(&project, "six", UUID, "1.16.0", &[]).unwrap(), + PipenvTarget::Fresh + ); + let (wiring, meta) = wire_default(&project, tmp.path()).await; + let keys: Vec<&str> = wiring.iter().filter_map(|w| w.key.as_deref()).collect(); + assert_eq!(keys, vec!["tests:six"]); + assert_eq!(meta.sections, vec!["tests"]); + let rewritten: Value = serde_json::from_str(&read_lock(tmp.path()).await).unwrap(); + assert_eq!(rewritten["docs"]["six"], older); + assert!(rewritten["tests"]["six"]["file"] + .as_str() + .is_some_and(|file| file.ends_with(REL_WHEEL))); + + // Wired: the other-release category does not break the in-sync + // re-run check. + let wired = load_pipenv_project(tmp.path()).await.unwrap(); + assert_eq!( + check_target_guards(&wired, "six", UUID, "1.16.0", &[]).unwrap(), + PipenvTarget::InSync + ); + + let reverted = revert_pipenv(&entry_for(wiring, meta), tmp.path(), false).await; + assert!(reverted.success, "{:?}", reverted.error); + assert!(reverted.warnings.is_empty(), "{:?}", reverted.warnings); + assert_eq!(read_lock(tmp.path()).await, before); + + // No category pins the patched release: still refused. + let mut lock: Value = serde_json::from_str(&before).unwrap(); + lock["tests"]["six"] = older; + let tmp = write_lock(&pipenv_dumps(&lock)).await; + let project = load_pipenv_project(tmp.path()).await.unwrap(); + let err = check_target_guards(&project, "six", UUID, "1.16.0", &[]).unwrap_err(); + assert_eq!(err.0, "pypi_pipenv_version_mismatch"); + } + /// A symlinked Pipfile.lock is refused before any write by both wire and /// revert: the rename-over would replace the link with a regular file /// and leave its target stale, and revert would never restore the link.