Skip to content

Commit e564c6d

Browse files
tmatDustinCampbell
andauthored
Avoid redirecting process output (#53539)
Co-authored-by: Dustin Campbell <dustin@teamcampbell.org>
1 parent 6e321aa commit e564c6d

File tree

5 files changed

+40
-26
lines changed

5 files changed

+40
-26
lines changed

src/Dotnet.Watch/Watch/Browser/BrowserLauncher.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,9 @@ internal sealed class BrowserLauncher(ILogger logger, IProcessOutputReporter pro
1616
private ImmutableHashSet<ProjectInstanceId> _browserLaunchAttempted = [];
1717

1818
/// <summary>
19-
/// Installs browser launch/reload trigger.
19+
/// Returns an output observing action that triggers the launch of the browser, or null if the browser should not be launched.
2020
/// </summary>
21-
public void InstallBrowserLaunchTrigger(
22-
ProcessSpec processSpec,
21+
public Action<OutputLine>? TryGetBrowserLaunchOutputObserver(
2322
ProjectGraphNode projectNode,
2423
ProjectOptions projectOptions,
2524
AbstractBrowserRefreshServer? server,
@@ -32,10 +31,10 @@ public void InstallBrowserLaunchTrigger(
3231
logger.LogError("Test requires browser to launch");
3332
}
3433

35-
return;
34+
return null;
3635
}
3736

38-
WebServerProcessStateObserver.Observe(projectNode, processSpec, url =>
37+
return WebServerProcessStateObserver.GetObserver(projectNode, url =>
3938
{
4039
if (projectOptions.IsMainProject &&
4140
ImmutableInterlocked.Update(ref _browserLaunchAttempted, static (set, key) => set.Add(key), projectNode.ProjectInstance.GetId()))

src/Dotnet.Watch/Watch/Process/ProcessSpec.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,25 @@ internal sealed class ProcessSpec
2424

2525
public string GetArgumentsDisplay()
2626
=> CommandLineUtilities.JoinArguments(Arguments ?? []);
27+
28+
/// <summary>
29+
/// Stream output lines to the process output reporter when
30+
/// - output observer is installed so that the output is also streamd to the console;
31+
/// - testing to synchonize the output of the process with the logger output, so that the printed lines don't interleave;
32+
/// unless the caller has already redirected the output (e.g. for Aspire child processes).
33+
///
34+
/// Do not redirect output otherwise as it disables the ability of the process to use Console APIs.
35+
/// </summary>
36+
public void RedirectOutput(Action<OutputLine>? outputObserver, IProcessOutputReporter outputReporter, EnvironmentOptions environmentOptions, string projectDisplayName)
37+
{
38+
if (environmentOptions.RunningAsTest || outputObserver != null)
39+
{
40+
OnOutput ??= line =>
41+
{
42+
outputReporter.ReportOutput(outputReporter.PrefixProcessOutput ? line with { Content = $"[{projectDisplayName}] {line.Content}" } : line);
43+
};
44+
45+
OnOutput += outputObserver;
46+
}
47+
}
2748
}

src/Dotnet.Watch/Watch/Process/ProjectLauncher.cs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,6 @@ public CompilationHandler CompilationHandler
6060
OnExit = onExit,
6161
};
6262

63-
// Stream output lines to the process output reporter.
64-
// The reporter synchronizes the output of the process with the logger output,
65-
// so that the printed lines don't interleave.
66-
// Only send the output to the reporter if no custom output handler was provided (e.g. for Aspire child processes).
67-
processSpec.OnOutput ??= line =>
68-
{
69-
context.ProcessOutputReporter.ReportOutput(context.ProcessOutputReporter.PrefixProcessOutput ? line with { Content = $"[{projectDisplayName}] {line.Content}" } : line);
70-
};
71-
7263
var environmentBuilder = new Dictionary<string, string>();
7364

7465
// initialize with project settings:
@@ -91,9 +82,10 @@ public CompilationHandler CompilationHandler
9182

9283
processSpec.Arguments = GetProcessArguments(projectOptions, environmentBuilder);
9384

94-
// Attach trigger to the process that detects when the web server reports to the output that it's listening.
95-
// Launches browser on the URL found in the process output for root projects.
96-
context.BrowserLauncher.InstallBrowserLaunchTrigger(processSpec, projectNode, projectOptions, clients.BrowserRefreshServer, cancellationToken);
85+
// Observes main project process output and launches browser when the URL is found in the output.
86+
var outputObserver = context.BrowserLauncher.TryGetBrowserLaunchOutputObserver(projectNode, projectOptions, clients.BrowserRefreshServer, cancellationToken);
87+
88+
processSpec.RedirectOutput(outputObserver, context.ProcessOutputReporter, context.EnvironmentOptions, projectDisplayName);
9789

9890
return await compilationHandler.TrackRunningProjectAsync(
9991
projectNode,

src/Dotnet.Watch/Watch/Process/WebServerProcessStateObserver.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ internal static partial class WebServerProcessStateObserver
2121
[GeneratedRegex(@"Login to the dashboard at (?<url>.*)\s*$", RegexOptions.Compiled)]
2222
private static partial Regex GetAspireDashboardUrlRegex();
2323

24-
public static void Observe(ProjectGraphNode serverProject, ProcessSpec serverProcessSpec, Action<string> onServerListening)
24+
public static Action<OutputLine> GetObserver(ProjectGraphNode serverProject, Action<string> onServerListening)
2525
{
2626
// Workaround for Aspire dashboard launching: scan for "Login to the dashboard at " prefix in the output and use the URL.
2727
// TODO: https://github.com/dotnet/sdk/issues/9038
@@ -30,7 +30,7 @@ public static void Observe(ProjectGraphNode serverProject, ProcessSpec serverPro
3030

3131
var _notified = false;
3232

33-
serverProcessSpec.OnOutput += line =>
33+
return line =>
3434
{
3535
if (_notified)
3636
{

src/Dotnet.Watch/dotnet-watch/Watch/DotNetWatcher.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ public static async Task WatchAsync(DotNetWatchContext context, CancellationToke
5858
{
5959
[EnvironmentVariables.Names.DotnetWatch] = "1",
6060
[EnvironmentVariables.Names.DotnetWatchIteration] = (iteration + 1).ToString(CultureInfo.InvariantCulture),
61-
},
62-
OnOutput = line => context.ProcessOutputReporter.ReportOutput(line)
61+
}
6362
};
6463

6564
var browserRefreshServer = projectRootNode != null && HotReloadAppModel.InferFromProject(context, projectRootNode) is WebApplicationAppModel webAppModel
@@ -68,15 +67,18 @@ public static async Task WatchAsync(DotNetWatchContext context, CancellationToke
6867

6968
browserRefreshServer?.ConfigureLaunchEnvironment(environmentBuilder, enableHotReload: false);
7069

71-
foreach (var (name, value) in environmentBuilder)
70+
Action<OutputLine>? outputObserver = null;
71+
if (projectRootNode != null)
7272
{
73-
processSpec.EnvironmentVariables.Add(name, value);
73+
Debug.Assert(context.MainProjectOptions != null);
74+
outputObserver = context.BrowserLauncher.TryGetBrowserLaunchOutputObserver(projectRootNode, context.MainProjectOptions, browserRefreshServer, shutdownCancellationToken);
7475
}
7576

76-
if (projectRootNode != null)
77+
processSpec.RedirectOutput(outputObserver, context.ProcessOutputReporter, context.EnvironmentOptions, projectRootNode?.GetDisplayName() ?? "");
78+
79+
foreach (var (name, value) in environmentBuilder)
7780
{
78-
Debug.Assert(context.MainProjectOptions != null);
79-
context.BrowserLauncher.InstallBrowserLaunchTrigger(processSpec, projectRootNode, context.MainProjectOptions, browserRefreshServer, shutdownCancellationToken);
81+
processSpec.EnvironmentVariables.Add(name, value);
8082
}
8183

8284
// Reset for next run

0 commit comments

Comments
 (0)