haskell / process

Library for dealing with system processes
http://hackage.haskell.org/package/process
Other
87 stars 82 forks source link

Shell commands invoke /bin/sh directly #253

Open madeline-os opened 2 years ago

madeline-os commented 2 years ago

I am a maintainer of the obelisk framework. This week, I had to debug a very strange issue where some shell escaping was "working on my machine"™, but not my friend's. obelisk provides an integrated development environment that uses nix to ensure consistent behavior across platforms. In particular, we try to lock down which shell that is used when we invoke other executables.

However, we cannot ensure that our dependencies that use System.Process choose a shell in a way we can modify: In particular, ghcid uses System.Process.shell, and so on my machine, where /bin/sh is bash, and my friend's machine, where /bin/sh is dash, the behavior of shell escaping is different, and this causes ghcid to fail to invoke it's --command argument correctly.

My gut is that, just as one would reflexively invoke #!/usr/bin/env bash in a shell script that one wants to be portable, one would want System.Process to invoke /usr/bin/env sh -c <cmd> instead of /bin/sh -c <cmd> directly. Otherwise, the user cannot control the shell that is used at all without invasively modifying their entire system environment. A user may be surprised that the shell that they use to run their haskell program is not the same shell used to invoke shell commands from within the program, for example. I was surprised, at least!

This would be a rather significant change to the behavior of shell commands, so I'm not sure if that this change is desirable. At the least, a warning about portability would be appreciated.

snoyberg commented 2 years ago

First, my general recommendation is to avoid using the shell function.

As for your comments: the only standards-compliant possibility is to call /bin/sh, POSIX does not require any other shell interpreter be present.

I'd certainly welcome a PR adding some warning documentation about this.

AresAndy commented 7 months ago

Hello, could you please consider exposing the shell execution command as part of the CreateProcess record? You could still have the POSIX compliant default set, and allow the developer to use another shell if necessary. Especially for Windows, defaulting to cmd.exe instead of powershell.exe not only is fairly archaic, but also very limiting

I could try to make a PR, but I'm not proficient enough in Haskell, plus the current code is fairly messy. I understand why it is like that, but I don't understand why this was "hard-coded" to some shells instead of others. The argument for compliance with POSIX holds only when the module is compiled for that family of systems. Windows, and other lesser known OSes are not

tomjaguarpaw commented 7 months ago

I'm not sure what you mean by "expose the shell execution command". Do you mean to make that a configurable parameter of CreateProcess? If so, can't you achieve the same by using proc instead of shell, as @snoyberg suggests above? Then you can choose whatever shell you like as the executable.

AresAndy commented 7 months ago

I'm not sure what you mean by "expose the shell execution command". Do you mean to make that a configurable parameter of CreateProcess? If so, can't you achieve the same by using proc instead of shell, as @snoyberg suggests above? Then you can choose whatever shell you like as the executable.

Yes, but that does not work when you want to perform say mkdir something under systems that do not ship such "commands" as standalone binaries. For instance: any not-POSIX-compliant OS.

Then you might recommend creating a temp file with all the commands to be executed by a specific shell and use that as input of a proc invocation.. All extra work that can be avoided by adding and handling a Maybe String to the CreateProcess data structure, that can override whatever hard-coded shell with a Just <something>

tomjaguarpaw commented 7 months ago

I don't understand. How would you run mkdir something using the current version of typed-process?

AresAndy commented 7 months ago

I don't understand. How would you run mkdir something using the current version of typed-process?

You would use proc. mkdir is a binary that can be found in $PATH, and something is an argument passed to that binary. But that is possible only on a POSIX compliant OS.

Both cmd and powershell implement mkdir internally within the shell, either as a command and/or cmdlet alias. There's no binary file available on $PATH and therefore proc could never invoke that, unless if somehow you were to run the shell interactively, send through the STDIN handle the command with arguments (if any) and wait on STDOUT, which needless to say is HIGHLY unpractical.. And prone to plausible failures too.

If on the other hand I could configure CreateProcess to spawn powershell directly, I could just do shell "New-Item" and then set the shell command to receive that string. Much more simple and platform/paradigm independent

tomjaguarpaw commented 7 months ago

But if you can do it by configuring CreateProcess to override the shell, can't you also do it using proc? I don't know the exact invocation, but maybe something like

proc "powershell.exe" ["New-Item"]