Skip to content

Commit

Permalink
Cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
tmat committed Oct 30, 2024
1 parent 971cbab commit b9bd37c
Show file tree
Hide file tree
Showing 14 changed files with 162 additions and 188 deletions.
10 changes: 0 additions & 10 deletions src/BuiltInTools/dotnet-watch/Aspire/AspireServiceFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,6 @@ public async ValueTask TerminateLaunchedProcessesAsync(CancellationToken cancell
}
}

/// <summary>
/// When shutting down we terminate all processes created by Aspire service first.
/// Then we terminate the AppHost (root project) without terminating the entire process tree,
/// so that we don't kill DCP before it has a chance to clean up resources.
/// </summary>
public bool TerminateEntireProcessTreeOnShutdown
// This does not work on OSX/Linux. /~https://github.com/dotnet/sdk/issues/44496
=> false; //!RuntimeInformation.IsOSPlatform(OSPlatform.Windows);

public IEnumerable<(string name, string value)> GetEnvironmentVariables()
=> _service.GetServerConnectionEnvironment().Select(kvp => (kvp.Key, kvp.Value));

Expand Down Expand Up @@ -255,7 +246,6 @@ private ProjectOptions GetProjectOptions(ProjectLaunchRequest projectLaunchInfo)
LaunchProfileName = projectLaunchInfo.LaunchProfile,
NoLaunchProfile = projectLaunchInfo.DisableLaunchProfile,
TargetFramework = null, // TODO: Should DCP protocol specify?
TerminateEntireProcessTreeOnShutdown = TerminateEntireProcessTreeOnShutdown,
};
}
}
Expand Down
1 change: 0 additions & 1 deletion src/BuiltInTools/dotnet-watch/CommandLineOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,5 @@ public ProjectOptions GetProjectOptions(string projectPath, string workingDirect
LaunchProfileName = LaunchProfileName,
NoLaunchProfile = NoLaunchProfile,
TargetFramework = TargetFramework,
TerminateEntireProcessTreeOnShutdown = true,
};
}
3 changes: 1 addition & 2 deletions src/BuiltInTools/dotnet-watch/DotNetWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
{
[EnvironmentVariables.Names.DotnetWatch] = "1",
[EnvironmentVariables.Names.DotnetWatchIteration] = (iteration + 1).ToString(CultureInfo.InvariantCulture),
},
TerminateEntireProcessTree = true,
}
};

var browserRefreshServer = (projectRootNode != null)
Expand Down
12 changes: 7 additions & 5 deletions src/BuiltInTools/dotnet-watch/HotReload/CompilationHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private DeltaApplier CreateDeltaApplier(ProjectGraphNode projectNode, BrowserRef
_ => new DefaultDeltaApplier(processReporter),
};

public async Task<RunningProject> TrackRunningProjectAsync(
public async Task<RunningProject?> TrackRunningProjectAsync(
ProjectGraphNode projectNode,
ProjectOptions projectOptions,
string namedPipeName,
Expand All @@ -132,18 +132,20 @@ public async Task<RunningProject> TrackRunningProjectAsync(
// It is important to first create the named pipe connection (delta applier is the server)
// and then start the process (named pipe client). Otherwise, the connection would fail.
deltaApplier.CreateConnection(namedPipeName, processCommunicationCancellationSource.Token);
var launchResult = new ProcessLaunchResult();

processSpec.OnExit += (_, _) =>
{
processExitedSource.Cancel();
return ValueTask.CompletedTask;
};

var launchResult = new ProcessLaunchResult();
var runningProcess = ProcessRunner.RunAsync(processSpec, processReporter, isUserApplication: true, launchResult, processTerminationSource.Token);

// RunAsync throws if process can't be launched, otherwise we must have a PID.
Debug.Assert(launchResult.ProcessId != null);
if (launchResult.ProcessId == null)
{
// error already reported
return null;
}

var capabilityProvider = deltaApplier.GetApplyUpdateCapabilitiesAsync(processCommunicationCancellationSource.Token);
var runningProject = new RunningProject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ internal interface IRuntimeProcessLauncher : IAsyncDisposable
{
IEnumerable<(string name, string value)> GetEnvironmentVariables();

/// <summary>
/// True if shutting down the root process should terminate its entire process tree.
/// </summary>
bool TerminateEntireProcessTreeOnShutdown { get; }

/// <summary>
/// Initiates shutdown. Terminates all created processes.
/// </summary>
Expand Down
13 changes: 0 additions & 13 deletions src/BuiltInTools/dotnet-watch/HotReload/ProjectLauncher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,11 @@ public EnvironmentOptions EnvironmentOptions
return null;
}

return await LaunchProcessAsync(projectOptions, projectNode, processTerminationSource, onOutput, restartOperation, build, cancellationToken);
}

public async ValueTask<RunningProject> LaunchProcessAsync(
ProjectOptions projectOptions,
ProjectGraphNode projectNode,
CancellationTokenSource processTerminationSource,
Action<OutputLine>? onOutput,
RestartOperation restartOperation,
bool build,
CancellationToken cancellationToken)
{
var processSpec = new ProcessSpec
{
Executable = EnvironmentOptions.MuxerPath,
WorkingDirectory = projectOptions.WorkingDirectory,
OnOutput = onOutput,
TerminateEntireProcessTree = projectOptions.TerminateEntireProcessTreeOnShutdown,
Arguments = build || projectOptions.Command is not ("run" or "test")
? [projectOptions.Command, .. projectOptions.CommandArguments]
: [projectOptions.Command, "--no-build", .. projectOptions.CommandArguments]
Expand Down
3 changes: 1 addition & 2 deletions src/BuiltInTools/dotnet-watch/HotReloadDotNetWatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ public override async Task WatchAsync(CancellationToken shutdownCancellationToke
var launcherEnvironment = runtimeProcessLauncher.GetEnvironmentVariables();
rootProjectOptions = rootProjectOptions with
{
LaunchEnvironmentVariables = [.. rootProjectOptions.LaunchEnvironmentVariables, .. launcherEnvironment],
TerminateEntireProcessTreeOnShutdown = runtimeProcessLauncher.TerminateEntireProcessTreeOnShutdown
LaunchEnvironmentVariables = [.. rootProjectOptions.LaunchEnvironmentVariables, .. launcherEnvironment]
};
}

Expand Down
1 change: 0 additions & 1 deletion src/BuiltInTools/dotnet-watch/Internal/IReporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public bool TryGetMessage(string? prefix, object?[] args, [NotNullWhen(true)] ou
public static readonly MessageDescriptor WaitingForChanges = new("Waiting for changes", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor LaunchedProcess = new("Launched '{0}' with arguments '{1}': process id {2}", "🚀", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor KillingProcess = new("Killing process {0}", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor KillingProcessTree = new("Killing entire process tree of process {0}", "⌚", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor HotReloadChangeHandled = new("Hot reload change handled in {0}ms.", "🔥", MessageSeverity.Verbose, s_id++);
public static readonly MessageDescriptor HotReloadSucceeded = new("Hot reload succeeded.", "🔥", MessageSeverity.Output, s_id++);
public static readonly MessageDescriptor BuildCompleted = new("Build completed.", "⌚", MessageSeverity.Verbose, s_id++);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ internal class MSBuildFileSetFactory(
{
capturedOutput.Add(line);
}
},
TerminateEntireProcessTree = true,
}
};

reporter.Verbose($"Running MSBuild target '{TargetName}' on '{rootProjectFile}'");
Expand Down
Loading

0 comments on commit b9bd37c

Please sign in to comment.