Use -e to set env variables (#45844)

Author: tmat

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);
 
     /// <summary>
     /// 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();

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)
         {
@@ -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<string> _buildArguments;
+        private readonly ProjectOptions _hostProjectOptions;
 
         /// <summary>
         /// 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<string> buildArguments)
+        public SessionManager(ProjectLauncher projectLauncher, ProjectOptions hostProjectOptions)
         {
             _projectLauncher = projectLauncher;
-            _buildArguments = buildArguments;
+            _hostProjectOptions = hostProjectOptions;
 
             _service = new AspireServerService(
                 this,
@@ -204,44 +204,64 @@ 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 = projectLaunchInfo.Environment?.Select(e => (e.Key, e.Value))?.ToArray() ?? [],
+                LaunchProfileName = projectLaunchInfo.LaunchProfile,
+                NoLaunchProfile = projectLaunchInfo.DisableLaunchProfile,
+                TargetFramework = _hostProjectOptions.TargetFramework,
+            };
+        }
+
+        // internal for testing
+        internal static IReadOnlyList<string> GetRunCommandArguments(ProjectLaunchRequest projectLaunchInfo, string? hostLaunchProfile)
         {
             var arguments = new List<string>
             {
                 "--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.Arguments != null)
+            if (projectLaunchInfo.DisableLaunchProfile)
+            {
+                arguments.Add("--no-launch-profile");
+            }
+            else if (!string.IsNullOrEmpty(projectLaunchInfo.LaunchProfile))
             {
-                arguments.AddRange(projectLaunchInfo.Arguments);
+                arguments.Add("--launch-profile");
+                arguments.Add(projectLaunchInfo.LaunchProfile);
+            }
+            else if (hostLaunchProfile != null)
+            {
+                arguments.Add("--launch-profile");
+                arguments.Add(hostLaunchProfile);
             }
 
-            return new()
+            if (projectLaunchInfo.Arguments != null)
             {
-                IsRootProject = false,
-                ProjectPath = projectLaunchInfo.ProjectPath,
-                WorkingDirectory = _projectLauncher.EnvironmentOptions.WorkingDirectory, // TODO: Should DCP protocol specify?
-                BuildArguments = _buildArguments, // TODO: Should DCP protocol specify?
-                Command = "run",
-                CommandArguments = arguments,
-                LaunchEnvironmentVariables = projectLaunchInfo.Environment?.Select(kvp => (kvp.Key, kvp.Value)).ToArray() ?? [],
-                LaunchProfileName = projectLaunchInfo.LaunchProfile,
-                NoLaunchProfile = projectLaunchInfo.DisableLaunchProfile,
-                TargetFramework = null, // TODO: Should DCP protocol specify?
-            };
+                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");
+                }
+            }
+
+            return arguments;
         }
     }
 
@@ -250,8 +270,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<string> buildArguments)
+    public IRuntimeProcessLauncher? TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, ProjectOptions hostProjectOptions)
         => projectNode.GetCapabilities().Contains(AppHostProjectCapability)
-            ? new SessionManager(projectLauncher, buildArguments)
+            ? new SessionManager(projectLauncher, hostProjectOptions)
             : null;
 }

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;

src/BuiltInTools/dotnet-watch/EnvironmentVariablesBuilder.cs

@@ -18,13 +18,6 @@ internal sealed class EnvironmentVariablesBuilder
         /// </summary>
         private readonly Dictionary<string, string> _variables = [];
 
-        /// <summary>
-        /// Environment variables passed as directives on command line (dotnet [env:name=value] run).
-        /// Currently, the effect is the same as setting <see cref="_variables"/> due to
-        /// https://github.com/dotnet/sdk/issues/40484
-        /// </summary>
-        private readonly Dictionary<string, string> _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<string, string> 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<string> 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));
+            }
         }
     }
 }

src/BuiltInTools/dotnet-watch/EnvironmentVariables_StartupHook.cs

@@ -13,12 +13,6 @@ public static partial class Names
         /// </summary>
         public const string DotnetWatchHotReloadNamedPipeName = "DOTNET_WATCH_HOTRELOAD_NAMEDPIPE_NAME";
 
-        /// <summary>
-        /// The full path to the process being launched by dotnet run.
-        /// Workaround for https://github.com/dotnet/sdk/issues/40484
-        /// </summary>
-        public const string DotnetWatchHotReloadTargetProcessPath = "DOTNET_WATCH_HOTRELOAD_TARGET_PROCESS_PATH";
-
         /// <summary>
         /// Enables logging from the client delta applier agent.
         /// </summary>

src/BuiltInTools/dotnet-watch/HotReload/IRuntimeProcessLauncherFactory.cs

@@ -12,5 +12,5 @@ namespace Microsoft.DotNet.Watch;
 /// </summary>
 internal interface IRuntimeProcessLauncherFactory
 {
-    public IRuntimeProcessLauncher? TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, IReadOnlyList<string> buildArguments);
+    public IRuntimeProcessLauncher? TryCreate(ProjectGraphNode projectNode, ProjectLauncher projectLauncher, ProjectOptions hostProjectOptions);
 }

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<string>()
+        {
+            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);
 

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();