From 9f27e4d975ebb0fc1ef156f33396b84295db533d Mon Sep 17 00:00:00 2001 From: Caelan Sayler Date: Fri, 23 May 2025 20:13:46 +0100 Subject: [PATCH] Fix: creating a locator should not affect static state --- src/lib-csharp/Locators/IVelopackLocator.cs | 6 ++++++ .../Locators/LinuxVelopackLocator.cs | 13 ++++-------- src/lib-csharp/Locators/OsxVelopackLocator.cs | 13 ++++-------- src/lib-csharp/Locators/VelopackLocator.cs | 21 +++++++++++++------ .../Locators/WindowsVelopackLocator.cs | 17 ++++++--------- .../Logging/CombinedVelopackLogger.cs | 9 ++++++++ test/TestApp/Program.cs | 2 +- 7 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/lib-csharp/Locators/IVelopackLocator.cs b/src/lib-csharp/Locators/IVelopackLocator.cs index 308e19f2..88f1c7b7 100644 --- a/src/lib-csharp/Locators/IVelopackLocator.cs +++ b/src/lib-csharp/Locators/IVelopackLocator.cs @@ -57,6 +57,12 @@ namespace Velopack.Locators /// IProcessImpl Process { get; } + /// + /// Add a logger to the list of loggers. This will be used to log messages from Velopack. + /// + /// + void AddLogger(IVelopackLogger logger); + /// /// Finds .nupkg files in the PackagesDir and returns a list of ReleaseEntryName objects. /// diff --git a/src/lib-csharp/Locators/LinuxVelopackLocator.cs b/src/lib-csharp/Locators/LinuxVelopackLocator.cs index 65768c8c..c61b2445 100644 --- a/src/lib-csharp/Locators/LinuxVelopackLocator.cs +++ b/src/lib-csharp/Locators/LinuxVelopackLocator.cs @@ -36,9 +36,6 @@ namespace Velopack.Locators /// public override string? Channel { get; } - /// - public override IVelopackLogger Log { get; } - /// public override string? AppTempDir => CreateSubDirIfDoesNotExist(TempUtil.GetDefaultTempBaseDirectory(), AppId); @@ -63,15 +60,13 @@ namespace Velopack.Locators if (!VelopackRuntimeInfo.IsLinux) throw new NotSupportedException($"Cannot instantiate {nameof(LinuxVelopackLocator)} on a non-linux system."); - var combinedLog = new CombinedVelopackLogger(); - combinedLog.Add(customLog); - Log = combinedLog; + CombinedLogger = new CombinedVelopackLogger(customLog); - Process = processImpl ??= new DefaultProcessImpl(combinedLog); + Process = processImpl ??= new DefaultProcessImpl(CombinedLogger); var ourPath = processImpl.GetCurrentProcessPath(); var currentProcessId = processImpl.GetCurrentProcessId(); - using var initLog = new CachedVelopackLogger(combinedLog); + using var initLog = new CachedVelopackLogger(CombinedLogger); initLog.Info($"Initializing {nameof(LinuxVelopackLocator)}"); var logFilePath = Path.Combine(Path.GetTempPath(), DefaultLoggingFileName); @@ -107,7 +102,7 @@ namespace Velopack.Locators try { var fileLog = new FileVelopackLogger(logFilePath, currentProcessId); - combinedLog.Add(fileLog); + CombinedLogger.Add(fileLog); } catch (Exception ex) { initLog.Error("Unable to create file logger: " + ex); } diff --git a/src/lib-csharp/Locators/OsxVelopackLocator.cs b/src/lib-csharp/Locators/OsxVelopackLocator.cs index 236d01df..dc0c6660 100644 --- a/src/lib-csharp/Locators/OsxVelopackLocator.cs +++ b/src/lib-csharp/Locators/OsxVelopackLocator.cs @@ -39,9 +39,6 @@ namespace Velopack.Locators /// public override string? PackagesDir => CreateSubDirIfDoesNotExist(CachesAppDir, "packages"); - /// - public override IVelopackLogger Log { get; } - private string? CachesAppDir => CreateSubDirIfDoesNotExist(CachesVelopackDir, AppId); private string? CachesVelopackDir => CreateSubDirIfDoesNotExist(CachesDir, "velopack"); private string? CachesDir => CreateSubDirIfDoesNotExist(LibraryDir, "Caches"); @@ -60,15 +57,13 @@ namespace Velopack.Locators if (!VelopackRuntimeInfo.IsOSX) throw new NotSupportedException($"Cannot instantiate {nameof(OsxVelopackLocator)} on a non-osx system."); - var combinedLog = new CombinedVelopackLogger(); - combinedLog.Add(customLog); - Log = combinedLog; + CombinedLogger = new CombinedVelopackLogger(customLog); - Process = processImpl ??= new DefaultProcessImpl(combinedLog); + Process = processImpl ??= new DefaultProcessImpl(CombinedLogger); var ourPath = processImpl.GetCurrentProcessPath(); var currentProcessId = processImpl.GetCurrentProcessId(); - using var initLog = new CachedVelopackLogger(combinedLog); + using var initLog = new CachedVelopackLogger(CombinedLogger); initLog.Info($"Initializing {nameof(OsxVelopackLocator)}"); string logFolder = Path.GetTempPath(); @@ -107,7 +102,7 @@ namespace Velopack.Locators try { var logFilePath = Path.Combine(logFolder, logFileName); var fileLog = new FileVelopackLogger(logFilePath, currentProcessId); - combinedLog.Add(fileLog); + CombinedLogger.Add(fileLog); } catch (Exception ex) { initLog.Error("Unable to create file logger: " + ex); } diff --git a/src/lib-csharp/Locators/VelopackLocator.cs b/src/lib-csharp/Locators/VelopackLocator.cs index b5bc03de..a4b6ee76 100644 --- a/src/lib-csharp/Locators/VelopackLocator.cs +++ b/src/lib-csharp/Locators/VelopackLocator.cs @@ -15,7 +15,7 @@ namespace Velopack.Locators public abstract class VelopackLocator : IVelopackLocator { private static IVelopackLocator? _current; - + /// /// The default log file name for Velopack. /// @@ -34,7 +34,8 @@ namespace Velopack.Locators get { if (_current == null) throw new InvalidOperationException( - $"No VelopackLocator has been set. Either call {nameof(VelopackApp)}.{nameof(VelopackApp.Build)}() or provide {nameof(IVelopackLocator)} as a method parameter."); + $"No VelopackLocator has been set. Either call {nameof(VelopackApp)}.{nameof(VelopackApp.Build)}().Run() " + + $"or provide {nameof(IVelopackLocator)} as a method parameter."); return _current; } } @@ -43,13 +44,13 @@ namespace Velopack.Locators public static IVelopackLocator CreateDefaultForPlatform(IProcessImpl? processImpl = null, IVelopackLogger? logger = null) { if (VelopackRuntimeInfo.IsWindows) - return _current = new WindowsVelopackLocator(processImpl, logger); + return new WindowsVelopackLocator(processImpl, logger); if (VelopackRuntimeInfo.IsOSX) - return _current = new OsxVelopackLocator(processImpl, logger); + return new OsxVelopackLocator(processImpl, logger); if (VelopackRuntimeInfo.IsLinux) - return _current = new LinuxVelopackLocator(processImpl, logger); + return new LinuxVelopackLocator(processImpl, logger); throw new PlatformNotSupportedException($"OS platform '{VelopackRuntimeInfo.SystemOs.GetOsLongName()}' is not supported."); } @@ -87,7 +88,7 @@ namespace Velopack.Locators public abstract string? Channel { get; } /// - public abstract IVelopackLogger Log { get; } + public virtual IVelopackLogger Log => ((IVelopackLogger?) CombinedLogger) ?? new NullVelopackLogger(); /// public virtual bool IsPortable => false; @@ -111,6 +112,14 @@ namespace Velopack.Locators /// public abstract SemanticVersion? CurrentlyInstalledVersion { get; } + internal CombinedVelopackLogger? CombinedLogger { get; set; } + + /// + public void AddLogger(IVelopackLogger logger) + { + CombinedLogger?.Add(logger); + } + /// public virtual List GetLocalPackages() { diff --git a/src/lib-csharp/Locators/WindowsVelopackLocator.cs b/src/lib-csharp/Locators/WindowsVelopackLocator.cs index dbbf4b9d..c69e90d9 100644 --- a/src/lib-csharp/Locators/WindowsVelopackLocator.cs +++ b/src/lib-csharp/Locators/WindowsVelopackLocator.cs @@ -37,9 +37,6 @@ namespace Velopack.Locators /// public override string? PackagesDir => _packagesDir.Value; - /// - public override IVelopackLogger Log { get; } - /// public override bool IsPortable => RootAppDir != null && File.Exists(Path.Combine(RootAppDir, ".portable")); @@ -56,16 +53,13 @@ namespace Velopack.Locators throw new NotSupportedException($"Cannot instantiate {nameof(WindowsVelopackLocator)} on a non-Windows system."); _packagesDir = new(GetPackagesDir); + CombinedLogger = new CombinedVelopackLogger(customLog); - var combinedLog = new CombinedVelopackLogger(); - combinedLog.Add(customLog); - Log = combinedLog; - - Process = processImpl ??= new DefaultProcessImpl(combinedLog); + Process = processImpl ??= new DefaultProcessImpl(CombinedLogger); var ourPath = processImpl.GetCurrentProcessPath(); var currentProcessId = processImpl.GetCurrentProcessId(); - using var initLog = new CachedVelopackLogger(combinedLog); + using var initLog = new CachedVelopackLogger(CombinedLogger); initLog.Info($"Initializing {nameof(WindowsVelopackLocator)}"); // We try various approaches here. Firstly, if Update.exe is in the parent directory, @@ -133,6 +127,7 @@ namespace Velopack.Locators Directory.CreateDirectory(TempAppRootDirectory); File.Copy(UpdateExePath, tempTargetUpdateExe); } + UpdateExePath = tempTargetUpdateExe; } @@ -142,7 +137,7 @@ namespace Velopack.Locators try { var logFilePath = Path.Combine(RootAppDir, DefaultLoggingFileName); var fileLog = new FileVelopackLogger(logFilePath, currentProcessId); - combinedLog.Add(fileLog); + CombinedLogger.Add(fileLog); //fileLogCreated = true; } catch (Exception ex) { fileLogException = ex; @@ -156,7 +151,7 @@ namespace Velopack.Locators var logFileName = String.IsNullOrEmpty(AppId) ? DefaultLoggingFileName : $"velopack_{AppId}.log"; var logFilePath = Path.Combine(Path.GetTempPath(), logFileName); var fileLog = new FileVelopackLogger(logFilePath, currentProcessId); - combinedLog.Add(fileLog); + CombinedLogger.Add(fileLog); } catch (Exception ex) { tempFileLogException = ex; } diff --git a/src/lib-csharp/Logging/CombinedVelopackLogger.cs b/src/lib-csharp/Logging/CombinedVelopackLogger.cs index 6d79e334..8947d221 100644 --- a/src/lib-csharp/Logging/CombinedVelopackLogger.cs +++ b/src/lib-csharp/Logging/CombinedVelopackLogger.cs @@ -8,6 +8,15 @@ namespace Velopack.Logging { private readonly List _loggers = new(); + public CombinedVelopackLogger(params IVelopackLogger?[] loggers) + { + foreach (var logger in loggers) { + if (logger != null) { + _loggers.Add(logger); + } + } + } + public void Log(VelopackLogLevel logLevel, string? message, Exception? exception) { foreach (var logger in _loggers) { diff --git a/test/TestApp/Program.cs b/test/TestApp/Program.cs index f8ee49af..22d67e43 100644 --- a/test/TestApp/Program.cs +++ b/test/TestApp/Program.cs @@ -4,7 +4,7 @@ using Velopack; using Velopack.Locators; using Velopack.Logging; -var locator = VelopackLocator.CreateDefaultForPlatform(new ConsoleVelopackLogger()); +var locator = VelopackLocator.CreateDefaultForPlatform(logger: new ConsoleVelopackLogger()); try { bool shouldExit = false;