From 28d26e9f451c57f93ccc9d90764180baedfb65d7 Mon Sep 17 00:00:00 2001 From: Jesse Braham Date: Fri, 24 May 2024 15:43:51 +0000 Subject: [PATCH] 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 | 77 ++++++++++++++-------------------------- xtask/src/lib.rs | 18 ++++++++++ xtask/src/main.rs | 73 +++++++++++++++++++------------------ 3 files changed, 83 insertions(+), 85 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69c82b211..a42a50ed7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -220,43 +220,7 @@ jobs: # -------------------------------------------------------------------------- # Lint - clippy-riscv: - 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: + clippy: 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 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 - - 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 + - run: cargo xtask fmt-packages --check + + # -------------------------------------------------------------------------- + # Tests hil: name: HIL Test | ${{ matrix.target.soc }} diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index cdb9ea15e..72af6bc0d 100644 --- a/xtask/src/lib.rs +++ b/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> { + 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 { #[derive(Debug, serde::Deserialize)] diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 0bfcd5e15..9d3b46fb1 100644 --- a/xtask/src/main.rs +++ b/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, } +#[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 = 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