Windows support

[Globegitter]

It seems when I try to use this on windows I just get the error spawn ENOENT. I am writing a cli tool that is working fine on Linux and Mac, but not on windows.

I am still running a few tests (slowly) but right now it seems like the error is due to this library. Will update if I make more progress.

Edit: Yep, changing to https://www.npmjs.com/package/superspawn fixes the issue.

[patrick-steele-idem]

I don't use Windows so this is not something I can test. If you want to submit a PR to support Windows then feel free, but this is not something I can work on. It looks like superspawn just adds some extra logic to resolve the executable on a Windows machine:
https://github.com/MarcDiethelm/superspawn/blob/dc06b8c3aa1ead2fef9e94f01c2702edbfbd5cdb/lib/index.js

[patrick-steele-idem]

Labeling as an enhancement since this module is just using require('child_process').spawn directly and it is not a bug in this module that is preventing it from working on Windows.

[patrick-steele-idem]

On second thought, I don't think logic to resolve a Windows executable belongs in this module. It is probably better for the caller to pre-resolve the executable before using this module. There's a lot of magic happening in superspawn and there is some synchronous blocking code when searching for executables.

[ben3005]

could this library use something like this:

https://github.com/IndigoUnited/node-cross-spawn

so that it works on windows?

[lshearer]

This module looks like the nicest promise implementation of spawn available, but the lack of Windows support is a deal breaker for us.

As @ben3005 mentioned, https://github.com/IndigoUnited/node-cross-spawn is a popular solution for the windows spawn issues and is a drop-in replacement for child_process.spawn. It would be awesome to have this integrated.

If bringing in a dependency is a concern, it would be nice to at least make the spawn method configurable, maybe something like:

const spawn = require('child-process-promise').useSpawn(require('cross-spawn')).spawn;

spawn(...).then(...)
...

That would let us easily use the two modules together, or potentially publish a second module that combines the two (e.g., cross-child-process-promise) to make things even cleaner.

[patrick-steele-idem]

I support the idea of making the spawn method configurable. I don't have a Windows machine or Windows VM set up, but if someone who does wants to contribute back I would be happy to review and merge if everything looks good. Also, if anyone has any experience on setting up automated tests in a Windows environment (e.g. AppVeyor) that would be very helpful for this project. Thanks for comments so far!

[borekb]

Seems to be working on Windows since TintypeMolly@8a08e9c, thanks!

It's README-worthy, IMO.

[zrisher]

Confirmed working for me on Windows as well, perhaps this issue can be closed?

[kachkaev]

@Globegitter can we close please this? I came across child-process-promise repo and saw this issue opened, so almost ignored the module.