Repository navigation
child_process,Windows: deprecate explicit use of cmd.exe #14157
Description
Activity
@nodejs/platform-windows
Reacted by Refael AckermannTechnically, this is correct, and I don't see a chance of breaking code. Some will probably break due to custom child process environments, but that's what deprecation is for.
So, for users, what this is effectively deprecating is using an environment without
%COMSPEC%.I'm -0 on this. While situations without
%COMSPEC%are probably quite messed up already,cmd.exeis a valid fallback and I don't see a pressing reason for deprecating, how this actually translates as an improvement for node.But discussion is always welcome!
I'm -0 on this. While situations without %COMSPEC% are probably quite messed up already, cmd.exe is a valid fallback and I don't see a pressing reason for deprecating, how this actually translates as an improvement for node.
But discussion is always welcome!
- I opened this as an issue rather than a PR mainly for discussion 😃
- Literal command strings IMHO are just bad and need a very good reason to exist.
- In this case it doesn't "solve", just improves the situation by ϵ%. Just as the environment could be missing %COMSPEC%, it could be missing %PATH% (or %WINDIR% so
%WINDIR%\system32\cmd.exeisn't any better), and also the system could be missingcmd.execompletely. So if we're just patching over a messed up situation with a only a slightly less messy fix, why bother 🤷♂️ - In general I'm just +0.5 on this whole thing, I just don't like literal command strings in the code
Reacted by João ReisClosing as it is inactive for around an year, feel free to re-open if there is anything outstanding.
https://github.com/nodejs/node/blob/master/lib/child_process.js#L444 has a fallback to use
cmd.exein caseprocess.env.ComSpecis falsy.This is redundant, fragile, and generally covers-up an invalid state (
%ComSpec%should always be defined and point to a valid shell executable).This code path should be deprecated according to the guide at https://github.com/nodejs/node/blob/master/COLLABORATOR_GUIDE.md#deprecations
and https://github.com/nodejs/node/blob/master/doc/api/deprecations.md
Ref: #14149 (comment)