From 7a73ae7cc0656956b13917e5458b117161401b4d Mon Sep 17 00:00:00 2001
From: Antoine Gersant <antoine.gersant@lesforges.org>
Date: Sat, 2 Jan 2021 16:03:51 -0800
Subject: [PATCH] Don't emit log file when running in foreground (-f on Linux,
 polaris-cli.exe on Windows) and --log is not set

---
 src/main.rs               | 36 +++++++++++++++++++++---------------
 src/options.rs            |  3 ++-
 src/paths.rs              | 19 ++++++++++++-------
 src/service/actix/test.rs |  2 +-
 4 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index 5fb2add..2495d1d 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -8,9 +8,9 @@ extern crate diesel_migrations;
 
 use anyhow::*;
 use log::info;
-use simplelog::{CombinedLogger, LevelFilter, TermLogger, TerminalMode, WriteLogger};
+use simplelog::{CombinedLogger, LevelFilter, SharedLogger, TermLogger, TerminalMode, WriteLogger};
 use std::fs;
-use std::path::PathBuf;
+use std::path::Path;
 
 mod app;
 mod db;
@@ -23,15 +23,15 @@ mod ui;
 mod utils;
 
 #[cfg(unix)]
-fn daemonize(foreground: bool, pid_file_path: &PathBuf) -> Result<()> {
+fn daemonize<T: AsRef<Path>>(foreground: bool, pid_file_path: T) -> Result<()> {
 	if foreground {
 		return Ok(());
 	}
-	if let Some(parent) = pid_file_path.parent() {
+	if let Some(parent) = pid_file_path.as_ref().parent() {
 		fs::create_dir_all(parent)?;
 	}
 	let daemonize = daemonize::Daemonize::new()
-		.pid_file(pid_file_path)
+		.pid_file(pid_file_path.as_ref())
 		.working_directory(".");
 	daemonize.start()?;
 	Ok(())
@@ -45,23 +45,29 @@ fn notify_ready() -> Result<()> {
 	Ok(())
 }
 
-fn init_logging(log_level: LevelFilter, log_file_path: &PathBuf) -> Result<()> {
+fn init_logging<T: AsRef<Path>>(log_level: LevelFilter, log_file_path: &Option<T>) -> Result<()> {
 	let log_config = simplelog::ConfigBuilder::new()
 		.set_location_level(LevelFilter::Error)
 		.build();
 
-	if let Some(parent) = log_file_path.parent() {
-		fs::create_dir_all(parent)?;
-	}
+	let mut loggers: Vec<Box<dyn SharedLogger>> = vec![TermLogger::new(
+		log_level,
+		log_config.clone(),
+		TerminalMode::Mixed,
+	)];
 
-	CombinedLogger::init(vec![
-		TermLogger::new(log_level, log_config.clone(), TerminalMode::Mixed),
-		WriteLogger::new(
+	if let Some(path) = log_file_path {
+		if let Some(parent) = path.as_ref().parent() {
+			fs::create_dir_all(parent)?;
+		}
+		loggers.push(WriteLogger::new(
 			log_level,
 			log_config.clone(),
-			fs::File::create(log_file_path)?,
-		),
-	])?;
+			fs::File::create(path)?,
+		));
+	}
+
+	CombinedLogger::init(loggers)?;
 
 	Ok(())
 }
diff --git a/src/options.rs b/src/options.rs
index 339338f..f3bc210 100644
--- a/src/options.rs
+++ b/src/options.rs
@@ -4,7 +4,6 @@ use std::path::PathBuf;
 
 pub struct CLIOptions {
 	pub show_help: bool,
-	#[cfg(unix)]
 	pub foreground: bool,
 	pub log_file_path: Option<PathBuf>,
 	#[cfg(unix)]
@@ -36,6 +35,8 @@ impl Manager {
 			show_help: matches.opt_present("h"),
 			#[cfg(unix)]
 			foreground: matches.opt_present("f"),
+			#[cfg(windows)]
+			foreground: !cfg!(feature = "ui"),
 			log_file_path: matches.opt_str("log").map(PathBuf::from),
 			#[cfg(unix)]
 			pid_file_path: matches.opt_str("pid").map(PathBuf::from),
diff --git a/src/paths.rs b/src/paths.rs
index d30ecb1..17f86b4 100644
--- a/src/paths.rs
+++ b/src/paths.rs
@@ -6,7 +6,7 @@ pub struct Paths {
 	pub cache_dir_path: PathBuf,
 	pub config_file_path: Option<PathBuf>,
 	pub db_file_path: PathBuf,
-	pub log_file_path: PathBuf,
+	pub log_file_path: Option<PathBuf>,
 	#[cfg(unix)]
 	pub pid_file_path: PathBuf,
 	pub swagger_dir_path: PathBuf,
@@ -22,7 +22,7 @@ impl Default for Paths {
 			cache_dir_path: ["."].iter().collect(),
 			config_file_path: None,
 			db_file_path: [".", "db.sqlite"].iter().collect(),
-			log_file_path: [".", "polaris.log"].iter().collect(),
+			log_file_path: Some([".", "polaris.log"].iter().collect()),
 			pid_file_path: [".", "polaris.pid"].iter().collect(),
 			swagger_dir_path: [".", "docs", "swagger"].iter().collect(),
 			web_dir_path: [".", "web"].iter().collect(),
@@ -40,7 +40,7 @@ impl Default for Paths {
 			cache_dir_path: install_directory.clone(),
 			config_file_path: None,
 			db_file_path: install_directory.join("db.sqlite"),
-			log_file_path: install_directory.join("polaris.log"),
+			log_file_path: Some(install_directory.join("polaris.log")),
 			swagger_dir_path: install_directory.join("swagger"),
 			web_dir_path: install_directory.join("web"),
 		}
@@ -62,7 +62,7 @@ impl Paths {
 			log_file_path: option_env!("POLARIS_LOG_DIR")
 				.map(PathBuf::from)
 				.map(|p| p.join("polaris.log"))
-				.unwrap_or(defaults.log_file_path),
+				.or(defaults.log_file_path),
 			#[cfg(unix)]
 			pid_file_path: option_env!("POLARIS_PID_DIR")
 				.map(PathBuf::from)
@@ -88,9 +88,6 @@ impl Paths {
 		if let Some(path) = &cli_options.database_file_path {
 			paths.db_file_path = path.clone();
 		}
-		if let Some(path) = &cli_options.log_file_path {
-			paths.log_file_path = path.clone();
-		}
 		#[cfg(unix)]
 		if let Some(path) = &cli_options.pid_file_path {
 			paths.pid_file_path = path.clone();
@@ -101,6 +98,14 @@ impl Paths {
 		if let Some(path) = &cli_options.web_dir_path {
 			paths.web_dir_path = path.clone();
 		}
+
+		let log_to_file = cli_options.log_file_path.is_some() || !cli_options.foreground;
+		if log_to_file {
+			paths.log_file_path = cli_options.log_file_path.clone().or(paths.log_file_path);
+		} else {
+			paths.log_file_path = None;
+		};
+
 		return paths;
 	}
 }
diff --git a/src/service/actix/test.rs b/src/service/actix/test.rs
index e832dfd..d4c218f 100644
--- a/src/service/actix/test.rs
+++ b/src/service/actix/test.rs
@@ -85,7 +85,7 @@ impl TestService for ActixTestService {
 			db_file_path: output_dir.join("db.sqlite"),
 			#[cfg(unix)]
 			pid_file_path: output_dir.join("polaris.pid"),
-			log_file_path: output_dir.join("polaris.log"),
+			log_file_path: None,
 			swagger_dir_path: ["docs", "swagger"].iter().collect(),
 			web_dir_path: ["test-data", "web"].iter().collect(),
 		};