Auto merge of #12085 - weihanglo:ci-check-version-bump, r=<try>

ci: check if a crate needs a version bump when source files change

Author: bors

.github/workflows/main.yml

@@ -43,6 +43,18 @@ jobs:
       - run: rustup update stable && rustup default stable
       - run: cargo stale-label
 
+  check-version-bump:
+    runs-on: ubuntu-latest
+    env:
+      BASE_SHA : ${{ github.event.pull_request.base.sha }}
+      COMMIT_SHA: ${{ github.sha }}
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0 # make `git diff` work
+      - run: rustup update stable && rustup default stable
+      - run: ci/validate-version-bump.sh
+
   # Ensure Cargo.lock is up-to-date
   lockfile:
     runs-on: ubuntu-latest
@@ -213,6 +225,7 @@ jobs:
     name: bors build finished
     needs:
     - build_std
+    - check-version-bump
     - docs
     - lockfile
     - resolver
@@ -229,6 +242,7 @@ jobs:
     name: bors build finished
     needs:
     - build_std
+    - check-version-bump
     - docs
     - lockfile
     - resolver

benches/capture/Cargo.toml

@@ -4,6 +4,7 @@ version = "0.1.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "Tool for capturing a real-world workspace for benchmarking."
+publish = false
 
 [dependencies]
 cargo_metadata.workspace = true

ci/validate-version-bump.sh

@@ -0,0 +1,47 @@
+#!/bin/bash
+# This script checks if a crate needs a version bump.
+#
+# At the time of writing, it doesn't check what kind of bump is required.
+# In the future, we could take SemVer compatibliity into account, like
+# integrating `cargo-semver-checks` of else
+#
+# Inputs:
+#     BASE_SHA    The commit SHA of the branch where the PR wants to merge into.
+#     COMMIT_SHA  The commit SHA that triggered the workflow.
+
+set -euo pipefail
+
+# When `BASE_SHA` is missing, we assume it is from bors merge commit,
+# so `HEAD~` should find the previous commit on master branch.
+base_sha=$(git rev-parse "${BASE_SHA:-HEAD~}")
+commit_sha=$(git rev-parse "${COMMIT_SHA:-HEAD}")
+
+echo "Base branch is $base_sha"
+echo "The current is $commit_sha"
+
+changed_crates=$(
+    git diff --name-only "$base_sha" "$commit_sha" -- crates/ credential/ benches/ \
+    | cut  -d'/' -f2 \
+    | sort -u
+)
+
+if  [ -z "$changed_crates" ]
+then
+    echo "No file changed in sub crates."
+    exit 0
+fi
+
+output=$(
+    echo "$changed_crates" \
+    | xargs printf -- '--package %s\n' \
+    | xargs cargo unpublished --check-version-bump
+)
+
+if  [ -z "$output" ]
+then
+    echo "No version bump needed for sub crates."
+    exit 0
+fi
+
+echo "$output"
+exit 1

crates/cargo-test-macro/Cargo.toml

@@ -7,6 +7,7 @@ homepage = "https://github.com/rust-lang/cargo"
 repository = "https://github.com/rust-lang/cargo"
 documentation = "https://github.com/rust-lang/cargo"
 description = "Helper proc-macro for Cargo's testsuite."
+publish = false
 
 [lib]
 proc-macro = true

crates/cargo-test-support/Cargo.toml

@@ -3,6 +3,7 @@ name = "cargo-test-support"
 version = "0.1.0"
 license = "MIT OR Apache-2.0"
 edition = "2021"
+publish = false
 
 [lib]
 doctest = false

crates/xtask-unpublished/src/xtask.rs

@@ -1,11 +1,23 @@
+use std::collections::HashSet;
+use std::fmt;
+use std::fmt::Write;
+
 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::*;
 
+type Record = (String, Option<String>, String, bool);
+
 pub fn cli() -> clap::Command {
     clap::Command::new("xtask-unpublished")
+        .arg(flag(
+            "check-version-bump",
+            "check if any version bump is needed",
+        ))
+        .arg_package_spec_simple("Package to inspect the published status")
         .arg(
             opt(
                 "verbose",
@@ -76,14 +88,24 @@ fn config_configure(config: &mut Config, args: &ArgMatches) -> CliResult {
 
 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 ws.members() {
+        for member in members_to_inspect {
             let name = member.name();
             let current = member.version();
             if member.publish() == &Some(vec![]) {
@@ -92,11 +114,8 @@ fn unpublished(args: &clap::ArgMatches, config: &mut cargo::util::Config) -> car
             }
 
             let version_req = format!("<={current}");
-            let query = cargo::core::dependency::Dependency::parse(
-                name,
-                Some(&version_req),
-                source_id.clone(),
-            )?;
+            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) {
@@ -107,50 +126,142 @@ fn unpublished(args: &clap::ArgMatches, config: &mut cargo::util::Config) -> car
                 }
             };
             if let Some(last) = possibilities.iter().map(|s| s.version()).max() {
-                if last != current {
-                    results.push((
-                        name.to_string(),
-                        Some(last.to_string()),
-                        current.to_string(),
-                    ));
-                } else {
-                    log::trace!("{name} {current} is published");
-                }
+                let published = last == current;
+                results.push((
+                    name.to_string(),
+                    Some(last.to_string()),
+                    current.to_string(),
+                    published,
+                ));
             } else {
-                results.push((name.to_string(), None, current.to_string()));
+                results.push((name.to_string(), None, current.to_string(), false));
             }
         }
     }
+    results.sort();
 
-    if !results.is_empty() {
-        results.insert(
-            0,
-            (
-                "name".to_owned(),
-                Some("published".to_owned()),
-                "current".to_owned(),
-            ),
-        );
-        results.insert(
-            1,
+    if results.is_empty() {
+        return Ok(());
+    }
+
+    let check_version_bump = args.flag("check-version-bump");
+
+    if check_version_bump {
+        output_version_bump_notice(&results);
+    }
+
+    output_table(results, check_version_bump)?;
+
+    Ok(())
+}
+
+/// Outputs a markdown table of publish status for each members.
+///
+/// ```text
+/// | name                            | crates.io | local  | published? |
+/// | ----                            | --------- | -----  | ---------- |
+/// | cargo                           | 0.70.1    | 0.72.0 | no         |
+/// | cargo-credential                | 0.1.0     | 0.2.0  | no         |
+/// | cargo-credential-1password      | 0.1.0     | 0.2.0  | no         |
+/// | cargo-credential-gnome-secret   | 0.1.0     | 0.2.0  | no         |
+/// | cargo-credential-macos-keychain | 0.1.0     | 0.2.0  | no         |
+/// | cargo-credential-wincred        | 0.1.0     | 0.2.0  | no         |
+/// | cargo-platform                  | 0.1.2     | 0.1.3  | no         |
+/// | cargo-util                      | 0.2.3     | 0.2.4  | no         |
+/// | crates-io                       | 0.36.0    | 0.36.1 | no         |
+/// | home                            | 0.5.5     | 0.5.6  | no         |
+/// ```
+fn output_table(results: Vec<Record>, check_version_bump: bool) -> fmt::Result {
+    let mut results: Vec<_> = results
+        .into_iter()
+        .filter(|(.., published)| !check_version_bump || *published)
+        .map(|e| {
             (
-                "====".to_owned(),
-                Some("=========".to_owned()),
-                "=======".to_owned(),
-            ),
-        );
+                e.0,
+                e.1.unwrap_or("-".to_owned()),
+                e.2,
+                if e.3 { "yes" } else { "no" }.to_owned(),
+            )
+        })
+        .collect();
+
+    if results.is_empty() {
+        return Ok(());
     }
-    for (name, last, current) in results {
-        if let Some(last) = last {
-            println!("{name} {last} {current}");
+
+    let header = (
+        "name".to_owned(),
+        "crates.io".to_owned(),
+        "local".to_owned(),
+        if check_version_bump {
+            "need version bump?"
         } else {
-            println!("{name} - {current}");
+            "published?"
         }
+        .to_owned(),
+    );
+    let separators = (
+        "-".repeat(header.0.len()),
+        "-".repeat(header.1.len()),
+        "-".repeat(header.2.len()),
+        "-".repeat(header.3.len()),
+    );
+    results.insert(0, header);
+    results.insert(1, separators);
+
+    let max_col_widths = results
+        .iter()
+        .map(|(name, last, local, bump)| (name.len(), last.len(), local.len(), bump.len()))
+        .reduce(|(c0, c1, c2, c3), (f0, f1, f2, f3)| {
+            (c0.max(f0), c1.max(f1), c2.max(f2), c3.max(f3))
+        })
+        .unwrap();
+
+    let print_space = |out: &mut dyn Write, n| {
+        for _ in 0..(n + 1) {
+            write!(out, " ")?;
+        }
+        fmt::Result::Ok(())
+    };
+
+    let out = &mut String::new();
+    for (name, last, local, bump) in results {
+        write!(out, "| {name}")?;
+        print_space(out, max_col_widths.0 - name.len())?;
+
+        write!(out, "| {last}")?;
+        print_space(out, max_col_widths.1 - last.len())?;
+
+        write!(out, "| {local}")?;
+        print_space(out, max_col_widths.2 - local.len())?;
+
+        write!(out, "| {bump}")?;
+        print_space(out, max_col_widths.3 - bump.len())?;
+
+        writeln!(out, "|")?;
     }
 
+    println!("{out}");
+
     Ok(())
 }
 
+fn output_version_bump_notice(results: &[Record]) {
+    let pkgs_need_bump = results
+        .iter()
+        .filter_map(|(name, .., published)| published.then_some(name.clone()))
+        .collect::<Vec<_>>();
+
+    if !pkgs_need_bump.is_empty() {
+        print!("### :warning: ");
+        println!("Require at least a patch version bump for each of the following packages:\n");
+        for pkg in pkgs_need_bump {
+            println!("* {pkg}");
+        }
+        println!()
+    }
+}
+
 #[test]
 fn verify_cli() {
     cli().debug_assert();

src/cargo/util/errors.rs

@@ -327,6 +327,12 @@ impl From<std::io::Error> for CliError {
     }
 }
 
+impl From<std::fmt::Error> for CliError {
+    fn from(err: std::fmt::Error) -> CliError {
+        CliError::new(err.into(), 1)
+    }
+}
+
 // =============================================================================
 // Construction helpers