sindresorhus
Executing "cmd.exe" on Windows never exits
On Windows, cmd.exe doesn't quit itself after running the required script. The /d /s /c options should be added. When execa run cmd.exe, it never exits and therefore never resolves the promise.
Reproduction
const execa = require("execa");
execa("cmd.exe", ["fixtures\\hello.cmd"])
.then(result => console.log(result)); // => This promise will never resolve
Technical
node-cross-spawn already add those options but only when on shell mode or on non .exe files.
This hangs because /c is missing. This is actually a core Node.js behavior, it does the same with:
childProcess.spawn('cmd.exe', ['anyFile.cmd'])
Now I actually don't think we should add /d /s /c to cmd.exe in this case. If users want to fire a *.cmd file, they should use the shell option. When they don't, they explicitly opt in for "I am going to call cmd.exe myself". Under those conditions, they might want to override the shell option logic, e.g. passing their own parameters to cmd.exe. I.e. I think it's good to not add parameters implicitly here. If they are using wrong CLI flags (e.g. missing /c), it would be on their responsibility, not the library (execa).
What do you think @GMartigny and @sindresorhus?
Since execa('anyFile.cmd') works as expected, I guess it's not a big deal. But is for some reason a user want to force cmd.exe against any command file and forget /c option, she's going to have a hard time finding what's wrong.
On the other hand if a user wants to fire cmd.exe and does not want to use /d, /s, /c or /q, firing cmd.exe directly is the way to go.
What do you think @sindresorhus?
I don't think we should do this. What we could do is to throw an error about using the shell option if something tries to pass in cmd.exe as the first argument.
This sounds good. This makes me wonder:
-
I still think some users might want to use their own parameters. There is an almost finalized PR in Node to support
option.shellbeing an array of strings (command + parameters), allowing users to customize shell parameters. I think once the PR is merged, we should allow the same for older Node.js versions. What do you think? -
Should we restrict error throwing to only
cmd.exeas first argument? Theshelloption translates tocmd.exe /d /s /con Windows, but tosh -con Unix. Those two are basically the same but with different shells. So shouldn't calling directlyshthrow as well? What aboutbash(which could be passed to theshelloption as well) and other shells? I don't think this issue is specific tocmd.exe, it's more about shell interpreters as a whole.
I think once the PR is merged, we should allow the same for older Node.js versions. What do you think?
I would prefer not bloating execa with such obscure feature. We already removed some much more commonly-used features in the name of simplifying and reducing the code-size.
Should we restrict error throwing to only cmd.exe as first argument? The shell option translates to cmd.exe /d /s /c on Windows, but to sh -c on Unix. Those two are basically the same but with different shells. So shouldn't calling directly sh throw as well? What about bash (which could be passed to the shell option as well) and other shells? I don't think this issue is specific to cmd.exe, it's more about shell interpreters as a whole.
Yes, but let's limit it to cmd.exe, cmd, sh, and bash.
As someone struggling to run powershell because of these opinionated decisions on cmd.exe being required, please don't ever force how to run a shell on users.
