Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node): Basic support for child_process.spawn #785

Merged
merged 27 commits into from
Apr 21, 2021

Conversation

uki00a
Copy link
Contributor

@uki00a uki00a commented Mar 4, 2021

Summary

This PR adds basic support for child_process.spawn based on the Deno.run API.

Related to denoland/deno#5376

Limitation

Currently, support for the shell option in Windows is quite limited due to escaping issues. For more details, see the following issues:

@CLAassistant
Copy link

CLAassistant commented Mar 4, 2021

CLA assistant check
All committers have signed the CLA.

@uki00a uki00a changed the title feat(node): Add child_process.spawn feat(node): Basic support for child_process.spawn Apr 3, 2021
exitCode = code;
});
await promise;
assertStrictEquals(exitCode, 0);
Copy link
Member

Choose a reason for hiding this comment

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

👍

@kt3k
Copy link
Member

kt3k commented Apr 15, 2021

@uki00a What do you think is the remaining task for the first pass?

node/module.ts Show resolved Hide resolved
@uki00a
Copy link
Contributor Author

uki00a commented Apr 15, 2021

@kt3k I'm still having one problem and not sure yet if that problem is caused by this PR's code or by the Deno.run API. 🤔

What is the problem?

There is a test.ts with the following contents:

import { spawn } from "./node/child_process.ts"

const deno = spawn("deno", ["--version"]);
let out = "";
deno.stdout!.on("data", (chunk) => {
  out += chunk;
});
deno.stderr!.on("data", (err) => console.error(err));
deno.on("exit", () => {
  console.log(out);
});

When I run this file, it works as intended:

$ deno run -A --unstable test.ts

deno 1.8.3 (release, x86_64-unknown-linux-gnu)
v8 9.0.257.3
typescript 4.2.2

Then, change the test.ts as follows:

import { spawn } from "./node/child_process.ts"

const git = spawn("git", ["--version"]);
let out = "";
git.stdout!.on("data", (chunk) => {
  out += chunk;
});
git.stderr!.on("data", (err) => console.error(err));
git.on("exit", () => {
  console.log(out);
});

When I run the test.ts, I get the following errors:

$ deno run -A --unstable test.ts

Uncaught Interrupted: operation canceled
Uncaught Interrupted: operation canceled

@uki00a uki00a marked this pull request as ready for review April 18, 2021 15:24
@uki00a
Copy link
Contributor Author

uki00a commented Apr 18, 2021

The problem causing the "operation canceled" error has been resolved by 9e38473. However, tests for the canary build seem to be failing... 🤔🧐

error: TS2322 [ERROR]: Type '{ transport: string; hostname: string; port: number; }' is not assignable to type 'Addr'.
  Object literal may only specify known properties, and 'hostname' does not exist in type 'Addr'.
      hostname: "",
      ~~~~~~~~~~~~
    at file:///Users/runner/work/deno_std/deno_std/http/_mock_conn.ts:8:7

@kt3k
Copy link
Member

kt3k commented Apr 19, 2021

Canary test is failing because of #860, and probably will be fixed by #861. No need of actions for now.

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

@uki00a Nice work! Thanks!

LGTM

@kt3k kt3k merged commit 2a0bf03 into denoland:main Apr 21, 2021
@uki00a uki00a deleted the implement-child-process-spawn branch April 21, 2021 12:13
@uki00a
Copy link
Contributor Author

uki00a commented Apr 21, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants