Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: internal
packages:
- "@azure-tools/typespec-python"
---

Use `cross-spawn` instead of `child_process.spawn` in the system requirements setup script. This removes the DEP0190 deprecation warning caused by `shell: true` while keeping support for executing `.cmd`/`.bat` wrappers (e.g. `python.cmd`) on Windows.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ChildProcess, spawn, SpawnOptions } from "child_process";
import type { ChildProcess, SpawnOptions } from "child_process";
import { spawn } from "cross-spawn";
import { coerce, satisfies } from "semver";

/*
Expand All @@ -11,26 +12,26 @@ const execute = (
options: MoreOptions = {},
): 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" });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is command going to be, is that cross platform compatible?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it can be py,python3, or python

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was .exe but let me look

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (options.onCreate) {
options.onCreate(cp);
}

if (options.onStdOutData) {
cp.stdout.on("data", options.onStdOutData);
cp.stdout?.on("data", options.onStdOutData);
}
if (options.onStdErrData) {
cp.stderr.on("data", options.onStdErrData);
cp.stderr?.on("data", options.onStdErrData);
}

let err = "";
let out = "";
let all = "";
cp.stderr.on("data", (chunk) => {
cp.stderr?.on("data", (chunk) => {
err += chunk;
all += chunk;
});
cp.stdout.on("data", (chunk) => {
cp.stdout?.on("data", (chunk) => {
out += chunk;
all += chunk;
});
Expand Down Expand Up @@ -111,7 +112,7 @@ const tryPython = async (
const result = await execute(command, [
...additionalArgs,
"-c",
`"${PRINT_PYTHON_VERSION_SCRIPT}"`,
PRINT_PYTHON_VERSION_SCRIPT,
]);
return validateVersionRequirement(resolution, result.stdout.trim(), requirement);
} catch (e) {
Expand Down
2 changes: 2 additions & 0 deletions packages/typespec-python/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
},
"dependencies": {
"@typespec/http-client-python": "catalog:",
"cross-spawn": "catalog:",
"semver": "catalog:",
"tsx": "catalog:"
},
Expand All @@ -76,6 +77,7 @@
"@azure-tools/typespec-azure-resource-manager": "workspace:^",
"@azure-tools/typespec-azure-rulesets": "workspace:^",
"@azure-tools/typespec-client-generator-core": "workspace:^",
"@types/cross-spawn": "catalog:",
"@types/node": "catalog:",
"@types/semver": "catalog:",
"@types/yargs": "catalog:",
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.