Log errors by default

Remove imperative `clap::Error::exit`.

Author: tamird

src/bin/bpf-linker.rs

@@ -6,10 +6,9 @@ extern crate aya_rustc_llvm_proxy;
 use std::{env, fs, io, path::PathBuf, str::FromStr};
 
 use bpf_linker::{Cpu, Linker, LinkerOptions, OptLevel, OutputType};
-use clap::Parser;
+use clap::{CommandFactory as _, Parser};
 use thiserror::Error;
-use tracing::{info, Level, Subscriber};
-use tracing_appender::non_blocking::WorkerGuard;
+use tracing::{info, Level};
 use tracing_subscriber::{fmt::MakeWriter, prelude::*, EnvFilter};
 use tracing_tree::HierarchicalLayer;
 
@@ -163,6 +162,12 @@ where
 }
 
 fn main() {
+    if let Err(e) = try_main() {
+        e.exit() // Needed for proper formatting.
+    }
+}
+
+fn try_main() -> clap::error::Result<()> {
     let args = env::args().map(|arg| {
         if arg == "-flavor" {
             "--flavor".to_string()
@@ -192,47 +197,48 @@ fn main() {
         export,
         fatal_errors,
         _debug,
-    } = Parser::parse_from(args);
+    } = Parser::try_parse_from(args)?;
+    let mut cmd = CommandLine::command();
 
     if inputs.is_empty() {
-        error("no input files", clap::error::ErrorKind::TooFewValues);
+        return Err(cmd.error(clap::error::ErrorKind::TooFewValues, "no input files"));
     }
 
     // Configure tracing.
-    if let Some(log_level) = log_level {
-        let subscriber_registry = tracing_subscriber::registry()
-            .with(EnvFilter::from_default_env().add_directive(log_level.into()));
-        let (subscriber, _guard): (
-            Box<dyn Subscriber + Send + Sync + 'static>,
-            Option<WorkerGuard>,
-        ) = match log_file {
+    let _guard = {
+        let filter = EnvFilter::from_default_env();
+        let filter = match log_level {
+            None => filter,
+            Some(log_level) => filter.add_directive(log_level.into()),
+        };
+        let subscriber_registry = tracing_subscriber::registry().with(filter);
+        match log_file {
             Some(log_file) => {
-                let file_appender = tracing_appender::rolling::never(
-                    log_file.parent().unwrap_or_else(|| {
-                        error("invalid log_file", clap::error::ErrorKind::InvalidValue)
-                    }),
-                    log_file.file_name().unwrap_or_else(|| {
-                        error("invalid log_file", clap::error::ErrorKind::InvalidValue)
-                    }),
-                );
-                let (non_blocking, _guard) = tracing_appender::non_blocking(file_appender);
+                let mut comps = log_file.components();
+                let file_name = comps
+                    .next_back()
+                    .and_then(|p| match p {
+                        std::path::Component::Normal(p) => Some(p),
+                        _ => None,
+                    })
+                    .ok_or_else(|| {
+                        cmd.error(clap::error::ErrorKind::InvalidValue, "invalid log_file")
+                    })?;
+                let parent = comps.as_path();
+                let file_appender = tracing_appender::rolling::never(parent, file_name);
+                let (non_blocking, guard) = tracing_appender::non_blocking(file_appender);
                 let subscriber = subscriber_registry
                     .with(tracing_layer(io::stdout))
                     .with(tracing_layer(non_blocking));
-
-                println!("Logging to stdout");
-                (Box::new(subscriber), Some(_guard))
+                tracing::subscriber::set_global_default(subscriber).map(|()| Some(guard))
             }
             None => {
                 let subscriber = subscriber_registry.with(tracing_layer(io::stderr));
-                println!("Logging to stderr");
-                (Box::new(subscriber), None)
+                tracing::subscriber::set_global_default(subscriber).map(|()| None)
             }
-        };
-        if let Err(e) = tracing::subscriber::set_global_default(subscriber) {
-            error(&e.to_string(), clap::error::ErrorKind::Format);
         }
     }
+    .map_err(|e| cmd.error(clap::error::ErrorKind::Format, e.to_string()))?;
 
     info!(
         "command line: {:?}",
@@ -242,9 +248,7 @@ fn main() {
     let export_symbols = export_symbols
         .map(fs::read_to_string)
         .transpose()
-        .unwrap_or_else(|e| {
-            error(&e.to_string(), clap::error::ErrorKind::Io);
-        });
+        .map_err(|e| cmd.error(clap::error::ErrorKind::Io, e.to_string()))?;
 
     // TODO: the data is owned by this call frame; we could make this zero-alloc.
     let export_symbols = export_symbols
@@ -284,23 +288,18 @@ fn main() {
         btf,
     });
 
-    if let Err(e) = linker.link() {
-        error(
-            &format!("{}; has_errors={}", e, linker.has_errors()),
-            clap::error::ErrorKind::Io,
-        );
-    }
+    linker
+        .link()
+        .map_err(|e| cmd.error(clap::error::ErrorKind::Io, e.to_string()))?;
 
     if fatal_errors && linker.has_errors() {
-        error(
-            "LLVM issued diagnostic with error severity",
+        return Err(cmd.error(
             clap::error::ErrorKind::Io,
-        );
+            "LLVM issued diagnostic with error severity",
+        ));
     }
-}
 
-fn error(desc: &str, kind: clap::error::ErrorKind) -> ! {
-    clap::Error::raw(kind, desc.to_string()).exit();
+    Ok(())
 }
 
 #[cfg(test)]

src/linker.rs

@@ -569,7 +569,6 @@ impl llvm::LLVMDiagnosticHandler for Linker {
         match severity {
             llvm_sys::LLVMDiagnosticSeverity::LLVMDSError => {
                 if MATCHERS.iter().any(|matcher| message.ends_with(matcher)) {
-                    warn!("llvm: {}", message);
                     return;
                 }
                 self.has_errors = true;