diff --git a/src/commands/check_workspace/mod.rs b/src/commands/check_workspace/mod.rs index a4c552e25..c4876b517 100644 --- a/src/commands/check_workspace/mod.rs +++ b/src/commands/check_workspace/mod.rs @@ -27,7 +27,7 @@ use crate::cli_args::{DiffOptions, DiffStrategy}; use crate::commands::check_workspace::binary::BinaryStore; use crate::crate_graph::{CrateGraph, FeatureResolution}; use crate::test_args::TestArgs; -use crate::utils::cargo::CrateChecker; +use crate::utils::cargo::{CRATES_IO, CrateChecker}; use crate::utils::docker::{Docker, RealHttpClient, RealOciClient}; use binary::PackageMetadataFslabsCiPublishBinary; use cargo::PackageMetadataFslabsCiPublishCargo; @@ -331,7 +331,10 @@ impl Result { registries.insert(r.clone()); } if publish.cargo.allow_public { - registries.insert("crates.io".to_string()); + // Cargo's spelling, so this collapses with any `crates-io` already + // contributed by package.publish above rather than sitting beside it + // as a second, unusable entry. + registries.insert(CRATES_IO.to_string()); } publish.cargo.registries = Some(registries); @@ -868,11 +871,7 @@ fn find_dev_dep_missing_registry_warnings( .cargo .registries .as_ref() - .map(|regs| { - regs.iter() - .filter(|r| *r != "crates-io") - .collect::>() - }) + .map(|regs| regs.iter().filter(|r| *r != CRATES_IO).collect::>()) .unwrap_or_default(); if private_registries.is_empty() { diff --git a/src/utils/cargo.rs b/src/utils/cargo.rs index d023091fa..4e0213509 100644 --- a/src/utils/cargo.rs +++ b/src/utils/cargo.rs @@ -23,6 +23,25 @@ use tokio::runtime::Handle; use toml_edit::{DocumentMut, Table, table, value}; use walkdir::WalkDir; +/// Cargo's own name for the public registry. It must be spelled exactly this +/// way: it is what `cargo publish --registry` accepts and what `package.publish` +/// is matched against. "crates.io" is not a legal registry name to cargo +/// ("invalid character `.` in registry name"), so it can never be used here. +pub const CRATES_IO: &str = "crates-io"; + +/// crates.io's sparse index. Cargo knows this natively and no `[registries]` +/// entry declares it, so nothing in the env or config chain can supply it. +/// Defaulted rather than required so consumers do not have to configure the +/// public registry to check whether a version is already on it. +const CRATES_IO_INDEX: &str = "sparse+https://index.crates.io/"; + +/// Index to fall back on when nothing in the env or config chain supplies one. +/// Kept separate from `CargoRegistry::new` so it is testable without depending +/// on the ambient `CARGO_REGISTRIES_*` vars, which CI does set. +fn default_index(registry_name: &str) -> Option<&'static str> { + (registry_name == CRATES_IO).then_some(CRATES_IO_INDEX) +} + #[derive(Serialize, Deserialize, Clone, Default, Debug)] pub struct CargoRegistry { pub name: String, @@ -89,6 +108,10 @@ impl CargoRegistry { }; config.merge(&CargoRegistry::new_from_env(name.clone())); config.merge(&CargoRegistry::new_from_config(name.clone())); + // Last, so an explicit argument, env var or config entry still wins. + if config.index.is_none() { + config.index = default_index(&config.name).map(str::to_string); + } if fetch_index { config.fetch_index()?; } @@ -104,7 +127,7 @@ impl CargoRegistry { let crate_url = env::var(format!("CARGO_REGISTRIES_{env_name}_CRATE_URL")).ok(); let token = env::var(format!("CARGO_REGISTRIES_{env_name}_TOKEN")).ok(); let user_agent = match name.as_str() { - "crates.io" => None, + CRATES_IO => None, _ => env::var(format!("CARGO_REGISTRIES_{env_name}_USER_AGENT")).ok(), }; @@ -1554,6 +1577,64 @@ dependencies = [ assert!(registry.is_sparse()); } + /// Regression: crates-io has no `[registries]` entry to read an index from, + /// so with nothing in the env chain it resolved with `index: None`, and + /// `check_crate_exists` had no index to query. + /// + /// Asserted on `default_index` rather than on a constructed registry: `new` + /// merges `CARGO_REGISTRIES_CRATES_IO_INDEX` first, and CI sets it, so the + /// same assertion through `new` passes locally and fails there. + #[test] + fn test_crates_io_defaults_to_its_sparse_index() { + assert_eq!(default_index(CRATES_IO), Some(CRATES_IO_INDEX)); + + let registry = CargoRegistry { + name: CRATES_IO.to_string(), + index: Some(CRATES_IO_INDEX.to_string()), + ..Default::default() + }; + assert!( + registry.is_sparse(), + "existence checks take the sparse path, so the default must be a sparse URL" + ); + } + + #[test] + fn test_no_default_index_for_other_registries() { + assert_eq!(default_index("fsl"), None); + } + + /// An explicitly passed index is never overwritten: `merge` only fills + /// fields that are still None, so this holds whatever the env supplies. + #[test] + fn test_explicit_index_beats_the_crates_io_default() { + let registry = CargoRegistry::new( + CRATES_IO.to_string(), + Some("sparse+https://mirror.example/".to_string()), + None, + None, + None, + None, + false, + ) + .unwrap(); + + assert_eq!( + registry.index.as_deref(), + Some("sparse+https://mirror.example/") + ); + } + + /// Cargo rejects a registry name containing `.`, so this constant is what + /// makes `cargo publish --registry` and `package.publish` matching work. + #[test] + fn test_crates_io_name_is_a_legal_cargo_registry_name() { + assert!( + !CRATES_IO.contains('.'), + "cargo: invalid character `.` in registry name" + ); + } + #[test] fn test_is_sparse_with_git_index() { let registry = CargoRegistry {