From 2b8c5579e54dab2377095040ac2989da23926a4f Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Wed, 8 Jan 2025 17:59:35 -0800 Subject: [PATCH 1/3] Use -e to set env variables --- .../DotNetDeltaApplier/StartupHook.cs | 11 ---- .../Aspire/AspireServiceFactory.cs | 62 ++++++++++++------- .../dotnet-watch/DotNetWatcher.cs | 2 +- .../EnvironmentVariablesBuilder.cs | 51 ++++----------- .../EnvironmentVariables_StartupHook.cs | 6 -- .../IRuntimeProcessLauncherFactory.cs | 2 +- .../dotnet-watch/HotReload/ProjectLauncher.cs | 48 +++++++------- .../dotnet-watch/HotReloadDotNetWatcher.cs | 2 +- ...cs => EnvironmentVariablesBuilderTests.cs} | 22 ++++--- .../Utilities/TestRuntimeProcessLauncher.cs | 2 +- 10 files changed, 95 insertions(+), 113 deletions(-) rename test/dotnet-watch.Tests/Internal/{EnvironmentVariablesBuilderTest.cs => EnvironmentVariablesBuilderTests.cs} (53%) diff --git a/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs b/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs index 34e11d46f42c..33d13ed999cf 100644 --- a/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs +++ b/src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs @@ -12,7 +12,6 @@ internal sealed class StartupHook { private static readonly bool s_logToStandardOutput = Environment.GetEnvironmentVariable(EnvironmentVariables.Names.HotReloadDeltaClientLogMessages) == "1"; private static readonly string s_namedPipeName = Environment.GetEnvironmentVariable(EnvironmentVariables.Names.DotnetWatchHotReloadNamedPipeName); - private static readonly string s_targetProcessPath = Environment.GetEnvironmentVariable(EnvironmentVariables.Names.DotnetWatchHotReloadTargetProcessPath); /// /// Invoked by the runtime when the containing assembly is listed in DOTNET_STARTUP_HOOKS. @@ -21,16 +20,6 @@ public static void Initialize() { var processPath = Environment.GetCommandLineArgs().FirstOrDefault(); - // Workaround for https://github.com/dotnet/sdk/issues/40484 - // When launching the application process dotnet-watch sets Hot Reload environment variables via CLI environment directives (dotnet [env:X=Y] run). - // Currently, the CLI parser sets the env variables to the dotnet.exe process itself, rather then to the target process. - // This may cause the dotnet.exe process to connect to the named pipe and break it for the target process. - if (!IsMatchingProcess(processPath, s_targetProcessPath)) - { - Log($"Ignoring process '{processPath}', expecting '{s_targetProcessPath}'"); - return; - } - Log($"Loaded into process: {processPath}"); ClearHotReloadEnvironmentVariables(); diff --git a/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs b/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs index d806dd11ba45..8dc1ead1645e 100644 --- a/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs +++ b/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs @@ -30,7 +30,7 @@ private readonly struct Session(string dcpId, string sessionId, RunningProject r private readonly ProjectLauncher _projectLauncher; private readonly AspireServerService _service; - private readonly IReadOnlyList _buildArguments; + private readonly ProjectOptions _hostProjectOptions; /// /// Lock to access: @@ -43,10 +43,10 @@ private readonly struct Session(string dcpId, string sessionId, RunningProject r private int _sessionIdDispenser; private volatile bool _isDisposed; - public SessionManager(ProjectLauncher projectLauncher, IReadOnlyList buildArguments) + public SessionManager(ProjectLauncher projectLauncher, ProjectOptions hostProjectOptions) { _projectLauncher = projectLauncher; - _buildArguments = buildArguments; + _hostProjectOptions = hostProjectOptions; _service = new AspireServerService( this, @@ -209,38 +209,56 @@ private ProjectOptions GetProjectOptions(ProjectLaunchRequest projectLaunchInfo) { "--project", projectLaunchInfo.ProjectPath, - // TODO: https://github.com/dotnet/sdk/issues/43946 - // Need to suppress launch profile for now, otherwise it would override the port set via env variable. - "--no-launch-profile", }; - //if (projectLaunchInfo.DisableLaunchProfile) - //{ - // arguments.Add("--no-launch-profile"); - //} - //else if (!string.IsNullOrEmpty(projectLaunchInfo.LaunchProfile)) - //{ - // arguments.Add("--launch-profile"); - // arguments.Add(projectLaunchInfo.LaunchProfile); - //} + // Implements https://github.com/dotnet/aspire/blob/main/docs/specs/IDE-execution.md#launch-profile-processing-project-launch-configuration + + if (projectLaunchInfo.DisableLaunchProfile) + { + arguments.Add("--no-launch-profile"); + } + else if (!string.IsNullOrEmpty(projectLaunchInfo.LaunchProfile)) + { + arguments.Add("--launch-profile"); + arguments.Add(projectLaunchInfo.LaunchProfile); + } + else if (!_hostProjectOptions.NoLaunchProfile && _hostProjectOptions.LaunchProfileName != null) + { + arguments.Add("--launch-profile"); + arguments.Add(_hostProjectOptions.LaunchProfileName); + } if (projectLaunchInfo.Arguments != null) { - arguments.AddRange(projectLaunchInfo.Arguments); + if (projectLaunchInfo.Arguments.Any()) + { + arguments.AddRange(projectLaunchInfo.Arguments); + } + else + { + // indicate that no arguments should be used even if launch profile specifies some: + arguments.Add("--no-launch-profile-arguments"); + } + } + + foreach (var (name, value) in projectLaunchInfo.Environment ?? []) + { + arguments.Add("-e"); + arguments.Add($"{name}={value}"); } return new() { IsRootProject = false, ProjectPath = projectLaunchInfo.ProjectPath, - WorkingDirectory = _projectLauncher.EnvironmentOptions.WorkingDirectory, // TODO: Should DCP protocol specify? - BuildArguments = _buildArguments, // TODO: Should DCP protocol specify? + WorkingDirectory = _projectLauncher.EnvironmentOptions.WorkingDirectory, + BuildArguments = _hostProjectOptions.BuildArguments, Command = "run", CommandArguments = arguments, - LaunchEnvironmentVariables = projectLaunchInfo.Environment?.Select(kvp => (kvp.Key, kvp.Value)).ToArray() ?? [], + LaunchEnvironmentVariables = [], LaunchProfileName = projectLaunchInfo.LaunchProfile, NoLaunchProfile = projectLaunchInfo.DisableLaunchProfile, - TargetFramework = null, // TODO: Should DCP protocol specify? + TargetFramework = _hostProjectOptions.TargetFramework, }; } } @@ -250,8 +268,8 @@ private ProjectOptions GetProjectOptions(ProjectLaunchRequest projectLaunchInfo) public static readonly AspireServiceFactory Instance = new(); public const string AppHostProjectCapability = "Aspire"; - public IRuntimeProcessLauncher? TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, IReadOnlyList buildArguments) + public IRuntimeProcessLauncher? TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, ProjectOptions hostProjectOptions) => projectNode.GetCapabilities().Contains(AppHostProjectCapability) - ? new SessionManager(projectLauncher, buildArguments) + ? new SessionManager(projectLauncher, hostProjectOptions) : null; } diff --git a/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs b/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs index 321ad2924813..8977641435c9 100644 --- a/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/DotNetWatcher.cs @@ -65,7 +65,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke ? await browserConnector.LaunchOrRefreshBrowserAsync(projectRootNode, processSpec, environmentBuilder, Context.RootProjectOptions, shutdownCancellationToken) : null; - environmentBuilder.ConfigureProcess(processSpec); + environmentBuilder.SetProcessEnvironmentVariables(processSpec); // Reset for next run buildEvaluator.RequiresRevaluation = false; diff --git a/src/BuiltInTools/dotnet-watch/EnvironmentVariablesBuilder.cs b/src/BuiltInTools/dotnet-watch/EnvironmentVariablesBuilder.cs index aec5634a4acf..987e9910ab31 100644 --- a/src/BuiltInTools/dotnet-watch/EnvironmentVariablesBuilder.cs +++ b/src/BuiltInTools/dotnet-watch/EnvironmentVariablesBuilder.cs @@ -18,13 +18,6 @@ internal sealed class EnvironmentVariablesBuilder /// private readonly Dictionary _variables = []; - /// - /// Environment variables passed as directives on command line (dotnet [env:name=value] run). - /// Currently, the effect is the same as setting due to - /// https://github.com/dotnet/sdk/issues/40484 - /// - private readonly Dictionary _directives = []; - public static EnvironmentVariablesBuilder FromCurrentEnvironment() { var builder = new EnvironmentVariablesBuilder(); @@ -42,57 +35,39 @@ public static EnvironmentVariablesBuilder FromCurrentEnvironment() return builder; } - public void SetDirective(string name, string value) - { - // should use DotNetStartupHookDirective - Debug.Assert(!name.Equals(EnvironmentVariables.Names.DotnetStartupHooks, StringComparison.OrdinalIgnoreCase)); - - _directives[name] = value; - } - public void SetVariable(string name, string value) { - // should use AspNetCoreHostingStartupAssembliesVariable + // should use AspNetCoreHostingStartupAssembliesVariable/DotNetStartupHookDirective Debug.Assert(!name.Equals(EnvironmentVariables.Names.AspNetCoreHostingStartupAssemblies, StringComparison.OrdinalIgnoreCase)); + Debug.Assert(!name.Equals(EnvironmentVariables.Names.DotnetStartupHooks, StringComparison.OrdinalIgnoreCase)); _variables[name] = value; } - public void ConfigureProcess(ProcessSpec processSpec) + public void SetProcessEnvironmentVariables(ProcessSpec processSpec) { - processSpec.Arguments = [.. GetCommandLineDirectives(), .. processSpec.Arguments ?? []]; - AddToEnvironment(processSpec.EnvironmentVariables); - } - - // for testing - internal void AddToEnvironment(Dictionary variables) - { - foreach (var (name, value) in _variables) - { - variables.Add(name, value); - } - - if (AspNetCoreHostingStartupAssembliesVariable is not []) + foreach (var (name, value) in GetEnvironment()) { - variables.Add(EnvironmentVariables.Names.AspNetCoreHostingStartupAssemblies, string.Join(AssembliesSeparator, AspNetCoreHostingStartupAssembliesVariable)); + processSpec.EnvironmentVariables.Add(name, value); } } - // for testing - internal IEnumerable GetCommandLineDirectives() + public IEnumerable<(string name, string value)> GetEnvironment() { - foreach (var (name, value) in _directives) + foreach (var (name, value) in _variables) { - yield return MakeDirective(name, value); + yield return (name, value); } if (DotNetStartupHookDirective is not []) { - yield return MakeDirective(EnvironmentVariables.Names.DotnetStartupHooks, string.Join(s_startupHooksSeparator, DotNetStartupHookDirective)); + yield return (EnvironmentVariables.Names.DotnetStartupHooks, string.Join(s_startupHooksSeparator, DotNetStartupHookDirective)); } - static string MakeDirective(string name, string value) - => $"[env:{name}={value}]"; + if (AspNetCoreHostingStartupAssembliesVariable is not []) + { + yield return (EnvironmentVariables.Names.AspNetCoreHostingStartupAssemblies, string.Join(AssembliesSeparator, AspNetCoreHostingStartupAssembliesVariable)); + } } } } diff --git a/src/BuiltInTools/dotnet-watch/EnvironmentVariables_StartupHook.cs b/src/BuiltInTools/dotnet-watch/EnvironmentVariables_StartupHook.cs index 6a9191c1dab7..7804e4b358fb 100644 --- a/src/BuiltInTools/dotnet-watch/EnvironmentVariables_StartupHook.cs +++ b/src/BuiltInTools/dotnet-watch/EnvironmentVariables_StartupHook.cs @@ -13,12 +13,6 @@ public static partial class Names /// public const string DotnetWatchHotReloadNamedPipeName = "DOTNET_WATCH_HOTRELOAD_NAMEDPIPE_NAME"; - /// - /// The full path to the process being launched by dotnet run. - /// Workaround for https://github.com/dotnet/sdk/issues/40484 - /// - public const string DotnetWatchHotReloadTargetProcessPath = "DOTNET_WATCH_HOTRELOAD_TARGET_PROCESS_PATH"; - /// /// Enables logging from the client delta applier agent. /// diff --git a/src/BuiltInTools/dotnet-watch/HotReload/IRuntimeProcessLauncherFactory.cs b/src/BuiltInTools/dotnet-watch/HotReload/IRuntimeProcessLauncherFactory.cs index 431e79830ae3..93d69f69db59 100644 --- a/src/BuiltInTools/dotnet-watch/HotReload/IRuntimeProcessLauncherFactory.cs +++ b/src/BuiltInTools/dotnet-watch/HotReload/IRuntimeProcessLauncherFactory.cs @@ -12,5 +12,5 @@ namespace Microsoft.DotNet.Watch; /// internal interface IRuntimeProcessLauncherFactory { - public IRuntimeProcessLauncher? TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, IReadOnlyList buildArguments); + public IRuntimeProcessLauncher? TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, ProjectOptions hostProjectOptions); } diff --git a/src/BuiltInTools/dotnet-watch/HotReload/ProjectLauncher.cs b/src/BuiltInTools/dotnet-watch/HotReload/ProjectLauncher.cs index 46a33b2b301d..4befce53dc52 100644 --- a/src/BuiltInTools/dotnet-watch/HotReload/ProjectLauncher.cs +++ b/src/BuiltInTools/dotnet-watch/HotReload/ProjectLauncher.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.Diagnostics; using System.Globalization; using Microsoft.Build.Graph; @@ -53,17 +54,12 @@ public EnvironmentOptions EnvironmentOptions { Executable = EnvironmentOptions.MuxerPath, WorkingDirectory = projectOptions.WorkingDirectory, - OnOutput = onOutput, - Arguments = [projectOptions.Command, "--no-build", .. projectOptions.CommandArguments] + OnOutput = onOutput }; var environmentBuilder = EnvironmentVariablesBuilder.FromCurrentEnvironment(); var namedPipeName = Guid.NewGuid().ToString(); - // Directives: - - // Variables: - foreach (var (name, value) in projectOptions.LaunchEnvironmentVariables) { // ignore dotnet-watch reserved variables -- these shouldn't be set by the project @@ -85,30 +81,36 @@ public EnvironmentOptions EnvironmentOptions // expect DOTNET_MODIFIABLE_ASSEMBLIES to be set in the blazor-devserver process, even though we are not performing Hot Reload in this process. // The value is converted to DOTNET-MODIFIABLE-ASSEMBLIES header, which is in turn converted back to environment variable in Mono browser runtime loader: // https://github.com/dotnet/runtime/blob/342936c5a88653f0f622e9d6cb727a0e59279b31/src/mono/browser/runtime/loader/config.ts#L330 - environmentBuilder.SetDirective(EnvironmentVariables.Names.DotnetModifiableAssemblies, "debug"); + environmentBuilder.SetVariable(EnvironmentVariables.Names.DotnetModifiableAssemblies, "debug"); if (injectDeltaApplier) { environmentBuilder.DotNetStartupHookDirective.Add(DeltaApplier.StartupHookPath); - environmentBuilder.SetDirective(EnvironmentVariables.Names.DotnetWatchHotReloadNamedPipeName, namedPipeName); - - // Do not ask agent to log to stdout until https://github.com/dotnet/sdk/issues/40484 is fixed. - // For now we need to set the env variable explicitly when we need to diagnose issue with the agent. - // Build targets might launch a process and read it's stdout. If the agent is loaded into such process and starts logging - // to stdout it might interfere with the expected output. - //if (context.Options.Verbose) - //{ - // environmentBuilder.SetVariable(EnvironmentVariables.Names.HotReloadDeltaClientLogMessages, "1"); - //} - - // TODO: workaround for https://github.com/dotnet/sdk/issues/40484 - var targetPath = projectNode.ProjectInstance.GetPropertyValue("RunCommand"); - environmentBuilder.SetVariable(EnvironmentVariables.Names.DotnetWatchHotReloadTargetProcessPath, targetPath); - Reporter.Verbose($"Target process is '{targetPath}'"); + environmentBuilder.SetVariable(EnvironmentVariables.Names.DotnetWatchHotReloadNamedPipeName, namedPipeName); + + if (context.Options.Verbose) + { + environmentBuilder.SetVariable(EnvironmentVariables.Names.HotReloadDeltaClientLogMessages, "1"); + } } var browserRefreshServer = await browserConnector.LaunchOrRefreshBrowserAsync(projectNode, processSpec, environmentBuilder, projectOptions, cancellationToken); - environmentBuilder.ConfigureProcess(processSpec); + + var arguments = new List() + { + projectOptions.Command, + "--no-build" + }; + + foreach (var (name, value) in environmentBuilder.GetEnvironment()) + { + arguments.Add("-e"); + arguments.Add($"{name}={value}"); + } + + arguments.AddRange(projectOptions.CommandArguments); + + processSpec.Arguments = arguments; var processReporter = new ProjectSpecificReporter(projectNode, Reporter); diff --git a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs index 4402c1fbf742..563fb89fc7f8 100644 --- a/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs +++ b/src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs @@ -111,7 +111,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke var rootProjectNode = evaluationResult.ProjectGraph.GraphRoots.Single(); - runtimeProcessLauncher = runtimeProcessLauncherFactory?.TryCreate(rootProjectNode, projectLauncher, rootProjectOptions.BuildArguments); + runtimeProcessLauncher = runtimeProcessLauncherFactory?.TryCreate(rootProjectNode, projectLauncher, rootProjectOptions); if (runtimeProcessLauncher != null) { var launcherEnvironment = runtimeProcessLauncher.GetEnvironmentVariables(); diff --git a/test/dotnet-watch.Tests/Internal/EnvironmentVariablesBuilderTest.cs b/test/dotnet-watch.Tests/Internal/EnvironmentVariablesBuilderTests.cs similarity index 53% rename from test/dotnet-watch.Tests/Internal/EnvironmentVariablesBuilderTest.cs rename to test/dotnet-watch.Tests/Internal/EnvironmentVariablesBuilderTests.cs index 75941f6762a6..b9b4b29ccce1 100644 --- a/test/dotnet-watch.Tests/Internal/EnvironmentVariablesBuilderTest.cs +++ b/test/dotnet-watch.Tests/Internal/EnvironmentVariablesBuilderTests.cs @@ -3,7 +3,7 @@ namespace Microsoft.DotNet.Watch.UnitTests { - public class EnvironmentVariablesBuilderTest + public class EnvironmentVariablesBuilderTests { [Fact] public void Value() @@ -12,10 +12,12 @@ public void Value() builder.DotNetStartupHookDirective.Add("a"); builder.AspNetCoreHostingStartupAssembliesVariable.Add("b"); - var values = new Dictionary(); - builder.AddToEnvironment(values); - AssertEx.SequenceEqual(["[env:DOTNET_STARTUP_HOOKS=a]"], builder.GetCommandLineDirectives()); - AssertEx.SequenceEqual([("ASPNETCORE_HOSTINGSTARTUPASSEMBLIES", "b")], values.Select(e => (e.Key, e.Value))); + var env = builder.GetEnvironment(); + AssertEx.SequenceEqual( + [ + ("DOTNET_STARTUP_HOOKS", "a"), + ("ASPNETCORE_HOSTINGSTARTUPASSEMBLIES", "b") + ], env); } [Fact] @@ -27,10 +29,12 @@ public void MultipleValues() builder.AspNetCoreHostingStartupAssembliesVariable.Add("b1"); builder.AspNetCoreHostingStartupAssembliesVariable.Add("b2"); - var values = new Dictionary(); - builder.AddToEnvironment(values); - AssertEx.SequenceEqual([$"[env:DOTNET_STARTUP_HOOKS=a1{Path.PathSeparator}a2]"], builder.GetCommandLineDirectives()); - AssertEx.SequenceEqual([("ASPNETCORE_HOSTINGSTARTUPASSEMBLIES", "b1;b2")], values.Select(e => (e.Key, e.Value))); + var env = builder.GetEnvironment(); + AssertEx.SequenceEqual( + [ + ("DOTNET_STARTUP_HOOKS", $"a1{Path.PathSeparator}a2"), + ("ASPNETCORE_HOSTINGSTARTUPASSEMBLIES", "b1;b2") + ], env); } } } diff --git a/test/dotnet-watch.Tests/Utilities/TestRuntimeProcessLauncher.cs b/test/dotnet-watch.Tests/Utilities/TestRuntimeProcessLauncher.cs index b4714791cc86..c10d2291f512 100644 --- a/test/dotnet-watch.Tests/Utilities/TestRuntimeProcessLauncher.cs +++ b/test/dotnet-watch.Tests/Utilities/TestRuntimeProcessLauncher.cs @@ -11,7 +11,7 @@ internal class TestRuntimeProcessLauncher(ProjectLauncher projectLauncher) : IRu { public class Factory(Action? initialize = null) : IRuntimeProcessLauncherFactory { - public IRuntimeProcessLauncher TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, IReadOnlyList buildArguments) + public IRuntimeProcessLauncher TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, ProjectOptions hostProjectOptions) { var service = new TestRuntimeProcessLauncher(projectLauncher); initialize?.Invoke(service); From 518a4fabce96871da45e68a41d641ceeaa0eb7dc Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Thu, 9 Jan 2025 15:58:41 -0800 Subject: [PATCH 2/3] Test --- .../Aspire/AspireServiceFactory.cs | 40 +++-- .../Aspire/AspireServiceFactoryTests.cs | 169 ++++++++++++++++++ 2 files changed, 193 insertions(+), 16 deletions(-) create mode 100644 test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs diff --git a/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs b/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs index 8dc1ead1645e..8eab6ce2df9f 100644 --- a/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs +++ b/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs @@ -12,7 +12,7 @@ namespace Microsoft.DotNet.Watch; internal class AspireServiceFactory : IRuntimeProcessLauncherFactory { - private sealed class SessionManager : IAspireServerEvents, IRuntimeProcessLauncher + internal sealed class SessionManager : IAspireServerEvents, IRuntimeProcessLauncher { private readonly struct Session(string dcpId, string sessionId, RunningProject runningProject, Task outputReader) { @@ -204,6 +204,26 @@ private async ValueTask TerminateSessionAsync(Session session, CancellationToken } private ProjectOptions GetProjectOptions(ProjectLaunchRequest projectLaunchInfo) + { + var hostLaunchProfile = _hostProjectOptions.NoLaunchProfile ? null : _hostProjectOptions.LaunchProfileName; + + return new() + { + IsRootProject = false, + ProjectPath = projectLaunchInfo.ProjectPath, + WorkingDirectory = _projectLauncher.EnvironmentOptions.WorkingDirectory, + BuildArguments = _hostProjectOptions.BuildArguments, + Command = "run", + CommandArguments = GetRunCommandArguments(projectLaunchInfo, hostLaunchProfile), + LaunchEnvironmentVariables = [], + LaunchProfileName = projectLaunchInfo.LaunchProfile, + NoLaunchProfile = projectLaunchInfo.DisableLaunchProfile, + TargetFramework = _hostProjectOptions.TargetFramework, + }; + } + + // internal for testing + internal static IReadOnlyList GetRunCommandArguments(ProjectLaunchRequest projectLaunchInfo, string? hostLaunchProfile) { var arguments = new List { @@ -222,10 +242,10 @@ private ProjectOptions GetProjectOptions(ProjectLaunchRequest projectLaunchInfo) arguments.Add("--launch-profile"); arguments.Add(projectLaunchInfo.LaunchProfile); } - else if (!_hostProjectOptions.NoLaunchProfile && _hostProjectOptions.LaunchProfileName != null) + else if (hostLaunchProfile != null) { arguments.Add("--launch-profile"); - arguments.Add(_hostProjectOptions.LaunchProfileName); + arguments.Add(hostLaunchProfile); } if (projectLaunchInfo.Arguments != null) @@ -247,19 +267,7 @@ private ProjectOptions GetProjectOptions(ProjectLaunchRequest projectLaunchInfo) arguments.Add($"{name}={value}"); } - return new() - { - IsRootProject = false, - ProjectPath = projectLaunchInfo.ProjectPath, - WorkingDirectory = _projectLauncher.EnvironmentOptions.WorkingDirectory, - BuildArguments = _hostProjectOptions.BuildArguments, - Command = "run", - CommandArguments = arguments, - LaunchEnvironmentVariables = [], - LaunchProfileName = projectLaunchInfo.LaunchProfile, - NoLaunchProfile = projectLaunchInfo.DisableLaunchProfile, - TargetFramework = _hostProjectOptions.TargetFramework, - }; + return arguments; } } diff --git a/test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs b/test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs new file mode 100644 index 000000000000..b05fab366e0a --- /dev/null +++ b/test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs @@ -0,0 +1,169 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using Aspire.Tools.Service; + +namespace Microsoft.DotNet.Watch.UnitTests; + +public class AspireServiceFactoryTests +{ + [Fact] + public void GetRunCommandArguments_Empty() + { + var request = new ProjectLaunchRequest() + { + Arguments = null, + DisableLaunchProfile = false, + LaunchProfile = null, + Environment = null, + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: null); + + AssertEx.SequenceEqual(["--project", "a.csproj"], args); + } + + [Fact] + public void GetRunCommandArguments_DisableLaunchProfile() + { + var request = new ProjectLaunchRequest() + { + Arguments = null, + DisableLaunchProfile = true, + LaunchProfile = "P", + Environment = [], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: "H"); + + AssertEx.SequenceEqual(["--project", "a.csproj", "--no-launch-profile" ], args); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + public void GetRunCommandArguments_NoLaunchProfile_HostProfile(string? launchProfile) + { + var request = new ProjectLaunchRequest() + { + Arguments = null, + DisableLaunchProfile = false, + LaunchProfile = launchProfile, + Environment = [], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: "H"); + + AssertEx.SequenceEqual(["--project", "a.csproj", "--launch-profile", "H"], args); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + public void GetRunCommandArguments_DisableLaunchProfile_HostProfile(string? launchProfile) + { + var request = new ProjectLaunchRequest() + { + Arguments = null, + DisableLaunchProfile = true, + LaunchProfile = launchProfile, + Environment = [], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: "H"); + + AssertEx.SequenceEqual(["--project", "a.csproj", "--no-launch-profile"], args); + } + + [Theory] + [InlineData("")] + [InlineData(null)] + public void GetRunCommandArguments_NoLaunchProfile_NoHostProfile(string? launchProfile) + { + var request = new ProjectLaunchRequest() + { + Arguments = null, + DisableLaunchProfile = false, + LaunchProfile = launchProfile, + Environment = [], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: null); + + AssertEx.SequenceEqual(["--project", "a.csproj"], args); + } + [Fact] + public void GetRunCommandArguments_LaunchProfile_NoArgs() + { + var request = new ProjectLaunchRequest() + { + Arguments = null, + DisableLaunchProfile = false, + LaunchProfile = "P", + Environment = [], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: "H"); + + AssertEx.SequenceEqual(["--project", "a.csproj", "--launch-profile", "P"], args); + } + + [Fact] + public void GetRunCommandArguments_LaunchProfile_EmptyArgs() + { + var request = new ProjectLaunchRequest() + { + Arguments = [], + DisableLaunchProfile = false, + LaunchProfile = "P", + Environment = [], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: "H"); + + AssertEx.SequenceEqual(["--project", "a.csproj", "--launch-profile", "P", "--no-launch-profile-arguments"], args); + } + + [Fact] + public void GetRunCommandArguments_LaunchProfile_NonEmptyArgs() + { + var request = new ProjectLaunchRequest() + { + Arguments = ["a", "b"], + DisableLaunchProfile = false, + LaunchProfile = "P", + Environment = [], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: "H"); + + AssertEx.SequenceEqual(["--project", "a.csproj", "--launch-profile", "P", "a", "b"], args); + } + + [Fact] + public void GetRunCommandArguments_EnvironmentVariables() + { + var request = new ProjectLaunchRequest() + { + Arguments = null, + DisableLaunchProfile = false, + LaunchProfile = null, + Environment = [KeyValuePair.Create("X", "1"), KeyValuePair.Create("Y", ""), KeyValuePair.Create("Z", "=")], + ProjectPath = "a.csproj" + }; + + var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: null); + + AssertEx.SequenceEqual(["--project", "a.csproj", "-e", "X=1", "-e", "Y=", "-e", "Z=="], args); + } +} From 2f028223cf0307870c584fed41fbee42fd5c41ff Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Fri, 10 Jan 2025 10:20:58 -0800 Subject: [PATCH 3/3] Fix --- .../dotnet-watch/Aspire/AspireServiceFactory.cs | 8 +------- .../Aspire/AspireServiceFactoryTests.cs | 17 ----------------- 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs b/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs index 8eab6ce2df9f..158a49274bf6 100644 --- a/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs +++ b/src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs @@ -215,7 +215,7 @@ private ProjectOptions GetProjectOptions(ProjectLaunchRequest projectLaunchInfo) BuildArguments = _hostProjectOptions.BuildArguments, Command = "run", CommandArguments = GetRunCommandArguments(projectLaunchInfo, hostLaunchProfile), - LaunchEnvironmentVariables = [], + LaunchEnvironmentVariables = projectLaunchInfo.Environment?.Select(e => (e.Key, e.Value))?.ToArray() ?? [], LaunchProfileName = projectLaunchInfo.LaunchProfile, NoLaunchProfile = projectLaunchInfo.DisableLaunchProfile, TargetFramework = _hostProjectOptions.TargetFramework, @@ -261,12 +261,6 @@ internal static IReadOnlyList GetRunCommandArguments(ProjectLaunchReques } } - foreach (var (name, value) in projectLaunchInfo.Environment ?? []) - { - arguments.Add("-e"); - arguments.Add($"{name}={value}"); - } - return arguments; } } diff --git a/test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs b/test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs index b05fab366e0a..084fcf8844a3 100644 --- a/test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs +++ b/test/dotnet-watch.Tests/Aspire/AspireServiceFactoryTests.cs @@ -149,21 +149,4 @@ public void GetRunCommandArguments_LaunchProfile_NonEmptyArgs() AssertEx.SequenceEqual(["--project", "a.csproj", "--launch-profile", "P", "a", "b"], args); } - - [Fact] - public void GetRunCommandArguments_EnvironmentVariables() - { - var request = new ProjectLaunchRequest() - { - Arguments = null, - DisableLaunchProfile = false, - LaunchProfile = null, - Environment = [KeyValuePair.Create("X", "1"), KeyValuePair.Create("Y", ""), KeyValuePair.Create("Z", "=")], - ProjectPath = "a.csproj" - }; - - var args = AspireServiceFactory.SessionManager.GetRunCommandArguments(request, hostLaunchProfile: null); - - AssertEx.SequenceEqual(["--project", "a.csproj", "-e", "X=1", "-e", "Y=", "-e", "Z=="], args); - } }