moonrepo / proto

A pluggable multi-language version manager.
https://moonrepo.dev/proto
MIT License
681 stars 34 forks source link

Windows shims show `Terminate batch job (Y/N)?` on ctrl-c #269

Closed rotu closed 11 months ago

rotu commented 1 year ago

What version?

0.21.0

Which command?

No response

What happened?

When launching a shim on Windows, interrupting with ctrl-c can result in the confusing output "Terminate batch job (Y/N)?".

To reproduce:

node -e "setTimeout(new Function, 30000);"

and stop the program with ctrl c.

Any logs?

No response

Operating system?

Windows

Architecture?

x64

rotu commented 1 year ago

Current viable workarounds:

  1. Use proto run directly: proto run node -- -e "setTimeout(new Function, 30000);"
  2. (if using cmd.exe) use DOSKEY to set up the shims instead:
    doskey /exename=cmd.exe npx=proto run node --bin npx.cmd -- $*
  3. (if using powershell) use a Function instead:
    function node { proto run node -- @args }
  4. Edit the shim to add an exit statement when node finishes, instead of returning control to the script:
    proto.exe run node --  %* & EXIT
milesj commented 1 year ago

I don't know windows very well, is the exit strategy really the best way to handle this?

rotu commented 1 year ago

I don't know windows very well, is the exit strategy really the best way to handle this?

I don't know Windows very well either. I think these are all limited workarounds, and that the best way to handle this is to not use cmd shims but to use exe executables instead.

The exit hack is a bad user experience if you're calling node from a cmd shell (since CALL node would terminates the shell when node ends) but great if you're using powershell or calling it from another executable.


Windows itself has some facility for shell aliases under Settings -> Apps -> Advanced app settings -> App execution aliases, which seem to get installed to C:\Users\dan\AppData\Local\Microsoft\WindowsApps. I think this is in UWP AppExecutionAlias.

But it looks like a lot of platform-specific stuff that may or may not be worth it, and is not as user-hackable as shims.

rotu commented 1 year ago

I've come up with an even better way of suppressing the prompt. It's ugly and I hate it, but it seems pretty effective:

setlocal enabledelayedexpansion 
proto.exe run node -- %* & set RC=!errorlevel! & call;
EXIT /b %RC%

On ctrl-c, the errorlevel gets set, but not immediately. That is, when doing

foo && echo success %ERRORLEVEL% || echo failure %ERRORLEVEL%
echo RC %ERRORLEVEL%

even though the return code of foo affects the branch taken, the error level is not yet updated. Fortunately, enabledelayedexpansion with !varname! allows us to get this value correctly. Then call; clobbers the exit code before cmd reacts to it, thus suppressing the prompt. Finally, we exit the batch script with the captured error code.

EDIT: this misbehaves with npx for some reason I don't understand. I should probably see what the npx kicker looks like after #268 before any more effort on this.

milesj commented 11 months ago

Figured this out here https://github.com/moonrepo/proto/pull/308

rotu commented 11 months ago

Figured this out here #308

I have my doubts that script-based shims can overcome all the idiosyncrasies of cmd and PowerShell. That said, if anyone can make it work, you can!

milesj commented 11 months ago

Actually stole it from here: https://github.com/pnpm/cmd-shim/issues/39

Looks like npm/pnpm/yarn are starting to use it too.

milesj commented 11 months ago

Ok out.

rotu commented 11 months ago

None of these are quite dealbreakers but here are issues I currently attribute to the use of shims:

Ok out.

Awesome! I tried out the fix and it definitely fixes this issue (and I think also fixes the more annoying bug where a node subprocess/server will stay alive after interrupting the parent).

milesj commented 11 months ago

deno and deno.exe resolve to different versions (various shims versus .proto\bin\deno.exe)

This is expected. Shims will dynamically change versions, while the exe is manually pinned to a version. We can't make the exe dynamic, so if you don't want the dynamic part, just don't use the shims in PATH.

Start-Process deno opens deno.ps1 in an editor versus Start-Process deno.exe which invokes the executable.

Do you have .ps1 in your PATHEXT variable? It should not exist by default. Sure it's not hitting deno.cmd?

milesj commented 11 months ago

In the long run, I want to ditch ps1/cmd shims, and somehow generate .exe files on demand. They would be simple wrappers for spawning a child process.

I'm pretty sure this is how chocolatey works.

rotu commented 11 months ago

We can't make the exe dynamic, so if you don't want the dynamic part, just don't use the shims in PATH.

It's the opposite - I do want the executable to be dynamic. But proto.exe is in .proto/bin, so if I want to get deno.exe out of my path, I have to manually clean out the unwanted executables from the .proto\bin folder.

Start-Process deno opens deno.ps1 in an editor versus Start-Process deno.exe which invokes the executable.

Do you have .ps1 in your PATHEXT variable? It should not exist by default. Sure it's not hitting deno.cmd?

Whether or not I put .ps1 in my PATHEXT variable, this is the case. Powershell has different handling so that ps1 files get put ahead of normal executable resolution when using the call operator/&, the dot sourcing operator/., and Invoke-Expression/iex. But scripts are still not executables, and Windows tries to make sure you never forget it!

image

milesj commented 11 months ago

It's the opposite - I do want the executable to be dynamic. But proto.exe is in .proto/bin, so if I want to get deno.exe out of my path, I have to manually clean out the unwanted executables from the .proto\bin folder.

You could move the proto binary somewhere else and just ignore that path.

Whether or not I put .ps1 in my PATHEXT variable, this is the case.

Why is windows so dumb 😢

rotu commented 11 months ago

You could move the proto binary somewhere else and just ignore that path.

I plan to remove that path from my $env:PATH and add an App Paths registry entry. This may or may not work.

Why is windows so dumb 😢

I bet it's because this way MS didn't have to worry about name collisions between Cmdlets and the pre-existing morass of executables and WSH scripts. Fun fact: when you say "pwsh" out loud, it's a fair approximation of the sound you get when you break windows!

rotu commented 11 months ago

The registry entry didn't work great. Wound up moving the executable to the shims folder and dropping the bin folder from my PATH