From b19b7a733d593cad792526914e77dbbdef69e0c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 09:23:06 +0000 Subject: [PATCH 1/2] Start fix for #1249 Assisted-by: Claude Code:claude-opus-5-5 From 8e23559cf87333aabbad07842185d6a35a507ca8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 09:34:14 +0000 Subject: [PATCH 2/2] Find a pin whose last line ends in a backslash A requirements file whose last line is `six==1.16.0 \` with nothing after it was read with the backslash still attached, so scan never asked the patch API about six and reported "No patches" while pip installed the unpatched release. pip strips that dangling backslash and reads the line as `six==1.16.0`; the shared requirements lexer now does the same, so lock-only discovery, the vendored planner and VEX discovery all see the pin, in the root file or a `-r` include, with LF or CRLF endings. Hosted rewrites keep refusing such a line with redirect_requirements_continuation; discovery now finds the pin, so a patched package gets that warning instead of silence. Fixes #1249 Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/e2e_redirect_yarn_classic_build.rs | 16 ++- .../tests/e2e_socket_yml_policy.rs | 8 +- .../tests/e2e_vendor_yarn_berry_build.rs | 47 ++++++- .../tests/hosted_memory_parity.rs | 121 ++++++++++++++---- .../tests/in_process_redirect.rs | 11 +- .../tests/in_process_redirect_pipenv.rs | 3 +- .../tests/in_process_redirect_pnpm.rs | 17 ++- .../tests/mode_migration_pypi.rs | 15 ++- .../tests/scan_requirements_lock_only.rs | 35 +++++ .../src/crawlers/cargo_crawler.rs | 3 +- crates/socket-patch-core/src/formats/mod.rs | 2 +- .../src/hosted/memory/mod.rs | 7 +- .../src/hosted/memory/roots.rs | 18 ++- .../src/patch/redirect/mod.rs | 27 ++-- .../src/patch/redirect/poetry.rs | 12 +- .../src/patch/redirect/upstream/cargo.rs | 5 +- .../src/utils/group_commit.rs | 25 +++- .../src/utils/requirements.rs | 30 ++++- .../src/vendor/lock_inventory/tests.rs | 35 ++++- crates/socket-patch-core/src/vendor/mod.rs | 2 +- .../src/vendor/yarn_classic_lock.rs | 6 +- crates/socket-patch-core/src/vex/product.rs | 4 +- 22 files changed, 366 insertions(+), 83 deletions(-) diff --git a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs index e36d0fbbf..7f277f70c 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_yarn_classic_build.rs @@ -900,7 +900,10 @@ async fn classic_offline_mirror_refuses_hosted_and_keeps_installs_working() { return; }; assert!( - fx.proj.join("mirror").join(format!("{DEP}-{DEP_VERSION}.tgz")).is_file(), + fx.proj + .join("mirror") + .join(format!("{DEP}-{DEP_VERSION}.tgz")) + .is_file(), "the fixture install must populate the offline mirror" ); let fresh = fx.tmp.path().join("fresh"); @@ -926,7 +929,11 @@ async fn classic_offline_mirror_refuses_hosted_and_keeps_installs_working() { String::from_utf8_lossy(&ci.stderr) ); assert!( - !fresh.join("node_modules").join(DEP).join("index.js").exists(), + !fresh + .join("node_modules") + .join(DEP) + .join("index.js") + .exists(), "yarn < 1.7 is expected to install nothing from the mirror" ); return; @@ -948,7 +955,10 @@ async fn classic_offline_mirror_refuses_hosted_and_keeps_installs_working() { ); let installed = std::fs::read(fresh.join("node_modules").join(DEP).join("index.js")).unwrap(); - assert_eq!(installed, fx.orig, "the untouched lock installs the upstream bytes"); + assert_eq!( + installed, fx.orig, + "the untouched lock installs the upstream bytes" + ); std::fs::remove_dir_all(fresh.join("node_modules")).unwrap(); } } diff --git a/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs b/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs index 16ea58d3b..f9322ae68 100644 --- a/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs +++ b/crates/socket-patch-cli/tests/e2e_socket_yml_policy.rs @@ -1100,7 +1100,9 @@ async fn agent_mode_retains_a_recorded_patch_the_policy_now_excludes() { async fn agent_mode_judges_nested_project_copies_by_their_own_root() { let server = MockServer::start().await; mount_api(&server, catalog()).await; - let repo = Repo::new(Some("version: 2\npatches:\n ignorePaths: [\"/services/legacy/\"]\n")); + let repo = Repo::new(Some( + "version: 2\npatches:\n ignorePaths: [\"/services/legacy/\"]\n", + )); // left-pad is installed in both web (admitted) and legacy (ignored). write_npm_root(&repo.dir("services/legacy"), &["gamma", "left-pad"]); let (code, doc) = scan_json(&repo.root, &server.uri(), &["--mode", "agent"], &[]); @@ -1169,7 +1171,9 @@ async fn agent_mode_judges_nested_project_copies_by_their_own_root() { // includePaths: the docs' headline example selects nested projects from // the repo root (which itself is not included). - let repo = Repo::new(Some("version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n")); + let repo = Repo::new(Some( + "version: 2\npatches:\n includePaths: [\"/services/legacy/\"]\n", + )); let (code, doc) = scan_json(&repo.root, &server.uri(), &["--mode", "agent"], &[]); assert_eq!(code, 0, "{doc:#}"); assert_eq!( diff --git a/crates/socket-patch-cli/tests/e2e_vendor_yarn_berry_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_yarn_berry_build.rs index b6edf019d..6555371a1 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_yarn_berry_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_yarn_berry_build.rs @@ -729,7 +729,14 @@ async fn run_berry_capstone(driver: VendorDriver, yarnrc_extra: &str) { fn git(cwd: &Path, args: &[&str]) -> Output { let out = Command::new("git") - .args(["-c", "user.name=t", "-c", "user.email=t@t", "-c", "init.defaultBranch=main"]) + .args([ + "-c", + "user.name=t", + "-c", + "user.email=t@t", + "-c", + "init.defaultBranch=main", + ]) .args(args) .current_dir(cwd) .output() @@ -810,16 +817,30 @@ fn yarn_berry_vendored_tarball_survives_a_tgz_gitignore_rule() { }; let (code, stdout, stderr) = run_socket( &proj, - &["vendor", "--json", "--offline", "--cwd", proj.to_str().unwrap()], + &[ + "vendor", + "--json", + "--offline", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 0, + "vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" ); - assert_eq!(code, 0, "vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}"); git(&proj, &["add", "-A"]); git(&proj, &["commit", "-qm", "vendored"]); let fresh = tmp.path().join("fresh"); git( tmp.path(), - &["clone", "-q", proj.to_str().unwrap(), fresh.to_str().unwrap()], + &[ + "clone", + "-q", + proj.to_str().unwrap(), + fresh.to_str().unwrap(), + ], ); let fresh_global = tmp.path().join("fresh-yarn-global"); let ci = corepack( @@ -854,14 +875,26 @@ fn yarn_berry_vendor_refuses_a_gitignored_socket_dir() { let pkg_before = std::fs::read(proj.join("package.json")).unwrap(); let (code, stdout, stderr) = run_socket( &proj, - &["vendor", "--json", "--offline", "--cwd", proj.to_str().unwrap()], + &[ + "vendor", + "--json", + "--offline", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 1, + "vendor must fail.\nstdout:\n{stdout}\nstderr:\n{stderr}" ); - assert_eq!(code, 1, "vendor must fail.\nstdout:\n{stdout}\nstderr:\n{stderr}"); assert!( stdout.contains("vendor_artifact_gitignored"), "refusal code expected:\n{stdout}" ); assert_eq!(std::fs::read(proj.join("yarn.lock")).unwrap(), lock_before); - assert_eq!(std::fs::read(proj.join("package.json")).unwrap(), pkg_before); + assert_eq!( + std::fs::read(proj.join("package.json")).unwrap(), + pkg_before + ); assert!(!proj.join(format!(".socket/vendor/npm/{UUID}")).exists()); } diff --git a/crates/socket-patch-cli/tests/hosted_memory_parity.rs b/crates/socket-patch-cli/tests/hosted_memory_parity.rs index e7f80a637..09245d1c1 100644 --- a/crates/socket-patch-cli/tests/hosted_memory_parity.rs +++ b/crates/socket-patch-cli/tests/hosted_memory_parity.rs @@ -1336,9 +1336,17 @@ async fn assert_pnpm_workspace_parity_with_roots( let through_selection = run_engine(&server, selected).await; for (how, memory) in [("straight", &straight), ("selection", &through_selection)] { let found: Vec<&str> = memory.projects.iter().map(|p| p.root.as_str()).collect(); - assert_eq!(found, roots, "{how}: member locks belong to the workspace root"); + assert_eq!( + found, roots, + "{how}: member locks belong to the workspace root" + ); for project in &memory.projects { - assert!(project.error.is_none(), "{how}: {}: {:?}", project.root, project.error); + assert!( + project.error.is_none(), + "{how}: {}: {:?}", + project.root, + project.error + ); } let project = memory .projects @@ -1455,8 +1463,8 @@ async fn parity_pnpm_member_branch_lock_refusal() { /// root lock alone. #[tokio::test] async fn parity_pnpm_stale_member_lock_under_a_shared_lock() { - let lock = String::from_utf8(read_fixture("redirect/npm/pnpm/basic/input/pnpm-lock.yaml")) - .unwrap(); + let lock = + String::from_utf8(read_fixture("redirect/npm/pnpm/basic/input/pnpm-lock.yaml")).unwrap(); let shared = lock.replacen(" .:\n", " .: {}\n packages/a:\n", 1); assert_ne!(shared, lock); let files = pnpm_workspace( @@ -1477,7 +1485,10 @@ async fn pnpm_memory_runs( server: &MockServer, files: &BTreeMap>, opts: &socket_patch_cli::hosted_memory::HostedScanOptions, -) -> [(&'static str, socket_patch_cli::hosted_memory::HostedScanOutput); 2] { +) -> [( + &'static str, + socket_patch_cli::hosted_memory::HostedScanOutput, +); 2] { let straight = run_engine(server, build_input(files, &[], opts.clone())).await; let mut selected = selected_input(files); let roots = selected.options.project_roots.take(); @@ -1494,13 +1505,21 @@ fn project_roots(output: &socket_patch_cli::hosted_memory::HostedScanOutput) -> } fn changed_paths(output: &socket_patch_cli::hosted_memory::HostedScanOutput) -> Vec<&str> { - output.changed_files.iter().map(|f| f.path.as_str()).collect() + output + .changed_files + .iter() + .map(|f| f.path.as_str()) + .collect() } async fn pnpm_server() -> MockServer { let dir = fixtures_root().join("redirect/npm/pnpm/basic"); let server = MockServer::start().await; - mount_api(&server, &patches_from_overrides(&dir.join("overrides.json"), Some(&server.uri()))).await; + mount_api( + &server, + &patches_from_overrides(&dir.join("overrides.json"), Some(&server.uri())), + ) + .await; server } @@ -1518,11 +1537,16 @@ async fn parity_pnpm_member_with_another_lock_keeps_its_root() { r#"{ "name": "a", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "a" } } }"#, )], ); - let (_, changed) = assert_pnpm_workspace_parity_with_roots(&files, true, &["", "packages/a"]).await; + let (_, changed) = + assert_pnpm_workspace_parity_with_roots(&files, true, &["", "packages/a"]).await; let paths: Vec<&str> = changed.keys().map(String::as_str).collect(); assert_eq!( paths, - ["packages/a/pnpm-lock.yaml", "packages/b/pnpm-lock.yaml", "pnpm-workspace.yaml"] + [ + "packages/a/pnpm-lock.yaml", + "packages/b/pnpm-lock.yaml", + "pnpm-workspace.yaml" + ] ); } @@ -1541,17 +1565,34 @@ async fn an_unconfirmed_pnpm_member_keeps_its_lock() { &["packages/a"], &[], ); - let mut no_manifest = pnpm_workspace(per_member, Some(PNPM_ROOT_ONLY_LOCK), &["packages/a"], &[]); + let mut no_manifest = + pnpm_workspace(per_member, Some(PNPM_ROOT_ONLY_LOCK), &["packages/a"], &[]); no_manifest.remove("packages/a/package.json"); - let shared_no_root_lock = pnpm_workspace("packages:\n - 'packages/*'\n", None, &["packages/a"], &[]); + let shared_no_root_lock = + pnpm_workspace("packages:\n - 'packages/*'\n", None, &["packages/a"], &[]); let readable = pnpm_workspace(per_member, Some(PNPM_ROOT_ONLY_LOCK), &["packages/a"], &[]); // (name, files, paths passed presence-only, expected roots) - type Case<'a> = (&'a str, &'a BTreeMap>, &'a [&'a str], &'a [&'a str]); + type Case<'a> = ( + &'a str, + &'a BTreeMap>, + &'a [&'a str], + &'a [&'a str], + ); let cases: [Case; 4] = [ ("unmodeled glob", &unmodeled, &[], &["", "packages/a"]), ("no manifest", &no_manifest, &[], &["", "packages/a"]), - ("shared, no root lock", &shared_no_root_lock, &[], &["packages/a"]), - ("unreadable root file", &readable, &["pnpm-workspace.yaml"], &["", "packages/a"]), + ( + "shared, no root lock", + &shared_no_root_lock, + &[], + &["packages/a"], + ), + ( + "unreadable root file", + &readable, + &["pnpm-workspace.yaml"], + &["", "packages/a"], + ), ]; for (name, files, present, roots) in cases { let mut files = files.clone(); @@ -1570,15 +1611,30 @@ async fn an_unconfirmed_pnpm_member_keeps_its_lock() { .unwrap(); let paths = changed_paths(&output); if trust { - let error = member.error.as_ref().unwrap_or_else(|| panic!("{name}: not refused")); - assert_eq!(error.code, "redirect_pnpm_settings_elsewhere", "{name}: {error:?}"); - assert!(!paths.iter().any(|p| p.starts_with("packages/a/")), "{name}: {paths:?}"); + let error = member + .error + .as_ref() + .unwrap_or_else(|| panic!("{name}: not refused")); + assert_eq!( + error.code, "redirect_pnpm_settings_elsewhere", + "{name}: {error:?}" + ); + assert!( + !paths.iter().any(|p| p.starts_with("packages/a/")), + "{name}: {paths:?}" + ); } else { assert!(member.error.is_none(), "{name}: {:?}", member.error); - assert!(paths.contains(&"packages/a/pnpm-lock.yaml"), "{name}: {paths:?}"); + assert!( + paths.contains(&"packages/a/pnpm-lock.yaml"), + "{name}: {paths:?}" + ); } assert!( - !output.warnings.iter().any(|w| w.code == "pnpm_member_lock_ignored"), + !output + .warnings + .iter() + .any(|w| w.code == "pnpm_member_lock_ignored"), "{name}: {:?}", output.warnings ); @@ -1651,7 +1707,11 @@ async fn a_pnpm_member_stays_a_root_when_socket_yml_leaves_the_workspace_root_ou ("direct", build_input(&files, &[], direct_opts)), ] { let output = run_engine(&server, input).await; - assert_eq!(project_roots(&output), ["packages/a", "packages/b"], "{how}"); + assert_eq!( + project_roots(&output), + ["packages/a", "packages/b"], + "{how}" + ); assert_eq!( changed_paths(&output), ["packages/a/pnpm-lock.yaml", "packages/b/pnpm-lock.yaml"], @@ -1668,8 +1728,10 @@ async fn a_pnpm_member_stays_a_root_when_socket_yml_leaves_the_workspace_root_ou #[tokio::test] async fn nested_pnpm_workspaces_demote_into_the_nearest_root() { let server = pnpm_server().await; - let per_member = |globs: &str| format!("packages:\n - '{globs}'\nsharedWorkspaceLockfile: false\n"); - let lock = String::from_utf8(read_fixture("redirect/npm/pnpm/basic/input/pnpm-lock.yaml")).unwrap(); + let per_member = + |globs: &str| format!("packages:\n - '{globs}'\nsharedWorkspaceLockfile: false\n"); + let lock = + String::from_utf8(read_fixture("redirect/npm/pnpm/basic/input/pnpm-lock.yaml")).unwrap(); let inner_ws = per_member("m/*"); let files = pnpm_workspace( &per_member("packages/*"), @@ -1685,14 +1747,22 @@ async fn nested_pnpm_workspaces_demote_into_the_nearest_root() { for (how, output) in &runs { assert_eq!(project_roots(output), ["", "packages/x"], "{how}"); for project in &output.projects { - assert!(project.error.is_none(), "{how}: {}: {:?}", project.root, project.error); + assert!( + project.error.is_none(), + "{how}: {}: {:?}", + project.root, + project.error + ); } let paths = changed_paths(output); for path in ["packages/x/m/a/pnpm-lock.yaml", "packages/x/pnpm-lock.yaml"] { assert!(paths.contains(&path), "{how}: {paths:?}"); } assert!( - !output.warnings.iter().any(|w| w.code == "conflicting_write"), + !output + .warnings + .iter() + .any(|w| w.code == "conflicting_write"), "{how}: {:?}", output.warnings ); @@ -1706,7 +1776,8 @@ async fn nested_pnpm_workspaces_demote_into_the_nearest_root() { #[tokio::test] async fn a_pnpm_workspace_file_listing_nothing_adds_no_root() { let server = pnpm_server().await; - let lock = String::from_utf8(read_fixture("redirect/npm/pnpm/basic/input/pnpm-lock.yaml")).unwrap(); + let lock = + String::from_utf8(read_fixture("redirect/npm/pnpm/basic/input/pnpm-lock.yaml")).unwrap(); let files: BTreeMap> = [ ("examples/pnpm-workspace.yaml", "packages: []\n"), ("examples/demo/package.json", r#"{ "name": "demo" }"#), diff --git a/crates/socket-patch-cli/tests/in_process_redirect.rs b/crates/socket-patch-cli/tests/in_process_redirect.rs index d43043dba..096018057 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect.rs @@ -2925,7 +2925,11 @@ async fn rush_pnpm_trust_warning_gives_rush_remedy() { "the Rush trust detail must name `{needle}`; got:\n{detail}" ); } - for needle in ["pnpm install --trust-lockfile", "--store-dir", "pnpm clean --lockfile"] { + for needle in [ + "pnpm install --trust-lockfile", + "--store-dir", + "pnpm clean --lockfile", + ] { assert!( !detail.contains(needle), "the Rush trust detail must not offer the pnpm-only `{needle}`; got:\n{detail}" @@ -2979,7 +2983,10 @@ async fn rush_rerun_on_redirected_locks_reissues_the_rush_remedy() { detail.contains("pnpm_config_trust_lockfile=true rush install"), "{detail}" ); - assert!(!detail.contains("pnpm install --trust-lockfile"), "{detail}"); + assert!( + !detail.contains("pnpm install --trust-lockfile"), + "{detail}" + ); assert!( !tmp.path().join("pnpm-workspace.yaml").exists(), "a Rush re-run must not create a root pnpm-workspace.yaml" diff --git a/crates/socket-patch-cli/tests/in_process_redirect_pipenv.rs b/crates/socket-patch-cli/tests/in_process_redirect_pipenv.rs index e31acf570..e4187d707 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pipenv.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pipenv.rs @@ -589,8 +589,7 @@ async fn live_pipfile_lock_conflict_vetoes_the_requirements_redirect() { #[tokio::test] #[serial] async fn platform_wheel_is_not_pinned_into_the_lock() { - assert_wheel_tag_is_not_pinned("cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64") - .await; + assert_wheel_tag_is_not_pinned("cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64").await; } /// #1048: a pure wheel bound to one interpreter (`cp311-none-any`) fails diff --git a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs index 182a80299..11d876428 100644 --- a/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs +++ b/crates/socket-patch-cli/tests/in_process_redirect_pnpm.rs @@ -432,7 +432,10 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { assert_eq!(code, 0, "scan --mode hosted should succeed on a BOM lock"); let lock = std::fs::read_to_string(&lock_path).unwrap(); assert!(lock.starts_with("\u{feff}lockfileVersion:"), "{lock}"); - assert!(lock.contains(HOSTED_URL), "the BOM lock is redirected: {lock}"); + assert!( + lock.contains(HOSTED_URL), + "the BOM lock is redirected: {lock}" + ); let ws_path = tmp.path().join("pnpm-workspace.yaml"); assert_eq!( std::fs::read_to_string(&ws_path).ok().as_deref(), @@ -447,7 +450,10 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { pristine, "rollback restores the BOM lock byte for byte" ); - assert!(!ws_path.exists(), "the auto-created workspace file goes too"); + assert!( + !ws_path.exists(), + "the auto-created workspace file goes too" + ); // A BOM workspace file whose first key is the user's opt-out: left // byte-identical (no duplicate `trustLockfile`), lock still redirected. @@ -473,7 +479,11 @@ async fn hosted_bom_lock_and_workspace_read_like_their_plain_twins() { "the lock is still redirected for {user_ws:?}" ); let ws = std::fs::read_to_string(tmp.path().join("pnpm-workspace.yaml")).unwrap(); - assert_eq!(ws, want.unwrap_or(user_ws), "workspace file for {user_ws:?}"); + assert_eq!( + ws, + want.unwrap_or(user_ws), + "workspace file for {user_ws:?}" + ); assert_eq!(ws.matches("trustLockfile").count(), 1, "{ws:?}"); } } @@ -1950,7 +1960,6 @@ async fn hosted_unknown_or_later_pnpm_still_gets_the_trust_scaffold() { } } - /// A pnpm workspace that keeps one lock per member /// (`sharedWorkspaceLockfile: false`): `packages/a` depends on the patched /// package directly, `packages/b` through `dep-b`, each through its own diff --git a/crates/socket-patch-cli/tests/mode_migration_pypi.rs b/crates/socket-patch-cli/tests/mode_migration_pypi.rs index 1812583b9..fb897ca00 100644 --- a/crates/socket-patch-cli/tests/mode_migration_pypi.rs +++ b/crates/socket-patch-cli/tests/mode_migration_pypi.rs @@ -1332,9 +1332,18 @@ async fn ledger_update_failure_changes_nothing() { set_mode(0o755); assert_eq!(code, 1, "{env:#}"); assert_eq!(env["status"], "error", "{env:#}"); - assert!(!env.to_string().contains("redirect_takeover_unpatched"), "{env:#}"); - assert_eq!(std::fs::read(root.join("requirements.txt")).unwrap(), vendored); - assert_eq!(std::fs::read(root.join(".socket/vendor/state.json")).unwrap(), state); + assert!( + !env.to_string().contains("redirect_takeover_unpatched"), + "{env:#}" + ); + assert_eq!( + std::fs::read(root.join("requirements.txt")).unwrap(), + vendored + ); + assert_eq!( + std::fs::read(root.join(".socket/vendor/state.json")).unwrap(), + state + ); assert!(root.join(format!(".socket/vendor/pypi/{UUID}")).exists()); } diff --git a/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs index 96eeaf2fe..47523f7a9 100644 --- a/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs +++ b/crates/socket-patch-cli/tests/scan_requirements_lock_only.rs @@ -17,6 +17,8 @@ //! (`-r ${REQDIR}/dev.txt`); //! * #1028: a `-r` that follows other options on the line //! (`--pre -r dev.txt`, `-i URL -r dev.txt`). +//! * #1249: a last line ending in a dangling `\` continuation +//! (`six==1.16.0 \` at EOF), which pip joins and installs. //! //! Driven through the built binary against a mock patch API; the //! assertion is what discovery sends to the batch endpoint and the @@ -341,3 +343,36 @@ async fn lock_only_scan_discovers_include_after_other_options() { .await; } } + +/// #1249: pip's `join_lines` strips the backslash of a continued last +/// line and yields it at EOF, so `name==X \` with nothing after it is +/// the pin `name==X`, in the root file or a `-r` include, LF or CRLF. +#[tokio::test] +async fn lock_only_scan_discovers_pin_with_dangling_eof_continuation() { + for root in [ + "sp-fixture-dangle==1.16.0 \\", + "sp-fixture-dangle==1.16.0 \\\n", + "sp-fixture-dangle==1.16.0 \\\r\n", + ] { + eprintln!("case {root:?}"); + assert_lock_only_discovers( + &[("requirements.txt", root)], + &["pkg:pypi/sp-fixture-dangle@1.16.0"], + ) + .await; + } + assert_lock_only_discovers( + &[ + ("requirements.txt", "-r dev.txt\n"), + ( + "dev.txt", + "sp-fixture-idna==3.4\nsp-fixture-dangle==1.16.0 \\\n", + ), + ], + &[ + "pkg:pypi/sp-fixture-idna@3.4", + "pkg:pypi/sp-fixture-dangle@1.16.0", + ], + ) + .await; +} diff --git a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs index 63a6cc5e8..97c831a10 100644 --- a/crates/socket-patch-core/src/crawlers/cargo_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/cargo_crawler.rs @@ -379,8 +379,7 @@ fn read_crate_cargo_toml(crate_path: &Path, dir_name: &str) -> Option<(String, S let content = crate::utils::fs::read_regular_to_string_sync(&cargo_toml_path).ok()?; // Fallback: parse directory name as - - package_name_version(&content) - .or_else(|| CargoCrawler::parse_dir_name_version(dir_name)) + package_name_version(&content).or_else(|| CargoCrawler::parse_dir_name_version(dir_name)) } impl Default for CargoCrawler { diff --git a/crates/socket-patch-core/src/formats/mod.rs b/crates/socket-patch-core/src/formats/mod.rs index 294f0f436..319e37621 100644 --- a/crates/socket-patch-core/src/formats/mod.rs +++ b/crates/socket-patch-core/src/formats/mod.rs @@ -29,8 +29,8 @@ pub(crate) mod bun; pub mod cargo; pub mod composer; -pub mod governing_locks; pub mod gem; +pub mod governing_locks; pub(crate) mod maven; pub(crate) mod nuget; pub mod pnpm; diff --git a/crates/socket-patch-core/src/hosted/memory/mod.rs b/crates/socket-patch-core/src/hosted/memory/mod.rs index 03ab697ce..2581df4c3 100644 --- a/crates/socket-patch-core/src/hosted/memory/mod.rs +++ b/crates/socket-patch-core/src/hosted/memory/mod.rs @@ -78,9 +78,9 @@ use crate::rollout::stage::{ classify, lookup_incomplete, mark_pinned, offers_from_results, Offers, RecordedIndex, Row, Stage, ROLLOUT_DEFERRED, }; +use crate::utils::purl_key::PurlKey; use discover::Provider; use stages::{Planned, RewriteRefused, Rewritten, StageOptions}; -use crate::utils::purl_key::PurlKey; /// `"+"`; the sha comes from the /// `SOCKET_PATCH_GIT_SHA` build-time variable. @@ -711,8 +711,9 @@ async fn engine( let candidates = roots::pnpm_member_candidates(&root_list, |path| files.contains_key(path), ecosystems); let pnpm_members = confirm_pnpm_members(&files, candidates.clone(), admitted).await; - let other_marker = - |root: &str| roots::has_other_root_marker(root, files.keys().map(String::as_str), ecosystems); + let other_marker = |root: &str| { + roots::has_other_root_marker(root, files.keys().map(String::as_str), ecosystems) + }; if !explicit_roots { root_list.retain(|root| { !pnpm_members.iter().any(|(member, _)| member == root) || other_marker(root) diff --git a/crates/socket-patch-core/src/hosted/memory/roots.rs b/crates/socket-patch-core/src/hosted/memory/roots.rs index 7b15a0a3e..f05d13cd9 100644 --- a/crates/socket-patch-core/src/hosted/memory/roots.rs +++ b/crates/socket-patch-core/src/hosted/memory/roots.rs @@ -295,11 +295,23 @@ mod tests { ); let mut own = paths.to_vec(); own.push("packages/a/yarn.lock"); - assert!(has_other_root_marker("packages/a", own.iter().copied(), None)); + assert!(has_other_root_marker( + "packages/a", + own.iter().copied(), + None + )); let cargo = ["cargo".to_string()]; - assert!(!has_other_root_marker("packages/a", own.iter().copied(), Some(&cargo))); + assert!(!has_other_root_marker( + "packages/a", + own.iter().copied(), + Some(&cargo) + )); own.push("packages/a/Cargo.lock"); - assert!(has_other_root_marker("packages/a", own.iter().copied(), Some(&cargo))); + assert!(has_other_root_marker( + "packages/a", + own.iter().copied(), + Some(&cargo) + )); } fn roots(paths: &[&str]) -> Vec { diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index a9a882c25..43dc7fa66 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -81,9 +81,9 @@ use crate::formats::yarn::patterns::{ use crate::formats::yarn::source::{classic_copy_source, CopySource}; use crate::formats::yarn::stanzas::{stanza_key, BerryStanzas}; #[cfg(test)] -mod pnpm_equivalence_tests; -#[cfg(test)] mod platform_wheel_tests; +#[cfg(test)] +mod pnpm_equivalence_tests; mod poetry; mod pypi_takeover; pub use pypi_takeover::preflight_pypi_takeover; @@ -579,7 +579,7 @@ pub fn rewrite_registry_redirect_with_pipenv_version( bun_lockb_present, &std::collections::BTreeSet::new(), &std::collections::BTreeSet::new(), - &yarnrc::OuterYarnMirror::default(), + &yarnrc::OuterYarnMirror::default(), ) } @@ -6949,8 +6949,10 @@ fn rewrite_maven_pom( // One pass over the pom's repositories: `(id, url)` of each, which also // answers the per-dep URL-refresh check below while the pom is still // unchanged (a no-op rescan then never re-scans the pom per dep). - let original_repos: Vec<(String, Option)> = - pom.as_deref().map(maven_repository_ids_and_urls).unwrap_or_default(); + let original_repos: Vec<(String, Option)> = pom + .as_deref() + .map(maven_repository_ids_and_urls) + .unwrap_or_default(); let hosted_repo_generations: std::collections::BTreeSet = original_repos .iter() .filter_map(|(id, _)| generation::pin_name_uuid(id, false).map(str::to_string)) @@ -11022,7 +11024,10 @@ mod tests { &[(YARNRC_REL, "yarn-offline-mirror: false\n")], &[(YARNRC_REL, "yarn-offline-mirror:\n")], &[(YARNRC_REL, "yarn-offline-mirror \"\"\n")], - &[(YARNRC_REL, "yarn-offline-mirror-pruning true\n# yarn-offline-mirror ./m\n")], + &[( + YARNRC_REL, + "yarn-offline-mirror-pruning true\n# yarn-offline-mirror ./m\n", + )], &[(npmrc::NPMRC_REL, "[scope]\nyarn-offline-mirror=./m\n")], &[ (YARNRC_REL, "yarn-offline-mirror false\n"), @@ -11038,7 +11043,10 @@ mod tests { let mut r = RewriteResult::default(); rewrite_yarn_classic(&files, std::slice::from_ref(&ovr), &mut r); assert!(r.warnings.is_empty(), "{rcs:?}: {:?}", r.warnings); - assert!(r.files["yarn.lock"].contains("http://p.test/lp.tgz"), "{rcs:?}"); + assert!( + r.files["yarn.lock"].contains("http://p.test/lp.tgz"), + "{rcs:?}" + ); assert!(r.refused_yarn_classic_uuids.is_empty(), "{rcs:?}"); } } @@ -11063,7 +11071,10 @@ mod tests { rewrite_yarn_classic(&files, std::slice::from_ref(&other), &mut r); assert!(r.refused_yarn_classic_uuids.is_empty()); assert_eq!( - r.warnings.iter().map(|w| w.code.as_str()).collect::>(), + r.warnings + .iter() + .map(|w| w.code.as_str()) + .collect::>(), ["redirect_yarn_classic_entry_not_found"] ); } diff --git a/crates/socket-patch-core/src/patch/redirect/poetry.rs b/crates/socket-patch-core/src/patch/redirect/poetry.rs index 7a1b32d92..5ee901ff6 100644 --- a/crates/socket-patch-core/src/patch/redirect/poetry.rs +++ b/crates/socket-patch-core/src/patch/redirect/poetry.rs @@ -89,7 +89,9 @@ pub(super) fn rewrite_poetry( new: Some(Value::String(new)), }); } - result.confirmed_python_lock_uuids.insert(dep.patch_uuid.clone()); + result + .confirmed_python_lock_uuids + .insert(dep.patch_uuid.clone()); if !stale_warned { if let Some(format) = *writer_format.get_or_insert_with(|| pre_1_4_writer(&content)) @@ -124,14 +126,18 @@ pub(super) fn rewrite_poetry( } // Already redirected to this artifact (idempotent re-scan). LockStep::Unchanged => { - result.confirmed_python_lock_uuids.insert(dep.patch_uuid.clone()); + result + .confirmed_python_lock_uuids + .insert(dep.patch_uuid.clone()); } LockStep::NotFound => result.warnings.push(RewriteWarning { code: "redirect_poetry_entry_not_found".into(), detail: format!("no {path} entry for {}@{}", dep.name, dep.version), }), LockStep::Refused(detail) => { - result.refused_python_lock_uuids.insert(dep.patch_uuid.clone()); + result + .refused_python_lock_uuids + .insert(dep.patch_uuid.clone()); result.warnings.push(RewriteWarning { code: "redirect_poetry_lock_unsupported".into(), detail: format!("{path}: {detail}"), diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/cargo.rs b/crates/socket-patch-core/src/patch/redirect/upstream/cargo.rs index 54d4c8030..2e9c01a90 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/cargo.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/cargo.rs @@ -489,6 +489,9 @@ mod tests { let other = format!("log = {{ version = \"0.4.20\", registry = \"socket-patch-{C}\" }}\n"); let (outcome, after) = restore_b(&manifest(&other), &config).await; assert_eq!(outcome.restored().count(), 1, "{:?}", outcome.pins); - assert_eq!(after.as_deref().map(str::trim_start), Some(block(C).as_str())); + assert_eq!( + after.as_deref().map(str::trim_start), + Some(block(C).as_str()) + ); } } diff --git a/crates/socket-patch-core/src/utils/group_commit.rs b/crates/socket-patch-core/src/utils/group_commit.rs index 3b483d55f..d82737fda 100644 --- a/crates/socket-patch-core/src/utils/group_commit.rs +++ b/crates/socket-patch-core/src/utils/group_commit.rs @@ -1335,7 +1335,10 @@ mod tests { (".socket/vendor/.gitattributes", true), (".socket/vendor/gradle/g/a/maven-metadata.xml", true), (".socket/vendor/gradle/g/a/1/a-1.jar", false), - (".socket/vendor/gradle/g/a/1/socket-patch.vendor.json", false), + ( + ".socket/vendor/gradle/g/a/1/socket-patch.vendor.json", + false, + ), (".socket/vendor/npm/u/left-pad-1.3.0.tgz", false), (".socket/manifest.json", false), ("packages/a/.socket/vendor/npm/u/a.tgz", false), @@ -1437,10 +1440,16 @@ mod tests { if keep { group.rollback_to(savepoint); group.commit().await.unwrap(); - assert!(unit.join("a.tgz").exists(), "a rolled-back removal is forgotten"); + assert!( + unit.join("a.tgz").exists(), + "a rolled-back removal is forgotten" + ); } else { drop(group); - assert!(unit.join("a.tgz").exists(), "a dropped group deletes nothing"); + assert!( + unit.join("a.tgz").exists(), + "a dropped group deletes nothing" + ); } } @@ -1449,8 +1458,14 @@ mod tests { remove_tree_and_prune(&unit, &socket).await.unwrap(); group.commit().await.unwrap(); assert!(!unit.exists()); - assert!(!socket.join("vendor").exists(), "the emptied levels are pruned"); - assert!(socket.join("apply.lock").exists(), "`.socket/` itself stays"); + assert!( + !socket.join("vendor").exists(), + "the emptied levels are pruned" + ); + assert!( + socket.join("apply.lock").exists(), + "`.socket/` itself stays" + ); } /// A journal the commit had to create `.socket/vendor/` for (a hosted diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index 1bc0c0029..d1a585819 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -242,9 +242,12 @@ pub(crate) fn logical_lines(content: &str) -> Vec { i += 1; physical.push(lines[i].to_string()); } + // A continued line with nothing after it is complete at EOF, and + // pip's join strips its backslash too (#1249). + let dangling = lines[i].trim_end().ends_with('\\') && !comment(i); let mut text = String::new(); for (k, pl) in physical.iter().enumerate() { - if k + 1 < physical.len() { + if k + 1 < physical.len() || dangling { // pip's join: the backslash and the newline vanish. text.push_str(pl.trim_end().strip_suffix('\\').unwrap_or(pl)); } else { @@ -757,6 +760,31 @@ mod tests { ); } + /// #1249: a continued last line with nothing after it is complete at + /// EOF, and pip's `join_lines` strips its backslash like any other. + #[test] + fn lexer_strips_a_dangling_continuation_at_eof() { + for content in ["six==1.16.0 \\", "six==1.16.0 \\\n", "six==1.16.0 \\\r\n"] { + let lines = logical_lines(content); + assert_eq!(lines.len(), 1, "{content:?}"); + assert_eq!(lines[0].text, "six==1.16.0 ", "{content:?}"); + assert_eq!( + lines[0].physical, + vec!["six==1.16.0 \\".to_string()], + "physical stays raw" + ); + assert_eq!( + exact_pin(strip_comment(&lines[0].text)), + Some(("six", "1.16.0")) + ); + } + let lines = logical_lines("six==1.16.0 \\\n --hash=sha256:abc \\\n"); + assert_eq!(lines.len(), 1); + assert_eq!(lines[0].text, "six==1.16.0 --hash=sha256:abc "); + // A comment's backslash is the comment's text, at EOF too. + assert_eq!(logical_lines("# note \\")[0].text, "# note \\"); + } + #[test] fn lexer_never_continues_a_comment_and_drops_one_leading_bom() { let lines = logical_lines("\u{feff}# note \\\nsix==1.0\n"); diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index b6bc8df76..979c394cd 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -999,7 +999,10 @@ async fn headerless_yarn_classic_lock_is_inventoried_as_classic_by_the_fallback( .filter(|l| !l.starts_with('#')) .collect::>() .join("\n"); - assert!(!headerless.contains("lockfile v1"), "fixture drops the header"); + assert!( + !headerless.contains("lockfile v1"), + "fixture drops the header" + ); write(tmp.path(), "yarn.lock", &headerless).await; let (flavor, entries) = inventory_npm_lock(tmp.path()).await.unwrap().unwrap(); assert_eq!(flavor, NpmLockFlavor::YarnClassic); @@ -3558,6 +3561,36 @@ async fn requirements_in_root_includes_are_inventoried() { assert_eq!(sorted_pairs(&in_memory), sorted_pairs(&entries)); } +/// #1249: pip's `join_lines` strips the backslash of a continued line +/// that is the file's last and flushes it at EOF, so `six==1.16.0 \` with +/// nothing after it installs six 1.16.0. Lock-only discovery reads it as +/// that pin, in the root file and in a `-r` include, LF and CRLF. +#[tokio::test] +async fn requirements_dangling_eof_continuation_is_inventoried() { + for root in ["six==1.16.0 \\", "six==1.16.0 \\\n", "six==1.16.0 \\\r\n"] { + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "requirements.txt", root).await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![("six".to_string(), "1.16.0".to_string())], + "{root:?}" + ); + } + + let tmp = tempfile::tempdir().unwrap(); + write(tmp.path(), "requirements.txt", "-r dev.txt\n").await; + write(tmp.path(), "dev.txt", "idna==3.4\nsix==1.16.0 \\\n").await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![ + ("idna".to_string(), "3.4".to_string()), + ("six".to_string(), "1.16.0".to_string()), + ] + ); +} + /// #721: pip decodes a requirements file by its BOM, so a UTF-16 root /// file and a UTF-16 include (what Windows PowerShell 5.1's `pip freeze >` /// writes) are inventoried like their UTF-8 text, on disk and in memory. diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index fed4ed7c9..68259ef15 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -129,8 +129,8 @@ pub use verify::{ }; // The hosted→vendored takeover refuses a berry project the backend would // refuse BEFORE it reverts the hosted redirect. -pub use npm_lock::npm_lock_vendor_preflight; pub use npm_common::npm_tarball_gitignore_preflight; +pub use npm_lock::npm_lock_vendor_preflight; pub use yarn_berry_lock::{yarn_berry_vendor_preflight, yarn_berry_vendor_target_preflight}; use std::collections::{HashMap, HashSet}; diff --git a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs index fb1fb2f17..4d6608fad 100644 --- a/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs +++ b/crates/socket-patch-core/src/vendor/yarn_classic_lock.rs @@ -1501,11 +1501,7 @@ left-pad@^1.3.0: .filter(|&c| c != "vendor_prebuilt_downloaded") .collect(); assert_eq!(codes, ["vendor_yarn_classic_non_registry_legacy_wiring"]); - let detail = &warnings - .iter() - .find(|w| w.code == codes[0]) - .unwrap() - .detail; + let detail = &warnings.iter().find(|w| w.code == codes[0]).unwrap().detail; assert!( detail.contains("host.test/fork") && detail.contains("vendor --revert") diff --git a/crates/socket-patch-core/src/vex/product.rs b/crates/socket-patch-core/src/vex/product.rs index 9eb7d9843..eee122b98 100644 --- a/crates/socket-patch-core/src/vex/product.rs +++ b/crates/socket-patch-core/src/vex/product.rs @@ -1425,7 +1425,9 @@ mod tests { async fn detect_git_remote_handles_non_existent_start_path() { let dir = tempfile::tempdir().unwrap(); let nonexistent = dir.path().join("does/not/exist"); - assert!(detect_git_remote(&nonexistent, &mut Vec::new()).await.is_none()); + assert!(detect_git_remote(&nonexistent, &mut Vec::new()) + .await + .is_none()); } /// B22: inside a submodule (`.git` is a `gitdir:` FILE), the product is