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

Avoid using system shell to start processes #523

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

justin-espedal
Copy link

This relates to HaxeFoundation/haxe#10389

This change allows Haxe's basic build tooling to be used even when the system shell is unavailable.

Sys.command runs processes through the system shell. The 2-argument form of new Process, on the other hand, starts processes without going through the shell.

This relates to HaxeFoundation/haxe#10389

This change allows Haxe's basic build tooling to be used even when the system shell is unavailable.

`Sys.command` runs processes through the system shell. The 2-argument form of `new Process`, on the other hand, starts processes without going through the shell.
@RealyUniqueName
Copy link
Member

This is not equal to Sys.command as it prints stdout data as it arrives, but accumulates stderr messages until the process is finished. Moreover stdin is completely ignored.

@justin-espedal
Copy link
Author

Yes, it's not quite equivalent. I had assumed that stdin is never used here, so I didn't pursue proper handling of in/out/error, but now that I consider it, perhaps that was a mistake owing to the specific subset of haxelibs I have experience with (and how I use those haxelibs).

Unfortunately, I'm not sure how to handle stdin/out/error properly without more complicated multi-threaded code to process all the streams at once, which I was hoping to avoid. But as far as I know, Neko doesn't expose anything to make this easy. It seems to me that it's between multi-threaded code on the Haxe side, or creating some new process-related function on Neko's side that handles this, right?

@back2dos
Copy link
Member

back2dos commented Oct 19, 2021

There are rather difficult problems to overcome here:

  1. On windows, haxe and neko can be .cmd files when installed through npm (via haxe or lix). Or they may be .bat files if the user has their own stuff set up for whatever reason. The resolution of executables other than .exe relies on the shell, meaning you'll have to resolve the right one via where (assuming it's even available to you - otherwise you'll have to locate it in cwd + PATH by hand).

  2. Dealing with stdio is important, because some haxelibs have interactive, e.g. hxcpp. A few libraries even call haxelib, e.g. a submission helper I created which ultimately calls haxelib and runs into Sys.getChar(false) which requires the haxelib process's stdin to be connected to a terminal - Sys.command accomplishes this if the current process's stdin is also connected to a terminal, because child processes created in this fashion inherit std. The current process API doesn't allow for it, but future APIs will.
    You could attempt to run haxelib on a platform where an API is available to inherit stdio to child processes, e.g. via libuv on hl or eval (although this means there'll be no binary per se, only a .bat file calling the haxe interpreter) or java (using a native image or a packaged executable with a JRE) or cs (not too familiar with .NET but I expect it has such capabilities).

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

Successfully merging this pull request may close these issues.

3 participants