diff --git a/.github/workflows/diff.yml b/.github/workflows/diff.yml index f1200e03..20b8ea50 100644 --- a/.github/workflows/diff.yml +++ b/.github/workflows/diff.yml @@ -5,6 +5,8 @@ on: jobs: generate: + name: | + Generate matrix. ${{ github.event.comment.user.name }}: ${{ github.event.comment.author_association}} runs-on: ubuntu-latest outputs: diffs: ${{ steps.regress-ci.outputs.diffs }} @@ -25,7 +27,7 @@ jobs: id: regress-ci env: GITHUB_COMMENT: ${{ github.event.comment.body }} - GITHUB_COMMENT_PR: ${{ github.event.comment.issue_url }} + GITHUB_COMMENT_PR: ${{ github.event.issue.number }} diff: runs-on: ubuntu-latest needs: [generate] @@ -50,16 +52,17 @@ jobs: with: tool: cargo-semver-checks - # if a new line is added here, make sure to update the `summary` job to reference the new step index - uses: taiki-e/install-action@v2 with: tool: git-delta - - run: cargo regress diff --use-pager-directly ${{ matrix.command }} + # if a new line is added here, make sure to update the `summary` job to reference the new step index + - run: cargo regress diff ${{ matrix.command }} + - run: cargo regress diff ${{ matrix.command }} env: GH_TOKEN: ${{ github.token }} GITHUB_PR: ${{ matrix.pr }} - GIT_PAGER: delta --hunk-header-style omit + GIT_PAGER: delta --raw summary: runs-on: ubuntu-latest needs: [diff, generate] @@ -68,9 +71,8 @@ jobs: - uses: actions/checkout@v4 - run: | - PR_ID=$(echo "${{ github.event.comment.issue_url }}" | grep -o '[0-9]\+$') gh run view ${{ github.run_id }} --json jobs | \ - jq -r '"Diff for [comment]("+$comment+")\n\n" + ([.jobs[] | select(.name | startswith("diff")) | "- [" + (.name | capture("\\((?[^,]+),.*") | .name) + "](" + .url + "?pr=" + $pr_id + "#step:7:45)"] | join("\n"))' --arg pr_id "$PR_ID" --arg comment "${{ github.event.comment.url }}"| \ - gh pr comment "$PR_ID" --body "$(< /dev/stdin)" + jq -r '"Diff for [comment]("+$comment+")\n\n" + ([.jobs[] | select(.name | startswith("diff")) | "- [" + (.name | capture("\\((?[^,]+),.*") | .name) + "](" + .url + "?pr=" + $pr_id + "#step:7:47)"] | join("\n"))' --arg pr_id "${{ github.event.issue.number }}" --arg comment "${{ github.event.comment.url }}"| \ + gh pr comment "${{ github.event.issue.number }}" --body "$(< /dev/stdin)" env: GH_TOKEN: ${{ github.token }} diff --git a/Cargo.lock b/Cargo.lock index 4299f182..86748504 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1158,6 +1158,12 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "shell-words" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24188a676b6ae68c3b2cb3a01be17fbf7240ce009799bb56d5b1409051e78fde" + [[package]] name = "slab" version = "0.4.9" @@ -1248,6 +1254,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "shell-words", "svd2rust", "syn 2.0.42", "thiserror", diff --git a/Cargo.toml b/Cargo.toml index b07a8b9a..28418c58 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,4 +73,10 @@ features = ["full","extra-traits"] [workspace] members = ["ci/svd2rust-regress"] default-members = ["."] -exclude = ["output"] +exclude = [ + "output", + # workaround for https://github.com/rust-lang/cargo/pull/12779, doesn't work for output though + # see https://github.com/rust-lang/cargo/issues/6009#issuecomment-1925445245 + "output/baseline/**", + "output/current/**" +] diff --git a/ci/svd2rust-regress/Cargo.toml b/ci/svd2rust-regress/Cargo.toml index 10709734..3b882db7 100644 --- a/ci/svd2rust-regress/Cargo.toml +++ b/ci/svd2rust-regress/Cargo.toml @@ -20,3 +20,4 @@ wildmatch = "2.1.1" which = "5.0.0" tracing = "0.1.40" tracing-subscriber = { version = "0.3.18", features = ["env-filter", "fmt"] } +shell-words = "1.1" diff --git a/ci/svd2rust-regress/src/ci.rs b/ci/svd2rust-regress/src/ci.rs index e8c4d2b1..4e4494a0 100644 --- a/ci/svd2rust-regress/src/ci.rs +++ b/ci/svd2rust-regress/src/ci.rs @@ -11,7 +11,7 @@ pub struct Ci { #[clap(env = "GITHUB_COMMENT")] pub comment: String, #[clap(env = "GITHUB_COMMENT_PR")] - pub comment_pr: String, + pub comment_pr: usize, } #[derive(serde::Serialize)] @@ -32,7 +32,7 @@ impl Ci { diffs.push(Diff { needs_semver_checks: command.contains("semver"), command: command.to_owned(), - pr: self.comment_pr.split('/').last().unwrap().parse()?, + pr: self.comment_pr, }); } let json = serde_json::to_string(&diffs)?; diff --git a/ci/svd2rust-regress/src/diff.rs b/ci/svd2rust-regress/src/diff.rs index 4d3d2a81..a2e132ff 100644 --- a/ci/svd2rust-regress/src/diff.rs +++ b/ci/svd2rust-regress/src/diff.rs @@ -8,7 +8,7 @@ use crate::Opts; #[derive(clap::Parser, Debug)] #[clap(name = "diff")] pub struct Diffing { - /// The base version of svd2rust to use and the command input, defaults to latest master build + /// The base version of svd2rust to use and the command input, defaults to latest master build: `@master` /// /// Change the base version by starting with `@` followed by the source. /// @@ -16,6 +16,7 @@ pub struct Diffing { #[clap(global = true, long = "baseline", alias = "base")] pub baseline: Option, + /// The head version of svd2rust to use and the command input, defaults to current pr: `@pr` #[clap(global = true, long = "current", alias = "head")] pub current: Option, @@ -27,14 +28,12 @@ pub struct Diffing { #[clap(global = true, long)] pub form_split: bool, - #[clap(subcommand)] - pub sub: Option, - #[clap(global = true, long, short = 'c')] pub chip: Vec, /// Filter by manufacturer, case sensitive, may be combined with other filters #[clap( + global = true, short = 'm', long = "manufacturer", ignore_case = true, @@ -44,6 +43,7 @@ pub struct Diffing { /// Filter by architecture, case sensitive, may be combined with other filters #[clap( + global = true, short = 'a', long = "architecture", ignore_case = true, @@ -54,20 +54,24 @@ pub struct Diffing { #[clap(global = true, long)] pub diff_folder: Option, - #[clap(hide = true, env = "GITHUB_PR")] + /// The pr number to use for `@pr`. If not set will try to get the current pr with the command `gh pr` + #[clap(env = "GITHUB_PR", global = true, long)] pub pr: Option, - #[clap(env = "GIT_PAGER", long)] + #[clap(env = "GIT_PAGER", global = true, long)] pub pager: Option, /// if set, will use pager directly instead of `git -c core.pager` - #[clap(long, short = 'P')] + #[clap(global = true, long, short = 'P')] pub use_pager_directly: bool, /// URL for SVD to download #[clap(global = true, long)] pub url: Option, + #[clap(subcommand)] + pub sub: Option, + #[clap(last = true)] pub last_args: Option, } diff --git a/ci/svd2rust-regress/src/main.rs b/ci/svd2rust-regress/src/main.rs index a8c1d246..0e0eb09a 100644 --- a/ci/svd2rust-regress/src/main.rs +++ b/ci/svd2rust-regress/src/main.rs @@ -50,7 +50,7 @@ pub fn get_cargo_workspace() -> &'static std::path::Path { } #[derive(clap::Parser, Debug)] -pub struct TestOpts { +pub struct TestAll { /// Run a long test (it's very long) #[clap(short = 'l', long)] pub long_test: bool, @@ -59,7 +59,7 @@ pub struct TestOpts { #[clap(short = 'c', long)] pub chip: Vec, - /// Filter by manufacturer, case sensitive, may be combined with other filters + /// Filter by manufacturer, may be combined with other filters #[clap( short = 'm', long = "manufacturer", @@ -68,7 +68,7 @@ pub struct TestOpts { )] pub mfgr: Option, - /// Filter by architecture, case sensitive, may be combined with other filters + /// Filter by architecture, may be combined with other filters #[clap( short = 'a', long = "architecture", @@ -104,7 +104,97 @@ pub struct TestOpts { // TODO: Compile svd2rust? } -impl TestOpts { +#[derive(clap::Parser, Debug)] +// TODO: Replace with https://github.com/clap-rs/clap/issues/2621 when available +#[group(id = "svd_source", required = true)] +pub struct Test { + /// Enable formatting with `rustfmt` + #[arg(short = 'f', long)] + pub format: bool, + + #[arg(long)] + /// Enable splitting `lib.rs` with `form` + pub form_lib: bool, + + #[arg( + short = 'm', + long = "manufacturer", + ignore_case = true, + value_parser = manufacturers(), + )] + /// Manufacturer + pub mfgr: Option, + #[arg( + short = 'a', + long = "architecture", + ignore_case = true, + value_parser = architectures(), + )] + /// Architecture + pub arch: Option, + #[arg(long, group = "svd_source", conflicts_with_all = ["svd_file"], requires = "arch")] + /// URL to SVD file to test + pub url: Option, + #[arg(long = "svd", group = "svd_source")] + /// Path to SVD file to test + pub svd_file: Option, + #[arg(long, group = "svd_source")] + /// Chip to use, use `--url` or `--svd-file` for another way to specify svd + pub chip: Option, + + /// Path to an `svd2rust` binary, relative or absolute. + /// Defaults to `target/release/svd2rust[.exe]` of this repository + /// (which must be already built) + #[clap(short = 'p', long = "svd2rust-path", default_value = default_svd2rust())] + pub current_bin_path: PathBuf, + #[clap(last = true)] + pub command: Option, +} + +impl Test { + fn run(&self, opts: &Opts) -> Result<(), anyhow::Error> { + match self { + Self { url: Some(url), .. } => {} + Self { + svd_file: Some(svd_file), + .. + } => {} + Self { + chip: Some(chip), .. + } => {} + _ => unreachable!("clap should not allow this"), + } + let test = if let (Some(url), Some(arch)) = (&self.url, &self.arch) { + tests::TestCase { + arch: svd2rust::Target::parse(&arch)?, + mfgr: tests::Manufacturer::Unknown, + chip: self + .chip + .as_deref() + .or_else(|| url.rsplit('/').next().and_then(|s| s.strip_suffix(".svd"))) + .ok_or_else(|| { + anyhow::anyhow!( + "could not figure out chip name, specify with `--chip `", + ) + })? + .to_owned(), + svd_url: Some(url.clone()), + should_pass: true, + run_when: tests::RunWhen::default(), + } + } else { + tests::tests(Some(&opts.test_cases))? + .iter() + .find(|t| self.chip.iter().any(|c| WildMatch::new(c).matches(&t.chip))) + .ok_or_else(|| anyhow::anyhow!("no test found for chip"))? + .to_owned() + }; + test.test(opts, &self.current_bin_path, self.command.as_deref())?; + Ok(()) + } +} + +impl TestAll { fn run(&self, opt: &Opts) -> Result<(), anyhow::Error> { let tests = tests::tests(Some(&opt.test_cases))? .iter() @@ -152,7 +242,7 @@ impl TestOpts { tests.par_iter().for_each(|t| { let start = Instant::now(); - match t.test(opt, self) { + match t.test(opt, &self.current_bin_path, self.command.as_deref()) { Ok(s) => { if let Some(stderrs) = s { let mut buf = String::new(); @@ -217,7 +307,8 @@ impl TestOpts { #[derive(clap::Subcommand, Debug)] pub enum Subcommand { Diff(Diffing), - Tests(TestOpts), + Tests(TestAll), + Test(Test), Ci(Ci), } @@ -256,7 +347,8 @@ pub struct Opts { impl Opts { const fn use_rustfmt(&self) -> bool { match self.subcommand { - Subcommand::Tests(TestOpts { format, .. }) + Subcommand::Tests(TestAll { format, .. }) + | Subcommand::Test(Test { format, .. }) | Subcommand::Diff(Diffing { format, .. }) | Subcommand::Ci(Ci { format, .. }) => format, } @@ -264,7 +356,8 @@ impl Opts { const fn use_form(&self) -> bool { match self.subcommand { - Subcommand::Tests(TestOpts { form_lib, .. }) + Subcommand::Tests(TestAll { form_lib, .. }) + | Subcommand::Test(Test { form_lib, .. }) | Subcommand::Diff(Diffing { form_split: form_lib, .. @@ -278,13 +371,10 @@ impl Opts { fn default_test_cases() -> std::ffi::OsString { std::env::var_os("CARGO_MANIFEST_DIR").map_or_else( || std::ffi::OsString::from("tests.yml".to_owned()), - |mut e| { - e.extend([std::ffi::OsStr::new("/tests.yml")]); - std::path::PathBuf::from(e) - .strip_prefix(std::env::current_dir().unwrap()) - .unwrap() - .to_owned() - .into_os_string() + |path| { + let path = std::path::PathBuf::from(path); + let path = path.join("tests.yml"); + path.to_owned().into_os_string() }, ) } @@ -414,6 +504,13 @@ fn main() -> Result<(), anyhow::Error> { } Subcommand::Diff(diff) => diff.run(&opt).with_context(|| "failed to run diff"), Subcommand::Ci(ci) => ci.run(&opt).with_context(|| "failed to run ci"), + Subcommand::Test(test) => { + anyhow::ensure!( + test.current_bin_path.exists(), + "svd2rust binary does not exist" + ); + test.run(&opt).with_context(|| "failed to run test") + } } } diff --git a/ci/svd2rust-regress/src/svd_test.rs b/ci/svd2rust-regress/src/svd_test.rs index 7f0f290f..6f1e0d9f 100644 --- a/ci/svd2rust-regress/src/svd_test.rs +++ b/ci/svd2rust-regress/src/svd_test.rs @@ -1,7 +1,7 @@ use anyhow::{anyhow, Context, Result}; use svd2rust::{util::ToSanitizedCase, Target}; -use crate::{command::CommandExt, tests::TestCase, Opts, TestOpts}; +use crate::{command::CommandExt, tests::TestCase, Opts, TestAll}; use std::io::prelude::*; use std::path::PathBuf; use std::process::Command; @@ -133,18 +133,15 @@ impl CommandHelper for Command { } impl TestCase { - #[tracing::instrument(skip(self, opts, test_opts), fields(name = %self.name()))] + #[tracing::instrument(skip(self, opts), fields(name = %self.name()))] pub fn test( &self, opts: &Opts, - test_opts: &TestOpts, + bin_path: &Path, + command: Option<&str>, ) -> Result>, TestError> { let (chip_dir, mut process_stderr_paths) = self - .setup_case( - &opts.output_dir, - &test_opts.current_bin_path, - test_opts.command.as_deref(), - ) + .setup_case(&opts.output_dir, bin_path, command) .with_context(|| anyhow!("when setting up case for {}", self.name()))?; // Run `cargo check`, capturing stderr to a log file let cargo_check_err_file = path_helper_base(&chip_dir, &["cargo-check.err.log"]); @@ -262,7 +259,9 @@ impl TestCase { let mut svd2rust_bin = Command::new(svd2rust_bin_path); if let Some(command) = command { if !command.is_empty() { - svd2rust_bin.arg(command); + svd2rust_bin.args( + shell_words::split(command).context("unable to split command into args")?, + ); } } svd2rust_bin @@ -355,7 +354,6 @@ impl TestCase { process_stderr_paths.push(rustfmt_err_file); } - tracing::info!("Done processing"); Ok((chip_dir, process_stderr_paths)) } } diff --git a/ci/svd2rust-regress/src/tests.rs b/ci/svd2rust-regress/src/tests.rs index 040b4c78..f1562a5d 100644 --- a/ci/svd2rust-regress/src/tests.rs +++ b/ci/svd2rust-regress/src/tests.rs @@ -66,7 +66,7 @@ pub enum RunWhen { Never, } -#[derive(serde::Serialize, serde::Deserialize, Clone)] +#[derive(serde::Serialize, serde::Deserialize, Clone, Debug)] pub struct TestCase { pub arch: Target, pub mfgr: Manufacturer,