From 4fbb37547d33591ab87a6a1f0a29a0dfbe5dabee Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Mon, 14 Sep 2026 13:58:34 -0400 Subject: [PATCH] Preserve pip patch markers and hash checks Keep environment markers ahead of hash options in vendored requirements. Strip hosted inline comments before appending hashes so pip enforces them. Cover marker evaluation with real installs. Assisted-by: Codex:gpt-6-astra --- .../tests/e2e_vendor_pypi_build.rs | 88 +++++++++++++++++++ .../src/patch/redirect/mod.rs | 27 +++++- .../src/vendor/pypi_requirements.rs | 6 +- 3 files changed, 117 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs index f4f4c4db..28febfb4 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs @@ -891,3 +891,91 @@ fn pip_requirements_vendor_fresh_checkout_no_index_and_revert() { ".socket/vendor must be fully removed after revert" ); } + +#[test] +#[serial_test::serial] +fn pip_vendored_requirements_evaluate_environment_markers() { + let python = find_python().expect("Python is required for the pip marker regression"); + let tmp = tempfile::tempdir().unwrap(); + for (label, marker, installed) in [ + ("excluded", "python_version < '2'", false), + ("included", "python_version >= '2'", true), + ] { + let project = tmp.path().join(label); + std::fs::create_dir_all(&project).unwrap(); + assert_tool_ok( + &tool(Path::new(python), &project, &["-m", "venv", ".venv"], &[]), + "create source venv", + ); + let venv = project.join(".venv"); + assert_tool_ok( + &tool( + &venv.join("bin/pip"), + &project, + &["install", "--disable-pip-version-check", "six==1.16.0"], + &[], + ), + "install upstream six", + ); + stage_patch(&project, &site_packages(&venv).join("six.py")); + let original = format!("six==1.16.0 ; {marker}\n"); + std::fs::write(project.join("requirements.txt"), &original).unwrap(); + let (code, stdout, stderr) = run_vendored(&VendorDriver::VendorOffline, &project); + assert_eq!(code, 0, "vendor failed: {stdout}\n{stderr}"); + assert_vendored_applied(&parse_envelope(&stdout)); + + let fresh = project.join("fresh"); + std::fs::create_dir_all(&fresh).unwrap(); + std::fs::copy( + project.join("requirements.txt"), + fresh.join("requirements.txt"), + ) + .unwrap(); + copy_dir_recursive(&project.join(".socket"), &fresh.join(".socket")); + assert_tool_ok( + &tool(Path::new(python), &fresh, &["-m", "venv", ".venv"], &[]), + "create fresh venv", + ); + let fresh_venv = fresh.join(".venv"); + assert_tool_ok( + &tool( + &fresh_venv.join("bin/pip"), + &fresh, + &[ + "install", + "--disable-pip-version-check", + "--no-index", + "--require-hashes", + "-r", + "requirements.txt", + ], + &[], + ), + "install vendored marker requirement", + ); + let probe = tool( + &fresh_venv.join("bin/python"), + &fresh, + &[ + "-c", + "import importlib.util; print(importlib.util.find_spec('six') is not None)", + ], + &[], + ); + assert_tool_ok(&probe, "inspect installed package"); + assert_eq!( + String::from_utf8_lossy(&probe.stdout).trim(), + if installed { "True" } else { "False" } + ); + if installed { + assert_eq!(python_oracle(&fresh_venv, &fresh), "1"); + } + let (code, stdout, stderr) = + run_socket(&project, &["vendor", "--revert", "--offline", "--json"]); + assert_eq!(code, 0, "revert failed: {stdout}\n{stderr}"); + assert_eq!( + std::fs::read_to_string(project.join("requirements.txt")).unwrap(), + original + ); + } +} diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0a8e7cd6..d0389daa 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -504,6 +504,7 @@ fn rewrite_pypi_requirements( } let name_re = Regex::new(r"^([A-Za-z0-9._-]+)\s*(?:[=<>~!]=?|@|;|\s|$)") .expect("static requirements-name regex is valid"); + let comment_re = Regex::new(r"\s+#.*$").expect("static requirements-comment regex is valid"); let mut lines: Vec = files["requirements.txt"] .split('\n') .map(|s| s.to_string()) @@ -550,7 +551,12 @@ fn rewrite_pypi_requirements( // BEFORE any per-requirement ` --` option. Grabbing to end-of-line // would swallow a previously appended `--hash=…` and duplicate it // on every re-run. - let req_part = line.split(" --").next().unwrap_or(line).trim_end(); + let uncommented = comment_re.replace(line, ""); + let req_part = uncommented + .split(" --") + .next() + .unwrap_or(&uncommented) + .trim_end(); let marker = match req_part.find(';') { Some(idx) => req_part[idx..].trim_end(), None => "", @@ -5247,6 +5253,25 @@ mod tests { ); } + #[test] + fn requirements_marker_comment_keeps_hash_active() { + let original = "requests==2.28.1 ; python_version >= \"3.7\" # explanation\n"; + let files = BTreeMap::from([("requirements.txt".to_string(), original.to_string())]); + let sha256 = "c".repeat(64); + let url = "https://patch.socket.dev/requests-2.28.1-py3-none-any.whl"; + let overrides = vec![pypi_override("requests", "2.28.1", url, &sha256)]; + let first = rewrite_registry_redirect(&files, &overrides); + let output = first.files.get("requirements.txt").expect("rewritten"); + assert_eq!( + output, + &format!("requests @ {url} ; python_version >= \"3.7\" --hash=sha256:{sha256}\n") + ); + let again = BTreeMap::from([("requirements.txt".to_string(), output.clone())]); + let second = rewrite_registry_redirect(&again, &overrides); + assert!(second.files.is_empty()); + assert!(second.edits.is_empty()); + } + const MAVEN_SUFFIXED: &str = "1.7.36-socket.aaaaaaaa"; /// A fail-closed override (suffixed version + jar/pom sha256 present). diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 76a92cfe..6af1a1ca 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -1,7 +1,7 @@ //! requirements.txt wiring (pip & `uv pip`). //! //! The spike-verified line shape is -//! `./ --hash=sha256:[ ; ] # socket-patch vendor: ==`: +//! `./[ ; ] --hash=sha256: # socket-patch vendor: ==`: //! both pip 26 and uv 0.11 accept the bare relative path (resolved against //! the INVOKING CWD, never the requirements-file dir — hence the documented //! root-only constraint), enforce the `--hash` pin (implicitly: any @@ -592,7 +592,7 @@ fn vendor_line( .unwrap_or_default(); let note = if transitive { " (transitive)" } else { "" }; format!( - "./{rel_wheel} --hash=sha256:{sha256_hex}{marker_part} # socket-patch vendor: {canon_name}=={version}{note}" + "./{rel_wheel}{marker_part} --hash=sha256:{sha256_hex} # socket-patch vendor: {canon_name}=={version}{note}" ) } @@ -1020,7 +1020,7 @@ mod tests { assert_eq!( read_root(tmp.path()).await, format!( - "./{REL_WHEEL} --hash=sha256:{SHA} ; python_version >= \"3.8\" # socket-patch vendor: six==1.16.0\n" + "./{REL_WHEEL} ; python_version >= \"3.8\" --hash=sha256:{SHA} # socket-patch vendor: six==1.16.0\n" ) ); }