From 632f3a82b70669e6ad5cb5ba2b15799c5c870296 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 26 Jul 2023 00:40:14 +0100 Subject: [PATCH 1/4] chore: new xtask `bump-check` This is a rewrite of old `ci/validate-version-bump.sh` in Rust. --- crates/xtask-bump-check/Cargo.toml | 13 ++ crates/xtask-bump-check/src/main.rs | 15 ++ crates/xtask-bump-check/src/xtask.rs | 287 +++++++++++++++++++++++++++ 3 files changed, 315 insertions(+) create mode 100644 crates/xtask-bump-check/Cargo.toml create mode 100644 crates/xtask-bump-check/src/main.rs create mode 100644 crates/xtask-bump-check/src/xtask.rs diff --git a/crates/xtask-bump-check/Cargo.toml b/crates/xtask-bump-check/Cargo.toml new file mode 100644 index 000000000000..3753a5ef4260 --- /dev/null +++ b/crates/xtask-bump-check/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "xtask-bump-check" +version = "0.0.0" +edition.workspace = true +publish = false + +[dependencies] +anyhow.workspace = true +cargo.workspace = true +clap.workspace = true +env_logger.workspace = true +git2.workspace = true +log.workspace = true diff --git a/crates/xtask-bump-check/src/main.rs b/crates/xtask-bump-check/src/main.rs new file mode 100644 index 000000000000..1942a3621cfb --- /dev/null +++ b/crates/xtask-bump-check/src/main.rs @@ -0,0 +1,15 @@ +mod xtask; + +fn main() { + env_logger::init_from_env("CARGO_LOG"); + let cli = xtask::cli(); + let matches = cli.get_matches(); + + let mut config = cargo::util::config::Config::default().unwrap_or_else(|e| { + let mut eval = cargo::core::shell::Shell::new(); + cargo::exit_with_error(e.into(), &mut eval) + }); + if let Err(e) = xtask::exec(&matches, &mut config) { + cargo::exit_with_error(e, &mut config.shell()) + } +} diff --git a/crates/xtask-bump-check/src/xtask.rs b/crates/xtask-bump-check/src/xtask.rs new file mode 100644 index 000000000000..77cc5236cc00 --- /dev/null +++ b/crates/xtask-bump-check/src/xtask.rs @@ -0,0 +1,287 @@ +//! ```text +//! NAME +//! xtask-bump-check +//! +//! SYNOPSIS +//! xtask-bump-check --baseline-rev +//! +//! DESCRIPTION +//! Checks if there is any member got changed since a base commit +//! but forgot to bump its version. +//! ``` + +use std::collections::HashMap; +use std::fmt::Write; +use std::fs; +use std::task; + +use cargo::core::dependency::Dependency; +use cargo::core::registry::PackageRegistry; +use cargo::core::Package; +use cargo::core::QueryKind; +use cargo::core::Registry; +use cargo::core::SourceId; +use cargo::core::Workspace; +use cargo::util::command_prelude::*; +use cargo::util::ToSemver; +use cargo::CargoResult; + +pub fn cli() -> clap::Command { + clap::Command::new("xtask-bump-check") + .arg( + opt( + "verbose", + "Use verbose output (-vv very verbose/build.rs output)", + ) + .short('v') + .action(ArgAction::Count) + .global(true), + ) + .arg_quiet() + .arg( + opt("color", "Coloring: auto, always, never") + .value_name("WHEN") + .global(true), + ) + .arg( + opt("baseline-rev", "Git revision to lookup for a baseline") + .action(ArgAction::Set) + .required(true), + ) + .arg(flag("frozen", "Require Cargo.lock and cache are up to date").global(true)) + .arg(flag("locked", "Require Cargo.lock is up to date").global(true)) + .arg(flag("offline", "Run without accessing the network").global(true)) + .arg(multi_opt("config", "KEY=VALUE", "Override a configuration value").global(true)) + .arg( + Arg::new("unstable-features") + .help("Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details") + .short('Z') + .value_name("FLAG") + .action(ArgAction::Append) + .global(true), + ) +} + +pub fn exec(args: &clap::ArgMatches, config: &mut cargo::util::Config) -> cargo::CliResult { + config_configure(config, args)?; + + bump_check(args, config)?; + + Ok(()) +} + +fn config_configure(config: &mut Config, args: &ArgMatches) -> CliResult { + let verbose = args.verbose(); + // quiet is unusual because it is redefined in some subcommands in order + // to provide custom help text. + let quiet = args.flag("quiet"); + let color = args.get_one::("color").map(String::as_str); + let frozen = args.flag("frozen"); + let locked = args.flag("locked"); + let offline = args.flag("offline"); + let mut unstable_flags = vec![]; + if let Some(values) = args.get_many::("unstable-features") { + unstable_flags.extend(values.cloned()); + } + let mut config_args = vec![]; + if let Some(values) = args.get_many::("config") { + config_args.extend(values.cloned()); + } + config.configure( + verbose, + quiet, + color, + frozen, + locked, + offline, + &None, + &unstable_flags, + &config_args, + )?; + Ok(()) +} + +/// Turns an arg into a commit object. +fn arg_to_commit<'a>( + args: &clap::ArgMatches, + repo: &'a git2::Repository, + name: &str, +) -> CargoResult> { + let arg = args.get_one::(name).map(String::as_str).unwrap(); + Ok(repo.revparse_single(arg)?.peel_to_commit()?) +} + +/// Checkouts a temporary workspace to do further version comparsions. +fn checkout_ws<'cfg, 'a>( + ws: &Workspace<'cfg>, + repo: &'a git2::Repository, + referenced_commit: &git2::Commit<'a>, +) -> CargoResult> { + let repo_path = repo.path().as_os_str().to_str().unwrap(); + // Put it under `target/cargo-` + let short_id = &referenced_commit.id().to_string()[..7]; + let checkout_path = ws.target_dir().join(format!("cargo-{short_id}")); + let checkout_path = checkout_path.as_path_unlocked(); + let _ = fs::remove_dir_all(checkout_path); + let new_repo = git2::build::RepoBuilder::new() + .clone_local(git2::build::CloneLocal::Local) + .clone(repo_path, checkout_path) + .unwrap(); + let obj = new_repo.find_object(referenced_commit.id(), None)?; + new_repo.reset(&obj, git2::ResetType::Hard, None)?; + Workspace::new(&checkout_path.join("Cargo.toml"), ws.config()) +} + +/// Get the current beta and stable branch in cargo repository. +/// +/// Assumptions: +/// +/// * The repository contains the full history of `origin/rust-1.*.0` branches. +/// * The version part of `origin/rust-1.*.0` always ends with a zero. +/// * The maximum version is for beta channel, and the second one is for stable. +fn beta_and_stable_branch(repo: &git2::Repository) -> CargoResult<[git2::Branch<'_>; 2]> { + let mut release_branches = Vec::new(); + for branch in repo.branches(Some(git2::BranchType::Remote))? { + let (branch, _) = branch?; + let Some(version) = branch.name()?.and_then(|n| n.strip_prefix("origin/rust-")) else { + continue; + }; + release_branches.push((version.to_semver()?, branch)); + } + release_branches.sort_unstable_by(|a, b| a.0.cmp(&b.0)); + + let beta = release_branches.pop().unwrap(); + let stable = release_branches.pop().unwrap(); + + assert_eq!(beta.0.major, 1); + assert_eq!(beta.0.patch, 0); + assert_eq!(stable.0.major, 1); + assert_eq!(stable.0.patch, 0); + assert_ne!(beta.0.minor, stable.0.minor); + + Ok([beta.1, stable.1]) +} + +/// Gets the referenced commit to compare if version bump needed. +/// +/// * When merging into nightly, check the version with beta branch +/// * When merging into beta, check the version with stable branch +/// * When merging into stable, check against crates.io registry directly +fn get_referenced_commit<'a>( + repo: &'a git2::Repository, + base: &git2::Commit<'a>, +) -> CargoResult>> { + let [beta, stable] = beta_and_stable_branch(&repo)?; + let rev_id = base.id(); + let stable_commit = stable.get().peel_to_commit()?; + let beta_commit = beta.get().peel_to_commit()?; + + let commit = if rev_id == stable_commit.id() { + None + } else if rev_id == beta_commit.id() { + Some(stable_commit) + } else { + Some(beta_commit) + }; + + Ok(commit) +} + +/// Compares version against published crates on crates.io. +/// +/// Assumption: We always release a version larger than all existing versions. +fn check_crates_io<'a>( + config: &Config, + published_members: &HashMap<&str, &'a Package>, + needs_bump: &mut Vec<&'a Package>, +) -> CargoResult<()> { + let source_id = SourceId::crates_io(config)?; + let mut registry = PackageRegistry::new(config)?; + let _lock = config.acquire_package_cache_lock()?; + registry.lock_patches(); + config.shell().status( + "BumpCheck", + format_args!("compare against `{}`", source_id.display_registry_name()), + )?; + for member in published_members.values() { + let name = member.name(); + let current = member.version(); + let version_req = format!("<={current}"); + let query = Dependency::parse(name, Some(&version_req), source_id)?; + let possibilities = loop { + // Exact to avoid returning all for path/git + match registry.query_vec(&query, QueryKind::Exact) { + task::Poll::Ready(res) => { + break res?; + } + task::Poll::Pending => registry.block_until_ready()?, + } + }; + let max_version = possibilities.iter().map(|s| s.version()).max(); + if max_version >= Some(current) { + needs_bump.push(member); + } + } + + Ok(()) +} + +/// Main entry of `xtask-bump-check`. +/// +/// Assumption: version number are incremental. We never have point release for old versions. +fn bump_check(args: &clap::ArgMatches, config: &mut cargo::util::Config) -> CargoResult<()> { + let ws = args.workspace(config)?; + let repo = git2::Repository::open(ws.root()).unwrap(); + let base_commit = arg_to_commit(args, &repo, "baseline-rev")?; + let referenced_commit = get_referenced_commit(&repo, &base_commit)?; + let published_members = HashMap::<&str, &Package>::from_iter( + ws.members() + .filter(|m| m.publish() != &Some(vec![])) // package.publish = false + .map(|m| (m.name().as_str(), m)), + ); + + let mut needs_bump = Vec::new(); + + check_crates_io(config, &published_members, &mut needs_bump)?; + + if let Some(commit) = referenced_commit.as_ref() { + config.shell().status( + "BumpCheck", + format_args!("compare aginst `{}`", commit.id()), + )?; + for member in checkout_ws(&ws, &repo, commit)?.members() { + let name = member.name().as_str(); + let Some(changed) = published_members.get(name) else { + log::trace!("skpping {name}, may be removed or not published"); + continue; + }; + + if changed.version() <= member.version() { + needs_bump.push(*changed); + } + } + } + + if needs_bump.is_empty() { + config + .shell() + .status("BumpCheck", "no version bump needed for member crates.")?; + return Ok(()); + } + + needs_bump.sort(); + needs_bump.dedup(); + + let mut msg = String::new(); + msg.push_str("Detected changes in these crates but no version bump found:\n"); + for pkg in needs_bump { + writeln!(&mut msg, " {}@{}", pkg.name(), pkg.version())?; + } + msg.push_str("\nPlease bump at least one patch version in each corresponding Cargo.toml."); + anyhow::bail!(msg) +} + +#[test] +fn verify_cli() { + cli().debug_assert(); +} From 94a6babba87d60d41b1f6a74102a2299ea12e177 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 26 Jul 2023 00:42:50 +0100 Subject: [PATCH 2/4] ci: integrate `xtask-bump-check` --- .cargo/config.toml | 1 + Cargo.lock | 12 +++++++++++ ci/validate-version-bump.sh | 41 +------------------------------------ 3 files changed, 14 insertions(+), 40 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index c4a595b6185e..268874484840 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -2,3 +2,4 @@ build-man = "run --package xtask-build-man --" stale-label = "run --package xtask-stale-label --" unpublished = "run --package xtask-unpublished --" +bump-check = "run --package xtask-bump-check --" diff --git a/Cargo.lock b/Cargo.lock index b92088f6d1da..b2c542168ddf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3766,6 +3766,18 @@ dependencies = [ name = "xtask-build-man" version = "0.0.0" +[[package]] +name = "xtask-bump-check" +version = "0.0.0" +dependencies = [ + "anyhow", + "cargo", + "clap", + "env_logger 0.10.0", + "git2", + "log", +] + [[package]] name = "xtask-stale-label" version = "0.0.0" diff --git a/ci/validate-version-bump.sh b/ci/validate-version-bump.sh index 9b54fdaaf766..db01ab19804d 100755 --- a/ci/validate-version-bump.sh +++ b/ci/validate-version-bump.sh @@ -19,43 +19,4 @@ head_sha=$(git rev-parse "${HEAD_SHA:-HEAD}") echo "Base branch is $base_sha" echo "Current head is $head_sha" -# Gets crate names of members that has been changed from $bash_sha to $head_sha. -changed_crates=$( - git diff --name-only "$base_sha" "$head_sha" -- crates/ credential/ benches/ \ - | cut -d'/' -f2 \ - | sort -u -) - -if [ -z "$changed_crates" ] -then - echo "No file changed in member crates." - exit 0 -fi - -# Checks publish status for only crates with code changes. -publish_status_table=$( - echo "$changed_crates" \ - | xargs printf -- '--package %s\n' \ - | xargs cargo unpublished -) - -# "yes" -> code changed but no version difference -> need a bump -# Prints 2nd column (sep by space), which is the name of the crate. -crates_need_bump=$( - echo "$publish_status_table" \ - | { grep '| yes ' || true; } \ - | awk '{print $2}' -) - -if [ -z "$crates_need_bump" ] -then - echo "No version bump needed for member crates." - exit 0 -fi - -echo "Detected changes in these crates but no version bump found:" -echo "$crates_need_bump" -echo -echo "Please bump at least one patch version for each corresponding Cargo.toml:" -echo 'Run "cargo unpublished" to read the publish status table for details.' -exit 1 +cargo bump-check --baseline-rev "$base_sha" From 3766372d9d139b9b5bb6dad6f07f045f57cbe165 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Wed, 26 Jul 2023 00:45:32 +0100 Subject: [PATCH 3/4] chore: remove `xtask-unpublished` --- .cargo/config.toml | 1 - Cargo.lock | 11 -- crates/xtask-unpublished/Cargo.toml | 12 -- crates/xtask-unpublished/src/main.rs | 15 -- crates/xtask-unpublished/src/xtask.rs | 200 -------------------------- 5 files changed, 239 deletions(-) delete mode 100644 crates/xtask-unpublished/Cargo.toml delete mode 100644 crates/xtask-unpublished/src/main.rs delete mode 100644 crates/xtask-unpublished/src/xtask.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index 268874484840..17d424a5d024 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,5 +1,4 @@ [alias] build-man = "run --package xtask-build-man --" stale-label = "run --package xtask-stale-label --" -unpublished = "run --package xtask-unpublished --" bump-check = "run --package xtask-bump-check --" diff --git a/Cargo.lock b/Cargo.lock index b2c542168ddf..1e71b7386872 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3785,17 +3785,6 @@ dependencies = [ "toml_edit", ] -[[package]] -name = "xtask-unpublished" -version = "0.0.0" -dependencies = [ - "anyhow", - "cargo", - "clap", - "env_logger 0.10.0", - "log", -] - [[package]] name = "yansi" version = "0.5.1" diff --git a/crates/xtask-unpublished/Cargo.toml b/crates/xtask-unpublished/Cargo.toml deleted file mode 100644 index 570a9a3ba1e0..000000000000 --- a/crates/xtask-unpublished/Cargo.toml +++ /dev/null @@ -1,12 +0,0 @@ -[package] -name = "xtask-unpublished" -version = "0.0.0" -edition.workspace = true -publish = false - -[dependencies] -anyhow.workspace = true -cargo.workspace = true -clap.workspace = true -env_logger.workspace = true -log.workspace = true diff --git a/crates/xtask-unpublished/src/main.rs b/crates/xtask-unpublished/src/main.rs deleted file mode 100644 index 1942a3621cfb..000000000000 --- a/crates/xtask-unpublished/src/main.rs +++ /dev/null @@ -1,15 +0,0 @@ -mod xtask; - -fn main() { - env_logger::init_from_env("CARGO_LOG"); - let cli = xtask::cli(); - let matches = cli.get_matches(); - - let mut config = cargo::util::config::Config::default().unwrap_or_else(|e| { - let mut eval = cargo::core::shell::Shell::new(); - cargo::exit_with_error(e.into(), &mut eval) - }); - if let Err(e) = xtask::exec(&matches, &mut config) { - cargo::exit_with_error(e, &mut config.shell()) - } -} diff --git a/crates/xtask-unpublished/src/xtask.rs b/crates/xtask-unpublished/src/xtask.rs deleted file mode 100644 index f1086951f8ae..000000000000 --- a/crates/xtask-unpublished/src/xtask.rs +++ /dev/null @@ -1,200 +0,0 @@ -//! `xtask-unpublished` outputs a table with publish status --- a local version -//! and a version on crates.io for comparisons. -//! -//! This aims to help developers check if there is any crate required a new -//! publish, as well as detect if a version bump is needed in CI pipeline. - -use std::collections::HashSet; - -use cargo::core::registry::PackageRegistry; -use cargo::core::QueryKind; -use cargo::core::Registry; -use cargo::core::SourceId; -use cargo::ops::Packages; -use cargo::util::command_prelude::*; - -pub fn cli() -> clap::Command { - clap::Command::new("xtask-unpublished") - .arg_package_spec_simple("Package to inspect the published status") - .arg( - opt( - "verbose", - "Use verbose output (-vv very verbose/build.rs output)", - ) - .short('v') - .action(ArgAction::Count) - .global(true), - ) - .arg_quiet() - .arg( - opt("color", "Coloring: auto, always, never") - .value_name("WHEN") - .global(true), - ) - .arg(flag("frozen", "Require Cargo.lock and cache are up to date").global(true)) - .arg(flag("locked", "Require Cargo.lock is up to date").global(true)) - .arg(flag("offline", "Run without accessing the network").global(true)) - .arg(multi_opt("config", "KEY=VALUE", "Override a configuration value").global(true)) - .arg( - Arg::new("unstable-features") - .help("Unstable (nightly-only) flags to Cargo, see 'cargo -Z help' for details") - .short('Z') - .value_name("FLAG") - .action(ArgAction::Append) - .global(true), - ) -} - -pub fn exec(args: &clap::ArgMatches, config: &mut cargo::util::Config) -> cargo::CliResult { - config_configure(config, args)?; - - unpublished(args, config)?; - - Ok(()) -} - -fn config_configure(config: &mut Config, args: &ArgMatches) -> CliResult { - let verbose = args.verbose(); - // quiet is unusual because it is redefined in some subcommands in order - // to provide custom help text. - let quiet = args.flag("quiet"); - let color = args.get_one::("color").map(String::as_str); - let frozen = args.flag("frozen"); - let locked = args.flag("locked"); - let offline = args.flag("offline"); - let mut unstable_flags = vec![]; - if let Some(values) = args.get_many::("unstable-features") { - unstable_flags.extend(values.cloned()); - } - let mut config_args = vec![]; - if let Some(values) = args.get_many::("config") { - config_args.extend(values.cloned()); - } - config.configure( - verbose, - quiet, - color, - frozen, - locked, - offline, - &None, - &unstable_flags, - &config_args, - )?; - Ok(()) -} - -fn unpublished(args: &clap::ArgMatches, config: &mut cargo::util::Config) -> cargo::CliResult { - let ws = args.workspace(config)?; - - let members_to_inspect: HashSet<_> = { - let pkgs = args.packages_from_flags()?; - if let Packages::Packages(_) = pkgs { - HashSet::from_iter(pkgs.get_packages(&ws)?) - } else { - HashSet::from_iter(ws.members()) - } - }; - - let mut results = Vec::new(); - { - let mut registry = PackageRegistry::new(config)?; - let _lock = config.acquire_package_cache_lock()?; - registry.lock_patches(); - let source_id = SourceId::crates_io(config)?; - - for member in members_to_inspect { - let name = member.name(); - let current = member.version(); - if member.publish() == &Some(vec![]) { - log::trace!("skipping {name}, `publish = false`"); - continue; - } - - let version_req = format!("<={current}"); - let query = - cargo::core::dependency::Dependency::parse(name, Some(&version_req), source_id)?; - let possibilities = loop { - // Exact to avoid returning all for path/git - match registry.query_vec(&query, QueryKind::Exact) { - std::task::Poll::Ready(res) => { - break res?; - } - std::task::Poll::Pending => registry.block_until_ready()?, - } - }; - let (last, published) = possibilities - .iter() - .map(|s| s.version()) - .max() - .map(|last| (last.to_string(), last == current)) - .unwrap_or(("-".to_string(), false)); - - results.push(vec![ - name.to_string(), - last, - current.to_string(), - if published { "yes" } else { "no" }.to_string(), - ]); - } - } - results.sort(); - - if results.is_empty() { - return Ok(()); - } - - results.insert( - 0, - vec![ - "name".to_owned(), - "crates.io".to_owned(), - "local".to_owned(), - "published?".to_owned(), - ], - ); - - output_table(results); - - Ok(()) -} - -/// Outputs a markdown table like this. -/// -/// ```text -/// | name | crates.io | local | published? | -/// |------------------|-----------|--------|------------| -/// | cargo | 0.70.1 | 0.72.0 | no | -/// | cargo-platform | 0.1.2 | 0.1.2 | yes | -/// | cargo-util | - | 0.2.4 | no | -/// | crates-io | 0.36.0 | 0.36.0 | yes | -/// | home | - | 0.5.6 | no | -/// ``` -fn output_table(table: Vec>) { - let header = table.first().unwrap(); - let paddings = table.iter().fold(vec![0; header.len()], |mut widths, row| { - for (width, field) in widths.iter_mut().zip(row) { - *width = usize::max(*width, field.len()); - } - widths - }); - - let print = |row: &[_]| { - for (field, pad) in row.iter().zip(&paddings) { - print!("| {field:pad$} "); - } - println!("|"); - }; - - print(header); - - paddings.iter().for_each(|fill| print!("|-{:- Date: Wed, 26 Jul 2023 01:24:29 +0100 Subject: [PATCH 4/4] ci: integrate `cargo-semver-checks` --- .github/workflows/main.yml | 8 +++++++- ci/validate-version-bump.sh | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6fd7b56f5a2d..61d11d54e17b 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -91,8 +91,14 @@ jobs: steps: - uses: actions/checkout@v3 with: - fetch-depth: 0 # make `git diff` work + fetch-depth: 0 - run: rustup update stable && rustup default stable + - name: Install cargo-semver-checks + run: | + mkdir installed-bins + curl -Lf https://github.com/obi1kenobi/cargo-semver-checks/releases/download/v0.22.1/cargo-semver-checks-x86_64-unknown-linux-gnu.tar.gz \ + | tar -xz --directory=./installed-bins + echo `pwd`/installed-bins >> $GITHUB_PATH - run: ci/validate-version-bump.sh test: diff --git a/ci/validate-version-bump.sh b/ci/validate-version-bump.sh index db01ab19804d..82645713e060 100755 --- a/ci/validate-version-bump.sh +++ b/ci/validate-version-bump.sh @@ -20,3 +20,4 @@ echo "Base branch is $base_sha" echo "Current head is $head_sha" cargo bump-check --baseline-rev "$base_sha" +cargo semver-checks --workspace --baseline-rev "$base_sha"