From af4ece4ee6b495b5832bf87b87757511e4e8879c Mon Sep 17 00:00:00 2001 From: Juraj Sadel Date: Tue, 25 Aug 2026 19:03:55 +0200 Subject: [PATCH] xtask: check crates.io for reserved versions before publishing --- xtask/Cargo.toml | 5 +- xtask/src/commands/release.rs | 2 + xtask/src/commands/release/bump_version.rs | 63 ++-- xtask/src/commands/release/execute_plan.rs | 35 ++- xtask/src/commands/release/plan.rs | 24 +- xtask/src/commands/release/publish_plan.rs | 20 +- xtask/src/commands/release/registry.rs | 345 +++++++++++++++++++++ 7 files changed, 469 insertions(+), 25 deletions(-) create mode 100644 xtask/src/commands/release/registry.rs diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 1d5b956b736..97a54a991d8 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -41,6 +41,9 @@ reqwest = { version = "0.12.12", features = [ # This pulls a gazillion crates - don't include it by default cargo-semver-checks = { version = "0.46.0", optional = true } +# Reads the crates.io index, including the yanked flag that cargo hides. +tame-index = { version = "0.25", features = ["sparse"], optional = true } + flate2 = { version = "1.1.1", optional = true } temp-file = { version = "0.1.9", optional = true } @@ -59,7 +62,7 @@ tempfile = "3" deploy-docs = ["dep:reqwest", "dep:kuchikiki"] preview-docs = ["dep:opener", "dep:rocket"] semver-checks = [ "dep:cargo-semver-checks", "dep:flate2", "dep:temp-file", "dep:regex" ] -release = ["semver-checks", "dep:opener", "dep:regex"] +release = ["semver-checks", "dep:opener", "dep:regex", "dep:tame-index"] report = ["dep:regex"] rel-check = ["dep:regex"] mcp = ["dep:inventory", "dep:rmcp", "dep:schemars", "dep:tokio", "dep:xtask-mcp-macros"] diff --git a/xtask/src/commands/release.rs b/xtask/src/commands/release.rs index 8a56ea67494..375b8e186bf 100644 --- a/xtask/src/commands/release.rs +++ b/xtask/src/commands/release.rs @@ -13,6 +13,8 @@ pub mod post_release; pub mod publish; #[cfg(feature = "release")] pub mod publish_plan; +#[cfg(feature = "release")] +pub mod registry; pub mod semver_check; pub mod tag_releases; diff --git a/xtask/src/commands/release/bump_version.rs b/xtask/src/commands/release/bump_version.rs index 61bd440cc63..be06480dacf 100644 --- a/xtask/src/commands/release/bump_version.rs +++ b/xtask/src/commands/release/bump_version.rs @@ -106,13 +106,16 @@ pub fn bump_version(workspace: &Path, args: BumpVersionArgs) -> Result<()> { // Bump the version for each given package: for package in args.packages { let mut package = CargoToml::new(workspace, package)?; - update_package(&mut package, &bump, false, false)?; + let new_version = do_version_bump(&package.package_version(), &bump) + .with_context(|| format!("Failed to bump version of {}", package.package))?; + update_package(&mut package, &new_version, false, false)?; } Ok(()) } -/// Update the specified package by bumping its version, updating its changelog, +/// Move the specified package to `new_version`, updating its changelog and +/// version placeholders along the way. /// /// `skip_dependent_rewrites` skips rewriting intra-workspace path-dep version /// requirements on sibling crates. Set this on backport patch releases: those @@ -122,16 +125,16 @@ pub fn bump_version(workspace: &Path, args: BumpVersionArgs) -> Result<()> { /// them anyway is pure churn. pub fn update_package( package: &mut CargoToml, - version: &VersionBump, + new_version: &semver::Version, dry_run: bool, skip_dependent_rewrites: bool, -) -> Result { +) -> Result<()> { check_crate_before_bumping(package)?; - let new_version = bump_crate_version(package, version, dry_run, skip_dependent_rewrites)?; - finalize_changelog(package, &new_version, dry_run)?; - finalize_placeholders(package, &new_version, dry_run)?; + bump_crate_version(package, new_version, dry_run, skip_dependent_rewrites)?; + finalize_changelog(package, new_version, dry_run)?; + finalize_placeholders(package, new_version, dry_run)?; - Ok(new_version) + Ok(()) } fn check_crate_before_bumping(manifest: &mut CargoToml) -> Result<()> { @@ -230,18 +233,16 @@ fn check_dependency_before_bumping(item: &Item) -> Result<()> { Ok(()) } -/// Bump the version of the specified package by the specified amount. +/// Write the given version into the package's manifest and into the manifests +/// of every workspace crate that depends on it. fn bump_crate_version( bumped_package: &mut CargoToml, - amount: &VersionBump, + version: &semver::Version, dry_run: bool, skip_dependent_rewrites: bool, -) -> Result { +) -> Result<()> { let prev_version = bumped_package.package_version(); - let version = do_version_bump(&prev_version, amount) - .with_context(|| format!("Failed to bump version of {}", bumped_package.package))?; - if dry_run { log::info!( "Dry run: would bump {} version to {version}", @@ -249,7 +250,7 @@ fn bump_crate_version( ); } else { log::info!("Update {} to {version}", bumped_package.package); - bumped_package.set_version(&version); + bumped_package.set_version(version); bumped_package.save()?; } @@ -258,7 +259,7 @@ fn bump_crate_version( " Skipping intra-workspace dependent rewrites for {}", bumped_package.package, ); - return Ok(version); + return Ok(()); } let package_name = bumped_package.package.to_string(); @@ -284,7 +285,7 @@ fn bump_crate_version( for dependent in tomls { let mut dependent = dependent?; - if dependent.change_version_of_dependency(&package_name, &version) { + if dependent.change_version_of_dependency(&package_name, version) { if dry_run { log::info!( " Dry run: would update {} in {}: ({prev_version} -> {version})", @@ -301,7 +302,7 @@ fn bump_crate_version( } } - Ok(version) + Ok(()) } /// Bump only the base version (`major.minor.patch`). @@ -597,6 +598,32 @@ mod tests { } } + /// The version handed to `update_package` must reach the manifest verbatim. + #[test] + fn update_package_writes_the_version_it_is_given() { + let workspace = tempfile::tempdir().unwrap(); + let package_dir = workspace.path().join(Package::EspSync.to_string()); + fs::create_dir(&package_dir).unwrap(); + fs::write( + package_dir.join("Cargo.toml"), + "[package]\nname = \"esp-sync\"\nversion = \"0.1.1\"\n", + ) + .unwrap(); + + let mut manifest = CargoToml::new(workspace.path(), Package::EspSync).unwrap(); + + // No VersionBump can reach 0.2.1 from 0.1.1 — Minor gives 0.2.0, Patch + // gives 0.1.2 — so this only passes if the version travels as data. + let resolved = semver::Version::parse("0.2.1").unwrap(); + update_package(&mut manifest, &resolved, false, true).unwrap(); + + let written = fs::read_to_string(package_dir.join("Cargo.toml")).unwrap(); + assert!( + written.contains(r#"version = "0.2.1""#), + "manifest did not receive the resolved version:\n{written}" + ); + } + #[test] fn test_rejected_dependencies() { let toml = r#" diff --git a/xtask/src/commands/release/execute_plan.rs b/xtask/src/commands/release/execute_plan.rs index 6840d088d12..9ecb036ba9f 100644 --- a/xtask/src/commands/release/execute_plan.rs +++ b/xtask/src/commands/release/execute_plan.rs @@ -10,7 +10,11 @@ use crate::{ commands::{ VersionBump, checker::generate_baseline, - release::plan::{PackagePlan, Plan}, + do_version_bump, + release::{ + plan::{PackagePlan, Plan}, + registry::RegistrySnapshot, + }, update_package, }, git::{current_branch, ensure_workspace_clean, get_remote_name_for}, @@ -28,6 +32,13 @@ pub struct ApplyPlanArgs { /// Instead of opening the pull request, just print base URL and body. #[arg(long)] manual_pull_request: bool, + + /// Do not ask crates.io which version numbers are already taken. + /// + /// The check needs network access. Skipping it means the release may be + /// prepared with a version that `cargo publish` will reject at the very end. + #[arg(long)] + skip_registry_check: bool, } /// Execute the release plan by making code changes, committing them to a new @@ -107,6 +118,13 @@ pub fn execute_plan(workspace: &Path, args: ApplyPlanArgs) -> Result<()> { println!("Dry run: would merge PR changelog entries into CHANGELOG.md / MIGRATING-*.md"); } + let snapshot = if args.skip_registry_check { + println!("Skipping the crates.io version check."); + RegistrySnapshot::skipped() + } else { + RegistrySnapshot::fetch(plan.packages.iter().map(|step| step.package))? + }; + // Make code changes, reusing the manifests validated above. Packages that // were already at their target version are stored as `None` and skipped. let skip_dependent_rewrites = plan.backport.is_some(); @@ -115,9 +133,20 @@ pub fn execute_plan(workspace: &Path, args: ApplyPlanArgs) -> Result<()> { continue; }; - let new_version = update_package( + let planned = do_version_bump(&package.package_version(), &step.bump) + .with_context(|| format!("Failed to bump version of {}", step.package))?; + let new_version = snapshot.next_free_version(step.package, &planned, &step.bump)?; + + if new_version != planned { + println!( + "{}: {planned} is reserved on crates.io, releasing {new_version} instead.", + step.package + ); + } + + update_package( &mut package, - &step.bump, + &new_version, !args.no_dry_run, skip_dependent_rewrites, )?; diff --git a/xtask/src/commands/release/plan.rs b/xtask/src/commands/release/plan.rs index c81efb21804..e0b95720ca4 100644 --- a/xtask/src/commands/release/plan.rs +++ b/xtask/src/commands/release/plan.rs @@ -15,7 +15,7 @@ use crate::{ VersionBump, checker::min_package_update, do_version_bump, - release::changelog_preview, + release::{changelog_preview, registry}, }, git::{BackportInfo, current_branch, parse_backport_branch}, metadata::Chip, @@ -29,6 +29,13 @@ pub struct PlanArgs { #[arg(long)] allow_non_main: bool, + /// Do not ask crates.io which version numbers are already taken. + /// + /// The check needs network access. Skipping it means the plan may pick a + /// version that `cargo publish` will reject at the very end of the release. + #[arg(long)] + skip_registry_check: bool, + /// The packages to be released. #[arg(value_enum, default_values_t = Package::iter())] packages: Vec, @@ -252,7 +259,7 @@ pub fn plan(workspace: &Path, args: PlanArgs) -> Result<()> { // after tweaks keeps targeting the same release branch. let slug = read_existing_slug(&plan_path)?.unwrap_or_else(generate_slug); - let plan = Plan { + let mut plan = Plan { base: current_branch, slug, backport: backport.clone(), @@ -308,6 +315,19 @@ pub fn plan(workspace: &Path, args: PlanArgs) -> Result<()> { .collect(), }; + if args.skip_registry_check { + println!("Skipping the crates.io version check."); + } else { + let snapshot = + registry::RegistrySnapshot::fetch(plan.packages.iter().map(|step| step.package))?; + + for step in plan.packages.iter_mut() { + step.new_version = + snapshot.next_free_version(step.package, &step.new_version, &step.bump)?; + step.tag_name = step.package.tag(&step.new_version); + } + } + log::debug!("Writing release plan to {}", plan_path.display()); let mut plan_header = String::from( diff --git a/xtask/src/commands/release/publish_plan.rs b/xtask/src/commands/release/publish_plan.rs index e77cffdfa11..ee83dc93d12 100644 --- a/xtask/src/commands/release/publish_plan.rs +++ b/xtask/src/commands/release/publish_plan.rs @@ -5,7 +5,10 @@ use clap::Args; use crate::{ cargo::{CargoArgsBuilder, CargoToml}, - commands::Plan, + commands::{ + Plan, + release::registry::{RegistrySnapshot, Slot}, + }, git::{current_branch, ensure_workspace_clean, get_remote_name_for}, }; @@ -47,6 +50,8 @@ pub fn publish_plan(workspace: &Path, args: PublishPlanArgs) -> Result<()> { }) .collect::>>()?; + let snapshot = RegistrySnapshot::fetch(plan.packages.iter().map(|step| step.package))?; + // Check that all packages are updated and ready to go. This is meant to prevent // publishing unupdated packages. for (step, toml) in plan.packages.iter().zip(tomls.iter()) { @@ -67,6 +72,19 @@ pub fn publish_plan(workspace: &Path, args: PublishPlanArgs) -> Result<()> { step.package ); } + + let slot = snapshot.slot(step.package, &step.new_version); + if slot != Slot::Free { + let yanked = match slot { + Slot::Yanked => " and yanked", + _ => "", + }; + bail!( + "{} {} is already published on crates.io{yanked}.", + step.package, + step.new_version, + ); + } } // Actually publish the packages. diff --git a/xtask/src/commands/release/registry.rs b/xtask/src/commands/release/registry.rs new file mode 100644 index 00000000000..523b999face --- /dev/null +++ b/xtask/src/commands/release/registry.rs @@ -0,0 +1,345 @@ +//! Queries crates.io for version numbers that are already spoken for. + +use std::collections::HashMap; + +use anyhow::{Context, Result, bail}; +use tame_index::{ + IndexKrate, + IndexLocation, + IndexUrl, + SparseIndex, + external::reqwest::blocking::ClientBuilder, + index::{FileLock, RemoteSparseIndex}, +}; + +use crate::{ + Package, + commands::{VersionBump, do_version_bump}, +}; + +/// Upper bound on how many times [`RegistrySnapshot::next_free_version`] will +/// step forward before giving up. +const MAX_STEPS: usize = 64; + +/// Whether crates.io will accept a given version number for a crate. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Slot { + /// Never used. The version is available. + Free, + /// Published and then withdrawn. crates.io keeps the number reserved + /// forever, and hides it from every cargo read path. + Yanked, + /// A resolvable release occupies the number. + Live, +} + +/// Pull the version/yanked pairs out of one index entry. +fn versions_of(krate: &IndexKrate) -> Vec<(semver::Version, bool)> { + krate + .versions + .iter() + .filter_map(|v| match v.version.parse::() { + Ok(parsed) => Some((parsed, v.is_yanked())), + Err(e) => { + // Pre-semver crates exist in the index. They cannot collide + // with anything we generate, so drop them rather than fail. + log::debug!("Ignoring unparseable index version {:?}: {e}", v.version); + None + } + }) + .collect() +} + +/// What crates.io already holds for a set of packages, fetched in one go. +#[derive(Debug, Default)] +pub struct RegistrySnapshot { + /// Per package: every version the index knows, and whether it is yanked. + taken: HashMap>, +} + +impl RegistrySnapshot { + /// A snapshot that knows nothing, so every version looks free. Used when + /// the caller opts out of the registry lookup. + pub fn skipped() -> Self { + Self::default() + } + + /// Look up every package in one batch. + pub fn fetch(packages: impl IntoIterator) -> Result { + let by_name = packages + .into_iter() + .map(|p| (p.to_string(), p)) + .collect::>(); + + if by_name.is_empty() { + return Ok(Self::default()); + } + + let index = SparseIndex::new(IndexLocation::new(IndexUrl::CratesIoSparse)) + .context("Failed to open the crates.io sparse index")?; + + let client = ClientBuilder::new() + .build() + .context("Failed to build an HTTP client for the crates.io index")?; + + let results = RemoteSparseIndex::new(index, client).krates( + by_name.keys().cloned().collect(), + false, + &FileLock::unlocked(), + ); + + let mut taken = HashMap::with_capacity(by_name.len()); + for (name, result) in results { + let krate = result + .with_context(|| format!("Failed to query the crates.io index for {name}"))?; + + match krate { + Some(krate) => { + taken.insert(by_name[&name], versions_of(&krate)); + } + None => log::debug!("{name} has never been published to crates.io"), + } + } + + Ok(Self { taken }) + } + + /// What crates.io holds for this exact version number. + pub fn slot(&self, package: Package, version: &semver::Version) -> Slot { + let Some((_, yanked)) = self + .taken + .get(&package) + .and_then(|versions| versions.iter().find(|(v, _)| v == version)) + else { + return Slot::Free; + }; + + if *yanked { Slot::Yanked } else { Slot::Live } + } + + /// Advance `planned` until it lands on a version crates.io has not seen. + pub fn next_free_version( + &self, + package: Package, + planned: &semver::Version, + bump: &VersionBump, + ) -> Result { + let step = match bump.pre { + Some(ref pre) => VersionBump::pre(pre.clone()), + None => VersionBump::patch(), + }; + + let mut version = planned.clone(); + + for _ in 0..MAX_STEPS { + match self.slot(package, &version) { + Slot::Free => return Ok(version), + Slot::Live => bail!( + "{package} {version} is already published on crates.io, but the workspace \ + expected it to be free. The version in Cargo.toml is behind the registry; \ + reconcile the two before releasing." + ), + Slot::Yanked => { + let next = do_version_bump(&version, &step)?; + log::warn!( + "{package} {version} was published and yanked. crates.io does not free \ + yanked version numbers, so the release moves to {next}." + ); + version = next; + } + } + } + + bail!( + "Could not find a free version for {package} within {MAX_STEPS} steps of {planned}. \ + Something is wrong with either the release plan or the crates.io index." + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::Version; + + /// A snapshot holding the given versions for [`Package::EspSync`]. + fn snapshot(versions: &[(&str, bool)]) -> RegistrySnapshot { + let taken = versions + .iter() + .map(|(v, yanked)| (v.parse().unwrap(), *yanked)) + .collect(); + + RegistrySnapshot { + taken: HashMap::from([(Package::EspSync, taken)]), + } + } + + /// One line of a crates.io index file: newline-delimited JSON, one object + /// per published version. Only the fields the parser requires are filled in. + fn index_line(version: &str, yanked: bool) -> String { + let cksum = "0".repeat(64); + format!( + r#"{{"name":"esp-sync","vers":"{version}","deps":[],"cksum":"{cksum}","features":{{}},"yanked":{yanked}}}"# + ) + } + + /// The esp-sync index as it stood when esp-rs/esp-hal#5385 was filed. + fn esp_sync_index() -> RegistrySnapshot { + let raw = [ + index_line("0.1.0", false), + index_line("0.2.0", true), + index_line("0.1.1", false), + ] + .join("\n"); + + let krate = IndexKrate::from_slice(raw.as_bytes()).unwrap(); + + RegistrySnapshot { + taken: HashMap::from([(Package::EspSync, versions_of(&krate))]), + } + } + + #[track_caller] + fn assert_free(planned: &str, bump: VersionBump, snapshot: &RegistrySnapshot, expected: &str) { + let planned = planned.parse().unwrap(); + let free = snapshot + .next_free_version(Package::EspSync, &planned, &bump) + .expect("expected a free version"); + assert_eq!(free.to_string(), expected); + } + + #[test] + fn free_slot_is_left_alone() { + assert_free( + "0.2.1", + VersionBump::minor(), + &snapshot(&[("0.2.0", true)]), + "0.2.1", + ); + assert_free( + "0.2.0", + VersionBump::minor(), + &RegistrySnapshot::default(), + "0.2.0", + ); + } + + #[test] + fn a_skipped_snapshot_never_moves_the_version() { + assert_free( + "0.2.0", + VersionBump::minor(), + &RegistrySnapshot::skipped(), + "0.2.0", + ); + } + + #[test] + fn yanked_slot_is_skipped() { + // esp-rs/esp-hal#5385: esp-sync 0.1.1 + Minor lands on the yanked 0.2.0. + assert_free( + "0.2.0", + VersionBump::minor(), + &snapshot(&[("0.1.0", false), ("0.1.1", false), ("0.2.0", true)]), + "0.2.1", + ); + } + + #[test] + fn consecutive_yanked_slots_are_skipped() { + assert_free( + "0.2.0", + VersionBump::minor(), + &snapshot(&[("0.2.0", true), ("0.2.1", true), ("0.2.2", true)]), + "0.2.3", + ); + } + + #[test] + fn pre_release_steps_the_counter() { + assert_free( + "1.1.0-beta.3", + VersionBump::pre("beta"), + &snapshot(&[("1.1.0-beta.3", true)]), + "1.1.0-beta.4", + ); + // Starting a fresh cycle on a bumped base steps the counter too. + assert_free( + "1.1.0-alpha.0", + VersionBump::base_and_pre(Version::Minor, "alpha"), + &snapshot(&[("1.1.0-alpha.0", true)]), + "1.1.0-alpha.1", + ); + } + + #[test] + fn other_packages_are_unaffected_by_a_reservation() { + // Reservations are per crate; esp-hal must not inherit esp-sync's. + let snapshot = snapshot(&[("0.2.0", true)]); + let planned = "0.2.0".parse().unwrap(); + + let free = snapshot + .next_free_version(Package::EspHal, &planned, &VersionBump::minor()) + .unwrap(); + + assert_eq!(free.to_string(), "0.2.0"); + } + + #[test] + fn yanked_flag_is_read_from_index_data() { + let snapshot = esp_sync_index(); + let slot = |v: &str| snapshot.slot(Package::EspSync, &v.parse().unwrap()); + + assert_eq!(slot("0.1.0"), Slot::Live); + assert_eq!(slot("0.1.1"), Slot::Live); + assert_eq!(slot("0.2.0"), Slot::Yanked); + assert_eq!(slot("0.2.1"), Slot::Free); + } + + #[test] + fn issue_5385_is_fixed_end_to_end() { + // The whole failure in one test: a Minor bump computed from the + // workspace alone lands on the reserved 0.2.0, so the release has to + // move to 0.2.1. + let snapshot = esp_sync_index(); + let bump = VersionBump::minor(); + + let planned = do_version_bump(&"0.1.1".parse().unwrap(), &bump).unwrap(); + assert_eq!(planned.to_string(), "0.2.0", "workspace-only bump"); + + let resolved = snapshot + .next_free_version(Package::EspSync, &planned, &bump) + .unwrap(); + assert_eq!(resolved.to_string(), "0.2.1", "after consulting crates.io"); + } + + /// Smoke test for the network path, which no other test covers. Assumes + /// esp-sync 0.2.0 stays yanked, which is not something anyone will undo. + /// + /// `cargo test -p esp-devtool --features release -- --ignored` + #[test] + #[ignore = "requires network access to crates.io"] + fn live_index_reports_the_yanked_esp_sync_release() { + let snapshot = RegistrySnapshot::fetch([Package::EspSync]).unwrap(); + let slot = |v: &str| snapshot.slot(Package::EspSync, &v.parse().unwrap()); + + assert_eq!(slot("0.2.0"), Slot::Yanked); + assert_eq!(slot("0.1.1"), Slot::Live); + } + + #[test] + fn live_release_in_the_way_is_an_error() { + let planned: semver::Version = "0.2.0".parse().unwrap(); + + for occupied in [ + // Planned slot itself is live. + snapshot(&[("0.2.0", false)]), + // Reached by stepping over a yanked slot. + snapshot(&[("0.2.0", true), ("0.2.1", false)]), + ] { + let result = + occupied.next_free_version(Package::EspSync, &planned, &VersionBump::minor()); + assert!(result.is_err(), "expected an error for {occupied:?}"); + } + } +}