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? |
| @@ -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).
There was a problem hiding this comment.
It looks like Split-Path has been around since PowerShell 1.0. We don't want to require version 6 of PowerShell, I think that's the .NET Core version that you have to install separately. I think we want this script to work on the PowerShell that's included with Windows. As far as I can tell it does. The lowest version of Windows we support is Server 2012 which has PowerShell 3, so I think we're good.
| @@ -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?
There was a problem hiding this comment.
If you're running PowerShell, then the ps1 script will take precedence over the .cmd script. So either way you just type dnx, and from PowerShell you'll get the ps1 script with improved CTRL+C handling, and from a cmd shell you'll get the .cmd script and the extra CTRL+C termination confirmation.
The only use of DnxShimSource is in GenerateInstallerLayout.targets and it has been updated to handle the items appropriately.
Co-authored-by: Jacques Eloff <joeloff@users.noreply.github.com>
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.