From 0e4ef094609c7a1fa6449ff5d415c99ae7ef03b3 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 25 Apr 2022 16:03:42 -0700 Subject: [PATCH 01/21] Add UseSystemd() IServiceCollection extension method --- .../Microsoft.Extensions.Hosting.Systemd.cs | 13 ++-- .../src/SystemdHostBuilderExtensions.cs | 63 ++++++++++++++----- .../tests/UseSystemdTests.cs | 26 ++++++-- 3 files changed, 76 insertions(+), 26 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs index bb70b756fc35a0..b574ff8438fc0a 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs @@ -8,6 +8,7 @@ namespace Microsoft.Extensions.Hosting { public static partial class SystemdHostBuilderExtensions { + public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseSystemd(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } public static Microsoft.Extensions.Hosting.IHostBuilder UseSystemd(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } } } @@ -31,11 +32,11 @@ public static partial class SystemdHelpers { public static bool IsSystemdService() { throw null; } } - [System.Runtime.Versioning.UnsupportedOSPlatform("android")] - [System.Runtime.Versioning.UnsupportedOSPlatform("browser")] - [System.Runtime.Versioning.UnsupportedOSPlatform("ios")] - [System.Runtime.Versioning.UnsupportedOSPlatform("maccatalyst")] - [System.Runtime.Versioning.UnsupportedOSPlatform("tvos")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("maccatalyst")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] public partial class SystemdLifetime : Microsoft.Extensions.Hosting.IHostLifetime, System.IDisposable { public SystemdLifetime(Microsoft.Extensions.Hosting.IHostEnvironment environment, Microsoft.Extensions.Hosting.IHostApplicationLifetime applicationLifetime, Microsoft.Extensions.Hosting.Systemd.ISystemdNotifier systemdNotifier, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { } @@ -43,7 +44,7 @@ public void Dispose() { } public System.Threading.Tasks.Task StopAsync(System.Threading.CancellationToken cancellationToken) { throw null; } public System.Threading.Tasks.Task WaitForStartAsync(System.Threading.CancellationToken cancellationToken) { throw null; } } - [System.Runtime.Versioning.UnsupportedOSPlatform("browser")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public partial class SystemdNotifier : Microsoft.Extensions.Hosting.Systemd.ISystemdNotifier { public SystemdNotifier() { } diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs index 60eb7db8b48a1b..4f0b9739b976d8 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs @@ -13,8 +13,8 @@ namespace Microsoft.Extensions.Hosting public static class SystemdHostBuilderExtensions { /// - /// Sets the host lifetime to , - /// provides notification messages for application started and stopping, + /// Configures the lifetime to + /// which provides notification messages for application started and stopping, /// and configures console logging to the systemd format. /// /// @@ -27,27 +27,62 @@ public static class SystemdHostBuilderExtensions /// notifications. See https://www.freedesktop.org/software/systemd/man/systemd.service.html. /// /// - /// The to use. - /// + /// The to configure. + /// The instance. public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) { if (SystemdHelpers.IsSystemdService()) { hostBuilder.ConfigureServices((hostContext, services) => { - services.Configure(options => - { - options.FormatterName = ConsoleFormatterNames.Systemd; - }); - - // IsSystemdService() will never return true for android/browser/iOS/tvOS -#pragma warning disable CA1416 // Validate platform compatibility - services.AddSingleton(); - services.AddSingleton(); -#pragma warning restore CA1416 // Validate platform compatibility + AddSystemdServices(services); }); } return hostBuilder; } + + /// + /// Configures the lifetime of the built from to + /// which provides notification messages for application started + /// and stopping, and configures console logging to the systemd format. + /// + /// + /// + /// This is context aware and will only activate if it detects the process is running + /// as a systemd Service. + /// + /// + /// The systemd service file must be configured with Type=notify to enable + /// notifications. See https://www.freedesktop.org/software/systemd/man/systemd.service.html. + /// + /// + /// + /// The used to build the . + /// For example, or the passed to the + /// callback. + /// The instance. + public static IServiceCollection UseSystemd(this IServiceCollection services) + { + if (SystemdHelpers.IsSystemdService()) + { + AddSystemdServices(services); + } + return services; + } + + private static void AddSystemdServices(IServiceCollection services) + { + services.Configure(options => + { + options.FormatterName = ConsoleFormatterNames.Systemd; + }); + + // IsSystemdService() will never return true for android/browser/iOS/tvOS +#pragma warning disable CA1416 // Validate platform compatibility + services.AddSingleton(); + services.AddSingleton(); +#pragma warning restore CA1416 // Validate platform compatibility + + } } } diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs index 328a1b870e04a5..73e001ed5e230b 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs @@ -12,16 +12,30 @@ public class UseSystemdTests [Fact] public void DefaultsToOffOutsideOfService() { - var host = new HostBuilder() + using IHost host = new HostBuilder() .UseSystemd() .Build(); - using (host) + var lifetime = host.Services.GetRequiredService(); + Assert.NotNull(lifetime); + Assert.IsNotType(lifetime); + } + + [Fact] + public void ServiceCollectionExtensoinMethodDefaultsToOffOutsideOfService() + { + HostApplicationBuilder builder = new HostApplicationBuilder(new HostApplicationBuilderSettings { - var lifetime = host.Services.GetRequiredService(); - Assert.NotNull(lifetime); - Assert.IsNotType(lifetime); - } + // Disable defaults that may not be supported on the testing platform like EventLogLoggerProvider. + DisableDefaults = true, + }); + builder.Services.UseSystemd(); + + using IHost host = builder.Build(); + + var lifetime = host.Services.GetRequiredService(); + Assert.NotNull(lifetime); + Assert.IsNotType(lifetime); } } } From 4c43f67ae4a3c9b4061b3c93e8bf4d48557b70fb Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 10:35:11 -0700 Subject: [PATCH 02/21] Add UseWindowsService() IServiceCollection extension method --- .../src/SystemdHostBuilderExtensions.cs | 19 +-- .../tests/UseSystemdTests.cs | 8 +- ...soft.Extensions.Hosting.WindowsServices.cs | 6 +- .../src/WindowsServiceLifetime.cs | 11 ++ ...owsServiceLifetimeHostBuilderExtensions.cs | 128 ++++++++++++---- .../src/WindowsServiceLifetimeOptions.cs | 6 + ...sions.Hosting.WindowsServices.Tests.csproj | 6 +- .../tests/UseWindowsServiceTests.cs | 143 +++++++++++++++++- 8 files changed, 279 insertions(+), 48 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs index 4f0b9739b976d8..f01fabc901a4eb 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs @@ -13,8 +13,8 @@ namespace Microsoft.Extensions.Hosting public static class SystemdHostBuilderExtensions { /// - /// Configures the lifetime to - /// which provides notification messages for application started and stopping, + /// Configures the lifetime to , + /// provides notification messages for application started and stopping, /// and configures console logging to the systemd format. /// /// @@ -28,14 +28,14 @@ public static class SystemdHostBuilderExtensions /// /// /// The to configure. - /// The instance. + /// The instance for chaining. public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) { if (SystemdHelpers.IsSystemdService()) { hostBuilder.ConfigureServices((hostContext, services) => { - AddSystemdServices(services); + AddSystemdLifetime(services); }); } return hostBuilder; @@ -43,7 +43,7 @@ public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) /// /// Configures the lifetime of the built from to - /// which provides notification messages for application started + /// provides notification messages for application started /// and stopping, and configures console logging to the systemd format. /// /// @@ -59,18 +59,19 @@ public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) /// /// The used to build the . /// For example, or the passed to the - /// callback. - /// The instance. + /// callback. + /// + /// The instance for chaining. public static IServiceCollection UseSystemd(this IServiceCollection services) { if (SystemdHelpers.IsSystemdService()) { - AddSystemdServices(services); + AddSystemdLifetime(services); } return services; } - private static void AddSystemdServices(IServiceCollection services) + private static void AddSystemdLifetime(IServiceCollection services) { services.Configure(options => { diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs index 73e001ed5e230b..7b0c51dbd4e7a8 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs @@ -17,24 +17,22 @@ public void DefaultsToOffOutsideOfService() .Build(); var lifetime = host.Services.GetRequiredService(); - Assert.NotNull(lifetime); Assert.IsNotType(lifetime); } [Fact] - public void ServiceCollectionExtensoinMethodDefaultsToOffOutsideOfService() + public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService() { - HostApplicationBuilder builder = new HostApplicationBuilder(new HostApplicationBuilderSettings + var builder = new HostApplicationBuilder(new HostApplicationBuilderSettings { // Disable defaults that may not be supported on the testing platform like EventLogLoggerProvider. DisableDefaults = true, }); - builder.Services.UseSystemd(); + builder.Services.UseSystemd(); using IHost host = builder.Build(); var lifetime = host.Services.GetRequiredService(); - Assert.NotNull(lifetime); Assert.IsNotType(lifetime); } } diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs index c22e9ccc2365e1..8a7245c8402034 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs @@ -8,6 +8,8 @@ namespace Microsoft.Extensions.Hosting { public static partial class WindowsServiceLifetimeHostBuilderExtensions { + public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } + public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action configure) { throw null; } public static Microsoft.Extensions.Hosting.IHostBuilder UseWindowsService(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } public static Microsoft.Extensions.Hosting.IHostBuilder UseWindowsService(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action configure) { throw null; } } @@ -21,10 +23,10 @@ namespace Microsoft.Extensions.Hosting.WindowsServices { public static partial class WindowsServiceHelpers { - [System.Runtime.Versioning.SupportedOSPlatformGuard("windows")] + [System.Runtime.Versioning.SupportedOSPlatformGuardAttribute("windows")] public static bool IsWindowsService() { throw null; } } - [System.Runtime.Versioning.SupportedOSPlatform("windows")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public partial class WindowsServiceLifetime : System.ServiceProcess.ServiceBase, Microsoft.Extensions.Hosting.IHostLifetime { public WindowsServiceLifetime(Microsoft.Extensions.Hosting.IHostEnvironment environment, Microsoft.Extensions.Hosting.IHostApplicationLifetime applicationLifetime, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory, Microsoft.Extensions.Options.IOptions optionsAccessor) { } diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs index b535f51f2beef8..bb800c89688e37 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs @@ -30,6 +30,17 @@ public WindowsServiceLifetime(IHostEnvironment environment, IHostApplicationLife ThrowHelper.ThrowIfNull(optionsAccessor); ThrowHelper.ThrowIfNull(windowsServiceOptionsAccessor); + // REVIEW: Should we just log a warning instead? + // Only validate content root for the new IServiceCollection overload of UseWindowsServices() for maximum backwards compatibility. + // The IHostBuilder overload sets the content root unlike the IServiceCollection overload, so it should be less likely to be wrong anyway. + // We could try normalizing the paths before comparing, but if the ContentRootPath has changed from it's AppContext.BaseDirectory default, + // there's probably a misconfiguration. + if (windowsServiceOptionsAccessor.Value.ValidateContentRoot && !string.Equals(environment.ContentRootPath, AppContext.BaseDirectory, StringComparison.Ordinal)) + { + throw new InvalidOperationException( + $"The IHostEnvironment.ConentRootPath value of '{environment.ContentRootPath}' must match the AppContext.BaseDirectory value of '{AppContext.BaseDirectory}' but is unequal."); + } + Environment = environment; ApplicationLifetime = applicationLifetime; Logger = loggerFactory.CreateLogger("Microsoft.Hosting.Lifetime"); diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs index 9f5aa6cc0b7324..febb072adb1a35 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.Hosting.WindowsServices; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.EventLog; +using Microsoft.Extensions.Options; namespace Microsoft.Extensions.Hosting { @@ -17,62 +18,137 @@ namespace Microsoft.Extensions.Hosting public static class WindowsServiceLifetimeHostBuilderExtensions { /// - /// Sets the host lifetime to WindowsServiceLifetime, sets the Content Root, - /// and enables logging to the event log with the application name as the default source name. + /// Sets the host lifetime to , sets the + /// to , and enables logging to the event log with the application name as the default source name. /// /// /// This is context aware and will only activate if it detects the process is running /// as a Windows Service. /// /// The to operate on. - /// The same instance of the for chaining. + /// The instance for chaining. public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder) { return UseWindowsService(hostBuilder, _ => { }); } /// - /// Sets the host lifetime to WindowsServiceLifetime, sets the Content Root, - /// and enables logging to the event log with the application name as the default source name. + /// Sets the host lifetime to , sets the + /// to , and enables logging to the event log with the application name as the default source name. /// /// /// This is context aware and will only activate if it detects the process is running /// as a Windows Service. /// /// The to operate on. - /// - /// The same instance of the for chaining. + /// An to configure the provided . + /// The instance for chaining. public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder, Action configure) { if (WindowsServiceHelpers.IsWindowsService()) { // Host.CreateDefaultBuilder uses CurrentDirectory for VS scenarios, but CurrentDirectory for services is c:\Windows\System32. hostBuilder.UseContentRoot(AppContext.BaseDirectory); - hostBuilder.ConfigureLogging((hostingContext, logging) => + hostBuilder.ConfigureServices(services => { - Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); + AddWindowsServiceLifetime(services, configure); + }); + } - logging.AddEventLog(); - }) - .ConfigureServices((hostContext, services) => - { - Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); + return hostBuilder; + } - services.AddSingleton(); - services.Configure(settings => - { - Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); + /// + /// Configures the lifetime of the built from to + /// , verifies the + /// is equal to , and enables logging to the event log with + /// the application name as the default source name. + /// + /// + /// This is context aware and will only activate if it detects the process is running + /// as a Windows Service. + /// + /// + /// The used to build the . + /// For example, or the passed to the + /// callback. + /// + /// The instance for chaining. + public static IServiceCollection UseWindowsService(this IServiceCollection services) + { + return UseWindowsService(services, _ => { }); + } - if (string.IsNullOrEmpty(settings.SourceName)) - { - settings.SourceName = hostContext.HostingEnvironment.ApplicationName; - } - }); - services.Configure(configure); - }); + /// + /// Configures the lifetime of the built from to + /// , verifies the + /// is equal to , and enables logging to the event log with + /// the application name as the default source name. + /// + /// + /// This is context aware and will only activate if it detects the process is running + /// as a Windows Service. + /// + /// + /// The used to build the . + /// For example, or the passed to the + /// callback. + /// + /// An to configure the provided . + /// The instance for chaining. + public static IServiceCollection UseWindowsService(this IServiceCollection services, Action configure) + { + if (WindowsServiceHelpers.IsWindowsService()) + { + UseWindowsServiceUnchecked(services, configure); } - return hostBuilder; + return services; + } + + // This is a separate method for testing. + private static void UseWindowsServiceUnchecked(IServiceCollection services, Action configure) + { + services.Configure(options => + { + // Host.CreateDefaultBuilder uses CurrentDirectory for VS scenarios, but CurrentDirectory for services is c:\Windows\System32. + options.ValidateContentRoot = true; + }); + AddWindowsServiceLifetime(services, configure); + } + + private static void AddWindowsServiceLifetime(IServiceCollection services, Action configure) + { + Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); + + services.AddLogging(logging => + { + Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); + logging.AddEventLog(); + }); + services.AddSingleton(); + services.AddSingleton, EventLogSettingsSetup>(); + services.Configure(configure); + } + + private sealed class EventLogSettingsSetup : IConfigureOptions + { + private readonly string? _applicationName; + + public EventLogSettingsSetup(IHostEnvironment environment) + { + _applicationName = environment.ApplicationName; + } + + public void Configure(EventLogSettings settings) + { + Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); + + if (string.IsNullOrEmpty(settings.SourceName)) + { + settings.SourceName = _applicationName; + } + } } } } diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs index 53a8dc0eb48074..d44a128298ba04 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs @@ -11,5 +11,11 @@ public class WindowsServiceLifetimeOptions /// The name used to identify the service to the system. /// public string ServiceName { get; set; } = string.Empty; + + // Used by the IServiceCollection overload of UseWindowsService to indicated that WindowsServiceLifetime + // should verify IHostEnvironment.ContentRootPath = AppContext.BaseDirectory (usually the default). + // This should also be the content root for the IHostBuilder overload unless it's been overridden, but + // we don't want to break people who might have successfully overridden IHostBuilder's ContentRoot. + internal bool ValidateContentRoot { get; set; } } } diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj index ef1085964e61e2..36f0edaf8a1058 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj @@ -1,4 +1,4 @@ - + $(NetCoreAppCurrent);$(NetFrameworkMinimum) @@ -9,4 +9,8 @@ + + + + diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs index 512c2c04b6ac0b..19c8197410aee0 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs @@ -2,26 +2,159 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.IO; +using System.Reflection; +using System.ServiceProcess; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting.Internal; +using Microsoft.Extensions.Hosting.WindowsServices; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.EventLog; +using Microsoft.Extensions.Options; using Xunit; namespace Microsoft.Extensions.Hosting { public class UseWindowsServiceTests { + private static MethodInfo? _useWindowsServiceUncheckedMethod = null; + + public static bool SupportsServiceBase + { + get + { + if (!PlatformDetection.IsWindows) + { + return false; + } + + try + { + new ServiceBase(); + } + catch (PlatformNotSupportedException) + { + // PlatformNotSupportedException : ServiceController enables manipulating and accessing Windows services and it is not applicable for other operating systems. + // at System.ServiceProcess.ServiceBase..ctor() in C:\dev\dotnet\runtime\artifacts\obj\System.ServiceProcess.ServiceController\Debug\net7.0\System.ServiceProcess.ServiceController.notsupported.cs:line 24 + // at Microsoft.Extensions.Hosting.WindowsServices.WindowsServiceLifetime..ctor(IHostEnvironment environment, IHostApplicationLifetime applicationLifetime, ILoggerFactory loggerFactory, IOptions`1 optionsAccessor, IOptions`1 windowsServiceOptionsAccessor) in C:\dev\dotnet\runtime\src\libraries\Microsoft.Extensions.Hosting.WindowsServices\src\WindowsServiceLifetime.cs:line 26 + + // REVIEW: This seems similar to https://github.com/dotnet/sdk/issues/16049 which was closed. I'm not sure why this is still happening in tests on Windows. + // net462 tests can construct ServiceBase just fine, but not net7.0. Does anyone know what the real issue is here? + return false; + } + + return true; + } + } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] public void DefaultsToOffOutsideOfService() { - var host = new HostBuilder() + using IHost host = new HostBuilder() .UseWindowsService() .Build(); - using (host) + var lifetime = host.Services.GetRequiredService(); + Assert.IsType(lifetime); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService() + { + var builder = new HostApplicationBuilder(); + + builder.Services.UseWindowsService(); + // No reason to write event logs in this test. Event log may be unsupported anyway. + builder.Logging.ClearProviders(); + + using IHost host = builder.Build(); + + var lifetime = host.Services.GetRequiredService(); + Assert.IsType(lifetime); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + public void ServiceCollectionExtensionMethodAddsWindowsServiceLifetimeInsideOfService() + { + var builder = new HostApplicationBuilder(); + + // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. + UseWindowsServiceUnchecked(builder.Services); + + Assert.Single(builder.Services, serviceDescriptor => + serviceDescriptor.ServiceType == typeof(IHostLifetime) && + serviceDescriptor.ImplementationType == typeof(WindowsServiceLifetime)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + public void ServiceCollectionExtensionMethodSetsEventLogSourceNameToApplicationNameInsideOfService() + { + string appName = Guid.NewGuid().ToString(); + + var builder = new HostApplicationBuilder(new HostApplicationBuilderSettings { - var lifetime = host.Services.GetRequiredService(); - Assert.IsType(lifetime); - } + ApplicationName = appName, + }); + + // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. + UseWindowsServiceUnchecked(builder.Services); + // No reason to write event logs in this test. Event log may be unsupported anyway. + builder.Logging.ClearProviders(); + + // Remove WindowsServiceLifetime descriptor so we can run this test even when SupportsServiceBase is false. + var lifetimeDescriptor = Assert.Single(builder.Services, serviceDescriptor => + serviceDescriptor.ServiceType == typeof(IHostLifetime) && + serviceDescriptor.ImplementationType == typeof(WindowsServiceLifetime)); + builder.Services.Remove(lifetimeDescriptor); + + using IHost host = builder.Build(); + + var eventLogSettings = host.Services.GetRequiredService>().Value; + Assert.Same(appName, eventLogSettings.SourceName); + } + + [ConditionalFact(typeof(UseWindowsServiceTests), nameof(SupportsServiceBase))] + public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() + { + var builder = new HostApplicationBuilder(); + + // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. + UseWindowsServiceUnchecked(builder.Services); + // No reason to write event logs in this test. + builder.Logging.ClearProviders(); + + using IHost host = builder.Build(); + + var lifetime = host.Services.GetRequiredService(); + Assert.IsType(lifetime); + } + + + [ConditionalFact(typeof(UseWindowsServiceTests), nameof(SupportsServiceBase))] + public void ServiceCollectionExtensionMethodThrowsGivenWrongContentRoot() + { + var builder = new HostApplicationBuilder(new HostApplicationBuilderSettings + { + ContentRootPath = Path.GetTempPath(), + }); + + // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. + UseWindowsServiceUnchecked(builder.Services); + // No reason to write event logs in this test. + builder.Logging.ClearProviders(); + + var ex = Assert.Throws(() => builder.Build()); + Assert.Equal($"The IHostEnvironment.ConentRootPath value of '{Path.GetTempPath()}' must match the AppContext.BaseDirectory value of '{AppContext.BaseDirectory}' but is unequal.", ex.Message); + } + + private void UseWindowsServiceUnchecked(IServiceCollection services, Action configure = null) + { + _useWindowsServiceUncheckedMethod ??= typeof(WindowsServiceLifetimeHostBuilderExtensions).GetMethod("UseWindowsServiceUnchecked", + BindingFlags.Static | BindingFlags.NonPublic, null, new[] { typeof(IServiceCollection), typeof(Action) }, null) + ?? throw new MissingMethodException(); + + configure ??= _ => { }; + _useWindowsServiceUncheckedMethod.Invoke(null, new object[] { services, configure }); } } } From f72c52a64d8e349dcf5574b51da3d9cc9e039634 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 12:53:06 -0700 Subject: [PATCH 03/21] Target Windows for UseWindowsServiceTests --- ...sions.Hosting.WindowsServices.Tests.csproj | 3 +- .../tests/UseWindowsServiceTests.cs | 41 +++---------------- 2 files changed, 8 insertions(+), 36 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj index 36f0edaf8a1058..93be9b87c967b5 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/Microsoft.Extensions.Hosting.WindowsServices.Tests.csproj @@ -1,7 +1,8 @@ - $(NetCoreAppCurrent);$(NetFrameworkMinimum) + + $(NetCoreAppCurrent)-windows;$(NetFrameworkMinimum) true diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs index 19c8197410aee0..5c347101bd7cdc 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs @@ -19,35 +19,7 @@ public class UseWindowsServiceTests { private static MethodInfo? _useWindowsServiceUncheckedMethod = null; - public static bool SupportsServiceBase - { - get - { - if (!PlatformDetection.IsWindows) - { - return false; - } - - try - { - new ServiceBase(); - } - catch (PlatformNotSupportedException) - { - // PlatformNotSupportedException : ServiceController enables manipulating and accessing Windows services and it is not applicable for other operating systems. - // at System.ServiceProcess.ServiceBase..ctor() in C:\dev\dotnet\runtime\artifacts\obj\System.ServiceProcess.ServiceController\Debug\net7.0\System.ServiceProcess.ServiceController.notsupported.cs:line 24 - // at Microsoft.Extensions.Hosting.WindowsServices.WindowsServiceLifetime..ctor(IHostEnvironment environment, IHostApplicationLifetime applicationLifetime, ILoggerFactory loggerFactory, IOptions`1 optionsAccessor, IOptions`1 windowsServiceOptionsAccessor) in C:\dev\dotnet\runtime\src\libraries\Microsoft.Extensions.Hosting.WindowsServices\src\WindowsServiceLifetime.cs:line 26 - - // REVIEW: This seems similar to https://github.com/dotnet/sdk/issues/16049 which was closed. I'm not sure why this is still happening in tests on Windows. - // net462 tests can construct ServiceBase just fine, but not net7.0. Does anyone know what the real issue is here? - return false; - } - - return true; - } - } - - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + [Fact] public void DefaultsToOffOutsideOfService() { using IHost host = new HostBuilder() @@ -58,7 +30,7 @@ public void DefaultsToOffOutsideOfService() Assert.IsType(lifetime); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + [Fact] public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService() { var builder = new HostApplicationBuilder(); @@ -73,7 +45,7 @@ public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService() Assert.IsType(lifetime); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + [Fact] public void ServiceCollectionExtensionMethodAddsWindowsServiceLifetimeInsideOfService() { var builder = new HostApplicationBuilder(); @@ -86,7 +58,7 @@ public void ServiceCollectionExtensionMethodAddsWindowsServiceLifetimeInsideOfSe serviceDescriptor.ImplementationType == typeof(WindowsServiceLifetime)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + [Fact] public void ServiceCollectionExtensionMethodSetsEventLogSourceNameToApplicationNameInsideOfService() { string appName = Guid.NewGuid().ToString(); @@ -113,7 +85,7 @@ public void ServiceCollectionExtensionMethodSetsEventLogSourceNameToApplicationN Assert.Same(appName, eventLogSettings.SourceName); } - [ConditionalFact(typeof(UseWindowsServiceTests), nameof(SupportsServiceBase))] + [Fact] public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() { var builder = new HostApplicationBuilder(); @@ -129,8 +101,7 @@ public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() Assert.IsType(lifetime); } - - [ConditionalFact(typeof(UseWindowsServiceTests), nameof(SupportsServiceBase))] + [Fact] public void ServiceCollectionExtensionMethodThrowsGivenWrongContentRoot() { var builder = new HostApplicationBuilder(new HostApplicationBuilderSettings From ab982957f517fa4a0c584528b2602ec9068a6de5 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 12:58:01 -0700 Subject: [PATCH 04/21] Stop throwing InvalidOperationExceptions from WindowsServiceLifetime --- .../src/WindowsServiceLifetime.cs | 11 ----------- .../tests/UseWindowsServiceTests.cs | 17 ----------------- 2 files changed, 28 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs index bb800c89688e37..b535f51f2beef8 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs @@ -30,17 +30,6 @@ public WindowsServiceLifetime(IHostEnvironment environment, IHostApplicationLife ThrowHelper.ThrowIfNull(optionsAccessor); ThrowHelper.ThrowIfNull(windowsServiceOptionsAccessor); - // REVIEW: Should we just log a warning instead? - // Only validate content root for the new IServiceCollection overload of UseWindowsServices() for maximum backwards compatibility. - // The IHostBuilder overload sets the content root unlike the IServiceCollection overload, so it should be less likely to be wrong anyway. - // We could try normalizing the paths before comparing, but if the ContentRootPath has changed from it's AppContext.BaseDirectory default, - // there's probably a misconfiguration. - if (windowsServiceOptionsAccessor.Value.ValidateContentRoot && !string.Equals(environment.ContentRootPath, AppContext.BaseDirectory, StringComparison.Ordinal)) - { - throw new InvalidOperationException( - $"The IHostEnvironment.ConentRootPath value of '{environment.ContentRootPath}' must match the AppContext.BaseDirectory value of '{AppContext.BaseDirectory}' but is unequal."); - } - Environment = environment; ApplicationLifetime = applicationLifetime; Logger = loggerFactory.CreateLogger("Microsoft.Hosting.Lifetime"); diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs index 5c347101bd7cdc..a09fbf52c31484 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs @@ -101,23 +101,6 @@ public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() Assert.IsType(lifetime); } - [Fact] - public void ServiceCollectionExtensionMethodThrowsGivenWrongContentRoot() - { - var builder = new HostApplicationBuilder(new HostApplicationBuilderSettings - { - ContentRootPath = Path.GetTempPath(), - }); - - // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. - UseWindowsServiceUnchecked(builder.Services); - // No reason to write event logs in this test. - builder.Logging.ClearProviders(); - - var ex = Assert.Throws(() => builder.Build()); - Assert.Equal($"The IHostEnvironment.ConentRootPath value of '{Path.GetTempPath()}' must match the AppContext.BaseDirectory value of '{AppContext.BaseDirectory}' but is unequal.", ex.Message); - } - private void UseWindowsServiceUnchecked(IServiceCollection services, Action configure = null) { _useWindowsServiceUncheckedMethod ??= typeof(WindowsServiceLifetimeHostBuilderExtensions).GetMethod("UseWindowsServiceUnchecked", From 4fbef8ae6149f0f2ceee3de7a41463a959d6e6ee Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 13:31:30 -0700 Subject: [PATCH 05/21] Don't default to CWD if in C:\Windows\system32 --- .../src/HostingHostBuilderExtensions.cs | 16 +++++++++++++--- .../tests/UnitTests/HostTests.cs | 17 +++++++++++++++++ ...crosoft.Extensions.Hosting.Unit.Tests.csproj | 9 ++++++++- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs index 4f68146976d93a..a04c46d46b99aa 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs @@ -197,10 +197,20 @@ public static IHostBuilder ConfigureDefaults(this IHostBuilder builder, string[] internal static void ApplyDefaultHostConfiguration(IConfigurationBuilder hostConfigBuilder, string[]? args) { - hostConfigBuilder.AddInMemoryCollection(new[] + // If we're running anywhere other than C:\Windows\system32, we default to using the CWD for the ContentRoot. + // However, since many things like Windows services and MSIX + + // In my testing, both Environment.CurrentDirectory and Environment.GetFolderPath(Environment.SpecialFolder.System) return the path without + // any trailing directory separator characters. I'm not even sure the casing can ever be different from these APIs, but I it makes sense to + // ignore case for Windows path comparisons given the file system is usually (always?) going to be case insensitive for the system path. + var cwd = Environment.CurrentDirectory; + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || !string.Equals(cwd, Environment.GetFolderPath(Environment.SpecialFolder.System), StringComparison.OrdinalIgnoreCase)) { - new KeyValuePair(HostDefaults.ContentRootKey, Directory.GetCurrentDirectory()) - }); + hostConfigBuilder.AddInMemoryCollection(new[] + { + new KeyValuePair(HostDefaults.ContentRootKey, cwd), + }); + } hostConfigBuilder.AddEnvironmentVariables(prefix: "DOTNET_"); if (args is { Length: > 0 }) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index f0e8662afcf93d..9629eba9646ceb 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -46,6 +46,23 @@ public void CreateDefaultBuilder_IncludesContentRootByDefault() Assert.Equal(expected, env.ContentRootPath); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWindowsSystemDirectory() + { + // Let's denormalize the system directory by making it C:\WINDOWS\SYSTEM32\ instead of C:\\Windows\\system32 + var systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System).ToUpper() + "\\"; + Directory.SetCurrentDirectory(systemDirectory); + var builder = Host.CreateDefaultBuilder(); + using var host = builder.Build(); + + var config = host.Services.GetRequiredService(); + var env = host.Services.GetRequiredService(); + + Assert.Null(config["ContentRoot"]); + Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); + Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); + } + [Fact] public void CreateDefaultBuilder_IncludesCommandLineArguments() { diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj index 3764ce16a0b64e..c9fc907ab78812 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj @@ -1,4 +1,4 @@ - + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent);$(NetFrameworkMinimum) @@ -7,6 +7,13 @@ true + + + + + + From 2fa20741d92187095913e15f51ebd7997a5bec0d Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 13:38:07 -0700 Subject: [PATCH 06/21] Use* to Add* renames --- .../ref/Microsoft.Extensions.Hosting.Systemd.cs | 2 +- .../src/SystemdHostBuilderExtensions.cs | 2 +- .../tests/UseSystemdTests.cs | 2 +- .../Microsoft.Extensions.Hosting.WindowsServices.cs | 2 +- .../src/WindowsServiceLifetimeHostBuilderExtensions.cs | 10 +++++----- .../tests/UseWindowsServiceTests.cs | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs index b574ff8438fc0a..33979d622b5380 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs @@ -9,7 +9,7 @@ namespace Microsoft.Extensions.Hosting public static partial class SystemdHostBuilderExtensions { public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseSystemd(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } - public static Microsoft.Extensions.Hosting.IHostBuilder UseSystemd(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } + public static Microsoft.Extensions.Hosting.IHostBuilder AddSystemd(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } } } namespace Microsoft.Extensions.Hosting.Systemd diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs index f01fabc901a4eb..32b139cca72b7e 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs @@ -62,7 +62,7 @@ public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) /// callback. /// /// The instance for chaining. - public static IServiceCollection UseSystemd(this IServiceCollection services) + public static IServiceCollection AddSystemd(this IServiceCollection services) { if (SystemdHelpers.IsSystemdService()) { diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs index 7b0c51dbd4e7a8..1feb5c333d3f05 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs @@ -13,7 +13,7 @@ public class UseSystemdTests public void DefaultsToOffOutsideOfService() { using IHost host = new HostBuilder() - .UseSystemd() + .AddSystemd() .Build(); var lifetime = host.Services.GetRequiredService(); diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs index 8a7245c8402034..48208866fae1d3 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs @@ -8,7 +8,7 @@ namespace Microsoft.Extensions.Hosting { public static partial class WindowsServiceLifetimeHostBuilderExtensions { - public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } + public static Microsoft.Extensions.DependencyInjection.IServiceCollection AddWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action configure) { throw null; } public static Microsoft.Extensions.Hosting.IHostBuilder UseWindowsService(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } public static Microsoft.Extensions.Hosting.IHostBuilder UseWindowsService(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action configure) { throw null; } diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs index febb072adb1a35..afff96f216e8ab 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs @@ -74,9 +74,9 @@ public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder, Acti /// callback. /// /// The instance for chaining. - public static IServiceCollection UseWindowsService(this IServiceCollection services) + public static IServiceCollection AddWindowsService(this IServiceCollection services) { - return UseWindowsService(services, _ => { }); + return AddWindowsService(services, _ => { }); } /// @@ -96,18 +96,18 @@ public static IServiceCollection UseWindowsService(this IServiceCollection servi /// /// An to configure the provided . /// The instance for chaining. - public static IServiceCollection UseWindowsService(this IServiceCollection services, Action configure) + public static IServiceCollection AddWindowsService(this IServiceCollection services, Action configure) { if (WindowsServiceHelpers.IsWindowsService()) { - UseWindowsServiceUnchecked(services, configure); + AddWindowsServiceUnchecked(services, configure); } return services; } // This is a separate method for testing. - private static void UseWindowsServiceUnchecked(IServiceCollection services, Action configure) + private static void AddWindowsServiceUnchecked(IServiceCollection services, Action configure) { services.Configure(options => { diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs index a09fbf52c31484..96d6aa5f182736 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs @@ -35,7 +35,7 @@ public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService() { var builder = new HostApplicationBuilder(); - builder.Services.UseWindowsService(); + builder.Services.AddWindowsService(); // No reason to write event logs in this test. Event log may be unsupported anyway. builder.Logging.ClearProviders(); From e4f3717e0ae7a9325c2849c6245e3ec65cae7579 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 13:41:24 -0700 Subject: [PATCH 07/21] Apply suggestions from code review Co-authored-by: Eric Erhardt Co-authored-by: Martin Costello --- .../src/SystemdHostBuilderExtensions.cs | 4 ++-- .../src/WindowsServiceLifetimeOptions.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs index 32b139cca72b7e..a56d69f385e2bd 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs @@ -43,7 +43,7 @@ public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) /// /// Configures the lifetime of the built from to - /// provides notification messages for application started + /// , provides notification messages for application started /// and stopping, and configures console logging to the systemd format. /// /// @@ -53,7 +53,7 @@ public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) /// /// /// The systemd service file must be configured with Type=notify to enable - /// notifications. See https://www.freedesktop.org/software/systemd/man/systemd.service.html. + /// notifications. See . /// /// /// diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs index d44a128298ba04..d35b2d195a7b4a 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs @@ -12,7 +12,7 @@ public class WindowsServiceLifetimeOptions /// public string ServiceName { get; set; } = string.Empty; - // Used by the IServiceCollection overload of UseWindowsService to indicated that WindowsServiceLifetime + // Used by the IServiceCollection overload of UseWindowsService to indicate that WindowsServiceLifetime // should verify IHostEnvironment.ContentRootPath = AppContext.BaseDirectory (usually the default). // This should also be the content root for the IHostBuilder overload unless it's been overridden, but // we don't want to break people who might have successfully overridden IHostBuilder's ContentRoot. From 9c32c8fae676cee5826536b6401521c7fc6832de Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 20:24:54 -0700 Subject: [PATCH 08/21] UseSystemd to AddSystemd ref fixup --- .../ref/Microsoft.Extensions.Hosting.Systemd.cs | 4 ++-- .../tests/UseSystemdTests.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs index 33979d622b5380..7d1462aba00056 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/ref/Microsoft.Extensions.Hosting.Systemd.cs @@ -8,8 +8,8 @@ namespace Microsoft.Extensions.Hosting { public static partial class SystemdHostBuilderExtensions { - public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseSystemd(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } - public static Microsoft.Extensions.Hosting.IHostBuilder AddSystemd(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } + public static Microsoft.Extensions.DependencyInjection.IServiceCollection AddSystemd(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } + public static Microsoft.Extensions.Hosting.IHostBuilder UseSystemd(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } } } namespace Microsoft.Extensions.Hosting.Systemd diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs index 1feb5c333d3f05..2b6e462a76a7b3 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs @@ -13,7 +13,7 @@ public class UseSystemdTests public void DefaultsToOffOutsideOfService() { using IHost host = new HostBuilder() - .AddSystemd() + .UseSystemd() .Build(); var lifetime = host.Services.GetRequiredService(); @@ -29,7 +29,7 @@ public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService() DisableDefaults = true, }); - builder.Services.UseSystemd(); + builder.Services.AddSystemd(); using IHost host = builder.Build(); var lifetime = host.Services.GetRequiredService(); From cc1ac58e47d728e381d984dfffa56182266b978b Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 22:01:55 -0700 Subject: [PATCH 09/21] UseWindowsService to AddWindowsService ref fixup --- .../ref/Microsoft.Extensions.Hosting.WindowsServices.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs index 48208866fae1d3..6b812681f73a02 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/ref/Microsoft.Extensions.Hosting.WindowsServices.cs @@ -9,7 +9,7 @@ namespace Microsoft.Extensions.Hosting public static partial class WindowsServiceLifetimeHostBuilderExtensions { public static Microsoft.Extensions.DependencyInjection.IServiceCollection AddWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services) { throw null; } - public static Microsoft.Extensions.DependencyInjection.IServiceCollection UseWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action configure) { throw null; } + public static Microsoft.Extensions.DependencyInjection.IServiceCollection AddWindowsService(this Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action configure) { throw null; } public static Microsoft.Extensions.Hosting.IHostBuilder UseWindowsService(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder) { throw null; } public static Microsoft.Extensions.Hosting.IHostBuilder UseWindowsService(this Microsoft.Extensions.Hosting.IHostBuilder hostBuilder, System.Action configure) { throw null; } } From 1f72a357759fcc3436f4e585cc46e498129e097c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 22:05:43 -0700 Subject: [PATCH 10/21] Remove extraneous code and outdated comments --- ...owsServiceLifetimeHostBuilderExtensions.cs | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs index afff96f216e8ab..6e7aad984027be 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs @@ -22,8 +22,7 @@ public static class WindowsServiceLifetimeHostBuilderExtensions /// to , and enables logging to the event log with the application name as the default source name. /// /// - /// This is context aware and will only activate if it detects the process is running - /// as a Windows Service. + /// This is context aware and will only activate if it detects the process is running as a Windows Service. /// /// The to operate on. /// The instance for chaining. @@ -33,8 +32,8 @@ public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder) } /// - /// Sets the host lifetime to , sets the - /// to , and enables logging to the event log with the application name as the default source name. + /// Sets the host lifetime to and enables logging to the event log with the application + /// name as the default source name. /// /// /// This is context aware and will only activate if it detects the process is running @@ -47,8 +46,6 @@ public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder, Acti { if (WindowsServiceHelpers.IsWindowsService()) { - // Host.CreateDefaultBuilder uses CurrentDirectory for VS scenarios, but CurrentDirectory for services is c:\Windows\System32. - hostBuilder.UseContentRoot(AppContext.BaseDirectory); hostBuilder.ConfigureServices(services => { AddWindowsServiceLifetime(services, configure); @@ -100,23 +97,12 @@ public static IServiceCollection AddWindowsService(this IServiceCollection servi { if (WindowsServiceHelpers.IsWindowsService()) { - AddWindowsServiceUnchecked(services, configure); + AddWindowsServiceLifetime(services, configure); } return services; } - // This is a separate method for testing. - private static void AddWindowsServiceUnchecked(IServiceCollection services, Action configure) - { - services.Configure(options => - { - // Host.CreateDefaultBuilder uses CurrentDirectory for VS scenarios, but CurrentDirectory for services is c:\Windows\System32. - options.ValidateContentRoot = true; - }); - AddWindowsServiceLifetime(services, configure); - } - private static void AddWindowsServiceLifetime(IServiceCollection services, Action configure) { Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows)); From decb55f4d2e018f9b36a555522e624988bb6f855 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 22:11:43 -0700 Subject: [PATCH 11/21] Fix more outdated comments --- .../WindowsServiceLifetimeHostBuilderExtensions.cs | 13 +++++-------- .../tests/UseWindowsServiceTests.cs | 10 +++++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs index 6e7aad984027be..d8cef616cefb47 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs @@ -18,8 +18,8 @@ namespace Microsoft.Extensions.Hosting public static class WindowsServiceLifetimeHostBuilderExtensions { /// - /// Sets the host lifetime to , sets the - /// to , and enables logging to the event log with the application name as the default source name. + /// Sets the host lifetime to and enables logging to the event log with + /// the application name as the default source name. /// /// /// This is context aware and will only activate if it detects the process is running as a Windows Service. @@ -57,9 +57,8 @@ public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder, Acti /// /// Configures the lifetime of the built from to - /// , verifies the - /// is equal to , and enables logging to the event log with - /// the application name as the default source name. + /// and enables logging to the event log with the application + /// name as the default source name. /// /// /// This is context aware and will only activate if it detects the process is running @@ -78,9 +77,7 @@ public static IServiceCollection AddWindowsService(this IServiceCollection servi /// /// Configures the lifetime of the built from to - /// , verifies the - /// is equal to , and enables logging to the event log with - /// the application name as the default source name. + /// and enables logging to the event log with the application name as the default source name. /// /// /// This is context aware and will only activate if it detects the process is running diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs index 96d6aa5f182736..c612ce3ccffe00 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs @@ -51,7 +51,7 @@ public void ServiceCollectionExtensionMethodAddsWindowsServiceLifetimeInsideOfSe var builder = new HostApplicationBuilder(); // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. - UseWindowsServiceUnchecked(builder.Services); + AddWindowsServiceLifetime(builder.Services); Assert.Single(builder.Services, serviceDescriptor => serviceDescriptor.ServiceType == typeof(IHostLifetime) && @@ -69,7 +69,7 @@ public void ServiceCollectionExtensionMethodSetsEventLogSourceNameToApplicationN }); // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. - UseWindowsServiceUnchecked(builder.Services); + AddWindowsServiceLifetime(builder.Services); // No reason to write event logs in this test. Event log may be unsupported anyway. builder.Logging.ClearProviders(); @@ -91,7 +91,7 @@ public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() var builder = new HostApplicationBuilder(); // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. - UseWindowsServiceUnchecked(builder.Services); + AddWindowsServiceLifetime(builder.Services); // No reason to write event logs in this test. builder.Logging.ClearProviders(); @@ -101,9 +101,9 @@ public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() Assert.IsType(lifetime); } - private void UseWindowsServiceUnchecked(IServiceCollection services, Action configure = null) + private void AddWindowsServiceLifetime(IServiceCollection services, Action configure = null) { - _useWindowsServiceUncheckedMethod ??= typeof(WindowsServiceLifetimeHostBuilderExtensions).GetMethod("UseWindowsServiceUnchecked", + _useWindowsServiceUncheckedMethod ??= typeof(WindowsServiceLifetimeHostBuilderExtensions).GetMethod("AddWindowsServiceLifetime", BindingFlags.Static | BindingFlags.NonPublic, null, new[] { typeof(IServiceCollection), typeof(Action) }, null) ?? throw new MissingMethodException(); From 155fbe88d8da3d0b4d8354a7beefcb4ca27a9b63 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 22:14:47 -0700 Subject: [PATCH 12/21] delete more --- .../src/WindowsServiceLifetimeOptions.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs index d35b2d195a7b4a..53a8dc0eb48074 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs @@ -11,11 +11,5 @@ public class WindowsServiceLifetimeOptions /// The name used to identify the service to the system. /// public string ServiceName { get; set; } = string.Empty; - - // Used by the IServiceCollection overload of UseWindowsService to indicate that WindowsServiceLifetime - // should verify IHostEnvironment.ContentRootPath = AppContext.BaseDirectory (usually the default). - // This should also be the content root for the IHostBuilder overload unless it's been overridden, but - // we don't want to break people who might have successfully overridden IHostBuilder's ContentRoot. - internal bool ValidateContentRoot { get; set; } } } From 55dcac62ad415390e783d569f1ed04ba3e9c9fc6 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 16 Jun 2022 22:19:24 -0700 Subject: [PATCH 13/21] Complete comment in ApplyDefaultHostConfiguration --- .../src/HostingHostBuilderExtensions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs index a04c46d46b99aa..48cf483103e02a 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs @@ -198,7 +198,9 @@ public static IHostBuilder ConfigureDefaults(this IHostBuilder builder, string[] internal static void ApplyDefaultHostConfiguration(IConfigurationBuilder hostConfigBuilder, string[]? args) { // If we're running anywhere other than C:\Windows\system32, we default to using the CWD for the ContentRoot. - // However, since many things like Windows services and MSIX + // However, since many things like Windows services and MSIX installers have C:\Windows\system32 as there CWD which is not likely + // to really be the home for things like appsettings.json, we skip changing the ContentRoot in that case. The non-"default" initial + // value for ContentRoot is AppContext.BaseDirectory (e.g. the executable path) which probably makes more sense than the system32. // In my testing, both Environment.CurrentDirectory and Environment.GetFolderPath(Environment.SpecialFolder.System) return the path without // any trailing directory separator characters. I'm not even sure the casing can ever be different from these APIs, but I it makes sense to From 4cbc772e20d5d68957da784d558946afc52209ec Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 17 Jun 2022 00:06:14 -0700 Subject: [PATCH 14/21] Keep the tests hygienic --- .../src/HostingHostBuilderExtensions.cs | 2 +- .../tests/UnitTests/HostTests.cs | 26 +++++++++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs index 48cf483103e02a..2f591e4dea877c 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs @@ -203,7 +203,7 @@ internal static void ApplyDefaultHostConfiguration(IConfigurationBuilder hostCon // value for ContentRoot is AppContext.BaseDirectory (e.g. the executable path) which probably makes more sense than the system32. // In my testing, both Environment.CurrentDirectory and Environment.GetFolderPath(Environment.SpecialFolder.System) return the path without - // any trailing directory separator characters. I'm not even sure the casing can ever be different from these APIs, but I it makes sense to + // any trailing directory separator characters. I'm not even sure the casing can ever be different from these APIs, but I think it makes sense to // ignore case for Windows path comparisons given the file system is usually (always?) going to be case insensitive for the system path. var cwd = Environment.CurrentDirectory; if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || !string.Equals(cwd, Environment.GetFolderPath(Environment.SpecialFolder.System), StringComparison.OrdinalIgnoreCase)) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index 9629eba9646ceb..82516c5ea628b6 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -49,18 +49,28 @@ public void CreateDefaultBuilder_IncludesContentRootByDefault() [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWindowsSystemDirectory() { + var originalCwd = Environment.CurrentDirectory; // Let's denormalize the system directory by making it C:\WINDOWS\SYSTEM32\ instead of C:\\Windows\\system32 var systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System).ToUpper() + "\\"; - Directory.SetCurrentDirectory(systemDirectory); - var builder = Host.CreateDefaultBuilder(); - using var host = builder.Build(); - var config = host.Services.GetRequiredService(); - var env = host.Services.GetRequiredService(); + try + { + Environment.CurrentDirectory = systemDirectory; + + var builder = Host.CreateDefaultBuilder(); + using var host = builder.Build(); - Assert.Null(config["ContentRoot"]); - Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); - Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); + var config = host.Services.GetRequiredService(); + var env = host.Services.GetRequiredService(); + + Assert.Null(config["ContentRoot"]); + Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); + Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); + } + finally + { + Environment.CurrentDirectory = originalCwd; + } } [Fact] From fd7bf278fdd8ef48ddb6fc125192ebea55d9e5f3 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 17 Jun 2022 00:27:04 -0700 Subject: [PATCH 15/21] less var --- .../tests/UseWindowsServiceTests.cs | 20 +++++++------------ .../src/HostingHostBuilderExtensions.cs | 2 +- .../tests/UnitTests/HostTests.cs | 10 +++++----- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs index c612ce3ccffe00..1fb2ade8a94079 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/tests/UseWindowsServiceTests.cs @@ -17,7 +17,7 @@ namespace Microsoft.Extensions.Hosting { public class UseWindowsServiceTests { - private static MethodInfo? _useWindowsServiceUncheckedMethod = null; + private static MethodInfo? _addWindowsServiceLifetimeMethod = null; [Fact] public void DefaultsToOffOutsideOfService() @@ -50,7 +50,7 @@ public void ServiceCollectionExtensionMethodAddsWindowsServiceLifetimeInsideOfSe { var builder = new HostApplicationBuilder(); - // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. + // Emulate calling builder.Services.AddWindowsService() from inside a Windows service. AddWindowsServiceLifetime(builder.Services); Assert.Single(builder.Services, serviceDescriptor => @@ -68,17 +68,11 @@ public void ServiceCollectionExtensionMethodSetsEventLogSourceNameToApplicationN ApplicationName = appName, }); - // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. + // Emulate calling builder.Services.AddWindowsService() from inside a Windows service. AddWindowsServiceLifetime(builder.Services); - // No reason to write event logs in this test. Event log may be unsupported anyway. + // No reason to write event logs in this test. builder.Logging.ClearProviders(); - // Remove WindowsServiceLifetime descriptor so we can run this test even when SupportsServiceBase is false. - var lifetimeDescriptor = Assert.Single(builder.Services, serviceDescriptor => - serviceDescriptor.ServiceType == typeof(IHostLifetime) && - serviceDescriptor.ImplementationType == typeof(WindowsServiceLifetime)); - builder.Services.Remove(lifetimeDescriptor); - using IHost host = builder.Build(); var eventLogSettings = host.Services.GetRequiredService>().Value; @@ -90,7 +84,7 @@ public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() { var builder = new HostApplicationBuilder(); - // Emulate calling builder.Services.UseWindowsService() from inside a Windows service. + // Emulate calling builder.Services.AddWindowsService() from inside a Windows service. AddWindowsServiceLifetime(builder.Services); // No reason to write event logs in this test. builder.Logging.ClearProviders(); @@ -103,12 +97,12 @@ public void ServiceCollectionExtensionMethodCanBeCalledOnDefaultConfiguration() private void AddWindowsServiceLifetime(IServiceCollection services, Action configure = null) { - _useWindowsServiceUncheckedMethod ??= typeof(WindowsServiceLifetimeHostBuilderExtensions).GetMethod("AddWindowsServiceLifetime", + _addWindowsServiceLifetimeMethod ??= typeof(WindowsServiceLifetimeHostBuilderExtensions).GetMethod("AddWindowsServiceLifetime", BindingFlags.Static | BindingFlags.NonPublic, null, new[] { typeof(IServiceCollection), typeof(Action) }, null) ?? throw new MissingMethodException(); configure ??= _ => { }; - _useWindowsServiceUncheckedMethod.Invoke(null, new object[] { services, configure }); + _addWindowsServiceLifetimeMethod.Invoke(null, new object[] { services, configure }); } } } diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs index 2f591e4dea877c..9d6aa528a193e6 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/src/HostingHostBuilderExtensions.cs @@ -205,7 +205,7 @@ internal static void ApplyDefaultHostConfiguration(IConfigurationBuilder hostCon // In my testing, both Environment.CurrentDirectory and Environment.GetFolderPath(Environment.SpecialFolder.System) return the path without // any trailing directory separator characters. I'm not even sure the casing can ever be different from these APIs, but I think it makes sense to // ignore case for Windows path comparisons given the file system is usually (always?) going to be case insensitive for the system path. - var cwd = Environment.CurrentDirectory; + string cwd = Environment.CurrentDirectory; if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || !string.Equals(cwd, Environment.GetFolderPath(Environment.SpecialFolder.System), StringComparison.OrdinalIgnoreCase)) { hostConfigBuilder.AddInMemoryCollection(new[] diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index 82516c5ea628b6..198b35da5b7cb1 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -49,16 +49,16 @@ public void CreateDefaultBuilder_IncludesContentRootByDefault() [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWindowsSystemDirectory() { - var originalCwd = Environment.CurrentDirectory; - // Let's denormalize the system directory by making it C:\WINDOWS\SYSTEM32\ instead of C:\\Windows\\system32 - var systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System).ToUpper() + "\\"; + string originalCwd = Environment.CurrentDirectory; + // Test that the path gets normalized before comparison. Use C:\WINDOWS\SYSTEM32\ instead of C:\Windows\system32. + string systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System).ToUpper() + "\\"; try { Environment.CurrentDirectory = systemDirectory; - var builder = Host.CreateDefaultBuilder(); - using var host = builder.Build(); + IHostBuilder builder = Host.CreateDefaultBuilder(); + using IHost host = builder.Build(); var config = host.Services.GetRequiredService(); var env = host.Services.GetRequiredService(); From aaf291e504c12ef888ff4237994ad77042aea13d Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 17 Jun 2022 10:19:23 -0700 Subject: [PATCH 16/21] Try fixing CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWin - It was failing on Windows.Nano.1809.Amd64.Open containers only. - Was it silently failing to change the CWD? --- .../tests/UnitTests/HostTests.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index 198b35da5b7cb1..d553eef84ef0a7 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -57,6 +57,21 @@ public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWin { Environment.CurrentDirectory = systemDirectory; + if (string.Equals(originalCwd, Environment.CurrentDirectory, StringComparison.OrdinalIgnoreCase)) + { + // Ignore cases where we cannot change the CWD to the system directory for the test. This appears to be the case for + // Windows.Nano.1809.Amd64.Open where we get the following failure from Assert.Null(config["ContentRoot"]). + // https://dev.azure.com/dnceng/public/_build/results?buildId=1830440&view=ms.vss-test-web.build-test-results-tab&runId=48453394&paneView=debug&resultId=102697 + // + // Assert.Null() Failure + // Expected: (null) + // Actual: C:\ + // + // I'm guessing C:\ is not really the system drive and is in fact the original CWD that couldn't be changed. + // If it was the system drive (and we could detect we're on Windows), then the configuration would be null. + return; + } + IHostBuilder builder = Host.CreateDefaultBuilder(); using IHost host = builder.Build(); @@ -65,7 +80,6 @@ public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWin Assert.Null(config["ContentRoot"]); Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); - Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); } finally { From 9bc02088dc404e29332dd6933c7dcbce3e98a475 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 17 Jun 2022 17:20:42 -0700 Subject: [PATCH 17/21] What's really going on? --- .../tests/UnitTests/HostTests.cs | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index d553eef84ef0a7..e48aca9653d582 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -57,27 +57,18 @@ public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWin { Environment.CurrentDirectory = systemDirectory; - if (string.Equals(originalCwd, Environment.CurrentDirectory, StringComparison.OrdinalIgnoreCase)) - { - // Ignore cases where we cannot change the CWD to the system directory for the test. This appears to be the case for - // Windows.Nano.1809.Amd64.Open where we get the following failure from Assert.Null(config["ContentRoot"]). - // https://dev.azure.com/dnceng/public/_build/results?buildId=1830440&view=ms.vss-test-web.build-test-results-tab&runId=48453394&paneView=debug&resultId=102697 - // - // Assert.Null() Failure - // Expected: (null) - // Actual: C:\ - // - // I'm guessing C:\ is not really the system drive and is in fact the original CWD that couldn't be changed. - // If it was the system drive (and we could detect we're on Windows), then the configuration would be null. - return; - } - IHostBuilder builder = Host.CreateDefaultBuilder(); using IHost host = builder.Build(); var config = host.Services.GetRequiredService(); var env = host.Services.GetRequiredService(); + // Temporary for diagnostics from Windows.Nano.1809.Amd64.Open containers on helix where config["ContentRoot"] is "C:\" + if (config["ContentRoot"] is not null) + { + throw new Exception($"ContentRoot: {config["ContentRoot"]}, originalCwd: {originalCwd}, systemDirectory: {systemDirectory}, Environment.CurrentDirectory: {Environment.CurrentDirectory}, Environment.SpecialFolder.System: {Environment.SpecialFolder.System}"); + } + Assert.Null(config["ContentRoot"]); Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); } From 29616866b16f8dcf3b51f3d6c5be33d45daaa341 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 17 Jun 2022 17:33:42 -0700 Subject: [PATCH 18/21] Use RemoteExecutor --- .../tests/UnitTests/HostTests.cs | 53 +++++++++++-------- ...osoft.Extensions.Hosting.Unit.Tests.csproj | 7 --- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index e48aca9653d582..94f768bf8143cb 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -9,8 +9,10 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; +using Microsoft.DotNet.RemoteExecutor; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Configuration.UserSecrets; using Microsoft.Extensions.DependencyInjection; @@ -46,36 +48,43 @@ public void CreateDefaultBuilder_IncludesContentRootByDefault() Assert.Equal(expected, env.ContentRootPath); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))] + public static bool IsWindowsAndRemotExecutorIsSupported => PlatformDetection.IsWindows && RemoteExecutor.IsSupported; + + [ConditionalFact(typeof(HostTests), nameof(IsWindowsAndRemotExecutorIsSupported))] public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWindowsSystemDirectory() { - string originalCwd = Environment.CurrentDirectory; - // Test that the path gets normalized before comparison. Use C:\WINDOWS\SYSTEM32\ instead of C:\Windows\system32. - string systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System).ToUpper() + "\\"; - - try + using var _ = RemoteExecutor.Invoke(() => { - Environment.CurrentDirectory = systemDirectory; + string originalCwd = Environment.CurrentDirectory; + // Test that the path gets normalized before comparison. Use C:\WINDOWS\SYSTEM32\ instead of C:\Windows\system32. + string systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System).ToUpper() + "\\"; + + try + { + Environment.CurrentDirectory = systemDirectory; - IHostBuilder builder = Host.CreateDefaultBuilder(); - using IHost host = builder.Build(); + IHostBuilder builder = Host.CreateDefaultBuilder(); + using IHost host = builder.Build(); - var config = host.Services.GetRequiredService(); - var env = host.Services.GetRequiredService(); + var config = host.Services.GetRequiredService(); + var env = host.Services.GetRequiredService(); + + // Temporary for diagnostics from Windows.Nano.1809.Amd64.Open containers on helix where config["ContentRoot"] is C:\. + // Maybe this doesn't happen anymore if the container doesn't support RemoteExecutor. + if (config["ContentRoot"] is not null) + { + throw new Exception($"ContentRoot: {config["ContentRoot"]}, originalCwd: {originalCwd}, systemDirectory: {systemDirectory}, Environment.CurrentDirectory: {Environment.CurrentDirectory}, Environment.SpecialFolder.System: {Environment.SpecialFolder.System}"); + } - // Temporary for diagnostics from Windows.Nano.1809.Amd64.Open containers on helix where config["ContentRoot"] is "C:\" - if (config["ContentRoot"] is not null) + Assert.Null(config["ContentRoot"]); + Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); + } + finally { - throw new Exception($"ContentRoot: {config["ContentRoot"]}, originalCwd: {originalCwd}, systemDirectory: {systemDirectory}, Environment.CurrentDirectory: {Environment.CurrentDirectory}, Environment.SpecialFolder.System: {Environment.SpecialFolder.System}"); + // I'm guessing that the RemoteExecutor means that this no longer matters, but I'm leaving it to be safe. + Environment.CurrentDirectory = originalCwd; } - - Assert.Null(config["ContentRoot"]); - Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); - } - finally - { - Environment.CurrentDirectory = originalCwd; - } + }); } [Fact] diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj index c9fc907ab78812..b5c760d44ca83e 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/Microsoft.Extensions.Hosting.Unit.Tests.csproj @@ -7,13 +7,6 @@ true - - - - - - From 2df3de4c3ecffd1faf7cdfcb8b37f5146ebbb431 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 17 Jun 2022 21:16:36 -0700 Subject: [PATCH 19/21] Update src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs --- .../Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index 94f768bf8143cb..8dd30049d9dfbf 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -73,7 +73,7 @@ public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWin // Maybe this doesn't happen anymore if the container doesn't support RemoteExecutor. if (config["ContentRoot"] is not null) { - throw new Exception($"ContentRoot: {config["ContentRoot"]}, originalCwd: {originalCwd}, systemDirectory: {systemDirectory}, Environment.CurrentDirectory: {Environment.CurrentDirectory}, Environment.SpecialFolder.System: {Environment.SpecialFolder.System}"); + throw new Exception($"ContentRoot: {config["ContentRoot"]}, originalCwd: {originalCwd}, systemDirectory: {systemDirectory}, Environment.CurrentDirectory: {Environment.CurrentDirectory}, Environment.SpecialFolder.System: {Environment.GetFolderPath(Environment.SpecialFolder.System)}"); } Assert.Null(config["ContentRoot"]); From ce29cd6e57523d665f3e486a7c98345fb86c585d Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 20 Jun 2022 14:16:55 -0500 Subject: [PATCH 20/21] Skip test on Windows nano server --- .../tests/UnitTests/HostTests.cs | 39 +++++++------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index 8dd30049d9dfbf..c4cb6ce4558108 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -55,35 +55,26 @@ public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWin { using var _ = RemoteExecutor.Invoke(() => { - string originalCwd = Environment.CurrentDirectory; - // Test that the path gets normalized before comparison. Use C:\WINDOWS\SYSTEM32\ instead of C:\Windows\system32. - string systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System).ToUpper() + "\\"; - - try + string systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System); + if (string.IsNullOrEmpty(systemDirectory)) { - Environment.CurrentDirectory = systemDirectory; + // Skip the environments (like Nano Server) where Environment.SpecialFolder.System returns empty - https://github.com/dotnet/runtime/issues/21430 + return; + } - IHostBuilder builder = Host.CreateDefaultBuilder(); - using IHost host = builder.Build(); + // Test that the path gets normalized before comparison. Use C:\WINDOWS\SYSTEM32\ instead of C:\Windows\system32. + systemDirectory = systemDirectory.ToUpper() + "\\"; - var config = host.Services.GetRequiredService(); - var env = host.Services.GetRequiredService(); + Environment.CurrentDirectory = systemDirectory; - // Temporary for diagnostics from Windows.Nano.1809.Amd64.Open containers on helix where config["ContentRoot"] is C:\. - // Maybe this doesn't happen anymore if the container doesn't support RemoteExecutor. - if (config["ContentRoot"] is not null) - { - throw new Exception($"ContentRoot: {config["ContentRoot"]}, originalCwd: {originalCwd}, systemDirectory: {systemDirectory}, Environment.CurrentDirectory: {Environment.CurrentDirectory}, Environment.SpecialFolder.System: {Environment.GetFolderPath(Environment.SpecialFolder.System)}"); - } + IHostBuilder builder = Host.CreateDefaultBuilder(); + using IHost host = builder.Build(); - Assert.Null(config["ContentRoot"]); - Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); - } - finally - { - // I'm guessing that the RemoteExecutor means that this no longer matters, but I'm leaving it to be safe. - Environment.CurrentDirectory = originalCwd; - } + var config = host.Services.GetRequiredService(); + var env = host.Services.GetRequiredService(); + + Assert.Null(config[HostDefaults.ContentRootKey]); + Assert.Equal(AppContext.BaseDirectory, env.ContentRootPath); }); } From 23e903df9fc925014ec4f8102e2c431308eb0739 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Mon, 20 Jun 2022 14:17:03 -0500 Subject: [PATCH 21/21] Respond to PR feedback --- .../src/SystemdHostBuilderExtensions.cs | 5 +++++ .../tests/UseSystemdTests.cs | 2 ++ .../src/WindowsServiceLifetimeHostBuilderExtensions.cs | 4 ++++ 3 files changed, 11 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs index a56d69f385e2bd..5be28337bbcef2 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdHostBuilderExtensions.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting.Systemd; using Microsoft.Extensions.Logging.Console; @@ -31,6 +32,8 @@ public static class SystemdHostBuilderExtensions /// The instance for chaining. public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) { + ThrowHelper.ThrowIfNull(hostBuilder); + if (SystemdHelpers.IsSystemdService()) { hostBuilder.ConfigureServices((hostContext, services) => @@ -64,6 +67,8 @@ public static IHostBuilder UseSystemd(this IHostBuilder hostBuilder) /// The instance for chaining. public static IServiceCollection AddSystemd(this IServiceCollection services) { + ThrowHelper.ThrowIfNull(services); + if (SystemdHelpers.IsSystemdService()) { AddSystemdLifetime(services); diff --git a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs index 2b6e462a76a7b3..948f9b4b527f16 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.Systemd/tests/UseSystemdTests.cs @@ -17,6 +17,7 @@ public void DefaultsToOffOutsideOfService() .Build(); var lifetime = host.Services.GetRequiredService(); + Assert.NotNull(lifetime); Assert.IsNotType(lifetime); } @@ -33,6 +34,7 @@ public void ServiceCollectionExtensionMethodDefaultsToOffOutsideOfService() using IHost host = builder.Build(); var lifetime = host.Services.GetRequiredService(); + Assert.NotNull(lifetime); Assert.IsNotType(lifetime); } } diff --git a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs index d8cef616cefb47..90e8035cadc458 100644 --- a/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs +++ b/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs @@ -44,6 +44,8 @@ public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder) /// The instance for chaining. public static IHostBuilder UseWindowsService(this IHostBuilder hostBuilder, Action configure) { + ThrowHelper.ThrowIfNull(hostBuilder); + if (WindowsServiceHelpers.IsWindowsService()) { hostBuilder.ConfigureServices(services => @@ -92,6 +94,8 @@ public static IServiceCollection AddWindowsService(this IServiceCollection servi /// The instance for chaining. public static IServiceCollection AddWindowsService(this IServiceCollection services, Action configure) { + ThrowHelper.ThrowIfNull(services); + if (WindowsServiceHelpers.IsWindowsService()) { AddWindowsServiceLifetime(services, configure);