update rust logger with feedback

src/bins/src/update.rs

@@ -133,7 +133,7 @@ fn main() -> Result<()> {
 
     let verbose = get_flag_or_false(&matches, "verbose");
     let log_file = matches.get_one("log");
-    let desired_log_file = log_file.cloned().unwrap_or(default_logfile_from_context(LocationContext::IAmUpdateExe));
+    let desired_log_file = log_file.cloned().unwrap_or(default_logfile_path(LocationContext::IAmUpdateExe));
     init_logging("update", Some(&desired_log_file), true, verbose);
 
     // change working directory to the parent directory of the exe

src/lib-cpp/src/lib.rs

@@ -16,8 +16,8 @@ use libc::{c_char, c_void, size_t};
 use log_derive::{logfn, logfn_inputs};
 use std::{ffi::CString, ptr};
 use velopack::{sources, ApplyWaitMode, Error as VelopackError, UpdateCheck, UpdateManager, VelopackApp};
-use velopack::locator::{LocationContext};
-use velopack::logging::{default_logfile_from_config, default_logfile_from_context, init_logging};
+use velopack::locator::LocationContext;
+use velopack::logging::{default_logfile_path, init_logging};
 
 /// Create a new FileSource update source for a given file path.
 #[no_mangle]
@@ -442,9 +442,9 @@ pub extern "C" fn vpkc_app_run(p_user_data: *mut c_void) {
     
     // init logging
     let log_file = if let Some(locator) = &app_options.locator {
-        default_logfile_from_config(locator)
+        default_logfile_path(locator)
     } else {
-        default_logfile_from_context(LocationContext::FromCurrentExe)
+        default_logfile_path(LocationContext::FromCurrentExe)
     };
     
     init_logging("lib-cpp", Some(&log_file), false, false);

src/lib-nodejs/velopack_nodeffi/src/lib.rs

@@ -7,7 +7,6 @@ use std::sync::{Arc, Mutex};
 use std::thread;
 use velopack::sources::*;
 use velopack::*;
-use velopack::logging::{default_logfile_from_config, default_logfile_from_context};
 
 struct UpdateManagerWrapper {
     manager: UpdateManager,
@@ -271,9 +270,9 @@ fn js_appbuilder_run(mut cx: FunctionContext) -> JsResult<JsUndefined> {
 
     // init logging
     let log_file = if let Some(locator) = &locator {
-        default_logfile_from_config(locator)
+        logging::default_logfile_path(locator)
     } else {
-        default_logfile_from_context(LocationContext::FromCurrentExe)
+        logging::default_logfile_path(LocationContext::FromCurrentExe)
     };
 
     logging::init_logging("lib-nodejs", Some(&log_file), false, false);

src/lib-rust/src/locator.rs

@@ -89,6 +89,27 @@ pub struct VelopackLocator {
     manifest: Manifest,
 }
 
+impl TryFrom<VelopackLocatorConfig> for VelopackLocator {
+    type Error = Error;
+    fn try_from(config: VelopackLocatorConfig) -> Result<Self, Self::Error> {
+        VelopackLocator::new(&config)
+    }
+}
+
+impl TryFrom<&VelopackLocatorConfig> for VelopackLocator {
+    type Error = Error;
+    fn try_from(config: &VelopackLocatorConfig) -> Result<Self, Self::Error> {
+        VelopackLocator::new(config)
+    }
+}
+
+impl TryFrom<LocationContext> for VelopackLocator {
+    type Error = Error;
+    fn try_from(context: LocationContext) -> Result<Self, Self::Error> {
+        auto_locate_app_manifest(context)
+    }
+}
+
 impl VelopackLocator {
     /// Creates a new VelopackLocator from the given paths, trying to auto-detect the manifest.
     pub fn new(config: &VelopackLocatorConfig) -> Result<VelopackLocator, Error>

src/lib-rust/src/logging.rs

@@ -1,23 +1,29 @@
-#![allow(unused_variables)]
-
-use crate::locator::{LocationContext, VelopackLocator, VelopackLocatorConfig};
+use crate::locator::VelopackLocator;
 use std::path::PathBuf;
 
-#[cfg(not(target_os = "windows"))]
-use std::path::Path;
-
 #[cfg(feature = "file-logging")]
 use simplelog::*;
 #[cfg(feature = "file-logging")]
 use time::format_description::{modifier, Component, FormatItem};
 
+static LOGGING_FILE_NAME: &str = "velopack.log";
+
 #[cfg(target_os = "linux")]
-fn default_file_linux() -> PathBuf {
-    Path::new("/tmp/velopack.log").to_path_buf()
+fn default_file_linux<L: TryInto<VelopackLocator>>(locator: L) -> PathBuf {
+    let file_name = match locator.try_into() {
+        Ok(locator) => format!("velopack_{}.log", locator.get_manifest_id()),
+        Err(_) => LOGGING_FILE_NAME.to_string(),
+    };
+    std::env::temp_dir().join(file_name)
 }
 
 #[cfg(target_os = "macos")]
-fn default_file_macos() -> PathBuf {
+fn default_file_macos<L: TryInto<VelopackLocator>>(locator: L) -> PathBuf {
+    let file_name = match locator.try_into() {
+        Ok(locator) => format!("velopack_{}.log", locator.get_manifest_id()),
+        Err(_) => LOGGING_FILE_NAME.to_string(),
+    };
+
     #[allow(deprecated)]
     let user_home = std::env::home_dir();
 
@@ -27,81 +33,42 @@ fn default_file_macos() -> PathBuf {
         lib_logs.push("Logs");
 
         if lib_logs.exists() {
-            lib_logs.push("velopack.log");
+            lib_logs.push(file_name);
             return lib_logs;
         }
     }
 
-    return Path::new("/tmp/velopack.log").to_path_buf();
+    std::env::temp_dir().join(file_name)
 }
 
-/// Default log location for Velopack on the current OS.
-#[allow(unused_variables)]
-pub fn default_logfile_from_locator(locator: VelopackLocator) -> PathBuf {
-    #[cfg(target_os = "windows")]
-    {
-        return locator.get_root_dir().join("Velopack.log");
-    }
-    #[cfg(target_os = "linux")]
-    {
-        return default_file_linux();
-    }
-    #[cfg(target_os = "macos")]
-    {
-        return default_file_macos();
-    }
-}
-
-/// Default log location for Velopack on the current OS.
-pub fn default_logfile_from_config(config: &VelopackLocatorConfig) -> PathBuf {
-    #[cfg(target_os = "windows")]
-    {
-        match VelopackLocator::new(config) {
-            Ok(locator) => {
-                return default_logfile_from_locator(locator);
+#[cfg(target_os = "windows")]
+fn default_file_windows<L: TryInto<VelopackLocator>>(locator: L) -> PathBuf {
+    match locator.try_into() {
+        Ok(locator) => {
+            let mut log_dir = locator.get_root_dir();
+            log_dir.push(LOGGING_FILE_NAME);
+            match std::fs::OpenOptions::new().write(true).create(true).open(&log_dir) {
+                Ok(_) => log_dir, // the desired location is writable
+                Err(_) => std::env::temp_dir().join(format!("velopack_{}.log", locator.get_manifest_id())), // fallback to temp dir with app name
             }
-            Err(e) => warn!("Could not auto-locate app manifest, writing log to current directory. ({})", e),
         }
-
-        // If we can't locate the current app, we write to the current directory.
-        let mut my_exe = std::env::current_exe().expect("Could not locate current executable");
-        my_exe.pop();
-        return my_exe.join("Velopack.log");
-    }
-    #[cfg(target_os = "linux")]
-    {
-        return default_file_linux();
-    }
-    #[cfg(target_os = "macos")]
-    {
-        return default_file_macos();
+        Err(_) => std::env::temp_dir().join(LOGGING_FILE_NAME), // fallback to temp dir shared filename
     }
 }
 
 /// Default log location for Velopack on the current OS.
-#[allow(unused_variables)]
-pub fn default_logfile_from_context(context: LocationContext) -> PathBuf {
+pub fn default_logfile_path<L: TryInto<VelopackLocator>>(locator: L) -> PathBuf {
     #[cfg(target_os = "windows")]
     {
-        match crate::locator::auto_locate_app_manifest(context) {
-            Ok(locator) => {
-                return default_logfile_from_locator(locator);
-            }
-            Err(e) => warn!("Could not auto-locate app manifest, writing log to current directory. ({})", e),
-        }
-
-        // If we can't locate the current app, we write to the current directory.
-        let mut my_exe = std::env::current_exe().expect("Could not locate current executable");
-        my_exe.pop();
-        return my_exe.join("Velopack.log");
+        return default_file_windows(locator);
     }
     #[cfg(target_os = "linux")]
     {
-        return default_file_linux();
+        return default_file_linux(locator);
     }
     #[cfg(target_os = "macos")]
     {
-        return default_file_macos();
+        return default_file_macos(locator);
     }
 }