fix: Resolve pre-commit check build target conflict with xtask

This commit addresses the issue where the pre-commit check for `cargo check` and `cargo clippy` was failing due to conflicts with the `xtask` module. The root cause was that the `xtask` module was being included in the build process, causing unexpected behavior and failures.

To resolve this, the following changes were made:

1. **Manual Exclusion of `xtask`**:
   - The `xtask` module is now manually excluded from the pre-commit checks by using the `-p` flag with `cargo check` and `cargo clippy`. This ensures that only the relevant workspace packages are checked and linted.

2. **Dynamic Package Selection**:
   - The list of workspace packages is dynamically retrieved using `cargo metadata` and filtered to exclude `xtask`. This approach avoids modifying `Cargo.toml` and ensures that the pre-commit hooks do not modify files during execution.

Signed-off-by: Zongyao Chen [email protected]

.pre-commit-config.yaml

@@ -34,14 +34,54 @@ repos:
       - id: cargo-check
         name: cargo check
         description: Check the package for errors.
-        entry: bash -c 'cargo check --target riscv64imac-unknown-none-elf --all --no-default-features'
+        entry: |
+          bash -c '
+            # Get all packages in the workspace
+            packages=$(cargo metadata --format-version 1 | jq -r ".packages[] | select(.name != \"xtask\") | .name")
+
+            # Check each package
+            for package in $packages; do
+              echo "Checking package: $package"
+              cargo check -p "$package" --target riscv64imac-unknown-none-elf
+              check_status=$?
+
+              # If the check fails, exit with the error code
+              if [ "$check_status" -ne 0 ]; then
+                echo "Package $package check failed, exit status: $check_status!"
+                exit $check_status
+              fi
+            done
+
+            echo "All packages checked successfully."
+            exit 0
+          '
         language: rust
         files: \.rs$
         pass_filenames: false
       - id: cargo-clippy
         name: cargo clippy
-        description: Lint rust sources
-        entry: bash -c 'cargo clippy --target riscv64imac-unknown-none-elf --all --no-default-features -- -D warnings'
+        description: Lint Rust sources.
+        entry: |
+          bash -c '
+            # Get all packages in the workspace
+            packages=$(cargo metadata --format-version 1 | jq -r ".packages[] | select(.name != \"xtask\") | .name")
+
+            # Lint each package
+            for package in $packages; do
+              echo "Linting package: $package"
+              cargo clippy -p "$package" --target riscv64imac-unknown-none-elf -- -D warnings
+              clippy_status=$?
+
+              # If the linting fails, exit with the error code
+              if [ "$clippy_status" -ne 0 ]; then
+                echo "Package $package clippy check failed, exit status: $clippy_status!"
+                exit $clippy_status
+              fi
+            done
+
+            echo "All packages linted successfully."
+            exit 0
+          '
         language: rust
         files: \.rs$
         pass_filenames: false

Cargo.toml

@@ -1,12 +1,6 @@
 [workspace]
 resolver = "2"
-members = [
-  "prototyper",
-  "bench-kernel",
-  "test-kernel",
-  "supervisor", 
-  "xtask"
-]
+members = ["prototyper", "bench-kernel", "test-kernel", "supervisor", "xtask"]
 
 [workspace.package]
 edition = "2021"

bench-kernel/src/main.rs

@@ -223,7 +223,7 @@ extern "C" fn rust_main(hartid: usize, dtb_pa: usize) -> ! {
     let frequency = tree.cpus.timebase_frequency;
     info!(
         r"
- ____                  _       _  __                    _ 
+ ____                  _       _  __                    _
 | __ )  ___ _ __   ___| |__   | |/ /___ _ __ _ __   ___| |
 |  _ \ / _ \ '_ \ / __| '_ \  | ' // _ \ '__| '_ \ / _ \ |
 | |_) |  __/ | | | (__| | | | | . \  __/ |  | | | |  __/ |
@@ -256,8 +256,8 @@ extern "C" fn rust_main(hartid: usize, dtb_pa: usize) -> ! {
     for i in 0..4 {
         info!("Test #{i} started");
         unsafe {
-            for i in 0..smp {
-                IPI_SENT[i].assume_init_mut().swap(false, Ordering::AcqRel);
+            for (i, ipi_sent) in IPI_SENT.iter_mut().enumerate().take(smp) {
+                ipi_sent.assume_init_mut().swap(false, Ordering::AcqRel);
                 if i != hartid {
                     while sbi::hart_get_status(i) != SUSPENDED {}
                 }

prototyper/src/fail.rs

@@ -2,11 +2,11 @@ use serde_device_tree::Dtb;
 
 use crate::dt;
 
-#[cfg(not(feature = "payload"))]
-use crate::sbi::reset;
 #[cfg(not(feature = "payload"))]
 use crate::firmware::dynamic;
 #[cfg(not(feature = "payload"))]
+use crate::sbi::reset;
+#[cfg(not(feature = "payload"))]
 use riscv::register::mstatus;
 
 // TODO: Need a better way to handle device tree parsing errors

prototyper/src/firmware/payload.rs

@@ -10,7 +10,6 @@ use super::BootInfo;
 pub fn is_boot_hart(_nonstandard_a2: usize) -> bool {
     static GENESIS: AtomicBool = AtomicBool::new(true);
     GENESIS.swap(false, Ordering::AcqRel)
-
 }
 
 pub fn get_boot_info(_nonstandard_a2: usize) -> BootInfo {

prototyper/src/main.rs

@@ -9,10 +9,10 @@ extern crate log;
 #[macro_use]
 mod macros;
 
-mod platform;
 mod dt;
 mod fail;
 mod firmware;
+mod platform;
 mod riscv_spec;
 mod sbi;
 

prototyper/src/platform/clint.rs

@@ -1,5 +1,5 @@
-use core::arch::asm;
 use aclint::SifiveClint;
+use core::arch::asm;
 use xuantie_riscv::peripheral::clint::THeadClint;
 
 use crate::sbi::ipi::IpiDevice;

prototyper/src/platform/mod.rs

@@ -196,7 +196,9 @@ impl Platform {
             use dt::Cpu;
             let cpu = cpu_iter.deserialize::<Cpu>();
             let hart_id = cpu.reg.iter().next().unwrap().0.start;
-            cpu_list.get_mut(hart_id).map(|x| *x = true);
+            if let Some(x) = cpu_list.get_mut(hart_id) {
+                *x = true;
+            }
         }
         self.info.cpu_enabled = Some(cpu_list);
     }

prototyper/src/sbi/console.rs

@@ -1,7 +1,7 @@
+use crate::platform::PLATFORM;
 use core::fmt::{self, Write};
 use rustsbi::{Console, Physical, SbiRet};
 use spin::Mutex;
-use crate::platform::PLATFORM;
 
 /// A trait that must be implemented by console devices to provide basic I/O functionality.
 pub trait ConsoleDevice {

prototyper/src/sbi/extensions.rs

@@ -71,7 +71,7 @@ pub fn init(cpus: &NodeSeq) {
             let isa_iter = cpu.isa.unwrap();
             let isa = isa_iter.iter().next().unwrap_or_default();
             Extension::ITER.iter().for_each(|ext| {
-                hart_exts[ext.index()] = isa.find(ext.as_str()).is_some();
+                hart_exts[ext.index()] = isa.contains(ext.as_str());
             })
         }
 
@@ -116,9 +116,11 @@ pub fn init(cpus: &NodeSeq) {
         unsafe {
             ROOT_STACK
                 .get_mut(hart_id)
-                .map(|stack| stack.hart_context().features = HartFeatures{
-                    extension: hart_exts,
-                    privileged_version: PrivilegedVersion::Version1_12,
+                .map(|stack| {
+                    stack.hart_context().features = HartFeatures {
+                        extension: hart_exts,
+                        privileged_version: PrivilegedVersion::Version1_12,
+                    }
                 })
                 .unwrap()
         }

prototyper/src/sbi/hsm.rs

@@ -232,7 +232,12 @@ impl rustsbi::Hsm for SbiHsm {
         use rustsbi::spec::hsm::suspend_type::{NON_RETENTIVE, RETENTIVE};
         if matches!(suspend_type, NON_RETENTIVE | RETENTIVE) {
             unsafe {
-                PLATFORM.sbi.ipi.as_ref().unwrap().clear_msip(current_hartid());
+                PLATFORM
+                    .sbi
+                    .ipi
+                    .as_ref()
+                    .unwrap()
+                    .clear_msip(current_hartid());
             }
             unsafe {
                 riscv::register::mie::set_msoft();

prototyper/src/sbi/ipi.rs

@@ -1,6 +1,3 @@
-use core::sync::atomic::Ordering::Relaxed;
-use rustsbi::{HartMask, SbiRet};
-use spin::Mutex;
 use crate::platform::PLATFORM;
 use crate::riscv_spec::{current_hartid, stimecmp};
 use crate::sbi::extensions::{hart_extension_probe, Extension};
@@ -8,6 +5,9 @@ use crate::sbi::hsm::remote_hsm;
 use crate::sbi::rfence;
 use crate::sbi::trap;
 use crate::sbi::trap_stack::ROOT_STACK;
+use core::sync::atomic::Ordering::Relaxed;
+use rustsbi::{HartMask, SbiRet};
+use spin::Mutex;
 
 /// IPI type for supervisor software interrupt.
 pub(crate) const IPI_TYPE_SSOFT: u8 = 1 << 0;
@@ -195,7 +195,7 @@ impl<T: IpiDevice> SbiIpi<T> {
     /// Get upper 32 bits of machine time.
     #[inline]
     pub fn get_timeh(&self) -> usize {
-        ( self.ipi_dev.lock().read_mtime() >> 32) as usize
+        (self.ipi_dev.lock().read_mtime() >> 32) as usize
     }
 
     /// Set machine software interrupt pending for hart.

prototyper/src/sbi/reset.rs

@@ -13,7 +13,7 @@ pub struct SbiReset<T: ResetDevice> {
     pub reset_dev: AtomicPtr<T>,
 }
 
-impl<'a, T: ResetDevice> SbiReset<T> {
+impl<T: ResetDevice> SbiReset<T> {
     pub fn new(reset_dev: AtomicPtr<T>) -> Self {
         Self { reset_dev }
     }

xtask/src/prototyper.rs

@@ -21,6 +21,7 @@ pub struct PrototyperArg {
 }
 
 #[must_use]
+#[rustfmt::skip] // "export_env!("PROTOTYPER_FDT_PATH" ?= fdt.unwrap());" is a macro, rustfmt will not format it correctly
 pub fn run(arg: &PrototyperArg) -> Option<ExitStatus> {
     let arch = "riscv64imac-unknown-none-elf";
     let fdt = arg.fdt.clone();
@@ -33,7 +34,7 @@ pub fn run(arg: &PrototyperArg) -> Option<ExitStatus> {
         .join(arch)
         .join("release");
 
-    cargo::Cargo::new("build")
+    let status = cargo::Cargo::new("build")
         .package("rustsbi-prototyper")
         .target(arch)
         .unstable("build-std", ["core"])
@@ -51,35 +52,40 @@ pub fn run(arg: &PrototyperArg) -> Option<ExitStatus> {
         .status()
         .ok()?;
 
-    let exit_status = Command::new("rust-objcopy")
-        .args(["-O", "binary"])
-        .arg("--binary-architecture=riscv64")
-        .arg(target_dir.join("rustsbi-prototyper"))
-        .arg(target_dir.join("rustsbi-prototyper.bin"))
-        .status()
-        .ok()?;
+    if status.success() {
+        let exit_status = Command::new("rust-objcopy")
+            .args(["-O", "binary"])
+            .arg("--binary-architecture=riscv64")
+            .arg(target_dir.join("rustsbi-prototyper"))
+            .arg(target_dir.join("rustsbi-prototyper.bin"))
+            .status()
+            .ok()?;
 
-    if arg.payload.is_some() {
-        fs::copy(
-            target_dir.join("rustsbi-prototyper"),
-            target_dir.join("rustsbi-prototyper-payload.elf"),
-        )
-        .ok()?;
-        fs::copy(
-            target_dir.join("rustsbi-prototyper.bin"),
-            target_dir.join("rustsbi-prototyper-payload.bin"),
-        )
-        .ok()?;
+        if arg.payload.is_some() {
+            fs::copy(
+                target_dir.join("rustsbi-prototyper"),
+                target_dir.join("rustsbi-prototyper-payload.elf"),
+            )
+            .ok()?;
+            fs::copy(
+                target_dir.join("rustsbi-prototyper.bin"),
+                target_dir.join("rustsbi-prototyper-payload.bin"),
+            )
+            .ok()?;
+        } else {
+            fs::copy(
+                target_dir.join("rustsbi-prototyper"),
+                target_dir.join("rustsbi-prototyper-dynamic.elf"),
+            )
+            .ok()?;
+            fs::copy(
+                target_dir.join("rustsbi-prototyper.bin"),
+                target_dir.join("rustsbi-prototyper-dynamic.bin"),
+            ).ok()?;
+        }
+        return Some(exit_status);
     } else {
-        fs::copy(
-            target_dir.join("rustsbi-prototyper"),
-            target_dir.join("rustsbi-prototyper-dynamic.elf"),
-        )
-        .ok()?;
-        fs::copy(
-            target_dir.join("rustsbi-prototyper.bin"),
-            target_dir.join("rustsbi-prototyper-dynamic.bin"),
-        ).ok()?;
+        eprintln!("Build failed with status: {:?}", status);
+        return Some(status);
     }
-    Some(exit_status)
 }