Browse Source

xtask: extract `Errors` type

This produces better errors in `cargo xtask public-api` because before
this change we'd print errors as display, which didn't show context.

Before:
```
Error: public API errors:
aya failed to check public API: public_api::Builder::build
aya-obj failed to check public API: public_api::Builder::build
aya-log failed to check public API: public_api::Builder::build
aya-log-common failed to check public API: public_api::Builder::build
aya-log-parser failed to check public API: public_api::Builder::build
aya-tool failed to check public API: public_api::Builder::build
aya-bpf failed to check public API: public_api::Builder::build
aya-bpf-bindings failed to check public API: public_api::Builder::build
aya-bpf-cty failed to check public API: public_api::Builder::build
aya-bpf-macros failed to check public API: public_api::Builder::build
aya-log-ebpf-macros failed to check public API: public_api::Builder::build
```

After:
```
Error: aya failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `struct`, `struct_field`, `union`, `enum`, `variant`, `function`, `typedef`, `opaque_ty`, `constant`, `trait`, `trait_alias`, `impl`, `static`, `foreign_type`, `macro`, `proc_attribute`, `proc_derive`, `assoc_const`, `assoc_type`, `primitive`, `keyword` at line 1 column 3082463
aya-obj failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `union`, `struct`, `struct_field`, `enum`, `variant`, `function`, `trait`, `trait_alias`, `impl`, `typedef`, `opaque_ty`, `constant`, `static`, `foreign_type`, `macro`, `proc_macro`, `primitive`, `assoc_const`, `assoc_type` at line 1 column 129355
aya-log failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `struct`, `struct_field`, `union`, `enum`, `variant`, `function`, `typedef`, `opaque_ty`, `constant`, `trait`, `trait_alias`, `impl`, `static`, `foreign_type`, `macro`, `proc_attribute`, `proc_derive`, `assoc_const`, `assoc_type`, `primitive`, `keyword` at line 1 column 311910
aya-log-common failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `union`, `struct`, `struct_field`, `enum`, `variant`, `function`, `trait`, `trait_alias`, `impl`, `typedef`, `opaque_ty`, `constant`, `static`, `foreign_type`, `macro`, `proc_macro`, `primitive`, `assoc_const`, `assoc_type` at line 1 column 130456
aya-log-parser failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `struct`, `struct_field`, `union`, `enum`, `variant`, `function`, `typedef`, `opaque_ty`, `constant`, `trait`, `trait_alias`, `impl`, `static`, `foreign_type`, `macro`, `proc_attribute`, `proc_derive`, `assoc_const`, `assoc_type`, `primitive`, `keyword` at line 1 column 204930
aya-tool failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `struct`, `struct_field`, `union`, `enum`, `variant`, `function`, `typedef`, `opaque_ty`, `constant`, `trait`, `trait_alias`, `impl`, `static`, `foreign_type`, `macro`, `proc_attribute`, `proc_derive`, `assoc_const`, `assoc_type`, `primitive`, `keyword` at line 1 column 234519
aya-bpf failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `struct`, `struct_field`, `union`, `enum`, `variant`, `function`, `typedef`, `opaque_ty`, `constant`, `trait`, `trait_alias`, `impl`, `static`, `foreign_type`, `macro`, `proc_attribute`, `proc_derive`, `assoc_const`, `assoc_type`, `primitive`, `keyword` at line 1 column 741697
aya-bpf-bindings failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `union`, `struct`, `struct_field`, `enum`, `variant`, `function`, `trait`, `trait_alias`, `impl`, `typedef`, `opaque_ty`, `constant`, `static`, `foreign_type`, `macro`, `proc_macro`, `primitive`, `assoc_const`, `assoc_type` at line 1 column 112445
aya-bpf-cty failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `union`, `struct`, `struct_field`, `enum`, `variant`, `function`, `trait`, `trait_alias`, `impl`, `typedef`, `opaque_ty`, `constant`, `static`, `foreign_type`, `macro`, `proc_macro`, `primitive`, `assoc_const`, `assoc_type` at line 1 column 315
aya-bpf-macros failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `struct`, `struct_field`, `union`, `enum`, `variant`, `function`, `typedef`, `opaque_ty`, `constant`, `trait`, `trait_alias`, `impl`, `static`, `foreign_type`, `macro`, `proc_attribute`, `proc_derive`, `assoc_const`, `assoc_type`, `primitive`, `keyword` at line 1 column 186226
aya-log-ebpf-macros failed to check public API

Caused by:
    0: public_api::Builder::build
    1: unknown variant `type_alias`, expected one of `module`, `extern_crate`, `import`, `struct`, `struct_field`, `union`, `enum`, `variant`, `function`, `typedef`, `opaque_ty`, `constant`, `trait`, `trait_alias`, `impl`, `static`, `foreign_type`, `macro`, `proc_attribute`, `proc_derive`, `assoc_const`, `assoc_type`, `primitive`, `keyword` at line 1 column 158055
```
Tamir Duberstein 1 year ago
parent
commit
782eb85d4a
3 changed files with 55 additions and 43 deletions
  1. 27 0
      xtask/src/lib.rs
  2. 26 23
      xtask/src/public_api.rs
  3. 2 20
      xtask/src/run.rs

+ 27 - 0
xtask/src/lib.rs

@@ -13,3 +13,30 @@ pub fn exec(cmd: &mut Command) -> Result<()> {
     }
     Ok(())
 }
+
+#[derive(Debug)]
+pub struct Errors<E>(Vec<E>);
+
+impl<E> Errors<E> {
+    pub fn new(errors: Vec<E>) -> Self {
+        Self(errors)
+    }
+}
+
+impl<E> std::fmt::Display for Errors<E>
+where
+    E: std::fmt::Debug,
+{
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let Self(errors) = self;
+        for (i, error) in errors.iter().enumerate() {
+            if i != 0 {
+                writeln!(f)?;
+            }
+            write!(f, "{:?}", error)?;
+        }
+        Ok(())
+    }
+}
+
+impl<E> std::error::Error for Errors<E> where E: std::fmt::Debug {}

+ 26 - 23
xtask/src/public_api.rs

@@ -10,6 +10,7 @@ use cargo_metadata::{Metadata, Package};
 use clap::Parser;
 use dialoguer::{theme::ColorfulTheme, Confirm};
 use diff::{lines, Result as Diff};
+use xtask::Errors;
 
 #[derive(Debug, Parser)]
 pub struct Options {
@@ -37,34 +38,36 @@ pub fn public_api(options: Options, metadata: Metadata) -> Result<()> {
         workspace_root,
         packages,
         ..
-    } = &metadata;
+    } = metadata;
 
-    let errors = packages
-        .iter()
-        .fold(String::new(), |mut buf, Package { name, publish, .. }| {
-            if !matches!(publish, Some(publish) if publish.is_empty()) {
-                match check_package_api(name, toolchain, bless, workspace_root.as_std_path()) {
-                    Ok(diff) => {
-                        if !diff.is_empty() {
-                            writeln!(
-                                &mut buf,
-                                "{name} public API changed; re-run with --bless. diff:\n{diff}"
-                            )
-                            .unwrap();
-                        }
-                    }
-                    Err(err) => {
-                        writeln!(&mut buf, "{name} failed to check public API: {err}").unwrap();
-                    }
+    let errors: Vec<_> = packages
+        .into_iter()
+        .map(|Package { name, publish, .. }| {
+            if matches!(publish, Some(publish) if publish.is_empty()) {
+                Ok(())
+            } else {
+                let diff = check_package_api(&name, toolchain, bless, workspace_root.as_std_path())
+                    .with_context(|| format!("{name} failed to check public API"))?;
+                if diff.is_empty() {
+                    Ok(())
+                } else {
+                    Err(anyhow::anyhow!(
+                        "{name} public API changed; re-run with --bless. diff:\n{diff}"
+                    ))
                 }
             }
-            buf
-        });
+        })
+        .filter_map(|result| match result {
+            Ok(()) => None,
+            Err(err) => Some(err),
+        })
+        .collect();
 
-    if !errors.is_empty() {
-        bail!("public API errors:\n{errors}")
+    if errors.is_empty() {
+        Ok(())
+    } else {
+        Err(Errors::new(errors).into())
     }
-    Ok(())
 }
 
 fn check_package_api(

+ 2 - 20
xtask/src/run.rs

@@ -13,7 +13,7 @@ use std::{
 use anyhow::{anyhow, bail, Context as _, Result};
 use cargo_metadata::{Artifact, CompilerMessage, Message, Target};
 use clap::Parser;
-use xtask::{exec, AYA_BUILD_INTEGRATION_BPF};
+use xtask::{exec, Errors, AYA_BUILD_INTEGRATION_BPF};
 
 #[derive(Parser)]
 enum Environment {
@@ -104,24 +104,6 @@ where
     Ok(executables)
 }
 
-#[derive(Debug)]
-struct Errors(Vec<anyhow::Error>);
-
-impl std::fmt::Display for Errors {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let Self(errors) = self;
-        for (i, error) in errors.iter().enumerate() {
-            if i != 0 {
-                writeln!(f)?;
-            }
-            write!(f, "{:?}", error)?;
-        }
-        Ok(())
-    }
-}
-
-impl std::error::Error for Errors {}
-
 /// Build and run the project.
 pub fn run(opts: Options) -> Result<()> {
     let Options {
@@ -547,7 +529,7 @@ pub fn run(opts: Options) -> Result<()> {
             if errors.is_empty() {
                 Ok(())
             } else {
-                Err(Errors(errors).into())
+                Err(Errors::new(errors).into())
             }
         }
     }