feat(logging): use tracing format layer + env filter

The comment here was resolved upstream in September 2021, so we can now apply a filter directly to a layer. This causes `WARN` (or more severe) logs to be written to stderr.

Author: arxanas

git-branchless-lib/src/core/effects.rs

@@ -1,12 +1,13 @@
 //! Wrappers around various side effects.
 
+use bstr::ByteSlice;
 use std::convert::TryInto;
 use std::fmt::{Debug, Write};
 use std::io::{stderr, stdout, Stderr, Stdout, Write as WriteIo};
 use std::mem::take;
 use std::sync::{Arc, Mutex, RwLock};
-use std::thread;
 use std::time::{Duration, Instant};
+use std::{io, thread};
 
 use indicatif::{MultiProgress, ProgressBar, ProgressDrawTarget, ProgressStyle};
 use itertools::Itertools;
@@ -919,7 +920,7 @@ impl Write for ErrorStream {
         match &self.dest {
             OutputDest::Stdout => {
                 self.buffer.push_str(s);
-                self.flush();
+                WriteProgress::flush(self);
             }
 
             OutputDest::Suppress => {
@@ -935,6 +936,33 @@ impl Write for ErrorStream {
     }
 }
 
+/// You probably don't want this. This implementation is only for `tracing`'s `fmt_layer`, because
+/// it needs a writer of type `io::Write`, but `Effects` normally uses its implementation of
+/// `fmt::Write`.
+impl io::Write for ErrorStream {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        match &self.dest {
+            OutputDest::Stdout => {
+                self.buffer.push_str(buf.to_str_lossy().as_ref());
+                Ok(buf.len())
+            }
+            OutputDest::Suppress => {
+                // Do nothing.
+                Ok(buf.len())
+            }
+            OutputDest::BufferForTest { stdout: _, stderr } => {
+                let mut buffer = stderr.lock().unwrap();
+                buffer.write(buf)
+            }
+        }
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        WriteProgress::flush(self);
+        Ok(())
+    }
+}
+
 impl Drop for ErrorStream {
     fn drop(&mut self) {
         WriteProgress::drop(self);

git-branchless/src/commands/mod.rs

@@ -37,6 +37,7 @@ use lib::core::rewrite::MergeConflictRemediation;
 use lib::git::Repo;
 use lib::git::RepoError;
 use lib::util::ExitCode;
+use tracing::level_filters::LevelFilter;
 use tracing_chrome::ChromeLayerBuilder;
 use tracing_error::ErrorLayer;
 use tracing_subscriber::fmt as tracing_fmt;
@@ -94,8 +95,6 @@ fn rewrite_args(args: Vec<OsString>) -> Vec<OsString> {
 /// Wrapper function for `main` to ensure that `Drop` is called for local
 /// variables, since `std::process::exit` will skip them.
 fn do_main_and_drop_locals() -> eyre::Result<i32> {
-    let _tracing_guard = install_tracing();
-
     let args = rewrite_args(std::env::args_os().collect_vec());
     let Opts {
         working_directory,
@@ -126,6 +125,8 @@ fn do_main_and_drop_locals() -> eyre::Result<i32> {
     };
     let effects = Effects::new(color);
 
+    let _tracing_guard = install_tracing(effects.clone());
+
     if let Some(ExitCode(exit_code)) = check_unsupported_config_options(&effects)? {
         let exit_code: i32 = exit_code.try_into()?;
         return Ok(exit_code);
@@ -406,27 +407,11 @@ pub fn main() {
 }
 
 #[must_use = "This function returns a guard object to flush traces. Dropping it immediately is probably incorrect. Make sure that the returned value lives until tracing has finished."]
-fn install_tracing() -> eyre::Result<impl Drop> {
-    let (filter_layer, fmt_layer) = match EnvFilter::try_from_default_env() {
-        Ok(filter_layer) => {
-            let fmt_layer = tracing_fmt::layer()
-                .with_span_events(tracing_fmt::format::FmtSpan::CLOSE)
-                .with_target(false);
-            (Some(filter_layer), Some(fmt_layer))
-        }
-        Err(_) => {
-            // We would like the filter layer to apply *only* to the formatting
-            // layer. That way, the logging output is suppressed, but we still
-            // get spantraces for use with `color-eyre`. However, it's currently
-            // not possible (?), at least not without writing some a custom
-            // subscriber. See https://github.com/tokio-rs/tracing/pull/1523
-            //
-            // The workaround is to only display logging messages if `RUST_LOG`
-            // is set (which is unfortunate, because we'll miss out on
-            // `WARN`-level messages by default).
-            (None, None)
-        }
-    };
+fn install_tracing(effects: Effects) -> eyre::Result<impl Drop> {
+    let env_filter = EnvFilter::builder()
+        .with_default_directive(LevelFilter::WARN.into())
+        .from_env_lossy();
+    let fmt_layer = tracing_fmt::layer().with_writer(move || effects.clone().get_error_stream());
 
     let (profile_layer, flush_guard): (_, Box<dyn Any>) = {
         // We may invoke a hook that calls back into `git-branchless`. In that case,
@@ -475,8 +460,7 @@ fn install_tracing() -> eyre::Result<impl Drop> {
 
     tracing_subscriber::registry()
         .with(ErrorLayer::default())
-        .with(filter_layer)
-        .with(fmt_layer)
+        .with(fmt_layer.with_filter(env_filter))
         .with(profile_layer)
         .try_init()?;
 

git-branchless/src/commands/reword.rs

@@ -8,7 +8,6 @@ use std::collections::{HashMap, HashSet};
 use std::convert::TryFrom;
 use std::fmt::Write;
 use std::fs::File;
-use std::io::Write as OtherWrite;
 use std::time::SystemTime;
 
 use bstr::{ByteSlice, ByteVec};
@@ -500,6 +499,7 @@ fn prepare_messages(
 
         let mut w = File::create(repo.get_path().join("REWORD_EDITMSG"))
             .context("Creating REWORD_EDITMSG file")?;
+        use std::io::Write;
         writeln!(
             &mut w,
             "{} This file was created by `git branchless reword` at {}\n\