diff --git a/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs b/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs index 169c36e1b..2733dd040 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs @@ -123,6 +123,11 @@ struct Shape { /// beside the Socket one (#679, #863): VEX must then attest nothing, and /// `remove` must merge the Socket block into the crates.io one. contest: Option<&'static str>, + /// Files `remove` restores to different but equivalent bytes: the + /// upstream restore drops the pin, so a declaration that named + /// crates.io explicitly (`registry = "crates-io"`, #386) comes back + /// without the redundant key, which cargo resolves identically. + restored: Vec<(&'static str, String)>, } fn run_socket(cwd: &Path, args: &[&str], cargo_home: &Path) -> (i32, String, String) { @@ -759,6 +764,11 @@ async fn run_shape(shape: Shape) -> Option<()> { // appended registry block with the same bytes whether or not the // original file ended in a newline — so an UNTERMINATED file can // only come back with its final newline (every other byte exact). + let bytes = &shape + .restored + .iter() + .find(|(path, _)| path == rel) + .map_or_else(|| bytes.clone(), |(_, text)| text.as_bytes().to_vec()); let want = match after.get(rel) { Some(got) if !bytes.ends_with(b"\n") && *got == [bytes.as_slice(), b"\n"].concat() => @@ -1004,6 +1014,7 @@ async fn cargo_hosted_multi_version_pins_each_declaration_and_removes_cleanly() refused: None, prefetch: "", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1029,10 +1040,59 @@ async fn cargo_hosted_legacy_config_is_restored_byte_for_byte() { refused: None, prefetch: "", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } +/// #386: `registry = "crates-io"` names Cargo's built-in crates.io +/// registry, so the declaration is a plain crates.io dependency. It was +/// refused as "another registry" and nothing was redirected; it must be +/// pinned in place, in both the inline and the table spelling. +#[tokio::test(flavor = "multi_thread")] +async fn cargo_hosted_explicit_crates_io_registry_is_pinned() { + for (tag, manifest, restored) in [ + ( + "crates-io-inline", + consumer_manifest("cfg-if = { version = \"1.0.4\", registry = \"crates-io\" }\n"), + consumer_manifest("cfg-if = \"1.0.4\"\n"), + ), + ( + "crates-io-table", + format!( + "{}\n[dependencies.cfg-if]\nversion = \"1.0.4\"\nregistry = \"crates-io\"\n", + consumer_manifest("") + ), + format!( + "{}\n[dependencies.cfg-if]\nversion = \"1.0.4\"\n", + consumer_manifest("") + ), + ), + ] { + let shape = Shape { + tag, + files: vec![ + ("Cargo.toml", manifest), + ("src/main.rs", "fn main() {}\n".to_string()), + ], + patches: vec![CFG_IF_1], + oracle: vec![( + "src/main.rs", + "fn main() { println!(\"{}\", cfg_if::socket_patched()); }\n".to_string(), + )], + crlf: false, + lockless: false, + refused: None, + prefetch: "", + contest: None, + restored: vec![("Cargo.toml", restored)], + }; + if run_shape(shape).await.is_none() { + return; + } + } +} + /// Bug H: a config ending in a blank line comes back byte-for-byte (the /// appended block's removal once normalized the trailing newline run). A /// config without a final newline comes back with every byte but that @@ -1069,6 +1129,7 @@ async fn cargo_hosted_config_trailing_bytes_are_restored() { refused: None, prefetch: "", contest: None, + restored: vec![], }; if run_shape(shape).await.is_none() { return; @@ -1104,6 +1165,7 @@ async fn cargo_hosted_same_line_in_two_sections_removes_cleanly() { refused: None, prefetch: "", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1145,6 +1207,7 @@ async fn cargo_hosted_workspace_member_declaration_is_pinned() { refused: None, prefetch: "", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1169,6 +1232,7 @@ async fn cargo_hosted_crlf_project_keeps_its_line_endings() { refused: None, prefetch: "", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1196,6 +1260,7 @@ async fn cargo_hosted_refuses_a_crate_another_crate_depends_on() { refused: Some("redirect_cargo_transitive_dependents"), prefetch: "", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1224,6 +1289,7 @@ async fn cargo_hosted_refuses_a_lockless_project_with_other_dependencies() { refused: Some("redirect_cargo_lockless_dependents"), prefetch: "", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1268,6 +1334,7 @@ async fn cargo_hosted_refuses_a_crate_the_root_manifest_patch_overrides() { refused: Some("redirect_cargo_dep_overridden"), prefetch: "cfg-if = \"=1.0.4\"\n", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1301,6 +1368,7 @@ async fn cargo_hosted_refuses_a_crate_a_config_patch_overrides() { refused: Some("redirect_cargo_dep_overridden"), prefetch: "cfg-if = \"=1.0.4\"\n", contest: None, + restored: vec![], }; let _ = run_shape(shape).await; } @@ -1324,6 +1392,7 @@ async fn cargo_hosted_contested_by_a_later_crates_io_copy() { lockless: false, refused: None, contest: Some("crc32fast = \"=1.5.0\""), + restored: vec![], prefetch: "", }; let _ = run_shape(shape).await; diff --git a/crates/socket-patch-cli/tests/mode_migration_cargo.rs b/crates/socket-patch-cli/tests/mode_migration_cargo.rs index 7904a7ae8..403d17c96 100644 --- a/crates/socket-patch-cli/tests/mode_migration_cargo.rs +++ b/crates/socket-patch-cli/tests/mode_migration_cargo.rs @@ -1348,9 +1348,11 @@ async fn takeover_over_unrewritable_spelling_keeps_vendored() { for (tag, spelling) in [ ("dotted", format!("{DEP}.version = \"1.0\"\n")), ("literal", format!("{DEP} = '1.0'\n")), + // An inline table spread over lines (#757). `registry = "crates-io"` + // was this case until #386 made hosted mode pin it. ( - "crates-io", - format!("{DEP} = {{ version = \"1.0\", registry = \"crates-io\" }}\n"), + "multi-line-inline", + format!("{DEP} = {{ version = \"1.0\", features = [\n] }}\n"), ), ] { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index b74b4ad87..c1de82d69 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -2612,11 +2612,21 @@ fn is_valid_gem_index_url(url: &str) -> bool { /// A registry name THIS rewriter owns: `socket-patch-`. An /// existing pin matching this grammar was written by a previous run and may be -/// superseded in place; any other registry pin is the user's and is refused. +/// superseded in place. Other registry names are the user's: only `crates-io` +/// is overwritten (see [`is_repinnable_registry_name`]), the rest refuse. fn is_socket_patch_registry_name(value: &str) -> bool { generation::pin_name_uuid(value, false).is_some() } +/// A `registry = …` value the Cargo.toml pin may overwrite in place: an +/// earlier `socket-patch-` pin (superseded), or `crates-io`, Cargo's +/// built-in name for crates.io, which resolves exactly like a declaration +/// with no `registry` key (#386). Any other name is a registry the user +/// chose, and the dep is refused. +fn is_repinnable_registry_name(value: &str) -> bool { + value == "crates-io" || is_socket_patch_registry_name(value) +} + /// The `socket-patch-` registry name pinning `crate_name` in this /// manifest, in EVERY declaration shape [`plan_cargo_toml`] writes: a /// `[…dependencies.]` header table (whose pin is a standalone @@ -3090,7 +3100,8 @@ enum CargoTomlAction { /// Plan the full-manifest pin: EVERY occurrence of `crate_name` across every /// dependency table gains `registry = ""`, an existing -/// `socket-patch-` pin is superseded in place, and any occurrence that +/// `socket-patch-` pin (or an explicit `registry = "crates-io"`, #386) +/// is overwritten in place, and any occurrence that /// cannot be handled refuses the whole dep. Nothing is applied unless every /// occurrence resolves. // The Cargo.toml planner's fixed probes, compiled once: `plan_cargo_toml` @@ -3320,7 +3331,7 @@ fn plan_cargo_toml( if ws { ws_entries.insert(key.clone(), CargoWorkspaceEntry::Pinned); } - } else if is_socket_patch_registry_name(&value) { + } else if is_repinnable_registry_name(&value) { let old_line = lines[line_idx]; let new_text = registry_val_re .replace(old_line, format!("registry = \"{reg}\"").as_str()) @@ -3434,7 +3445,7 @@ fn plan_cargo_toml( if workspace { ws_entries.insert(key.clone(), CargoWorkspaceEntry::Pinned); } - } else if is_socket_patch_registry_name(&value) { + } else if is_repinnable_registry_name(&value) { let new_text = registry_val_re .replace(raw, format!("registry = \"{reg}\"").as_str()) .into_owned(); @@ -22267,6 +22278,67 @@ packages: assert!(r.confirmed_cargo_uuids.is_empty()); } + /// #386: `crates-io` is Cargo's built-in name for crates.io, so a + /// declaration that names it explicitly is a plain crates.io dependency. + /// The pin replaces the value in place (inline, table and quoted-key + /// spellings) instead of refusing it as another registry, and a re-run + /// over the rewritten manifest is a silent no-op. + #[test] + fn cargo_explicit_crates_io_registry_is_pinned_in_place() { + let reg = cargo_reg(); + for (declaration, expected) in [ + ( + "[dependencies]\nserde = { version = \"1.0.190\", registry = \"crates-io\" }\n" + .to_string(), + format!("[dependencies]\nserde = {{ version = \"1.0.190\", registry = \"{reg}\" }}\n"), + ), + ( + "[dependencies]\nserde = { registry = 'crates-io', version = \"1.0.190\" } # explicit\n" + .to_string(), + format!("[dependencies]\nserde = {{ registry = \"{reg}\", version = \"1.0.190\" }} # explicit\n"), + ), + ( + "[dependencies.serde]\nversion = \"1.0.190\"\nregistry = \"crates-io\"\n".to_string(), + format!("[dependencies.serde]\nversion = \"1.0.190\"\nregistry = \"{reg}\"\n"), + ), + ( + "[dependencies.alias]\npackage = \"serde\"\nversion = \"1.0.190\"\n\"registry\" = \"crates-io\"\n" + .to_string(), + format!("[dependencies.alias]\npackage = \"serde\"\nversion = \"1.0.190\"\nregistry = \"{reg}\"\n"), + ), + ] { + let manifest = format!("[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n{declaration}"); + let r = rewrite_registry_redirect(&cargo_files(&manifest), &[cargo_sparse_override()]); + assert!(r.warnings.is_empty(), "{declaration}: {:?}", r.warnings); + let toml = r.files.get("Cargo.toml").expect("manifest pinned"); + assert!( + toml.ends_with(&expected), + "{declaration}: crates-io must be replaced in place:\n{toml}" + ); + assert!(!toml.contains("crates-io"), "{toml}"); + assert_eq!(cargo_socket_registry_pin(toml, "serde"), Some(reg.clone())); + assert!(r.confirmed_cargo_uuids.contains(CARGO_UUID)); + let toml_edits: Vec<_> = r.edits.iter().filter(|e| e.path == "Cargo.toml").collect(); + assert_eq!(toml_edits.len(), 1, "{:?}", r.edits); + assert!( + toml_edits[0] + .original + .as_ref() + .and_then(Value::as_str) + .is_some_and(|s| s.contains("crates-io")), + "the edit's original records the crates-io spelling: {:?}", + toml_edits[0] + ); + + let mut rerun_files = cargo_files(&manifest); + rerun_files.extend(r.files); + let rerun = rewrite_registry_redirect(&rerun_files, &[cargo_sparse_override()]); + assert!(rerun.files.is_empty(), "{:?}", rerun.files.keys()); + assert!(rerun.warnings.is_empty(), "{:?}", rerun.warnings); + assert!(rerun.confirmed_cargo_uuids.contains(CARGO_UUID)); + } + } + /// The TABLE-FORM twins of the inline-table registry matrix: an existing /// socket pin is superseded in place, a current pin is recognized as /// Already (silent, still confirmed), and a foreign pin refuses the whole 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 5fbafc1ac..8d95e5e4d 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/cargo.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/cargo.rs @@ -9,7 +9,9 @@ //! always crates.io's. A declaration's original spelling is not recorded: //! the shorthand `name = { version = "…", registry = "…" }` the rewriter //! produces from `name = "…"` collapses back to that shorthand, and every -//! other form just loses the pin. +//! other form just loses the pin. That includes a pin that overwrote an +//! explicit `registry = "crates-io"` (#386): the declaration comes back +//! without the redundant key, which cargo resolves the same way. use std::collections::{BTreeMap, BTreeSet}; @@ -330,14 +332,22 @@ static SHORTHAND_RE: std::sync::LazyLock = std::sync::LazyLock::new(|| { }); /// The line with its `registry = ""` pin removed: `Some(None)` drops -/// the whole line (the table form's inserted `registry = …` line), +/// the whole line (the table form's `registry = …` line; a trailing +/// comment on it survives as a comment-only line), /// `Some(Some(line))` is the unpinned declaration, `None` a spelling this /// restore does not recognize. fn unpin_line(line: &str, reg: &str) -> Option> { let trimmed = line.trim(); let body = trimmed.split('#').next().unwrap_or(trimmed).trim(); if body == format!("registry = \"{reg}\"") { - return Some(None); + // The pin may have overwritten a user's commented line in place + // (a superseded pin, or `registry = "crates-io"`): the comment + // stays, on its own line. + // Sliced from the raw line, so a CRLF file's `\r` stays on it. + return Some(line.find('#').map(|hash| { + let indent = &line[..line.len() - line.trim_start().len()]; + format!("{indent}{}", &line[hash..]) + })); } if let Some(c) = SHORTHAND_RE.captures(line) { if &c[3] == reg { @@ -506,6 +516,21 @@ mod tests { ); } + /// A table-form pin that overwrote a commented `registry = "crates-io"` + /// line (#386) or an earlier pin keeps the user's comment on restore. + #[test] + fn table_form_line_keeps_its_comment() { + assert_eq!( + unpin_line(&format!(" registry = \"{REG}\" # stay on crates.io"), REG), + Some(Some(" # stay on crates.io".to_string())) + ); + // A CRLF manifest is split on `\n`: the kept line keeps its `\r`. + assert_eq!( + unpin_line(&format!("registry = \"{REG}\" # note\r"), REG), + Some(Some("# note\r".to_string())) + ); + } + #[test] fn appended_block_leaves_the_config_byte_identical() { let original = "[net]\ngit-fetch-with-cli = true\n";