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 e91fcde29..c3b431d85 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_pypi_build.rs @@ -828,6 +828,124 @@ fn uv_vendor_fresh_checkout_frozen_offline_and_revert() { ); } +/// Vendor then revert six on a REAL uv project whose existing sources use +/// `sources_spelling`; the unwind must be silent, byte-identical, and leave +/// a pair `uv lock --check` accepts (#544 dotted keys, #524 sub-tables). +fn uv_sources_spelling_round_trip(tag: &str, sources_spelling: &str) { + let Some((uv, python)) = capstone_uv(tag) else { + return; + }; + bake_leak_guards(); + let tmp = tempfile::tempdir().unwrap(); + let proj = tmp.path().join("proj"); + std::fs::create_dir_all(&proj).unwrap(); + let cache = tmp.path().join("uv-cache"); + let mut cache_env: Vec<(&str, &str)> = vec![("UV_CACHE_DIR", cache.to_str().unwrap())]; + if let Some(py) = python.as_deref() { + cache_env.push(("UV_PYTHON", py)); + } + if !setup_uv_six_project(&uv, &proj, &cache_env, tag) { + return; + } + // Re-lock with a second, user-authored source in the spelling under + // test (a direct wheel URL, so no local build backend is needed). + let idna = "https://files.pythonhosted.org/packages/e5/3e/741d8c82801c347547f8a2a06aa57dbb1992be9e948df2ea0eda2c8b79e8/idna-3.7-py3-none-any.whl"; + std::fs::write( + proj.join("pyproject.toml"), + format!( + "[project]\nname = \"vendor-capstone\"\nversion = \"0.1.0\"\nrequires-python = \">=3.9\"\ndependencies = [\"six==1.16.0\", \"idna\"]\n\n{}", + sources_spelling.replace("{IDNA}", idna) + ), + ) + .unwrap(); + let relock = tool(&uv, &proj, &["lock", "-q"], &cache_env); + if !relock.status.success() { + println!( + "SKIP e2e_vendor_pypi_build({tag}): `uv lock` with the idna source failed:\n{}", + String::from_utf8_lossy(&relock.stderr) + ); + return; + } + assert_tool_ok(&tool(&uv, &proj, &["sync", "-q"], &cache_env), "uv sync"); + let installed_six = site_packages(&proj.join(".venv")).join("six.py"); + stage_patch(&proj, &installed_six); + let pyproject_before = std::fs::read(proj.join("pyproject.toml")).unwrap(); + let uvlock_before = std::fs::read(proj.join("uv.lock")).unwrap(); + + let (code, stdout, stderr) = run_vendored(&VendorDriver::VendorOffline, &proj); + assert_eq!( + code, 0, + "vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert_vendored_applied(&parse_envelope(&stdout)); + let check = tool(&uv, &proj, &["lock", "--check"], &cache_env); + assert_tool_ok(&check, "`uv lock --check` on the wired pair"); + + let (code, stdout, stderr) = run_socket( + &proj, + &[ + "vendor", + "--revert", + "--json", + "--cwd", + proj.to_str().unwrap(), + ], + ); + assert_eq!( + code, 0, + "revert failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let renv = parse_envelope(&stdout); + assert_eq!(renv["status"], "success", "revert envelope: {renv}"); + assert!( + !stdout.contains("vendor_lock_entry_drifted"), + "socket-patch's own sources line is not drift: {renv}" + ); + assert_eq!( + String::from_utf8(std::fs::read(proj.join("pyproject.toml")).unwrap()).unwrap(), + String::from_utf8(pyproject_before).unwrap(), + "revert must restore pyproject.toml byte-identical" + ); + assert_eq!( + std::fs::read(proj.join("uv.lock")).unwrap(), + uvlock_before, + "revert must restore uv.lock byte-identical" + ); + let check = tool(&uv, &proj, &["lock", "--check"], &cache_env); + assert_tool_ok(&check, "`uv lock --check` after the revert"); + assert!( + !proj.join(".socket/vendor").exists(), + ".socket/vendor must be fully removed after revert" + ); +} + +#[test] +#[serial_test::serial] +fn uv_vendor_revert_dotted_sources_key() { + uv_sources_spelling_round_trip( + "uv-dotted-sources", + "[tool.uv]\nsources.idna = { url = \"{IDNA}\" }\n", + ); +} + +#[test] +#[serial_test::serial] +fn uv_vendor_revert_dotted_sources_url_key() { + uv_sources_spelling_round_trip( + "uv-dotted-sources-url", + "[tool.uv]\nsources.idna.url = \"{IDNA}\"\n", + ); +} + +#[test] +#[serial_test::serial] +fn uv_vendor_revert_sub_table_sources() { + uv_sources_spelling_round_trip( + "uv-sub-table-sources", + "[tool.uv.sources.idna]\nurl = \"{IDNA}\"\n", + ); +} + /// `get --mode vendored` twin of the uv capstone above (v3.6): the /// SAME vendor engine and wiring, driven through get's uuid path — exempt /// from installed narrowing, so only the mocked `view/{uuid}` route is diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/uv.rs b/crates/socket-patch-core/src/patch/redirect/upstream/uv.rs index 321b01de7..187e35964 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/uv.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/uv.rs @@ -1127,6 +1127,12 @@ fn restore_metadata(meta: &mut Metadata, hit: &Hit, ctx: &Ctx<'_>) -> bool { uv.remove("sources"); } } + // Adding a key to a header-less parent (one only implied by + // `[tool.uv.sources.]` sub-tables) printed its header; once just + // sub-tables remain, the header is the rewrite's own bytes (#524). + if let Some(Item::Table(sources)) = uv.get_mut("sources") { + hide_header_over_sub_tables(sources); + } let mut removed = false; if let Some(index) = pushed { if let Some(overrides) = uv @@ -1143,12 +1149,29 @@ fn restore_metadata(meta: &mut Metadata, hit: &Hit, ctx: &Ctx<'_>) -> bool { if uv.is_empty() { tool.remove("uv"); } + if let Some(Item::Table(uv)) = tool.get_mut("uv") { + hide_header_over_sub_tables(uv); + } if tool.is_empty() { meta.doc.remove("tool"); } removed } +/// Make a standard table implicit again when it holds only sub-tables, so +/// it renders as the `[parent.]` headers alone — the spelling it had +/// before the hosted rewrite added (and restore removed) a key under it. +fn hide_header_over_sub_tables(table: &mut toml_edit::Table) { + if !table.is_dotted() + && !table.is_empty() + && table + .iter() + .all(|(_, item)| matches!(item, Item::Table(t) if !t.is_dotted())) + { + table.set_implicit(true); + } +} + #[cfg(test)] mod tests { use super::*; @@ -1188,6 +1211,88 @@ mod tests { assert!(spec_clauses("x @ https://h/x.whl").is_err()); } + const HOSTED_SIX: &str = "https://patch.socket.dev/patch/pypi/six/1.16.0/g/e828efa5-5c6d-43f3-9909-03f5ac232b98/six-1.16.0-py2.py3-none-any.whl"; + + /// Hosted rewrite of `six` into `original`, then `restore_metadata`: + /// the pyproject must come back byte-identically (#524). + fn assert_metadata_round_trips(original: &str) { + use crate::utils::python_lock::ArtifactSource; + let rewritten = crate::utils::python_script::rewrite_project_metadata( + original, + "six", + "1.16.0", + ArtifactSource::Url(HOSTED_SIX), + ) + .unwrap() + .expect("the rewrite adds a source"); + assert!(rewritten.contains(HOSTED_SIX), "{rewritten}"); + let mut meta = Metadata { + rel: "pyproject.toml".into(), + text: rewritten.clone(), + script: false, + doc: rewritten.parse().unwrap(), + }; + let hit = Hit { + index: 0, + uuid: "e828efa5-5c6d-43f3-9909-03f5ac232b98".into(), + name: "six".into(), + version: "1.16.0".into(), + }; + let client = super::super::UpstreamClient::new(true); + let ctx = Ctx { + client: &client, + origins: &[], + bun_lockb: false, + }; + restore_metadata(&mut meta, &hit, &ctx); + assert_eq!( + meta.render().unwrap(), + original, + "rewritten was:\n{rewritten}" + ); + } + + const SUB_TABLE_SOURCES: &str = "[tool.uv.sources.idna]\nurl = \"https://files.pythonhosted.org/packages/e5/3e/idna-3.7-py3-none-any.whl\"\n"; + + #[test] + fn restore_drops_sources_header_made_explicit_over_sub_tables() { + assert_metadata_round_trips(&format!( + "[project]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\"six==1.16.0\", \"idna==3.7\"]\n\n{SUB_TABLE_SOURCES}" + )); + } + + #[test] + fn restore_drops_headers_made_explicit_over_sub_tables_transitive() { + // six is transitive, so the rewrite also adds an override under the + // header-less `[tool.uv]` parent. + assert_metadata_round_trips(&format!( + "[project]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\"idna==3.7\"]\n\n{SUB_TABLE_SOURCES}" + )); + } + + #[test] + fn restore_drops_sources_header_made_explicit_over_sub_tables_crlf() { + assert_metadata_round_trips( + &format!( + "[project]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\"six==1.16.0\", \"idna==3.7\"]\n\n{SUB_TABLE_SOURCES}" + ) + .replace('\n', "\r\n"), + ); + } + + #[test] + fn restore_keeps_user_sources_spellings() { + let head = "[project]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\"six==1.16.0\", \"idna==3.7\"]\n\n"; + for sources in [ + "[tool.uv.sources]\nidna = { url = \"https://h/idna-3.7-py3-none-any.whl\" }\n", + "[tool.uv]\nsources.idna = { url = \"https://h/idna-3.7-py3-none-any.whl\" }\n", + "[tool.uv]\nsources.idna.url = \"https://h/idna-3.7-py3-none-any.whl\"\n", + "[tool.uv]\ndev-dependencies = []\n\n[tool.uv.sources.idna]\nurl = \"https://h/idna-3.7-py3-none-any.whl\"\n", + ] { + assert_metadata_round_trips(&format!("{head}{sources}")); + } + } + #[test] fn spec_style_joins() { let clauses = vec![">=1".to_string(), "<2".to_string()]; diff --git a/crates/socket-patch-core/src/vendor/pypi_uv.rs b/crates/socket-patch-core/src/vendor/pypi_uv.rs index 57cbe3750..3b602ba6b 100644 --- a/crates/socket-patch-core/src/vendor/pypi_uv.rs +++ b/crates/socket-patch-core/src/vendor/pypi_uv.rs @@ -495,12 +495,12 @@ pub(super) async fn wire_uv( // ── pyproject.toml (computed in memory; committed before the lock) ──── let mut doc = (*p.pyproject).clone(); - let had_uv_table = doc.get("tool").and_then(|t| item_get(t, "uv")).is_some(); - let created_sources_table = doc - .get("tool") - .and_then(|t| item_get(t, "uv")) - .and_then(|u| item_get(u, "sources")) - .is_none(); + let uv_item = doc.get("tool").and_then(|t| item_get(t, "uv")); + let had_uv_table = uv_item.is_some(); + // A header-less parent (implied by `[tool.uv.sources.]` sub-tables) + // prints its own header once we add a key to it, so that header is ours + // to remove on revert exactly as if we had created the table (#524). + let created_sources_table = header_is_ours(uv_item.and_then(|u| item_get(u, "sources"))); if class == UvDepClass::Transitive { // uv 0.2.35–0.5.3 do NOT apply [tool.uv.sources] to @@ -611,6 +611,21 @@ pub(super) async fn wire_uv( // toml_edit re-emits every newline as LF; a CRLF pyproject would come // back all-LF (whole-file churn, and revert splices never restore it). let new_pyproject = preserve_line_endings(&p.pyproject_text, doc.to_string()); + // toml_edit writes each added key in the spelling its parent already + // uses (`sources.six = …` under a dotted `[tool.uv]` key, `uv.…` under + // `[tool]`), so record the line as it actually rendered: revert splices + // it out by exact match (#544). + for rec in wiring + .iter_mut() + .filter(|r| r.file == "pyproject.toml" && r.action == WiringAction::Added) + { + let Some(fragment) = rec.new.as_ref().and_then(serde_json::Value::as_str) else { + continue; + }; + if let Some(line) = rendered_key_line(&p.pyproject_text, &new_pyproject, fragment) { + rec.new = Some(serde_json::Value::String(line)); + } + } // ── uv.lock text surgery (fully computed before any write) ──────────── let mut new_lock = p.lock_text.clone(); @@ -1001,6 +1016,34 @@ fn newline_of(text: &str) -> &'static str { } } +/// Whether a header for this `[tool.uv…]` table would be socket-patch's own +/// bytes once a key is added: the table is absent, or exists only +/// implicitly (no header of its own, just `[….]` sub-tables). A dotted +/// or explicit table already has the user's spelling and is never ours. +fn header_is_ours(item: Option<&Item>) -> bool { + match item { + None => true, + Some(Item::Table(t)) => t.is_implicit() && !t.is_dotted(), + Some(_) => false, + } +} + +/// The line of `new` that renders the added `fragment` (`key = value`), +/// including any dotted-key prefix toml_edit gave it, without its line +/// ending. Only lines absent from `old` count, so a user's identical line +/// is never mistaken for ours. +fn rendered_key_line(old: &str, new: &str, fragment: &str) -> Option { + // `str::lines` drops a trailing `\r` with the `\n`. + let old_lines: std::collections::HashSet<&str> = old.lines().collect(); + new.lines() + .find(|line| { + line.strip_suffix(fragment) + .is_some_and(|prefix| prefix.is_empty() || prefix.ends_with('.')) + && !old_lines.contains(line) + }) + .map(str::to_string) +} + /// Walk/create the table chain, marking CREATED intermediates implicit so /// they never render stray `[tool]` headers. fn ensure_table<'a>( @@ -5686,4 +5729,182 @@ six = { path = ".socket/vendor/pypi/9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f/six-1.1 ); assert_eq!(lock, relocked, "the live lock is left alone"); } + + /// Wire `pyproject` against `lock`, check the sources entry landed in + /// the file's own spelling, then revert and require a silent, + /// byte-identical round trip (#544, #524). + async fn assert_sources_spelling_round_trips(pyproject: &str, lock: &str, wired_line: &str) { + let tmp = write_pair(pyproject, lock).await; + let p = load_uv_project(tmp.path()).await.unwrap(); + let (wiring, meta, _) = wire_uv( + &p, + tmp.path(), + "six", + "1.16.0", + REL_WHEEL, + WHEEL_NAME, + WHEEL_SHA, + "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f", + ) + .await + .unwrap(); + let (wired, _) = read_pair(tmp.path()).await; + assert!( + wired + .lines() + .any(|l| l.trim_end_matches('\r') == wired_line), + "expected {wired_line:?} in the wired pyproject:\n{wired}" + ); + let entry = entry_for(wiring, meta); + let outcome = revert_uv(&entry, tmp.path(), false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!( + outcome.warnings.is_empty(), + "socket-patch's own line is not drift: {:?}", + outcome.warnings + ); + let (reverted, reverted_lock) = read_pair(tmp.path()).await; + assert_eq!( + reverted, pyproject, + "pyproject.toml must round-trip byte-identically" + ); + assert_eq!(reverted_lock, lock); + } + + fn six_source_line(prefix: &str) -> String { + format!("{prefix}six = {{ path = \"{REL_WHEEL}\" }}") + } + + /// #544: `[tool.uv]` + `sources. = { … }` makes the vendored entry + /// print as `sources.six = { … }`; revert must find that line. + #[tokio::test] + async fn revert_round_trips_dotted_sources_under_tool_uv() { + let pyproject = format!( + "{DIRECT_REGISTRY_PYPROJECT}\n[tool.uv]\nsources.localpkg = {{ path = \"./localpkg\" }}\n" + ); + assert_sources_spelling_round_trips( + &pyproject, + DIRECT_REGISTRY_LOCK, + &six_source_line("sources."), + ) + .await; + } + + /// #544: the fully dotted `sources..path = …` spelling. + #[tokio::test] + async fn revert_round_trips_dotted_sources_path_key() { + let pyproject = format!( + "{DIRECT_REGISTRY_PYPROJECT}\n[tool.uv]\nsources.localpkg.path = \"./localpkg\"\n" + ); + assert_sources_spelling_round_trips( + &pyproject, + DIRECT_REGISTRY_LOCK, + &six_source_line("sources."), + ) + .await; + } + + /// #544 (follow-up comment): `[tool]` + `uv.sources. = { … }`. + #[tokio::test] + async fn revert_round_trips_dotted_uv_sources_under_tool() { + let pyproject = format!( + "{DIRECT_REGISTRY_PYPROJECT}\n[tool]\nuv.sources.localpkg = {{ path = \"./localpkg\" }}\n" + ); + assert_sources_spelling_round_trips( + &pyproject, + DIRECT_REGISTRY_LOCK, + &six_source_line("uv.sources."), + ) + .await; + } + + /// #544: a root-level `tool.uv.sources.` dotted key. + #[tokio::test] + async fn revert_round_trips_root_dotted_tool_uv_sources() { + let pyproject = format!( + "tool.uv.sources.localpkg = {{ path = \"./localpkg\" }}\n\n{DIRECT_REGISTRY_PYPROJECT}" + ); + assert_sources_spelling_round_trips( + &pyproject, + DIRECT_REGISTRY_LOCK, + &six_source_line("tool.uv.sources."), + ) + .await; + } + + /// #544 on a CRLF checkout: the recorded line must still match. + #[tokio::test] + async fn revert_round_trips_dotted_sources_crlf() { + let pyproject = format!( + "{DIRECT_REGISTRY_PYPROJECT}\n[tool.uv]\nsources.localpkg = {{ path = \"./localpkg\" }}\n" + ) + .replace('\n', "\r\n"); + assert_sources_spelling_round_trips( + &pyproject, + DIRECT_REGISTRY_LOCK, + &six_source_line("sources."), + ) + .await; + } + + /// #544, transitive leg: with a dotted `uv` key under `[tool]`, the + /// override prints as `uv.override-dependencies = […]` and must revert. + #[tokio::test] + async fn revert_round_trips_dotted_override_under_tool() { + let pyproject = format!( + "{TRANSITIVE_REGISTRY_PYPROJECT}\n[tool]\nuv.sources.localpkg = {{ path = \"./localpkg\" }}\n" + ); + assert_sources_spelling_round_trips( + &pyproject, + TRANSITIVE_REGISTRY_LOCK, + "uv.override-dependencies = [\"six==1.16.0\"]", + ) + .await; + } + + /// #524: only `[tool.uv.sources.]` sub-tables, no header. Wiring + /// prints an explicit `[tool.uv.sources]` header; revert must drop it. + #[tokio::test] + async fn revert_drops_header_made_explicit_over_sub_tables() { + let pyproject = format!( + "{DIRECT_REGISTRY_PYPROJECT}\n[tool.uv.sources.localpkg]\npath = \"./localpkg\"\n" + ); + assert_sources_spelling_round_trips(&pyproject, DIRECT_REGISTRY_LOCK, &six_source_line("")) + .await; + } + + /// #524, transitive leg with a sub-table-only sources parent. + #[tokio::test] + async fn revert_drops_header_made_explicit_over_sub_tables_override() { + let pyproject = format!( + "{TRANSITIVE_REGISTRY_PYPROJECT}\n[tool.uv.sources.localpkg]\npath = \"./localpkg\"\n" + ); + assert_sources_spelling_round_trips( + &pyproject, + TRANSITIVE_REGISTRY_LOCK, + &six_source_line(""), + ) + .await; + } + + /// #524 on a CRLF checkout. + #[tokio::test] + async fn revert_drops_header_made_explicit_over_sub_tables_crlf() { + let pyproject = format!( + "{DIRECT_REGISTRY_PYPROJECT}\n[tool.uv.sources.localpkg]\npath = \"./localpkg\"\n" + ) + .replace('\n', "\r\n"); + assert_sources_spelling_round_trips(&pyproject, DIRECT_REGISTRY_LOCK, &six_source_line("")) + .await; + } + + /// Control: a user-authored explicit `[tool.uv.sources]` header stays. + #[tokio::test] + async fn revert_keeps_user_authored_sources_header() { + let pyproject = format!( + "{DIRECT_REGISTRY_PYPROJECT}\n[tool.uv.sources]\nlocalpkg = {{ path = \"./localpkg\" }}\n" + ); + assert_sources_spelling_round_trips(&pyproject, DIRECT_REGISTRY_LOCK, &six_source_line("")) + .await; + } } diff --git a/crates/socket-patch-core/src/vendor/toml_surgery.rs b/crates/socket-patch-core/src/vendor/toml_surgery.rs index 23c26154a..0b1777008 100644 --- a/crates/socket-patch-core/src/vendor/toml_surgery.rs +++ b/crates/socket-patch-core/src/vendor/toml_surgery.rs @@ -264,6 +264,9 @@ pub(super) fn remove_exact_line(text: &str, line: &str) -> Option { /// Drop a `[header]` whose section holds only blank lines, plus its /// preceding blank separator. A non-empty section is left untouched. +/// Blank lines before a following header are that header's own separator +/// and stay, so a header made explicit above `[header.]` sub-tables +/// comes out without eating the sub-table's spacing. /// Spliced by byte span so every other byte — including CRLF endings in a /// user-authored pyproject.toml — survives verbatim. pub(super) fn remove_table_if_empty(text: &str, header: &str) -> String { @@ -278,6 +281,9 @@ pub(super) fn remove_table_if_empty(text: &str, header: &str) -> String { } end += 1; } + if end < index.len() { + end = h + 1; + } let mut start = h; if start > 0 && index[start - 1].1.trim().is_empty() { start -= 1; @@ -483,12 +489,19 @@ mod tests { let keep_blanks = "x = 1\n\n[tool.uv]\n\ndev = true\n"; assert_eq!(remove_table_if_empty(keep_blanks, "[tool.uv]"), keep_blanks); // A section holding ONLY blank lines is empty too — the headline - // documented case. The splice consumes the section's trailing blanks - // up to the next header, so the blank separator before [next] does - // not survive (pinning current behavior). + // documented case. The blanks before [next] are its separator and + // survive; the header and its own preceding separator go. assert_eq!( remove_table_if_empty("x = 1\n\n[tool.uv]\n\n\n[next]\na = 1\n", "[tool.uv]"), - "x = 1\n[next]\na = 1\n" + "x = 1\n\n\n[next]\na = 1\n" + ); + // A header made explicit above its own sub-table (#524). + assert_eq!( + remove_table_if_empty( + "x = 1\n\n[tool.uv.sources]\n\n[tool.uv.sources.a]\npath = \"a\"\n", + "[tool.uv.sources]" + ), + "x = 1\n\n[tool.uv.sources.a]\npath = \"a\"\n" ); // Blank-only section at EOF: header, its blanks, and the preceding // separator are all dropped.