Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use -e to set env variables #45844

Open
wants to merge 2 commits into
base: release/9.0.2xx
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions src/BuiltInTools/DotNetDeltaApplier/StartupHook.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
Expand Down
88 changes: 57 additions & 31 deletions src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -204,44 +204,70 @@ 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<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.DisableLaunchProfile)
{
arguments.Add("--no-launch-profile");
}
else if (!string.IsNullOrEmpty(projectLaunchInfo.LaunchProfile))
{
arguments.Add("--launch-profile");
arguments.Add(projectLaunchInfo.LaunchProfile);
}
else if (hostLaunchProfile != null)
{
arguments.Add("--launch-profile");
arguments.Add(hostLaunchProfile);
}

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");
}
}

return new()
foreach (var (name, value) in projectLaunchInfo.Environment ?? [])
{
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?
};
arguments.Add("-e");
arguments.Add($"{name}={value}");
}

return arguments;
}
}

Expand All @@ -250,8 +276,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;
}
2 changes: 1 addition & 1 deletion src/BuiltInTools/dotnet-watch/DotNetWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
51 changes: 13 additions & 38 deletions src/BuiltInTools/dotnet-watch/EnvironmentVariablesBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
48 changes: 25 additions & 23 deletions src/BuiltInTools/dotnet-watch/HotReload/ProjectLauncher.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading
Loading