Improve handling of CTRL+C when dnx is used on Windows #49633
Conversation
Copilot prompt: Add code to #file:'dnx.cs' to launch a new process. It should launch the dotnet executable in the same directory as the current program. It should pass "dnx" as the first argument, and then add all the other arguments that were passed to the current program.
Use top level statements and argument escaper
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the Windows batch-based dnx script with a native AOT executable shim to avoid the “Terminate batch job” prompt when pressing CTRL+C in cmd.exe. On non-Windows platforms it continues to use the existing shell script.
- Renames MSBuild target
LayoutDnxScript→LayoutDnxShimand copies the AOT shim on Windows. - Adds a new
dnxproject (NativeAOT shim) and includes it in the solution/redist. - Implements
dnx.csshim logic and updates documentation.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Layout/redist/targets/GenerateInstallerLayout.targets | Renamed script target to shim and updated copy logic. |
| src/Layout/redist/redist.csproj | Added reference to new dnx shim project. |
| src/Layout/dnx/dnx.csproj | New project for AOT shim with publish settings. |
| src/Layout/dnx/dnx.cs | Shim entrypoint forwarding args to dotnet dnx. |
| src/Layout/dnx/README.md | Documentation for shim rationale and behavior. |
| sdk.slnx | Included dnx shim project in solution folder. |
Comments suppressed due to low confidence (1)
src/Layout/dnx/dnx.cs:1
- This new shim changes signal propagation and argument forwarding; please add automated tests to verify correct CTRL+C behavior on Windows and ensure arguments are passed intact.
// Licensed to the .NET Foundation under one or more agreements.
| @@ -0,0 +1,6 @@ | |||
| # dnx.ps1 | |||
There was a problem hiding this comment.
I don't use PowerShell much and I just generated this script by asking copilot to translate the batch script to PowerShell. So I'd appreciate review of the script from someone more familiar with PowerShell.
|
Since these will be shipping, shouldn't we include the MIT license header in these files? |
|
Do we need to add the .ps1 to the list of files to be signed? |
| <DnxShimSource Condition="$([MSBuild]::IsOSPlatform('WINDOWS'))" Include="dnx.cmd" /> | ||
| <DnxShimSource Condition="$([MSBuild]::IsOSPlatform('WINDOWS'))" Include="dnx.ps1" /> |
There was a problem hiding this comment.
I think we use Windows everywhere - plus if we're building on a platform that's case-sensitive, this feels like the safer approach.
| <DnxShimSource Condition="$([MSBuild]::IsOSPlatform('WINDOWS'))" Include="dnx.cmd" /> | |
| <DnxShimSource Condition="$([MSBuild]::IsOSPlatform('WINDOWS'))" Include="dnx.ps1" /> | |
| <DnxShimSource Condition="$([MSBuild]::IsOSPlatform('Windows'))" Include="dnx.cmd" /> | |
| <DnxShimSource Condition="$([MSBuild]::IsOSPlatform('Windows'))" Include="dnx.ps1" /> |
There was a problem hiding this comment.
MSBuild folds this to upper case anyway
https://github.com/dotnet/msbuild/blob/a7a4d5af02be5aa6dc93a492d6d03056dc811388/src/Build/Evaluation/IntrinsicFunctions.cs#L560-L563
There was a problem hiding this comment.
Or you could use $([System.OperatingSystem]::IsWindows()), which is special-cased to return true on .NET Framework. System.OperatingSystem property functions
| @@ -0,0 +1,6 @@ | |||
| # dnx.ps1 | |||
There was a problem hiding this comment.
Should we add a #Requires -Version 6.0
to make sure powershell supports Split-Path?
@Args was also added in 3.0 (2012 powershell).
| @@ -0,0 +1,6 @@ | |||
| # dnx.ps1 | |||
| # PowerShell script to launch dotnet.exe with 'dnx' and all passed arguments | |||
| $scriptDir = Split-Path -Parent $MyInvocation.MyCommand.Definition | |||
There was a problem hiding this comment.
Users will need to select the ps1 script instead of the cmd, right? Is it ok that DnxShimSource can contain 2 possible shim sources now, and is there any logic that might need updating because of this?
Fixes #49623
There is apparently no way to avoid the "Terminate batch job" behavior when running a batch script with cmd.exe (see microsoft/terminal#217). We could add an .exe shim instead of a .cmd file, but a NativeAOT executable to accomplish this would add about 1.5 MB to the payload.
So instead, this PR adds a
dnx.ps1script in addition todnx.cmd. This fixes the behavior for people using PowerShell as their command interpreter. It doesn't fix the issue for people usingcmd. It looks like Yarn went with this option.