From 0876bac6c52f3170aa324faee1e709a7628e5e78 Mon Sep 17 00:00:00 2001 From: Scott Mabin Date: Mon, 14 Apr 2025 12:23:54 +0100 Subject: [PATCH] xtask: refactor feature selection and package validation (#3358) * apply_feature_rules applies more things * apply features in one place only, fix missing features and clippy warnings * move various logic to package enum, re-add the ability to test packages with custom feature sets * small cleanup * simplify msrv check, fix CI * review feedback * try and fix msrv check * rebase fixups * use msrv toolchain in check --- .github/workflows/ci.yml | 37 +-- esp-config/src/generate/mod.rs | 2 +- esp-config/src/generate/validator.rs | 38 ++- esp-config/src/generate/value.rs | 2 +- .../src/{blocking_main.rs => blocking.rs} | 0 esp-hal-procmacros/src/lib.rs | 29 +- esp-hal/Cargo.toml | 3 - esp-hal/src/soc/esp32/psram.rs | 2 +- esp-metadata/src/generate_cfg.rs | 18 +- xtask/src/documentation.rs | 56 +--- xtask/src/lib.rs | 154 ++++++++-- xtask/src/main.rs | 280 ++++-------------- 12 files changed, 259 insertions(+), 362 deletions(-) rename esp-hal-procmacros/src/{blocking_main.rs => blocking.rs} (100%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c47472ce..5b40eba55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -133,39 +133,18 @@ jobs: components: rust-src - uses: Swatinem/rust-cache@v2 - # Verify the MSRV for all RISC-V chips. + - name: Stable toolchain checks + run: rustc +stable --version --verbose + - name: esp toolchain checks + run: rustc +esp --version --verbose + + # Verify the MSRV for all chips by running a lint session - name: msrv RISCV (esp-hal) run: | - cargo xtask build-package --features=esp32c2,ci --target=riscv32imc-unknown-none-elf esp-hal - cargo xtask build-package --features=esp32c3,ci --target=riscv32imc-unknown-none-elf esp-hal - cargo xtask build-package --features=esp32c6,ci --target=riscv32imac-unknown-none-elf esp-hal - cargo xtask build-package --features=esp32h2,ci --target=riscv32imac-unknown-none-elf esp-hal - - - name: msrv RISCV (esp-wifi) - run: | - cargo xtask build-package --features=esp32c2,wifi,ble,esp-hal/unstable --target=riscv32imc-unknown-none-elf esp-wifi - cargo xtask build-package --features=esp32c3,wifi,ble,esp-hal/unstable --target=riscv32imc-unknown-none-elf esp-wifi - cargo xtask build-package --features=esp32c6,wifi,ble,esp-hal/unstable --target=riscv32imac-unknown-none-elf esp-wifi - cargo xtask build-package --features=esp32h2,ble,esp-hal/unstable --target=riscv32imac-unknown-none-elf esp-wifi - - # Verify the MSRV for all Xtensa chips: + cargo +stable xtask lint-packages --chips esp32c2,esp32c3,esp32c6,esp32h2 - name: msrv Xtensa (esp-hal) run: | - cargo xtask build-package --toolchain=esp --features=esp32,ci --target=xtensa-esp32-none-elf esp-hal - cargo xtask build-package --toolchain=esp --features=esp32s2,ci --target=xtensa-esp32s2-none-elf esp-hal - cargo xtask build-package --toolchain=esp --features=esp32s3,ci --target=xtensa-esp32s3-none-elf esp-hal - - - name: msrv Xtensa (esp-wifi) - run: | - cargo xtask build-package --toolchain=esp --features=esp32,wifi,ble,esp-hal/unstable --target=xtensa-esp32-none-elf esp-wifi - cargo xtask build-package --toolchain=esp --features=esp32s2,wifi,esp-hal/unstable --target=xtensa-esp32s2-none-elf esp-wifi - cargo xtask build-package --toolchain=esp --features=esp32s3,wifi,ble,esp-hal/unstable --target=xtensa-esp32s3-none-elf esp-wifi - - - name: msrv (esp-lp-hal) - run: | - cargo xtask build-package --features=esp32c6 --target=riscv32imac-unknown-none-elf esp-lp-hal - cargo xtask build-package --features=esp32s2 --target=riscv32imc-unknown-none-elf esp-lp-hal - cargo xtask build-package --features=esp32s3 --target=riscv32imc-unknown-none-elf esp-lp-hal + cargo +esp xtask lint-packages --chips esp32,esp32s2,esp32s3 # -------------------------------------------------------------------------- # Format diff --git a/esp-config/src/generate/mod.rs b/esp-config/src/generate/mod.rs index 3377fd27b..5f41c3a0d 100644 --- a/esp-config/src/generate/mod.rs +++ b/esp-config/src/generate/mod.rs @@ -90,7 +90,7 @@ pub fn generate_config( continue; } markdown::write_doc_table_line(&mut doc_table, name, option); - markdown::write_summary_table_line(&mut selected_config, &name, value); + markdown::write_summary_table_line(&mut selected_config, name, value); } write_out_file(format!("{file_name}_config_table.md"), doc_table); diff --git a/esp-config/src/generate/validator.rs b/esp-config/src/generate/validator.rs index 3be813623..5b327ecea 100644 --- a/esp-config/src/generate/validator.rs +++ b/esp-config/src/generate/validator.rs @@ -4,6 +4,8 @@ use serde::Serialize; use super::{snake_case, value::Value, Error}; +type CustomValidatorFn = Box Result<(), Error>>; + /// Configuration value validation functions. #[derive(Serialize)] pub enum Validator { @@ -22,13 +24,10 @@ pub enum Validator { /// type. #[serde(serialize_with = "serialize_custom")] #[serde(untagged)] - Custom(Box Result<(), Error>>), + Custom(CustomValidatorFn), } -pub(crate) fn serialize_custom( - _: &Box Result<(), Error>>, - serializer: S, -) -> Result +pub(crate) fn serialize_custom(_: &CustomValidatorFn, serializer: S) -> Result where S: serde::Serializer, { @@ -75,25 +74,22 @@ impl Validator { config_key: &str, actual_value: &Value, ) { - match self { - Validator::Enumeration(values) => { - for possible_value in values { - writeln!( - stdout, - "cargo:rustc-check-cfg=cfg({config_key}_{})", - snake_case(possible_value) - ) - .ok(); - } - + if let Validator::Enumeration(values) = self { + for possible_value in values { writeln!( stdout, - "cargo:rustc-cfg={config_key}_{}", - snake_case(&actual_value.to_string()) + "cargo:rustc-check-cfg=cfg({config_key}_{})", + snake_case(possible_value) ) .ok(); } - _ => (), + + writeln!( + stdout, + "cargo:rustc-cfg={config_key}_{}", + snake_case(&actual_value.to_string()) + ) + .ok(); } } } @@ -109,9 +105,9 @@ pub(crate) fn enumeration(values: &Vec, value: &Value) -> Result<(), Err Ok(()) } else { - return Err(Error::parse( + Err(Error::parse( "Validator::Enumeration can only be used with string values", - )); + )) } } diff --git a/esp-config/src/generate/value.rs b/esp-config/src/generate/value.rs index c0e5203f9..5747cca4e 100644 --- a/esp-config/src/generate/value.rs +++ b/esp-config/src/generate/value.rs @@ -33,7 +33,7 @@ impl Value { [b'0', b'x', ..] => i128::from_str_radix(&s[2..], 16), [b'0', b'o', ..] => i128::from_str_radix(&s[2..], 8), [b'0', b'b', ..] => i128::from_str_radix(&s[2..], 2), - _ => i128::from_str_radix(&s, 10), + _ => s.parse(), } .map_err(|_| Error::parse(format!("Expected valid intger value, found: '{s}'")))?; diff --git a/esp-hal-procmacros/src/blocking_main.rs b/esp-hal-procmacros/src/blocking.rs similarity index 100% rename from esp-hal-procmacros/src/blocking_main.rs rename to esp-hal-procmacros/src/blocking.rs diff --git a/esp-hal-procmacros/src/lib.rs b/esp-hal-procmacros/src/lib.rs index b6dd22f0c..ca67396a6 100644 --- a/esp-hal-procmacros/src/lib.rs +++ b/esp-hal-procmacros/src/lib.rs @@ -8,27 +8,24 @@ //! //! - Placing statics and functions into RAM //! - Marking interrupt handlers -//! - Automatically creating an `embassy` executor instance and spawning the -//! defined entry point +//! - Blocking and Async `#[main]` macros //! //! These macros offer developers a convenient way to control memory placement //! and define interrupt handlers in their embedded applications, allowing for //! optimized memory usage and precise handling of hardware interrupts. //! //! Key Components: -//! - [`interrupt`](attr.interrupt.html) - Attribute macro for marking -//! interrupt handlers. Interrupt handlers are used to handle specific -//! hardware interrupts generated by peripherals. +//! - [`handler`](macro@handler) - Attribute macro for marking interrupt +//! handlers. Interrupt handlers are used to handle specific hardware +//! interrupts generated by peripherals. //! -//! The macro allows users to specify the interrupt name explicitly or use -//! the function name to match the interrupt. -//! - [`main`](attr.main.html) - Creates a new `executor` instance and declares -//! an application entry point spawning the corresponding function body as an -//! async task. -//! - [`ram`](attr.ram.html) - Attribute macro for placing statics and -//! functions into specific memory sections, such as SRAM or RTC RAM (slow or -//! fast) with different initialization options. See its documentation for -//! details. +//! - [`ram`](macro@ram) - Attribute macro for placing statics and functions +//! into specific memory sections, such as SRAM or RTC RAM (slow or fast) +//! with different initialization options. See its documentation for details. +//! +//! - [`embassy::main`](macro@embassy_main) - Creates a new `executor` instance +//! and declares an application entry point spawning the corresponding +//! function body as an async task. //! //! ## Examples //! @@ -49,7 +46,7 @@ use proc_macro::TokenStream; -mod blocking_main; +mod blocking; mod builder; #[cfg(feature = "embassy")] mod embassy; @@ -209,7 +206,7 @@ pub fn embassy_main(args: TokenStream, item: TokenStream) -> TokenStream { /// ``` #[proc_macro_attribute] pub fn blocking_main(args: TokenStream, input: TokenStream) -> TokenStream { - blocking_main::main(args, input) + blocking::main(args, input) } /// Automatically implement the [Builder Lite] pattern for a struct. diff --git a/esp-hal/Cargo.toml b/esp-hal/Cargo.toml index 1ac883382..371b73a5c 100644 --- a/esp-hal/Cargo.toml +++ b/esp-hal/Cargo.toml @@ -146,9 +146,6 @@ defmt = [ ## Use externally connected PSRAM (`quad` by default, can be configured to `octal` via ESP_HAL_CONFIG_PSRAM_MODE) psram = [] -# This feature is intended for testing; you probably don't want to enable it: -ci = ["defmt", "bluetooth"] - #! ### Unstable APIs #! Unstable APIs are drivers and features that are not yet ready for general use. #! They may be incomplete, have bugs, or be subject to change without notice. diff --git a/esp-hal/src/soc/esp32/psram.rs b/esp-hal/src/soc/esp32/psram.rs index 65c80b1d3..358706204 100644 --- a/esp-hal/src/soc/esp32/psram.rs +++ b/esp-hal/src/soc/esp32/psram.rs @@ -889,7 +889,7 @@ pub(crate) mod utils { // Enable MOSI spi.user().modify(|_, w| w.usr_mosi().clear_bit()); // Load send buffer - let len = (p_in_data.tx_data_bit_len + 31) / 32; + let len = p_in_data.tx_data_bit_len.div_ceil(32); if !p_tx_val.is_null() { for i in 0..len { spi.w(0) diff --git a/esp-metadata/src/generate_cfg.rs b/esp-metadata/src/generate_cfg.rs index 7c4255dbe..f2927fa63 100644 --- a/esp-metadata/src/generate_cfg.rs +++ b/esp-metadata/src/generate_cfg.rs @@ -100,7 +100,7 @@ pub enum Chip { } impl Chip { - pub fn target(&self) -> &str { + pub fn target(&self) -> &'static str { use Chip::*; match self { @@ -118,7 +118,7 @@ impl Chip { matches!(self, Esp32c6 | Esp32s2 | Esp32s3) } - pub fn lp_target(&self) -> Result<&str> { + pub fn lp_target(&self) -> Result<&'static str> { use Chip::*; match self { @@ -186,6 +186,20 @@ impl Config { } } + /// Create an empty configuration + pub fn empty() -> Self { + Self { + device: Device { + name: "".to_owned(), + arch: Arch::RiscV, + cores: Cores::Single, + peripherals: Vec::new(), + symbols: Vec::new(), + memory: Vec::new(), + }, + } + } + /// The name of the device. pub fn name(&self) -> String { self.device.name.clone() diff --git a/xtask/src/documentation.rs b/xtask/src/documentation.rs index aa89ac6d8..d2eb9a4d3 100644 --- a/xtask/src/documentation.rs +++ b/xtask/src/documentation.rs @@ -58,7 +58,7 @@ pub fn build_documentation( _ => vec![Chip::Esp32c6], } } else { - log::warn!("Package '{package}' does not have chip features, ignoring argument"); + log::debug!("Package '{package}' does not have chip features, ignoring argument"); vec![] }; @@ -97,7 +97,7 @@ fn build_documentation_for_package( // Ensure that the package/chip combination provided are valid: if let Some(chip) = chip { - if let Err(err) = crate::validate_package_chip(package, &chip) { + if let Err(err) = package.validate_package_chip(&chip) { log::warn!("{err}"); return Ok(()); } @@ -192,15 +192,17 @@ fn cargo_doc(workspace: &Path, package: Package, chip: Option) -> Result

) -> Result

Vec { - let chip_name = &config.name(); - - let mut features = vec![]; - match package { - Package::EspBacktrace => features.push("defmt".to_owned()), - Package::EspConfig => features.push("build".to_owned()), - Package::EspHal => { - features.push("unstable".to_owned()); - features.push("ci".to_owned()); - match chip_name.as_str() { - "esp32" => features.push("psram".to_owned()), - "esp32s2" => features.push("psram".to_owned()), - "esp32s3" => features.push("psram".to_owned()), - _ => {} - }; - } - Package::EspWifi => { - features.push("esp-hal/unstable".to_owned()); - if config.contains("wifi") { - features.push("wifi".to_owned()); - features.push("esp-now".to_owned()); - features.push("sniffer".to_owned()); - features.push("smoltcp/proto-ipv4".to_owned()); - features.push("smoltcp/proto-ipv6".to_owned()); - } - if config.contains("ble") { - features.push("ble".to_owned()); - } - if config.contains("wifi") && config.contains("ble") { - features.push("coex".to_owned()); - } - } - Package::EspHalEmbassy | Package::EspIeee802154 => { - features.push("esp-hal/unstable".to_owned()); - }, - _ => {} - } - - features -} - fn patch_documentation_index_for_package( workspace: &Path, package: &Package, @@ -454,7 +414,7 @@ fn generate_documentation_meta_for_package( for chip in chips { // Ensure that the package/chip combination provided are valid: - crate::validate_package_chip(&package, chip)?; + package.validate_package_chip(chip)?; // Build the context object required for rendering this particular build's // information on the documentation index: diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index 8022258f6..2d157a536 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -6,10 +6,10 @@ use std::{ process::Command, }; -use anyhow::{ensure, Context, Result}; +use anyhow::{anyhow, Context, Result}; use cargo::CargoAction; use clap::ValueEnum; -use esp_metadata::Chip; +use esp_metadata::{Chip, Config}; use strum::{Display, EnumIter, IntoEnumIterator as _}; use crate::{cargo::CargoArgsBuilder, firmware::Metadata}; @@ -96,6 +96,135 @@ impl Package { matches!(self, EspBuild | EspConfig | EspMetadata) } + + /// Given a device config, return the features which should be enabled for + /// this package. + pub fn feature_rules(&self, config: &Config) -> Vec { + let mut features = vec![]; + match self { + Package::EspBacktrace => features.push("defmt".to_owned()), + Package::EspConfig => features.push("build".to_owned()), + Package::EspHal => { + features.push("unstable".to_owned()); + if config.contains("psram") { + // TODO this doesn't test octal psram (since `ESP_HAL_CONFIG_PSRAM_MODE` + // defaults to `quad`) as it would require a separate build + features.push("psram".to_owned()) + } + if config.contains("usb0") { + features.push("usb-otg".to_owned()); + } + if config.contains("bt") { + features.push("bluetooth".to_owned()); + } + } + Package::EspWifi => { + features.push("esp-hal/unstable".to_owned()); + features.push("defmt".to_owned()); + if config.contains("wifi") { + features.push("wifi".to_owned()); + features.push("esp-now".to_owned()); + features.push("sniffer".to_owned()); + features.push("smoltcp/proto-ipv4".to_owned()); + features.push("smoltcp/proto-ipv6".to_owned()); + } + if config.contains("ble") { + features.push("ble".to_owned()); + } + if config.contains("coex") { + features.push("coex".to_owned()); + } + } + Package::EspHalProcmacros => { + features.push("embassy".to_owned()); + } + Package::EspHalEmbassy => { + features.push("esp-hal/unstable".to_owned()); + features.push("defmt".to_owned()); + features.push("executors".to_owned()); + } + Package::EspIeee802154 => { + features.push("defmt".to_owned()); + features.push("esp-hal/unstable".to_owned()); + } + Package::EspLpHal => { + if config.contains("lp_core") { + features.push("embedded-io".to_owned()); + } + } + Package::EspPrintln => { + features.push("auto".to_owned()); + features.push("defmt-espflash".to_owned()); + } + Package::EspStorage => { + features.push("storage".to_owned()); + features.push("nor-flash".to_owned()); + features.push("low-level".to_owned()); + } + Package::EspBootloaderEspIdf => { + features.push("defmt".to_owned()); + features.push("validation".to_owned()); + } + Package::EspAlloc => { + features.push("defmt".to_owned()); + } + _ => {} + } + + features + } + + /// Additional feature rules to test subsets of features for a package. + pub fn lint_feature_rules(&self, _config: &Config) -> Vec> { + let mut cases = Vec::new(); + + match self { + Package::EspWifi => { + // minimal set of features that when enabled _should_ still compile + cases.push(vec![ + "esp-hal/unstable".to_owned(), + "builtin-scheduler".to_owned(), + ]); + } + _ => {} + } + + cases + } + + /// Return the target triple for a given package/chip pair. + pub fn target_triple(&self, chip: &Chip) -> Result<&'static str> { + if *self == Package::EspLpHal { + chip.lp_target() + } else { + Ok(chip.target()) + } + } + + /// Validate that the specified chip is valid for the specified package. + pub fn validate_package_chip(&self, chip: &Chip) -> Result<()> { + let device = Config::for_chip(chip); + + let check = match self { + Package::EspIeee802154 => device.contains("ieee802154"), + Package::EspLpHal => chip.has_lp_core(), + Package::XtensaLx | Package::XtensaLxRt | Package::XtensaLxRtProcMacros => { + chip.is_xtensa() + } + Package::EspRiscvRt => chip.is_riscv(), + _ => true, + }; + + if check { + Ok(()) + } else { + Err(anyhow!( + "Invalid chip provided for package '{}': '{}'", + self, + chip + )) + } + } } #[derive(Debug, Clone, Copy, Display, ValueEnum)] @@ -536,24 +665,3 @@ pub fn package_version(workspace: &Path, package: Package) -> Result PathBuf { PathBuf::from(path.to_str().unwrap().to_string().replace("\\\\?\\", "")) } - -/// Return the target triple for a given package/chip pair. -pub fn target_triple(package: Package, chip: &Chip) -> Result<&str> { - if package == Package::EspLpHal { - chip.lp_target() - } else { - Ok(chip.target()) - } -} - -/// Validate that the specified chip is valid for the specified package. -pub fn validate_package_chip(package: &Package, chip: &Chip) -> Result<()> { - ensure!( - *package != Package::EspLpHal || chip.has_lp_core(), - "Invalid chip provided for package '{}': '{}'", - package, - chip - ); - - Ok(()) -} diff --git a/xtask/src/main.rs b/xtask/src/main.rs index c92db9a51..0fb724220 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -7,12 +7,11 @@ use std::{ use anyhow::{bail, ensure, Context as _, Result}; use clap::{Args, Parser}; -use esp_metadata::{Arch, Chip, Config}; +use esp_metadata::{Chip, Config}; use strum::IntoEnumIterator; use xtask::{ cargo::{CargoAction, CargoArgsBuilder}, firmware::Metadata, - target_triple, Package, Version, }; @@ -165,7 +164,7 @@ struct LintPackagesArgs { packages: Vec, /// Lint for a specific chip - #[arg(long, value_enum, default_values_t = Chip::iter())] + #[arg(long, value_enum, value_delimiter = ',', default_values_t = Chip::iter())] chips: Vec, /// Automatically apply fixes @@ -253,7 +252,7 @@ fn main() -> Result<()> { fn examples(workspace: &Path, mut args: ExampleArgs, action: CargoAction) -> Result<()> { // Ensure that the package/chip combination provided are valid: - xtask::validate_package_chip(&args.package, &args.chip)?; + args.package.validate_package_chip(&args.chip)?; // If the 'esp-hal' package is specified, what we *really* want is the // 'examples' package instead: @@ -305,7 +304,7 @@ fn build_examples( out_path: PathBuf, ) -> Result<()> { // Determine the appropriate build target for the given package and chip: - let target = target_triple(args.package, &args.chip)?; + let target = args.package.target_triple(&args.chip)?; if examples .iter() @@ -346,7 +345,7 @@ fn build_examples( fn run_example(args: ExampleArgs, examples: Vec, package_path: &Path) -> Result<()> { // Determine the appropriate build target for the given package and chip: - let target = target_triple(args.package, &args.chip)?; + let target = args.package.target_triple(&args.chip)?; // Filter the examples down to only the binary we're interested in, assuming it // actually supports the specified chip: @@ -375,7 +374,7 @@ fn run_example(args: ExampleArgs, examples: Vec, package_path: &Path) fn run_examples(args: ExampleArgs, examples: Vec, package_path: &Path) -> Result<()> { // Determine the appropriate build target for the given package and chip: - let target = target_triple(args.package, &args.chip)?; + let target = args.package.target_triple(&args.chip)?; // Filter the examples down to only the binaries we're interested in let mut examples: Vec = examples @@ -451,7 +450,7 @@ fn tests(workspace: &Path, args: TestArgs, action: CargoAction) -> Result<()> { let package_path = xtask::windows_safe_path(&workspace.join("hil-test")); // Determine the appropriate build target for the given package and chip: - let target = target_triple(Package::HilTest, &args.chip)?; + let target = Package::HilTest.target_triple(&args.chip)?; // Load all tests which support the specified chip and parse their metadata: let mut tests = xtask::firmware::load(&package_path.join("tests"))? @@ -636,188 +635,36 @@ fn lint_packages(workspace: &Path, args: LintPackagesArgs) -> Result<()> { let mut packages = args.packages; packages.sort(); - for package in packages { - let path = workspace.join(package.to_string()); - + for package in packages.iter().filter(|p| p.is_published()) { // Unfortunately each package has its own unique requirements for // building, so we need to handle each individually (though there // is *some* overlap) - for chip in &args.chips { let device = Config::for_chip(chip); - match package { - Package::EspBacktrace => { - lint_package( - chip, - &path, - &[ - "--no-default-features", - &format!("--target={}", chip.target()), - ], - &[&format!("{chip},defmt")], - args.fix, - package.build_on_host(), - )?; + if let Err(_) = package.validate_package_chip(chip) { + continue; + } + + let feature_sets = [ + vec![package.feature_rules(device)], // initially test all features + package.lint_feature_rules(device), // add separate test cases + ] + .concat(); + + for mut features in feature_sets { + if package.has_chip_features() { + features.push(device.name()) } - Package::EspHal => { - let mut features = format!("{chip},ci,unstable"); - - // Cover all esp-hal features where a device is supported - if device.contains("usb0") { - features.push_str(",usb-otg") - } - if device.contains("bt") { - features.push_str(",bluetooth") - } - if device.contains("psram") { - // TODO this doesn't test octal psram (since `ESP_HAL_CONFIG_PSRAM_MODE` - // defaults to `quad`) as it would require a separate build - features.push_str(",psram") - } - - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[&features], - args.fix, - package.build_on_host(), - )?; - } - - Package::EspHalEmbassy => { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[&format!("{chip},executors,defmt,esp-hal/unstable")], - args.fix, - package.build_on_host(), - )?; - } - - Package::EspIeee802154 => { - if device.contains("ieee802154") { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[&format!("{chip},defmt,esp-hal/unstable")], - args.fix, - package.build_on_host(), - )?; - } - } - Package::EspLpHal => { - if device.contains("lp_core") { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.lp_target().unwrap())], - &[&format!("{chip},embedded-io")], - args.fix, - package.build_on_host(), - )?; - } - } - - Package::EspPrintln => { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[&format!("{chip},defmt-espflash")], - args.fix, - package.build_on_host(), - )?; - } - - Package::EspRiscvRt => { - if matches!(device.arch(), Arch::RiscV) { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[""], - args.fix, - package.build_on_host(), - )?; - } - } - - Package::EspStorage => { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[&format!("{chip},storage,nor-flash,low-level")], - args.fix, - package.build_on_host(), - )?; - } - - Package::EspWifi => { - let minimal_features = format!("{chip},esp-hal/unstable,builtin-scheduler"); - let mut all_features = minimal_features.clone(); - - all_features.push_str(",defmt"); - - if device.contains("wifi") { - all_features.push_str(",esp-now,sniffer") - } - if device.contains("bt") { - all_features.push_str(",ble") - } - if device.contains("coex") { - all_features.push_str(",coex") - } - lint_package( - chip, - &path, - &[ - &format!("--target={}", chip.target()), - "--no-default-features", - ], - &[&minimal_features, &all_features], - args.fix, - package.build_on_host(), - )?; - } - - Package::XtensaLx => { - if matches!(device.arch(), Arch::Xtensa) { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[""], - args.fix, - package.build_on_host(), - )? - } - } - - Package::XtensaLxRt => { - if matches!(device.arch(), Arch::Xtensa) { - lint_package( - chip, - &path, - &[&format!("--target={}", chip.target())], - &[&format!("{chip}")], - args.fix, - package.build_on_host(), - )? - } - } - - // We will *not* check the following packages with `clippy`; this - // may or may not change in the future: - Package::Examples | Package::HilTest | Package::QaTest => {} - - // By default, no `clippy` arguments are required: - _ => lint_package(chip, &path, &[], &[], args.fix, package.build_on_host())?, + lint_package( + workspace, + *package, + chip, + &["--no-default-features"], + &features, + args.fix, + )?; } } } @@ -826,54 +673,53 @@ fn lint_packages(workspace: &Path, args: LintPackagesArgs) -> Result<()> { } fn lint_package( + workspace: &Path, + package: Package, chip: &Chip, - path: &Path, args: &[&str], - feature_sets: &[&str], + features: &[String], fix: bool, - build_on_host: bool, ) -> Result<()> { - for features in feature_sets { - log::info!( - "Linting package: {} ({}, features: {})", - path.display(), - chip, - features - ); + log::info!( + "Linting package: {} ({}, features: {:?})", + package, + chip, + features + ); - let builder = CargoArgsBuilder::default().subcommand("clippy"); + let path = workspace.join(package.to_string()); - let mut builder = if chip.is_xtensa() { - let builder = if build_on_host { - builder - } else { - builder.arg("-Zbuild-std=core,alloc") - }; + let mut builder = CargoArgsBuilder::default().subcommand("clippy"); + let mut builder = if !package.build_on_host() { + if chip.is_xtensa() { // We only overwrite Xtensas so that externally set nightly/stable toolchains // are not overwritten. - builder.toolchain("esp") - } else { - builder - }; - - for arg in args { - builder = builder.arg(arg.to_string()); + builder = builder.arg("-Zbuild-std=core,alloc"); + builder = builder.toolchain("esp"); } - builder = builder.arg(format!("--features={features}")); + builder.target(package.target_triple(chip)?) + } else { + builder + }; - let builder = if fix { - builder.arg("--fix").arg("--lib").arg("--allow-dirty") - } else { - builder.arg("--").arg("-D").arg("warnings").arg("--no-deps") - }; - - let cargo_args = builder.build(); - - xtask::cargo::run_with_env(&cargo_args, path, [("CI", "1")], false)?; + for arg in args { + builder = builder.arg(arg.to_string()); } + builder = builder.arg(format!("--features={}", features.join(","))); + + let builder = if fix { + builder.arg("--fix").arg("--lib").arg("--allow-dirty") + } else { + builder.arg("--").arg("-D").arg("warnings").arg("--no-deps") + }; + + let cargo_args = builder.build(); + + xtask::cargo::run_with_env(&cargo_args, &path, [("CI", "1")], false)?; + Ok(()) } @@ -964,7 +810,7 @@ fn run_doc_tests(workspace: &Path, args: ExampleArgs) -> Result<()> { // Determine the appropriate build target, and cargo features for the given // package and chip: - let target = target_triple(args.package, &chip)?; + let target = args.package.target_triple(&chip)?; let features = vec![chip.to_string(), "unstable".to_string()]; // We need `nightly` for building the doc tests, unfortunately: