don't clobber a previously set level filter

There's a bit of a race with the `log` API for initializing because
it covers two atomic values. It's possible to observe a logger and
attempt to use it before the max level is set. This change doesn't
really make that race any racier than it was already, it just prevents
the surprising behaviour of clobbering a previously set max level
filter even if the logger itself isn't set.

Author: KodrAus

Cargo.toml

@@ -33,5 +33,9 @@ harness = false
 name = "log-in-log"
 harness = false
 
+[[test]]
+name = "init-twice-retains-filter"
+harness = false
+
 [features]
 default = ["termcolor", "atty", "humantime", "regex"]

src/lib.rs

@@ -646,8 +646,14 @@ impl Builder {
     pub fn try_init(&mut self) -> Result<(), SetLoggerError> {
         let logger = self.build();
 
-        log::set_max_level(logger.filter());
-        log::set_boxed_logger(Box::new(logger))
+        let max_level = logger.filter();
+        let r = log::set_boxed_logger(Box::new(logger));
+
+        if r.is_ok() {
+            log::set_max_level(max_level);
+        }
+
+        r
     }
 
     /// Initializes the global logger with the built env logger.

tests/init-twice-retains-filter.rs

@@ -0,0 +1,40 @@
+extern crate log;
+extern crate env_logger;
+
+use std::process;
+use std::env;
+use std::str;
+
+fn main() {
+    if env::var("YOU_ARE_TESTING_NOW").is_ok() {
+        // Init from the env (which should set the max level to `Debug`)
+        env_logger::init();
+
+        assert_eq!(log::LevelFilter::Debug, log::max_level());
+
+        // Init again using a different max level
+        // This shouldn't clobber the level that was previously set
+        env_logger::Builder::new()
+            .parse("info")
+            .try_init()
+            .unwrap_err();
+
+        assert_eq!(log::LevelFilter::Debug, log::max_level());
+        return
+    }
+
+    let exe = env::current_exe().unwrap();
+    let out = process::Command::new(exe)
+        .env("YOU_ARE_TESTING_NOW", "1")
+        .env("RUST_LOG", "debug")
+        .output()
+        .unwrap_or_else(|e| panic!("Unable to start child process: {}", e));
+    if out.status.success() {
+        return
+    }
+
+    println!("test failed: {}", out.status);
+    println!("--- stdout\n{}", str::from_utf8(&out.stdout).unwrap());
+    println!("--- stderr\n{}", str::from_utf8(&out.stderr).unwrap());
+    process::exit(1);
+}