Repository navigation
fix(typespec-python): remove shell:true from spawn to eliminate DEP0190 deprecation warning - #5001
Libba Lawrence (l0lawrence) with Copilot wants to merge 3 commits into
Conversation
|
Azure Pipelines: 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
…P0190 deprecation warning Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com>
| ): Promise<ExecResult> => { | ||
| return new Promise((resolve, reject) => { | ||
| const cp = spawn(command, cmdlineargs, { ...options, stdio: "pipe", shell: true }); | ||
| const cp = spawn(command, cmdlineargs, { ...options, stdio: "pipe" }); |
There was a problem hiding this comment.
what is command going to be, is that cross platform compatible?
There was a problem hiding this comment.
I think it can be py,python3, or python
There was a problem hiding this comment.
But isn't it python.cmd on windows. I think this is the reason shell was set to true so it works on windows. It is good to remove shell: true but I think need to use crossspawn package instead or need to be smarter about finding the command
There was a problem hiding this comment.
I thought it was .exe but let me look
There was a problem hiding this comment.
ok just wanted to make sure it does actually work across platforms, windows is super annoying with what you can call with spawn. If it does then this looks good
commit: |
…ments.ts Restores Windows support for executing .cmd/.bat wrappers (e.g. python.cmd) that was lost when shell:true was removed, while keeping the DEP0190 deprecation warning fixed. cross-spawn handles arg escaping, so manual quoting is not needed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a400ee01-6085-4bac-a9a0-e3ac44faadca
|
Hi @Copilot. Your PR has had no update for 14 days and it is marked as a stale PR. If it is not updated within 14 days, the PR will automatically be closed. If you want to refresh the PR, please remove the |
|
Hi @Copilot. This PR will be closed since it has had no update for 28 days. If this change is still relevant, please reopen the PR. |
Node.js DEP0190 warns that passing an args array to
spawnwithshell: trueis insecure — args are concatenated rather than escaped. This warning was being emitted during both theinstallandpreparesteps oftypespec-python.Changes
system-requirements.ts—execute(): Removeshell: truefrom thespawnoptions. Arguments are already passed as an array, so the shell is unnecessary.system-requirements.ts—tryPython(): Remove the wrapping double-quotes aroundPRINT_PYTHON_VERSION_SCRIPT. Those quotes were shell-interpreted (and stripped) whenshell: truewas active. Without the shell, they would be passed as literal"characters to Python's-cflag, breaking version detection.