Fix Pipenv refusing split-version categories (#1616)

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::<String>()
+                    .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"));
+}

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 `==<other>` 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() {

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`