diff --git a/.cargo/config.toml b/.cargo/config.toml index c4a595b6185e..17d424a5d024 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,4 +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/.github/workflows/main.yml b/.github/workflows/main.yml index 3deae63555f0..64368241b1c9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -56,8 +56,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/Cargo.lock b/Cargo.lock index 8d6fef0a37a4..586ed619a748 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3760,23 +3760,24 @@ name = "xtask-build-man" version = "0.0.0" [[package]] -name = "xtask-stale-label" -version = "0.0.0" -dependencies = [ - "toml_edit", -] - -[[package]] -name = "xtask-unpublished" +name = "xtask-bump-check" version = "0.0.0" dependencies = [ "anyhow", "cargo", "clap 4.3.3", "env_logger 0.10.0", + "git2", "log", ] +[[package]] +name = "xtask-stale-label" +version = "0.0.0" +dependencies = [ + "toml_edit", +] + [[package]] name = "yansi" version = "0.5.1" diff --git a/ci/validate-version-bump.sh b/ci/validate-version-bump.sh index 9b54fdaaf766..bb9cd44d881f 100755 --- a/ci/validate-version-bump.sh +++ b/ci/validate-version-bump.sh @@ -19,43 +19,5 @@ 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" --head-rev "$head_sha" +cargo semver-checks --workspace --baseline-rev "$base_sha" diff --git a/crates/xtask-unpublished/Cargo.toml b/crates/xtask-bump-check/Cargo.toml similarity index 80% rename from crates/xtask-unpublished/Cargo.toml rename to crates/xtask-bump-check/Cargo.toml index 541a34dea11e..f399fb65c991 100644 --- a/crates/xtask-unpublished/Cargo.toml +++ b/crates/xtask-bump-check/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "xtask-unpublished" +name = "xtask-bump-check" version = "0.0.0" edition = "2021" publish = false @@ -9,4 +9,5 @@ anyhow.workspace = true cargo.workspace = true clap.workspace = true env_logger.workspace = true +git2.workspace = true log.workspace = true diff --git a/crates/xtask-unpublished/src/main.rs b/crates/xtask-bump-check/src/main.rs similarity index 100% rename from crates/xtask-unpublished/src/main.rs rename to crates/xtask-bump-check/src/main.rs diff --git a/crates/xtask-bump-check/src/xtask.rs b/crates/xtask-bump-check/src/xtask.rs new file mode 100644 index 000000000000..ad3f61be4112 --- /dev/null +++ b/crates/xtask-bump-check/src/xtask.rs @@ -0,0 +1,338 @@ +//! ```text +//! NAME +//! xtask-bump-check +//! +//! SYNOPSIS +//! xtask-bump-check --baseline-rev --head-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::path::Path; +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( + opt("head-rev", "The HEAD Git revision") + .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) +} + +/// Lists all changed workspace members between two commits. +/// +/// Assumption: Paths of workspace members. See `member_dirs`. +fn changed<'r, 'ws>( + ws: &'ws Workspace<'_>, + repo: &'r git2::Repository, + base_commit: &git2::Commit<'r>, + head: &git2::Commit<'r>, +) -> CargoResult> { + let member_dirs = ["crates", "credential", "benches"]; + let ws_members: HashMap<&str, &Package> = + HashMap::from_iter(ws.members().map(|m| (m.name().as_str(), m))); + let base_tree = base_commit.as_object().peel_to_tree()?; + let head_tree = head.as_object().peel_to_tree()?; + let diff = repo + .diff_tree_to_tree(Some(&base_tree), Some(&head_tree), Default::default()) + .unwrap(); + + let mut changed_members = HashMap::new(); + + for delta in diff.deltas() { + let mut insert_changed = |path: &Path| { + if !member_dirs.iter().any(|dir| path.starts_with(dir)) { + return; + } + let Some(member) = path.components().nth(1) else { + log::trace!("skipping {path:?}, not a valid member path"); + return; + }; + let name = member.as_os_str().to_str().unwrap(); + let Some((name, pkg)) = ws_members.get_key_value(name) else { + log::trace!("skipping {name:?}, not found in workspace"); + return; + }; + if pkg.publish() == &Some(vec![]) { + log::trace!("skipping {name}, `publish = false`"); + return; + } + changed_members.insert(*name, *pkg); + }; + + insert_changed(delta.old_file().path().unwrap()); + insert_changed(delta.new_file().path().unwrap()); + } + + Ok(changed_members) +} + +/// 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, + changed_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 changed_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 head_commit = arg_to_commit(args, &repo, "head-rev")?; + let referenced_commit = get_referenced_commit(&repo, &base_commit)?; + let changed_members = changed(&ws, &repo, &base_commit, &head_commit)?; + + let mut needs_bump = Vec::new(); + + check_crates_io(config, &changed_members, &mut needs_bump)?; + + if let Some(commit) = referenced_commit.as_ref() { + config.shell().status( + "BumpCheck", + format_args!("compare against `{}`", commit.id()), + )?; + for member in checkout_ws(&ws, &repo, commit)?.members() { + let name = member.name().as_str(); + let Some(changed) = changed_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(); +} 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!("|-{:-