Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

After upgrading to 7.1.0, zx start powershell as shell instead of Bash #523

Closed
budarin opened this issue Oct 6, 2022 · 29 comments
Closed

Comments

@budarin
Copy link

budarin commented Oct 6, 2022

Expected Behavior

run Bash as shell

we share the project between Windows, MacOs and Linux platforms, and we can't write scripts for Windows and Linux separately (we have a lot of them)!

Actual Behavior

starts powerShell instead

Steps to Reproduce the Problem

  1. write code
import { $ } from 'zx/core';

await $`rm -rf ./dist/client/assets.json`;
  1. run the code
zx script.mjs
  1. get the error
$ yarn zx ./script.mjs
yarn run v1.22.15
$ D:\Projects\raketa\raketa-web-app-boilerplate\node_modules\.bin\zx ./script.mjs
$ rm -rf ./dist/client/assets.json
Remove-Item : A parameter cannot be found that matches parameter name 'rf'.
At line:1 char:4
+ rm -rf ./dist/client/assets.json
+    ~~~
    + CategoryInfo          : InvalidArgument: (:) [Remove-Item], ParameterBindingException
    + FullyQualifiedErrorId : NamedParameterNotFound,Microsoft.PowerShell.Commands.RemoveItemCommand
 
Error: Remove-Item : A parameter cannot be found that matches parameter name 'rf'.
At line:1 char:4
+ rm -rf ./dist/client/assets.json
+    ~~~
    + CategoryInfo          : InvalidArgument: (:) [Remove-Item], ParameterBindingException
    + FullyQualifiedErrorId : NamedParameterNotFound,Microsoft.PowerShell.Commands.RemoveItemCommand
 
    at file:///D:/Projects/raketa/raketa-web-app-boilerplate/script.mjs:3:8
    exit code: 1

while with zx@7.0.8 everything works well

$ yarn run v1.22.15
$ D:\Projects\raketa\raketa-web-app-boilerplate\node_modules\.bin\zx ./script.mjs
$ rm -rf ./dist/client/assets.json
Done in 1.11s.

Specifications

  • Version: 7.1.0
  • Platform: Windows 10
@budarin
Copy link
Author

budarin commented Oct 6, 2022

maybe still try to fix the problems with the "dollar" sign in bash instead of simply replacing the execution environment with powershell?

The Linux world does not have posershell, and the commands in powershell and bash are different.

Scripts are used on bash/sh in projects precisely because they can be used uniformly on different platforms without changes, and now we have to write scripts separately for Windows and all other platforms separately

@antonmedv
Copy link
Collaborator

to fix the problems with the "dollar" sign

$ is not an error. It's bash special syntax for strings. Apparently, you used cmd.exe. For me it's much easier to add support for pwsh. Excaping in cmd.exe is bad.

@budarin
Copy link
Author

budarin commented Oct 6, 2022

to fix the problems with the "dollar" sign

$ is not an error. It's bash special syntax for strings. Apparently, you used cmd.exe. For me it's much easier to add support for pwsh. Excaping in cmd.exe is bad.

I wrote this after reading the contents of the last release, where this reason was given as the reason for switching to PowerShell

@ziimakc
Copy link

ziimakc commented Oct 6, 2022

Same issue, how to switch to bash forcibly?

@budarin
Copy link
Author

budarin commented Oct 6, 2022

Same issue, how to switch to bash forcibly?

in every script write by hand

if ( process.platform === 'win32' ) $.shell = 'bash'

@antonmedv
Copy link
Collaborator

Actually we can check for bash on windows automatically too!

@antonmedv
Copy link
Collaborator

Will do it.

@antonmedv
Copy link
Collaborator

Fix https://github.com/google/zx/actions/runs/3202574452/jobs/5231685644

Try it:

npm i zx@dev

@budarin
Copy link
Author

budarin commented Oct 7, 2022

zx@dev

it works!

@louisgv
Copy link

louisgv commented Oct 9, 2022

FYI, the new patch doesn't check if the bash is WSL or a TTY sim (like git bash). In my case, bash points to WSL environment which has its own file system, thus screwed up the pathing of my script.

Apparently related to: #398

@antonmedv
Copy link
Collaborator

check if the bash is WSL or a TTY sim (like git bash).

Is there a way to check it? We dan update the logic.

@louisgv
Copy link

louisgv commented Oct 10, 2022

check if the bash is WSL or a TTY sim (like git bash).

Is there a way to check it? We dan update the logic.

We can check if the user is running under wsl by using microsoft/WSL#423 (comment)

What's tricky is that there are several run cases:

  • Invoking wsl under windows mounted system
  • Invoking wsl under WSL VM

Most user don't want the first, but they would want the 2nd. So it's really is a pairs checking for WSL and for maybe:

  • cwd: the mount point on each system might be diff
  • windows specific shell cmd: "get-command bash" and look at the source might give us a indicator that it's either git bash vs wsl

@antonmedv
Copy link
Collaborator

Invoking wsl under WSL VM

zx will use bash in wsl right now.

@louisgv
Copy link

louisgv commented Oct 10, 2022

Invoking wsl under WSL VM

zx will use bash in wsl right now.

Just to illustrate the difference between the cases I mentioned above, here's a specific example:

  1. git introduced fsmonitor
  2. host user has git-window installed (with fsmonitor enabled)
  3. host user also has WSL installed, and inside the WSL shell, also has git, with fsmonitor enabled
  4. when host uses zx to run git, it uses the wsl's bash env, which uses the wsl's git, which points fsmonitor to its own cache instead of the host's git-window's fsmonitor's cache -> resulting in mismatching cache. This causes tools to not recognize that file has been checked in (bc the fsmonitor cache is separated) -> git status shows wrong results (bc it's using fsmonitor cache from wsl)

Invoking WSL bash under WSL VM is not the same as invoking WSL bash in the host's system - the tooling will have mismatch paths and side effects. I do think it is one hell of an abomination for a shell environment, but welp they did made WSL and people apparently love Windows

Most user don't want the first, but they would want the 2nd

I think this sentence I made is a bit confusing. When the tooling has no side effect (like just a CLI that grep files for url), zx in both cases works fine. However, when there are side effect such as fs caching/fsmonitor (git case above, or pnpm global cache), it's important to differentiate where WSL bash is being run.

Right now, zx uses WSL's bash under the host's environment (which introduces the example issue above) IF I run the zx script under the host environment.

If I'm just testing zx inside a WSL VM, it works but then I'd rather bootup linux.

@antonmedv
Copy link
Collaborator

I see. Let’s try to fix it. Maybe you can create a PR?

@Maikel-Nait
Copy link

After upgrading to 7.1.1, seems I'm hitting this problem. zx then uses my WSL bash shell by default, and doing a simple "yarn" command yields the error /mnt/c/Program Files/nodejs/yarn: 11: exec: node: not found

In other words, I still have to resort to the original hack ... :-D

@antonmedv
Copy link
Collaborator

And what zx should us in this case? Powershell?

@Maikel-Nait
Copy link

Honestly, I'm not familiar with all this WSL / bash paths entanglement. louisgv seems much more clear about it. For me the hack is good enough. I'm just using Windows for development, but the final code will run on a Linux machine, so no big issues for me.

@antonmedv antonmedv reopened this Oct 19, 2022
@google google deleted a comment from mnpenner Oct 25, 2022
@rxliuli
Copy link

rxliuli commented Nov 18, 2022

Is there any progress? I expect to go to git bash by default on windows to run as a shell, most developers will install and use git, after all github needs it

@antonmedv
Copy link
Collaborator

Well, windows does not have bash by default. Only powershell.

@JenieX
Copy link

JenieX commented Feb 10, 2023

Man, I almost lost my mind trying to figure it out. Thank you budarin, that did solve it for me.

$.shell = 'C:\\Program Files\\Git\\bin\\bash.exe';

I think it must be at least noted in the main readme file that this package doesn't use bash. It would be better to set bash by default though.

@antonmedv
Copy link
Collaborator

Bash is default by default. Unfortunately on windows everybody install bash in different ways.

@JenieX
Copy link

JenieX commented Feb 10, 2023

Bash is default by default. Unfortunately on windows everybody install bash in different ways.

Never mind. I just noticed it is noted in the main page. 😄

@kcaswick
Copy link

What about checking what platform bash is built for, and skipping it if it is a Linux version when process.platform === 'win32'?

It would require 1 extra invocation of bash. Only using bash when bash -c '[[ $OSTYPE != "linux"* ]]' (per SO) is true seems simplest, though a reasonable alternative would be bash -c '[[ $OSTYPE == msys* || $OSTYPE == cygwin* || $OSTYPE == win32* ]]'. Anyone know a reason one of these would fail, but not the other?

PS> gcm -all bash

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
Application     bash.exe                                           10.0.1904… C:\WINDOWS\system32\bash.exe

PS> C:\WINDOWS\system32\bash.exe --version
GNU bash, version 5.1.16(1)-release (x86_64-redhat-linux-gnu)
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
PS> C:\WINDOWS\system32\bash.exe -c 'echo $OSTYPE'
linux-gnu
PS> C:\WINDOWS\system32\bash.exe -c '[[ $OSTYPE != "linux"* ]]' ; $?                            
False
PS> C:\WINDOWS\system32\bash.exe -c '[[ $OSTYPE == msys* || $OSTYPE == cygwin* || $OSTYPE == win32* ]]' ; $?
False
PS> & $env:USERPROFILE\AppData\Local\Programs\Git\bin\bash --version
GNU bash, version 5.1.16(1)-release (x86_64-pc-msys)
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
PS> & $env:USERPROFILE\AppData\Local\Programs\Git\bin\bash -c 'echo $OSTYPE'  
msys
PS> & $env:USERPROFILE\AppData\Local\Programs\Git\bin\bash -c '[[ $OSTYPE != "linux"* ]]' ; $? 
True
PS> & $env:USERPROFILE\AppData\Local\Programs\Git\bin\bash -c '[[ $OSTYPE == msys* || $OSTYPE == cygwin* || $OSTYPE == win32* ]]' ; $?
True
PS> & $env:USERPROFILE\AppData\Local\Programs\Git\usr\bin\bash --version
GNU bash, version 5.1.16(1)-release (x86_64-pc-msys)
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
PS> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.2.9
PSEdition                      Core
GitCommitId                    7.2.9
OS                             Microsoft Windows 10.0.19045
Platform                       Win32NT

@Artoria2e5
Copy link

Artoria2e5 commented Aug 28, 2023

Bash is a bad idea too, because Cygwin/MSYS2 argument parsing is not what nodejs/uvlib expects. Try to coax echo.exe from MSYS into printing out C:\" from outside of Cygwin/MSYS2 and you will be very surprised. See the patchset linked on mirror/newlib-cygwin#5 (no, don't tell me to revive that patch, I am the one that needs reviving first) and https://github.com/Artoria2e5/node/blob/c0a6aff35cd3ff6d3bd0e3687776158e97466c93/doc/api/child_process.md#windows-command-line.

On the other hand at least you get the same shell to do loops with. That's good.

@Sojaner
Copy link

Sojaner commented Feb 8, 2024

Might not be very relevant, but I personally use the approach used by the C# port https://github.com/Cysharp/ProcessX where it does Windows -> "cmd /c", Linux -> "(which bash) -c".
And in the rest of the code, I keep track of which platform I'm on and use the available commands however limited.

@kcaswick
Copy link

kcaswick commented Feb 8, 2024

@Sojaner How do you handle the quoting issues with using \, ^, &, ", etc.?

It looks like Process X let an issue about how to include newlines go stale and get auto-closed. At a glance, it looks like they just ignore that cmd and bash handle escaping differently.

@Sojaner
Copy link

Sojaner commented Feb 9, 2024

@kcaswick You are correct! That's a big problem with cmd and it makes the scenarios for windows very limited. But that's the same with batch scripts!
I don't write scripts for windows that often, but almost every time I end up with a "batch script" with some JavaScript goods from zx to make life a little easier!

@antonmedv
Copy link
Collaborator

Fixed in v8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants