From e0fa2816fad4bc8c08a8d2d2bc6e2fb48017540e Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Wed, 17 Apr 2024 16:55:34 +0200 Subject: [PATCH 1/4] Fix spawn calls for node LTS versions --- .../src/application-process.ts | 20 +++++++++++++++---- dev-packages/cli/bin/theia-patch.js | 7 +++---- .../src/native-webpack-plugin.ts | 4 ---- .../src/package-re-exports.ts | 3 ++- packages/process/package.json | 2 +- packages/process/src/node/pseudo-pty.ts | 2 ++ yarn.lock | 10 +++++----- 7 files changed, 29 insertions(+), 19 deletions(-) diff --git a/dev-packages/application-manager/src/application-process.ts b/dev-packages/application-manager/src/application-process.ts index 47fda1c743ba1..89e81a381887a 100644 --- a/dev-packages/application-manager/src/application-process.ts +++ b/dev-packages/application-manager/src/application-process.ts @@ -32,11 +32,17 @@ export class ApplicationProcess { ) { } spawn(command: string, args?: string[], options?: cp.SpawnOptions): cp.ChildProcess { - return cp.spawn(command, args || [], Object.assign({}, this.defaultOptions, options)); + return cp.spawn(command, args || [], Object.assign({}, this.defaultOptions, { + ...options, + shell: true + })); } fork(modulePath: string, args?: string[], options?: cp.ForkOptions): cp.ChildProcess { - return cp.fork(modulePath, args, Object.assign({}, this.defaultOptions, options)); + return cp.fork(modulePath, args, Object.assign({}, this.defaultOptions, { + ...options, + shell: true + })); } canRun(command: string): boolean { @@ -44,13 +50,19 @@ export class ApplicationProcess { } run(command: string, args: string[], options?: cp.SpawnOptions): Promise { - const commandProcess = this.spawnBin(command, args, options); + const commandProcess = this.spawnBin(command, args, { + ...options, + shell: true + }); return this.promisify(command, commandProcess); } spawnBin(command: string, args: string[], options?: cp.SpawnOptions): cp.ChildProcess { const binPath = this.resolveBin(command); - return this.spawn(binPath, args, options); + return this.spawn(binPath, args, { + ...options, + shell: true + }); } protected resolveBin(command: string): string { diff --git a/dev-packages/cli/bin/theia-patch.js b/dev-packages/cli/bin/theia-patch.js index 5c9e7ad2ad828..a6a511fa9b2dd 100755 --- a/dev-packages/cli/bin/theia-patch.js +++ b/dev-packages/cli/bin/theia-patch.js @@ -19,7 +19,7 @@ const path = require('path'); const cp = require('child_process'); -const patchPackage= require.resolve('patch-package'); +const patchPackage = require.resolve('patch-package'); console.log(`patch-package = ${patchPackage}`); const patchesDir = path.join('.', 'node_modules', '@theia', 'cli', 'patches'); @@ -28,10 +28,9 @@ console.log(`patchesdir = ${patchesDir}`); const env = Object.assign({}, process.env); -const scriptProcess = cp.exec(`node ${patchPackage} --patch-dir "${patchesDir}"`, { +const scriptProcess = cp.exec(`node "${patchPackage}" --patch-dir "${patchesDir}"`, { cwd: process.cwd(), - env, - + env }); scriptProcess.stdout.pipe(process.stdout); diff --git a/dev-packages/native-webpack-plugin/src/native-webpack-plugin.ts b/dev-packages/native-webpack-plugin/src/native-webpack-plugin.ts index eafa1a01ac386..7ff3c13e20fcc 100644 --- a/dev-packages/native-webpack-plugin/src/native-webpack-plugin.ts +++ b/dev-packages/native-webpack-plugin/src/native-webpack-plugin.ts @@ -124,10 +124,6 @@ export class NativeWebpackPlugin { const dllFile = require.resolve('node-pty/build/Release/winpty.dll'); const targetDllFile = path.join(targetDirectory, 'winpty.dll'); await this.copyExecutable(dllFile, targetDllFile); - } else { - const sourceFile = require.resolve('node-pty/build/Release/spawn-helper'); - const targetFile = path.join(targetDirectory, 'spawn-helper'); - await this.copyExecutable(sourceFile, targetFile); } } diff --git a/dev-packages/private-re-exports/src/package-re-exports.ts b/dev-packages/private-re-exports/src/package-re-exports.ts index 40aebc7a91db4..65f0fddfc4750 100644 --- a/dev-packages/private-re-exports/src/package-re-exports.ts +++ b/dev-packages/private-re-exports/src/package-re-exports.ts @@ -177,7 +177,8 @@ export class PackageReExports { ELECTRON_RUN_AS_NODE: '1' }, encoding: 'utf8', - stdio: ['ignore', 'pipe', 'inherit'] + stdio: ['ignore', 'pipe', 'inherit'], + shell: true }); const [packageRoot, reExports] = JSON.parse(stdout) as [string, ReExport[]]; return new PackageReExports(packageName, packageRoot, reExports); diff --git a/packages/process/package.json b/packages/process/package.json index 736f1b4b13ae0..713d6dee59f44 100644 --- a/packages/process/package.json +++ b/packages/process/package.json @@ -4,7 +4,7 @@ "description": "Theia process support.", "dependencies": { "@theia/core": "1.48.0", - "node-pty": "0.11.0-beta17", + "node-pty": "1.0.0", "string-argv": "^0.1.1", "tslib": "^2.6.2" }, diff --git a/packages/process/src/node/pseudo-pty.ts b/packages/process/src/node/pseudo-pty.ts index 245a66dd2b211..b44cb5f7a126b 100644 --- a/packages/process/src/node/pseudo-pty.ts +++ b/packages/process/src/node/pseudo-pty.ts @@ -51,4 +51,6 @@ export class PseudoPty implements IPty { pause(): void { } resume(): void { } + + clear(): void { } } diff --git a/yarn.lock b/yarn.lock index 489221288bada..900f027315dd8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8594,12 +8594,12 @@ node-preload@^0.2.1: dependencies: process-on-spawn "^1.0.0" -node-pty@0.11.0-beta17: - version "0.11.0-beta17" - resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-0.11.0-beta17.tgz#7df6a60dced6bf7a3a282b65cf51980c68954af6" - integrity sha512-JALo4LgYKmzmmXI23CIfS6DpCuno647YJpNg3RT6jCKTHWrt+RHeB6JAlb/pJG9dFNSeaiIAWD+0waEg2AzlfA== +node-pty@1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-1.0.0.tgz#7daafc0aca1c4ca3de15c61330373af4af5861fd" + integrity sha512-wtBMWWS7dFZm/VgqElrTvtfMq4GzJ6+edFI0Y0zyzygUSZMgZdraDUMUhCIvkjhJjme15qWmbyJbtAx4ot4uZA== dependencies: - nan "^2.14.0" + nan "^2.17.0" node-releases@^2.0.14: version "2.0.14" From edc63d89c87ff34ec6eb0849120eaf7038eab2b6 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Thu, 18 Apr 2024 14:19:00 +0200 Subject: [PATCH 2/4] Test error --- packages/process/package.json | 2 +- .../process/src/common/process-manager-types.ts | 1 + packages/process/src/node/process.ts | 4 ++-- packages/process/src/node/terminal-process.ts | 16 +++++++++++++++- .../terminal/src/node/shell-terminal-server.ts | 4 +++- yarn.lock | 8 ++++---- 6 files changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/process/package.json b/packages/process/package.json index 713d6dee59f44..0606b5165ef95 100644 --- a/packages/process/package.json +++ b/packages/process/package.json @@ -4,7 +4,7 @@ "description": "Theia process support.", "dependencies": { "@theia/core": "1.48.0", - "node-pty": "1.0.0", + "node-pty": "1.1.0-beta5", "string-argv": "^0.1.1", "tslib": "^2.6.2" }, diff --git a/packages/process/src/common/process-manager-types.ts b/packages/process/src/common/process-manager-types.ts index 01653487f8e6b..a8af2d0585a6a 100644 --- a/packages/process/src/common/process-manager-types.ts +++ b/packages/process/src/common/process-manager-types.ts @@ -42,6 +42,7 @@ export interface IProcessExitEvent { * Data emitted when a process has been successfully started. */ export interface IProcessStartEvent { + readonly pid?: number; } /** diff --git a/packages/process/src/node/process.ts b/packages/process/src/node/process.ts index 490ee09015563..a30e7b38dd81c 100644 --- a/packages/process/src/node/process.ts +++ b/packages/process/src/node/process.ts @@ -123,8 +123,8 @@ export abstract class Process implements ManagedProcess { return this.closeEmitter.event; } - protected emitOnStarted(): void { - this.startEmitter.fire({}); + protected emitOnStarted(event?: IProcessStartEvent): void { + this.startEmitter.fire(event ?? {}); } /** diff --git a/packages/process/src/node/terminal-process.ts b/packages/process/src/node/terminal-process.ts index f6879af71a08d..500b6d83f3aa6 100644 --- a/packages/process/src/node/terminal-process.ts +++ b/packages/process/src/node/terminal-process.ts @@ -25,6 +25,7 @@ import { DevNullStream } from './dev-null-stream'; import { signame } from './utils'; import { PseudoPty } from './pseudo-pty'; import { Writable } from 'stream'; +import * as fs from 'fs'; export const TerminalProcessOptions = Symbol('TerminalProcessOptions'); export interface TerminalProcessOptions extends ProcessOptions { @@ -144,13 +145,26 @@ export class TerminalProcess extends Process { * @returns the terminal PTY and a stream by which it may be sent input */ private createPseudoTerminal(command: string, options: TerminalProcessOptions, ringBuffer: MultiRingBuffer): { terminal: IPty | undefined, inputStream: Writable } { + // Only test the command file on non-Windows platforms. + // On Windows, calling `spawn` on an invalid file throws an error. On Linux/macOS, it does not. + if (!isWindows) { + try { + const stat = fs.statSync(command); + if (stat.isDirectory()) { + throw new Error('Permission denied'); + } + } catch (error) { + throw new Error('File not found: ' + command); + } + } + const terminal = spawn( command, (isWindows && options.commandLine) || options.args || [], options.options || {} ); - process.nextTick(() => this.emitOnStarted()); + setImmediate(() => this.emitOnStarted({ pid: terminal.pid })); // node-pty actually wait for the underlying streams to be closed before emitting exit. // We should emulate the `exit` and `close` sequence. diff --git a/packages/terminal/src/node/shell-terminal-server.ts b/packages/terminal/src/node/shell-terminal-server.ts index 63eded6a30618..bb2270f162574 100644 --- a/packages/terminal/src/node/shell-terminal-server.ts +++ b/packages/terminal/src/node/shell-terminal-server.ts @@ -69,7 +69,9 @@ export class ShellTerminalServer extends BaseTerminalServer implements IShellTer private spawnAsPromised(command: string, args: string[]): Promise { return new Promise((resolve, reject) => { let stdout = ''; - const child = cp.spawn(command, args); + const child = cp.spawn(command, args, { + shell: true + }); if (child.pid) { child.stdout.on('data', (data: Buffer) => { stdout += data.toString(); diff --git a/yarn.lock b/yarn.lock index 900f027315dd8..c438266377b01 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8594,10 +8594,10 @@ node-preload@^0.2.1: dependencies: process-on-spawn "^1.0.0" -node-pty@1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-1.0.0.tgz#7daafc0aca1c4ca3de15c61330373af4af5861fd" - integrity sha512-wtBMWWS7dFZm/VgqElrTvtfMq4GzJ6+edFI0Y0zyzygUSZMgZdraDUMUhCIvkjhJjme15qWmbyJbtAx4ot4uZA== +node-pty@1.1.0-beta5: + version "1.1.0-beta5" + resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-1.1.0-beta5.tgz#364386b7058a93070234064f13164ec1ef914993" + integrity sha512-j3QdgFHnLY0JWxztrvM3g67RaQLOGvytv+C6mFu0PqD+JILlzqfwuoyqRqVxdZZjoOTUXPfSRj1qPVCaCH+eOw== dependencies: nan "^2.17.0" From 80ccead1f2cf358eb291a8cfd0094fe7c4a6a478 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Wed, 24 Apr 2024 12:34:56 +0000 Subject: [PATCH 3/4] Revert version to earlier --- packages/process/package.json | 2 +- .../process/src/common/process-manager-types.ts | 1 - packages/process/src/node/process.ts | 4 ++-- packages/process/src/node/terminal-process.ts | 16 +--------------- yarn.lock | 8 ++++---- 5 files changed, 8 insertions(+), 23 deletions(-) diff --git a/packages/process/package.json b/packages/process/package.json index 0606b5165ef95..ca4e0cd22b8c3 100644 --- a/packages/process/package.json +++ b/packages/process/package.json @@ -4,7 +4,7 @@ "description": "Theia process support.", "dependencies": { "@theia/core": "1.48.0", - "node-pty": "1.1.0-beta5", + "node-pty": "0.11.0-beta24", "string-argv": "^0.1.1", "tslib": "^2.6.2" }, diff --git a/packages/process/src/common/process-manager-types.ts b/packages/process/src/common/process-manager-types.ts index a8af2d0585a6a..01653487f8e6b 100644 --- a/packages/process/src/common/process-manager-types.ts +++ b/packages/process/src/common/process-manager-types.ts @@ -42,7 +42,6 @@ export interface IProcessExitEvent { * Data emitted when a process has been successfully started. */ export interface IProcessStartEvent { - readonly pid?: number; } /** diff --git a/packages/process/src/node/process.ts b/packages/process/src/node/process.ts index a30e7b38dd81c..490ee09015563 100644 --- a/packages/process/src/node/process.ts +++ b/packages/process/src/node/process.ts @@ -123,8 +123,8 @@ export abstract class Process implements ManagedProcess { return this.closeEmitter.event; } - protected emitOnStarted(event?: IProcessStartEvent): void { - this.startEmitter.fire(event ?? {}); + protected emitOnStarted(): void { + this.startEmitter.fire({}); } /** diff --git a/packages/process/src/node/terminal-process.ts b/packages/process/src/node/terminal-process.ts index 500b6d83f3aa6..f6879af71a08d 100644 --- a/packages/process/src/node/terminal-process.ts +++ b/packages/process/src/node/terminal-process.ts @@ -25,7 +25,6 @@ import { DevNullStream } from './dev-null-stream'; import { signame } from './utils'; import { PseudoPty } from './pseudo-pty'; import { Writable } from 'stream'; -import * as fs from 'fs'; export const TerminalProcessOptions = Symbol('TerminalProcessOptions'); export interface TerminalProcessOptions extends ProcessOptions { @@ -145,26 +144,13 @@ export class TerminalProcess extends Process { * @returns the terminal PTY and a stream by which it may be sent input */ private createPseudoTerminal(command: string, options: TerminalProcessOptions, ringBuffer: MultiRingBuffer): { terminal: IPty | undefined, inputStream: Writable } { - // Only test the command file on non-Windows platforms. - // On Windows, calling `spawn` on an invalid file throws an error. On Linux/macOS, it does not. - if (!isWindows) { - try { - const stat = fs.statSync(command); - if (stat.isDirectory()) { - throw new Error('Permission denied'); - } - } catch (error) { - throw new Error('File not found: ' + command); - } - } - const terminal = spawn( command, (isWindows && options.commandLine) || options.args || [], options.options || {} ); - setImmediate(() => this.emitOnStarted({ pid: terminal.pid })); + process.nextTick(() => this.emitOnStarted()); // node-pty actually wait for the underlying streams to be closed before emitting exit. // We should emulate the `exit` and `close` sequence. diff --git a/yarn.lock b/yarn.lock index c438266377b01..8b45a3820e7f0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8594,10 +8594,10 @@ node-preload@^0.2.1: dependencies: process-on-spawn "^1.0.0" -node-pty@1.1.0-beta5: - version "1.1.0-beta5" - resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-1.1.0-beta5.tgz#364386b7058a93070234064f13164ec1ef914993" - integrity sha512-j3QdgFHnLY0JWxztrvM3g67RaQLOGvytv+C6mFu0PqD+JILlzqfwuoyqRqVxdZZjoOTUXPfSRj1qPVCaCH+eOw== +node-pty@0.11.0-beta24: + version "0.11.0-beta24" + resolved "https://registry.yarnpkg.com/node-pty/-/node-pty-0.11.0-beta24.tgz#084841017187656edaf14b459946c4a1d7cf8392" + integrity sha512-CzItw3hitX+wnpw9dHA/A+kcbV7ETNKrsyQJ+s0ZGzsu70+CSGuIGPLPfMnAc17vOrQktxjyRQfaqij75GVJFw== dependencies: nan "^2.17.0" From 55dff38524b81eda71def78ee55f28cab730bae6 Mon Sep 17 00:00:00 2001 From: Mark Sujew Date: Thu, 25 Apr 2024 12:59:38 +0000 Subject: [PATCH 4/4] Review comments --- .../application-manager/src/application-process.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/dev-packages/application-manager/src/application-process.ts b/dev-packages/application-manager/src/application-process.ts index 89e81a381887a..2befd37bcfc50 100644 --- a/dev-packages/application-manager/src/application-process.ts +++ b/dev-packages/application-manager/src/application-process.ts @@ -39,10 +39,7 @@ export class ApplicationProcess { } fork(modulePath: string, args?: string[], options?: cp.ForkOptions): cp.ChildProcess { - return cp.fork(modulePath, args, Object.assign({}, this.defaultOptions, { - ...options, - shell: true - })); + return cp.fork(modulePath, args, Object.assign({}, this.defaultOptions, options)); } canRun(command: string): boolean { @@ -50,10 +47,7 @@ export class ApplicationProcess { } run(command: string, args: string[], options?: cp.SpawnOptions): Promise { - const commandProcess = this.spawnBin(command, args, { - ...options, - shell: true - }); + const commandProcess = this.spawnBin(command, args, options); return this.promisify(command, commandProcess); }