awesome-cross-platform-nodejs icon indicating copy to clipboard operation
awesome-cross-platform-nodejs copied to clipboard

what's broken in spawn?

Open bcoe opened this issue 6 years ago • 12 comments

@sindresorhus, @satazor in newish versions of Node.js, what issues continue to exist in spawn that would motivate the use of execa or cross-spawn. I wonder if:

  1. some of the historic bugs have been addressed in Node.js.
  2. if they haven't, could they be?

CC: @boneskull

bcoe avatar Apr 04 '19 17:04 bcoe

Hey @bcoe! You may find what issues cross-spawn solves here: https://github.com/moxystudio/node-cross-spawn#why

Also, we explain why options.shell is not an alternative: https://github.com/moxystudio/node-cross-spawn#using-optionsshell-as-an-alternative-to-cross-spawn

satazor avatar Apr 04 '19 17:04 satazor

One big reason for the use of cross-spawn for me is being able to use shabangs on Windows.

ehmicky avatar Apr 04 '19 17:04 ehmicky

I feel like PATHEXT might be fixed, or at least I saw a bug issue on Node.js closed related to it...

@satazor thanks for the list, it's basically a laundry list if what the Node.js tooling working group should be fixing in spawn.

@ehmicky I think we could probably update the documentation to reference @satazor's list, vs., the stale information in that section of the document.

bcoe avatar Apr 04 '19 17:04 bcoe

@bcoe I might be wrong, but the PATHEXT is still an issue. Do you have any link/references?

satazor avatar Apr 04 '19 17:04 satazor

@satazor we referenced this issue for PATHEXT in the doc, which has since been closed:

https://github.com/nodejs/node-v0.x-archive/issues/2318

bcoe avatar Apr 04 '19 17:04 bcoe

The issue was closed by this comment: https://github.com/nodejs/node-v0.x-archive/issues/2318#issuecomment-264476003. If you go through the comments history, the only suggestion people gave was to use options.shell true but it's not really a solution because of https://github.com/moxystudio/node-cross-spawn#using-optionsshell-as-an-alternative-to-cross-spawn.

satazor avatar Apr 04 '19 17:04 satazor

Also options.shell: true fires the command through a shell instead of directly calling it. This is less secure, slower and less cross-platform: https://github.com/sindresorhus/execa/pull/182

ehmicky avatar Apr 04 '19 17:04 ehmicky

I just tried PATHEXT with Node 11.13.0 on Windows 10 and child_process.spawn(). I can confirm it does not work unless options.shell: true is used.

ehmicky avatar Apr 04 '19 18:04 ehmicky

execa lists many of the reasons for its existence: https://github.com/sindresorhus/execa#why (There are however many more things than listed there.)

sindresorhus avatar Apr 05 '19 05:04 sindresorhus

List of cross-platform issues with child_process.spawn() (and related methods):

  1. Shabangs are not supported on Windows. I.e. you need to do spawn('node', ['./node_script.js']) instead of spawn('./node_script.js')
  2. on Unix, foreground child processes (detached: false, the default value) continue running even if their parent exits. On Windows, they don't.
  3. PATHEXT does not work on Windows. I.e. you need to do spawn('./script.bat') instead of spawn('./script') even if PATHEXT includes .bat.
  4. when using shell: true, child_process.stdout includes cmd.exe prompt and command on Windows because Node.js fires cmd.exe without the /q flag.
  5. windowsHide defaults to false where it probably should default to true instead.

Issues 1) and 3) are solved with node-cross-spawn. All of those issues are solved with execa.

Extra information on 3): it can be solved by using a shell interpreter (shell: true option). However this is:

  • less secure: it allows for command injection by passing arguments like $(rm -rf /) or && rm -rf /
  • less cross-platform: it encourages using shell-specific features such as globbing, single quote escaping or even semicolons which won't work on cmd.exe.
  • slower as it goes through an extra step (the shell interpreter)

Just to be complete, execa also brings the following features which are not cross-platform-related:

  • promise interface. Synchronous methods are still available (execa.sync()).
  • can run locally installed binaries.
  • timeout option.
  • can pass stdin as a string or buffer, instead of only as a stream.
  • returns an all property with interleaved stdout and stderr.
  • strip final newline (optionally).
  • has more descriptive error messages (which is very valuable in testing).

Overall I think using execa still makes lots of sense in both production code and in tests, but obviously I am little biased :)

ehmicky avatar Apr 05 '19 09:04 ehmicky

@sindresorhus @ehmicky, I'd advocate that we write-up a proposal here under initiatives, which outlines some of the issues being shimmed by exec alternatives; some of these cross-platform issues (I think) should be fixed in Node.js itself (and this is exactly the goal of the Tooling Working Group).

What do you think @boneskull?

Also, looping in @iansu, I think this could be some incredible projects for getting a few commits into Node.js.

bcoe avatar Apr 05 '19 19:04 bcoe

For the 5 points I was mentioning:

  1. I could see how some might object to making shebangs work on Windows. Windows uses file associations instead. And this is not a problem if the caller specifies the program to run explicitly. Finally, this is only a problem with cmd.exe: I think if someone's using msys/MinGW shebangs might work. However others might find it convenient (which is one of the main features of node-cross-spawn).
  2. This is already documented on the Node.js website. There also already was a GitHub issue but it did not end up fixing the difference of behavior between Windows and Unix.
  3. There already was a GitHub issue and PR but it did not end up fixing the problem.
  4. This one I could not find any GitHub issue. So I created one.
  5. There already was a PR but it was rejected.

Based on that, I am wondering whether most of those proposals would move forward, considering they were already somewhat attempted at being fixed? However having any cross-platform issues fixed in core Node.js seems like the best approach.

Please let me know if anyone has other cross-platform issues in mind when it comes to child_process.

ehmicky avatar Apr 07 '19 11:04 ehmicky