Use esp toolchain for all clippy checks, add --check option to fmt-packages subcommand in xtask (#1593)

* Update `fmt-packages` subcommand to allow checking formatting as well

* Use the `esp` toolchain for all `clippy` checks in CI

.github/workflows/ci.yml

@@ -220,43 +220,7 @@ jobs:
   # --------------------------------------------------------------------------
   # Lint
 
-  clippy-riscv:
+  clippy:
-    runs-on: ubuntu-latest
-
-    steps:
-      - uses: actions/checkout@v4
-      - uses: dtolnay/rust-toolchain@v1
-        with:
-          toolchain: nightly
-          targets: riscv32imc-unknown-none-elf,riscv32imac-unknown-none-elf,riscv32imafc-unknown-none-elf
-          components: clippy,rust-src
-      - uses: Swatinem/rust-cache@v2
-
-      # Run 'cargo clippy' on all packages targeting RISC-V:
-      ## esp-hal:
-      - name: clippy (esp-hal, esp32c2)
-        run: cd esp-hal && cargo clippy --features=esp32c2 --target=riscv32imc-unknown-none-elf -- -D warnings
-      - name: clippy (esp-hal, esp32c3)
-        run: cd esp-hal && cargo clippy --features=esp32c3 --target=riscv32imc-unknown-none-elf -- -D warnings
-      - name: clippy (esp-hal, esp32c6)
-        run: cd esp-hal && cargo clippy --features=esp32c6 --target=riscv32imac-unknown-none-elf -- -D warnings
-      - name: clippy (esp-hal, esp32h2)
-        run: cd esp-hal && cargo clippy --features=esp32h2 --target=riscv32imac-unknown-none-elf -- -D warnings
-      ## esp-hal-smartled:
-      - name: clippy (esp-hal-smartled)
-        run: cd esp-hal-smartled && cargo clippy --features=esp32c6 --target=riscv32imac-unknown-none-elf -- -D warnings
-      ## esp-lp-hal:
-      - name: clippy (esp-lp-hal, esp32c6)
-        run: cd esp-lp-hal && cargo clippy --features=esp32c6 --target=riscv32imac-unknown-none-elf -- -D warnings
-      - name: clippy (esp-lp-hal, esp32s2)
-        run: cd esp-lp-hal && cargo clippy --features=esp32s2 --target=riscv32imc-unknown-none-elf -- -D warnings
-      - name: clippy (esp-lp-hal, esp32s3)
-        run: cd esp-lp-hal && cargo clippy --features=esp32s3 --target=riscv32imc-unknown-none-elf -- -D warnings
-      # esp-riscv-rt:
-      - name: clippy (esp-riscv-rt)
-        run: cd esp-riscv-rt && cargo clippy --target=riscv32imc-unknown-none-elf -- -D warnings
-
-  clippy-xtensa:
     runs-on: ubuntu-latest
 
     steps:
@@ -267,13 +231,34 @@ jobs:
           ldproxy: false
       - uses: Swatinem/rust-cache@v2
 
-      # Run 'cargo clippy' on all packages targeting Xtensa:
+      ## esp-hal:
       - name: clippy (esp-hal, esp32)
         run: cd esp-hal && cargo clippy -Zbuild-std=core --features=esp32 --target=xtensa-esp32-none-elf -- -D warnings
+      - name: clippy (esp-hal, esp32c2)
+        run: cd esp-hal && cargo clippy -Zbuild-std=core --features=esp32c2 --target=riscv32imc-unknown-none-elf -- -D warnings
+      - name: clippy (esp-hal, esp32c3)
+        run: cd esp-hal && cargo clippy -Zbuild-std=core --features=esp32c3 --target=riscv32imc-unknown-none-elf -- -D warnings
+      - name: clippy (esp-hal, esp32c6)
+        run: cd esp-hal && cargo clippy -Zbuild-std=core --features=esp32c6 --target=riscv32imac-unknown-none-elf -- -D warnings
+      - name: clippy (esp-hal, esp32h2)
+        run: cd esp-hal && cargo clippy -Zbuild-std=core --features=esp32h2 --target=riscv32imac-unknown-none-elf -- -D warnings
       - name: clippy (esp-hal, esp32s2)
         run: cd esp-hal && cargo clippy -Zbuild-std=core --features=esp32s2 --target=xtensa-esp32s2-none-elf -- -D warnings
       - name: clippy (esp-hal, esp32s3)
         run: cd esp-hal && cargo clippy -Zbuild-std=core --features=esp32s3 --target=xtensa-esp32s3-none-elf -- -D warnings
+      ## esp-hal-smartled:
+      - name: clippy (esp-hal-smartled)
+        run: cd esp-hal-smartled && cargo clippy -Zbuild-std=core --features=esp32c6 --target=riscv32imac-unknown-none-elf -- -D warnings
+      ## esp-lp-hal:
+      - name: clippy (esp-lp-hal, esp32c6)
+        run: cd esp-lp-hal && cargo clippy -Zbuild-std=core --features=esp32c6 --target=riscv32imac-unknown-none-elf -- -D warnings
+      - name: clippy (esp-lp-hal, esp32s2)
+        run: cd esp-lp-hal && cargo clippy -Zbuild-std=core --features=esp32s2 --target=riscv32imc-unknown-none-elf -- -D warnings
+      - name: clippy (esp-lp-hal, esp32s3)
+        run: cd esp-lp-hal && cargo clippy -Zbuild-std=core --features=esp32s3 --target=riscv32imc-unknown-none-elf -- -D warnings
+      # esp-riscv-rt:
+      - name: clippy (esp-riscv-rt)
+        run: cd esp-riscv-rt && cargo clippy -Zbuild-std=core --target=riscv32imc-unknown-none-elf -- -D warnings
 
   rustfmt:
     runs-on: ubuntu-latest
@@ -289,18 +274,10 @@ jobs:
       - uses: Swatinem/rust-cache@v2
 
       # Check the formatting of all packages:
-      - name: rustfmt (esp-hal)
+      - run: cargo xtask fmt-packages --check
-        run: cargo fmt --all --manifest-path=esp-hal/Cargo.toml -- --check
+
-      - name: rustfmt (esp-hal-procmacros)
+  # --------------------------------------------------------------------------
-        run: cargo fmt --all --manifest-path=esp-hal-procmacros/Cargo.toml -- --check
+  # Tests
-      - name: rustfmt (esp-hal-smartled)
-        run: cargo fmt --all --manifest-path=esp-hal-smartled/Cargo.toml -- --check
-      - name: rustfmt (esp-lp-hal)
-        run: cargo fmt --all --manifest-path=esp-lp-hal/Cargo.toml -- --check
-      - name: rustfmt (esp-riscv-rt)
-        run: cargo fmt --all --manifest-path=esp-riscv-rt/Cargo.toml -- --check
-      - name: rustfmt (examples)
-        run: cargo fmt --all --manifest-path=examples/Cargo.toml -- --check
 
   hil:
     name: HIL Test | ${{ matrix.target.soc }}

xtask/src/lib.rs

@@ -545,6 +545,24 @@ pub fn generate_efuse_table(
 // ----------------------------------------------------------------------------
 // Helper Functions
 
+/// Return a (sorted) list of paths to each valid Cargo package in the
+/// workspace.
+pub fn package_paths(workspace: &Path) -> Result<Vec<PathBuf>> {
+    let mut paths = Vec::new();
+    for entry in fs::read_dir(workspace)? {
+        let entry = entry?;
+        if entry.file_type()?.is_dir() {
+            if entry.path().join("Cargo.toml").exists() {
+                paths.push(entry.path());
+            }
+        }
+    }
+
+    paths.sort();
+
+    Ok(paths)
+}
+
 /// Parse the version from the specified package's Cargo manifest.
 pub fn package_version(workspace: &Path, package: Package) -> Result<semver::Version> {
     #[derive(Debug, serde::Deserialize)]

xtask/src/main.rs

@@ -7,7 +7,13 @@ use std::{
 use anyhow::{bail, Result};
 use clap::{Args, Parser};
 use strum::IntoEnumIterator;
-use xtask::{cargo::CargoAction, Chip, Metadata, Package, Version};
+use xtask::{
+    cargo::{CargoAction, CargoArgsBuilder},
+    Chip,
+    Metadata,
+    Package,
+    Version,
+};
 
 // ----------------------------------------------------------------------------
 // Command-line Interface
@@ -25,7 +31,7 @@ enum Cli {
     /// Bump the version of the specified package(s).
     BumpVersion(BumpVersionArgs),
     /// Format all packages in the workspace with rustfmt
-    FmtPackages,
+    FmtPackages(FmtPackagesArgs),
     /// Generate the eFuse fields source file from a CSV.
     GenerateEfuseFields(GenerateEfuseFieldsArgs),
     /// Run the given example for the specified chip.
@@ -100,6 +106,13 @@ struct BumpVersionArgs {
     packages: Vec<Package>,
 }
 
+#[derive(Debug, Args)]
+struct FmtPackagesArgs {
+    /// Run in 'check' mode; exists with 0 if formatted correctly, 1 otherwise
+    #[arg(long)]
+    check: bool,
+}
+
 #[derive(Debug, Args)]
 struct GenerateEfuseFieldsArgs {
     /// Path to the local ESP-IDF repository.
@@ -135,11 +148,11 @@ fn main() -> Result<()> {
         Cli::BuildPackage(args) => build_package(&workspace, args),
         Cli::BuildTests(args) => tests(&workspace, args, CargoAction::Build),
         Cli::BumpVersion(args) => bump_version(&workspace, args),
-        Cli::FmtPackages => fmt_packages(&workspace),
+        Cli::FmtPackages(args) => fmt_packages(&workspace, args),
         Cli::GenerateEfuseFields(args) => generate_efuse_src(&workspace, args),
+        Cli::RunElfs(args) => run_elfs(args),
         Cli::RunExample(args) => examples(&workspace, args, CargoAction::Run),
         Cli::RunTests(args) => tests(&workspace, args, CargoAction::Run),
-        Cli::RunElfs(args) => run_elfs(args),
     }
 }
 
@@ -390,6 +403,27 @@ fn generate_efuse_src(workspace: &Path, args: GenerateEfuseFieldsArgs) -> Result
     Ok(())
 }
 
+fn fmt_packages(workspace: &Path, args: FmtPackagesArgs) -> Result<()> {
+    for path in xtask::package_paths(workspace)? {
+        log::info!("Formatting package: {}", path.display());
+
+        let mut cargo_args = CargoArgsBuilder::default()
+            .toolchain("nightly")
+            .subcommand("fmt")
+            .arg("--all")
+            .build();
+
+        if args.check {
+            cargo_args.push("--".into());
+            cargo_args.push("--check".into());
+        }
+
+        xtask::cargo::run(&cargo_args, &path)?;
+    }
+
+    Ok(())
+}
+
 fn run_elfs(args: RunElfArgs) -> Result<()> {
     let mut failed: Vec<String> = Vec::new();
     for elf in fs::read_dir(&args.path)? {
@@ -430,37 +464,6 @@ fn run_elfs(args: RunElfArgs) -> Result<()> {
     Ok(())
 }
 
-fn fmt_packages(workspace: &Path) -> Result<()> {
-    // Ensure that `rustfmt` is installed
-    if Command::new("rustfmt").arg("--version").output().is_err() {
-        bail!("The 'rustfmt' command is not installed, exiting");
-    }
-
-    // Iterate over all Cargo.toml files in the workspace and format the projects
-    for entry in fs::read_dir(workspace)? {
-        let entry = entry?;
-        if entry.file_type()?.is_dir() {
-            let manifest_path = entry.path().join("Cargo.toml");
-            if manifest_path.exists() {
-                // Run `cargo fmt` on the project
-                let status = Command::new("cargo")
-                    .arg("+nightly")
-                    .arg("fmt")
-                    .arg("--all")
-                    .arg("--manifest-path")
-                    .arg(manifest_path.to_str().unwrap())
-                    .status()?;
-
-                if !status.success() {
-                    bail!("Formatting failed for {}", manifest_path.display());
-                }
-            }
-        }
-    }
-
-    Ok(())
-}
-
 // ----------------------------------------------------------------------------
 // Helper Functions