From 60e05a023597bd475ead2a3275fb933e1d8cd4a8 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 16 Aug 2021 18:23:20 -0700 Subject: [PATCH 01/27] Get working prototype This version allows for opening folders and files, including with the -a flag --- src/vs/code/node/cli.ts | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index de50b33b195c8..5de47796924fb 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -5,7 +5,7 @@ import { ChildProcess, spawn, SpawnOptions } from 'child_process'; import { chmodSync, existsSync, readFileSync, statSync, truncateSync, unlinkSync } from 'fs'; -import { homedir } from 'os'; +import { homedir, platform } from 'os'; import type { ProfilingSession, Target } from 'v8-inspect-profiler'; import { Event } from 'vs/base/common/event'; import { isAbsolute, join } from 'vs/base/common/path'; @@ -337,7 +337,42 @@ export async function main(argv: string[]): Promise { options['stdio'] = 'ignore'; } - const child = spawn(process.execPath, argv.slice(2), options); + let child: ChildProcess; + if (platform() !== 'darwin') { + child = spawn(process.execPath, argv.slice(2), options); + + if (args.wait && waitMarkerFilePath) { + return new Promise(resolve => { + + // Complete when process exits + child.once('exit', () => resolve(undefined)); + + // Complete when wait marker file is deleted + whenDeleted(waitMarkerFilePath!).then(resolve, resolve); + }).then(() => { + + // Make sure to delete the tmp stdin file if we have any + if (stdinFilePath) { + unlinkSync(stdinFilePath); + } + }); + } + } else { + const envVars: string[] = []; + for (const envKey in env) { + // if (envKey === '_') { + // continue; + // } + const value = env[envKey]; + envVars.push('--env'); + envVars.push(envKey + '=' + value ?? ''); + } + const argsArr = ['-b', 'com.microsoft.VSCode', ...envVars, '-n', '--args', ...argv.slice(2)]; + // console.log('open ' + argsArr.join(' ')); + // console.log(JSON.stringify(argsArr)); + + child = spawn('open', argsArr, options); + } return Promise.all(processCallbacks.map(callback => callback(child))); } From de25b0dcb1c88dcecc3b580083f45cfde3c2a0ee Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 16 Aug 2021 18:44:15 -0700 Subject: [PATCH 02/27] Ignore underscore env var --- src/vs/code/node/cli.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 5de47796924fb..0ed0e43dd8d15 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -360,9 +360,9 @@ export async function main(argv: string[]): Promise { } else { const envVars: string[] = []; for (const envKey in env) { - // if (envKey === '_') { - // continue; - // } + if (envKey === '_') { + continue; + } const value = env[envKey]; envVars.push('--env'); envVars.push(envKey + '=' + value ?? ''); From e2d4dcf9286b6e33d37d547b5d3c21a1a9c31394 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Tue, 17 Aug 2021 17:54:13 -0700 Subject: [PATCH 03/27] Draft: Handle --status and --verbose --- src/vs/code/node/cli.ts | 70 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 6 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 0ed0e43dd8d15..825dc3f39c6be 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -4,8 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import { ChildProcess, spawn, SpawnOptions } from 'child_process'; -import { chmodSync, existsSync, readFileSync, statSync, truncateSync, unlinkSync } from 'fs'; -import { homedir, platform } from 'os'; +import { chmodSync, closeSync, existsSync, openSync, readFileSync, readSync, statSync, truncateSync, unlinkSync } from 'fs'; +import { homedir, platform, tmpdir } from 'os'; import type { ProfilingSession, Target } from 'v8-inspect-profiler'; import { Event } from 'vs/base/common/event'; import { isAbsolute, join } from 'vs/base/common/path'; @@ -358,6 +358,7 @@ export async function main(argv: string[]): Promise { }); } } else { + const requiresWait = args['telemetry'] || args['status'] || args['verbose']; const envVars: string[] = []; for (const envKey in env) { if (envKey === '_') { @@ -367,11 +368,68 @@ export async function main(argv: string[]): Promise { envVars.push('--env'); envVars.push(envKey + '=' + value ?? ''); } - const argsArr = ['-b', 'com.microsoft.VSCode', ...envVars, '-n', '--args', ...argv.slice(2)]; - // console.log('open ' + argsArr.join(' ')); - // console.log(JSON.stringify(argsArr)); - + const openArgs: string[] = ['-n']; + + let tmpfile = ''; + let tmpfileError = ''; + if (requiresWait) { + const time = new Date().getTime(); + tmpfile = `${tmpdir()}/vscode-wait-transfer-${time}.log`; + tmpfileError = `${tmpdir()}/vscode-wait-transfer-error-${time}.log`; + writeFileSync(tmpfile, ''); + openArgs.push('-W'); + openArgs.push('--stdout', tmpfile); + openArgs.push('--stderr', tmpfileError); + argv.push('--wait'); + } + const argsArr: string[] = []; + if (process.execPath.startsWith('code')) { + argsArr.push('-a', process.execPath); + } else { + // running from OSS, launch stable + argsArr.push('-b', 'com.microsoft.VSCode'); + } + argsArr.push(...envVars, ...openArgs, '--args', ...argv.slice(2)); child = spawn('open', argsArr, options); + + if (requiresWait) { + const openPromise = (child: ChildProcess) => new Promise((c) => { + console.log('Temp file location: ' + tmpfile); + if (args['verbose']) { + const stream = openSync(tmpfile, 'r'); + const bufferSize = 50; + const buffer = Buffer.alloc(bufferSize); + const interval = setInterval(() => { + const readAmount = readSync(stream, buffer, 0, bufferSize, null); + process.stdout.write(buffer.toString(undefined, 0, readAmount)); + }, 50); + child.on('exit', () => { + setTimeout(() => { + clearTimeout(interval); + closeSync(stream); + c(); + }, 1500); + }); + } else if (args['status']) { + child.on('exit', () => { + const buffer = readFileSync(tmpfile); + let bufferContents = buffer.toString().trim(); + console.log(bufferContents); + c(); + }); + } else { + // TODO: find telemetry file + // We currently get the following error: + /* vscode/.build/electron/Code\ -\ OSS.app/Contents/MacOS/Electron vscode/out/cli.js --telemetry + ENOENT: no such file or directory, open '/Users/raymondzhao/work/vscode/telemetry-core.json' + */ + } + }).then(() => { + unlinkSync(tmpfile); + unlinkSync(tmpfileError); + }); + processCallbacks.push(openPromise); + } } return Promise.all(processCallbacks.map(callback => callback(child))); From e0a81a37093ac1d7d0c66b4052e941a043390166 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Thu, 19 Aug 2021 15:09:36 -0700 Subject: [PATCH 04/27] Add draft for telemetry, add --exec-path --- src/vs/code/node/cli.ts | 18 ++++-------------- src/vs/platform/environment/common/argv.ts | 1 + src/vs/platform/environment/node/argv.ts | 1 + .../platform/environment/node/userDataPath.js | 5 +++++ 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 825dc3f39c6be..041d84ee9c0b9 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -358,7 +358,7 @@ export async function main(argv: string[]): Promise { }); } } else { - const requiresWait = args['telemetry'] || args['status'] || args['verbose']; + const requiresWait = args['status'] || args['verbose']; const envVars: string[] = []; for (const envKey in env) { if (envKey === '_') { @@ -383,12 +383,8 @@ export async function main(argv: string[]): Promise { argv.push('--wait'); } const argsArr: string[] = []; - if (process.execPath.startsWith('code')) { - argsArr.push('-a', process.execPath); - } else { - // running from OSS, launch stable - argsArr.push('-b', 'com.microsoft.VSCode'); - } + const execPathToUse = args['exec-path'] ?? process.execPath; + argsArr.push('-a', execPathToUse); argsArr.push(...envVars, ...openArgs, '--args', ...argv.slice(2)); child = spawn('open', argsArr, options); @@ -410,19 +406,13 @@ export async function main(argv: string[]): Promise { c(); }, 1500); }); - } else if (args['status']) { + } else { child.on('exit', () => { const buffer = readFileSync(tmpfile); let bufferContents = buffer.toString().trim(); console.log(bufferContents); c(); }); - } else { - // TODO: find telemetry file - // We currently get the following error: - /* vscode/.build/electron/Code\ -\ OSS.app/Contents/MacOS/Electron vscode/out/cli.js --telemetry - ENOENT: no such file or directory, open '/Users/raymondzhao/work/vscode/telemetry-core.json' - */ } }).then(() => { unlinkSync(tmpfile); diff --git a/src/vs/platform/environment/common/argv.ts b/src/vs/platform/environment/common/argv.ts index 70a7eb4b86e41..bb82658069b9a 100644 --- a/src/vs/platform/environment/common/argv.ts +++ b/src/vs/platform/environment/common/argv.ts @@ -85,6 +85,7 @@ export interface NativeParsedArgs { 'sync'?: 'on' | 'off'; '__sandbox'?: boolean; 'logsPath'?: string; + 'exec-path'?: string; // chromium command line args: https://electronjs.org/docs/all#supported-chrome-command-line-switches 'no-proxy-server'?: boolean; diff --git a/src/vs/platform/environment/node/argv.ts b/src/vs/platform/environment/node/argv.ts index 823ef14c42aa9..24ed695ba8813 100644 --- a/src/vs/platform/environment/node/argv.ts +++ b/src/vs/platform/environment/node/argv.ts @@ -120,6 +120,7 @@ export const OPTIONS: OptionDescriptions> = { 'open-devtools': { type: 'boolean' }, '__sandbox': { type: 'boolean' }, 'logsPath': { type: 'string' }, + 'exec-path': { type: 'string' }, // chromium flags 'no-proxy-server': { type: 'boolean' }, diff --git a/src/vs/platform/environment/node/userDataPath.js b/src/vs/platform/environment/node/userDataPath.js index 1f7196c33e162..9cd5538f51d1b 100644 --- a/src/vs/platform/environment/node/userDataPath.js +++ b/src/vs/platform/environment/node/userDataPath.js @@ -60,6 +60,11 @@ return path.join(appDataPath, productName); } + // 3. Support debugging mac OSS CLI + if (process.platform === 'darwin' && cliArgs['exec-path']) { + return cliArgs['exec-path']; + } + // With Electron>=13 --user-data-dir switch will be propagated to // all processes https://github.com/electron/electron/blob/1897b14af36a02e9aa7e4d814159303441548251/shell/browser/electron_browser_client.cc#L546-L553 // Check VSCODE_PORTABLE and VSCODE_APPDATA before this case to get correct values. From bf31afeca3e5ce62c96d985e09208cfc6813bd94 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Thu, 19 Aug 2021 16:05:32 -0700 Subject: [PATCH 05/27] Get --telemetry working with --exec-path --- src/vs/code/node/cliProcessMain.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vs/code/node/cliProcessMain.ts b/src/vs/code/node/cliProcessMain.ts index c35bbc0f4094a..3e1b624973d4a 100644 --- a/src/vs/code/node/cliProcessMain.ts +++ b/src/vs/code/node/cliProcessMain.ts @@ -229,7 +229,10 @@ class CliMain extends Disposable { // Telemetry else if (this.argv['telemetry']) { - console.log(await buildTelemetryMessage(environmentService.appRoot, environmentService.extensionsPath)); + const rootToUse = this.argv['exec-path'] ? + join(this.argv['exec-path'], '/Contents/Resources/app') : + environmentService.appRoot; + console.log(await buildTelemetryMessage(rootToUse, environmentService.extensionsPath)); } } From 9fb94dd5840cbcd2a36e1528e6fcbf904c2a1da2 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Thu, 19 Aug 2021 16:24:50 -0700 Subject: [PATCH 06/27] Add support for stdin arg --- src/vs/code/node/cli.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 041d84ee9c0b9..29e6a4a95a665 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -358,7 +358,7 @@ export async function main(argv: string[]): Promise { }); } } else { - const requiresWait = args['status'] || args['verbose']; + const requiresWait = args['status'] || args['verbose'] || hasReadStdinArg; const envVars: string[] = []; for (const envKey in env) { if (envKey === '_') { @@ -406,7 +406,7 @@ export async function main(argv: string[]): Promise { c(); }, 1500); }); - } else { + } else if (args['status']) { child.on('exit', () => { const buffer = readFileSync(tmpfile); let bufferContents = buffer.toString().trim(); From 9459ab6fb49a44d0df559695ba0c005a7f87e4f0 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 23 Aug 2021 11:24:12 -0700 Subject: [PATCH 07/27] Apply PR feedback --- src/vs/code/node/cli.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 29e6a4a95a665..58b76fd94a1ab 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -361,6 +361,7 @@ export async function main(argv: string[]): Promise { const requiresWait = args['status'] || args['verbose'] || hasReadStdinArg; const envVars: string[] = []; for (const envKey in env) { + // Skip this envKey, otherwise we get a malformed argument warning in the console if (envKey === '_') { continue; } @@ -377,6 +378,7 @@ export async function main(argv: string[]): Promise { tmpfile = `${tmpdir()}/vscode-wait-transfer-${time}.log`; tmpfileError = `${tmpdir()}/vscode-wait-transfer-error-${time}.log`; writeFileSync(tmpfile, ''); + writeFileSync(tmpfileError, ''); openArgs.push('-W'); openArgs.push('--stdout', tmpfile); openArgs.push('--stderr', tmpfileError); @@ -390,28 +392,20 @@ export async function main(argv: string[]): Promise { if (requiresWait) { const openPromise = (child: ChildProcess) => new Promise((c) => { - console.log('Temp file location: ' + tmpfile); - if (args['verbose']) { + if (args['verbose'] || args['status']) { const stream = openSync(tmpfile, 'r'); - const bufferSize = 50; + const bufferSize = 500; const buffer = Buffer.alloc(bufferSize); const interval = setInterval(() => { const readAmount = readSync(stream, buffer, 0, bufferSize, null); process.stdout.write(buffer.toString(undefined, 0, readAmount)); - }, 50); + }, 0); child.on('exit', () => { setTimeout(() => { clearTimeout(interval); closeSync(stream); c(); - }, 1500); - }); - } else if (args['status']) { - child.on('exit', () => { - const buffer = readFileSync(tmpfile); - let bufferContents = buffer.toString().trim(); - console.log(bufferContents); - c(); + }, 500); }); } }).then(() => { From ae84da589f5cb516aaff3956fc69a9e09c279d41 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 23 Aug 2021 19:10:02 -0700 Subject: [PATCH 08/27] Take away exec path, use code-cli.sh to debug --- src/vs/code/node/cli.ts | 13 +++++++++++-- src/vs/code/node/cliProcessMain.ts | 5 +---- src/vs/platform/environment/common/argv.ts | 1 - src/vs/platform/environment/node/argv.ts | 1 - src/vs/platform/environment/node/userDataPath.js | 5 ----- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 58b76fd94a1ab..d28c239fee2ca 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -8,7 +8,7 @@ import { chmodSync, closeSync, existsSync, openSync, readFileSync, readSync, sta import { homedir, platform, tmpdir } from 'os'; import type { ProfilingSession, Target } from 'v8-inspect-profiler'; import { Event } from 'vs/base/common/event'; -import { isAbsolute, join } from 'vs/base/common/path'; +import { isAbsolute, join, resolve } from 'vs/base/common/path'; import { IProcessEnvironment, isWindows } from 'vs/base/common/platform'; import { randomPort } from 'vs/base/common/ports'; import { isString } from 'vs/base/common/types'; @@ -385,9 +385,18 @@ export async function main(argv: string[]): Promise { argv.push('--wait'); } const argsArr: string[] = []; - const execPathToUse = args['exec-path'] ?? process.execPath; + const isDev = env['NODE_ENV'] === 'development'; + // When we're in development mode, call the OSS app rather than the OSS app's Electron + const execPathToUse = isDev ? resolve(join(process.execPath, '../../..')) : process.execPath; argsArr.push('-a', execPathToUse); argsArr.push(...envVars, ...openArgs, '--args', ...argv.slice(2)); + if (isDev) { + // If we're in development mode, replace the . arg with the + // vscode source arg. Because the OSS app isn't bundled, + // it needs the vscode source arg to launch properly. + const launchDirIndex = argsArr.indexOf('.'); + argsArr[launchDirIndex] = resolve(join(execPathToUse, '../../..')); + } child = spawn('open', argsArr, options); if (requiresWait) { diff --git a/src/vs/code/node/cliProcessMain.ts b/src/vs/code/node/cliProcessMain.ts index 3e1b624973d4a..c35bbc0f4094a 100644 --- a/src/vs/code/node/cliProcessMain.ts +++ b/src/vs/code/node/cliProcessMain.ts @@ -229,10 +229,7 @@ class CliMain extends Disposable { // Telemetry else if (this.argv['telemetry']) { - const rootToUse = this.argv['exec-path'] ? - join(this.argv['exec-path'], '/Contents/Resources/app') : - environmentService.appRoot; - console.log(await buildTelemetryMessage(rootToUse, environmentService.extensionsPath)); + console.log(await buildTelemetryMessage(environmentService.appRoot, environmentService.extensionsPath)); } } diff --git a/src/vs/platform/environment/common/argv.ts b/src/vs/platform/environment/common/argv.ts index bb82658069b9a..70a7eb4b86e41 100644 --- a/src/vs/platform/environment/common/argv.ts +++ b/src/vs/platform/environment/common/argv.ts @@ -85,7 +85,6 @@ export interface NativeParsedArgs { 'sync'?: 'on' | 'off'; '__sandbox'?: boolean; 'logsPath'?: string; - 'exec-path'?: string; // chromium command line args: https://electronjs.org/docs/all#supported-chrome-command-line-switches 'no-proxy-server'?: boolean; diff --git a/src/vs/platform/environment/node/argv.ts b/src/vs/platform/environment/node/argv.ts index 24ed695ba8813..823ef14c42aa9 100644 --- a/src/vs/platform/environment/node/argv.ts +++ b/src/vs/platform/environment/node/argv.ts @@ -120,7 +120,6 @@ export const OPTIONS: OptionDescriptions> = { 'open-devtools': { type: 'boolean' }, '__sandbox': { type: 'boolean' }, 'logsPath': { type: 'string' }, - 'exec-path': { type: 'string' }, // chromium flags 'no-proxy-server': { type: 'boolean' }, diff --git a/src/vs/platform/environment/node/userDataPath.js b/src/vs/platform/environment/node/userDataPath.js index 9cd5538f51d1b..1f7196c33e162 100644 --- a/src/vs/platform/environment/node/userDataPath.js +++ b/src/vs/platform/environment/node/userDataPath.js @@ -60,11 +60,6 @@ return path.join(appDataPath, productName); } - // 3. Support debugging mac OSS CLI - if (process.platform === 'darwin' && cliArgs['exec-path']) { - return cliArgs['exec-path']; - } - // With Electron>=13 --user-data-dir switch will be propagated to // all processes https://github.com/electron/electron/blob/1897b14af36a02e9aa7e4d814159303441548251/shell/browser/electron_browser_client.cc#L546-L553 // Check VSCODE_PORTABLE and VSCODE_APPDATA before this case to get correct values. From f32a1717a1737e6e4803cfd875ac7748ae1b0def Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 30 Aug 2021 17:51:15 -0700 Subject: [PATCH 09/27] Remove vscode wait flag for verbose Open's wait flag is enough for --verbose, --status, and - arguments. --- src/vs/code/node/cli.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index d28c239fee2ca..aa50622887bb2 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -382,7 +382,6 @@ export async function main(argv: string[]): Promise { openArgs.push('-W'); openArgs.push('--stdout', tmpfile); openArgs.push('--stderr', tmpfileError); - argv.push('--wait'); } const argsArr: string[] = []; const isDev = env['NODE_ENV'] === 'development'; From 551082a7c81c86c91e36f7850e2030bf3345eed0 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 30 Aug 2021 18:13:50 -0700 Subject: [PATCH 10/27] Apply more PR feedback --- src/vs/code/node/cli.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index aa50622887bb2..7f296c4166020 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -358,17 +358,7 @@ export async function main(argv: string[]): Promise { }); } } else { - const requiresWait = args['status'] || args['verbose'] || hasReadStdinArg; - const envVars: string[] = []; - for (const envKey in env) { - // Skip this envKey, otherwise we get a malformed argument warning in the console - if (envKey === '_') { - continue; - } - const value = env[envKey]; - envVars.push('--env'); - envVars.push(envKey + '=' + value ?? ''); - } + const requiresWait = verbose || hasReadStdinArg; const openArgs: string[] = ['-n']; let tmpfile = ''; @@ -388,7 +378,7 @@ export async function main(argv: string[]): Promise { // When we're in development mode, call the OSS app rather than the OSS app's Electron const execPathToUse = isDev ? resolve(join(process.execPath, '../../..')) : process.execPath; argsArr.push('-a', execPathToUse); - argsArr.push(...envVars, ...openArgs, '--args', ...argv.slice(2)); + argsArr.push(...openArgs, '--args', ...argv.slice(2)); if (isDev) { // If we're in development mode, replace the . arg with the // vscode source arg. Because the OSS app isn't bundled, @@ -400,14 +390,14 @@ export async function main(argv: string[]): Promise { if (requiresWait) { const openPromise = (child: ChildProcess) => new Promise((c) => { - if (args['verbose'] || args['status']) { + if (verbose) { const stream = openSync(tmpfile, 'r'); const bufferSize = 500; const buffer = Buffer.alloc(bufferSize); const interval = setInterval(() => { const readAmount = readSync(stream, buffer, 0, bufferSize, null); process.stdout.write(buffer.toString(undefined, 0, readAmount)); - }, 0); + }, 50); child.on('exit', () => { setTimeout(() => { clearTimeout(interval); From 9c681c6c19d4e81e7a1370a29bd6d9d3236d2866 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Tue, 31 Aug 2021 10:51:07 -0700 Subject: [PATCH 11/27] Add draft of file watcher impl --- src/vs/code/node/cli.ts | 44 ++++++++----------------- src/vs/code/node/cliVerboseLogger.ts | 48 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 31 deletions(-) create mode 100644 src/vs/code/node/cliVerboseLogger.ts diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 7f296c4166020..45fb181d3288c 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ChildProcess, spawn, SpawnOptions } from 'child_process'; -import { chmodSync, closeSync, existsSync, openSync, readFileSync, readSync, statSync, truncateSync, unlinkSync } from 'fs'; +import { chmodSync, existsSync, readFileSync, statSync, truncateSync, unlinkSync } from 'fs'; import { homedir, platform, tmpdir } from 'os'; import type { ProfilingSession, Target } from 'v8-inspect-profiler'; import { Event } from 'vs/base/common/event'; @@ -14,6 +14,7 @@ import { randomPort } from 'vs/base/common/ports'; import { isString } from 'vs/base/common/types'; import { whenDeleted, writeFileSync } from 'vs/base/node/pfs'; import { findFreePort } from 'vs/base/node/ports'; +import { CliVerboseLogger } from 'vs/code/node/cliVerboseLogger'; import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; import { buildHelpMessage, buildVersionMessage, OPTIONS } from 'vs/platform/environment/node/argv'; import { addArg, parseCLIProcessArgv } from 'vs/platform/environment/node/argvHelper'; @@ -361,17 +362,17 @@ export async function main(argv: string[]): Promise { const requiresWait = verbose || hasReadStdinArg; const openArgs: string[] = ['-n']; - let tmpfile = ''; - let tmpfileError = ''; + let tmpStdoutLogger: CliVerboseLogger | undefined; + let tmpStderrLogger: CliVerboseLogger | undefined; if (requiresWait) { const time = new Date().getTime(); - tmpfile = `${tmpdir()}/vscode-wait-transfer-${time}.log`; - tmpfileError = `${tmpdir()}/vscode-wait-transfer-error-${time}.log`; - writeFileSync(tmpfile, ''); - writeFileSync(tmpfileError, ''); + const tmpStdoutName = `${tmpdir()}/vscode-wait-transfer-${time}.log`; + const tmpStderrName = `${tmpdir()}/vscode-wait-transfer-error-${time}.log`; + tmpStdoutLogger = new CliVerboseLogger(tmpStdoutName); + tmpStderrLogger = new CliVerboseLogger(tmpStderrName); openArgs.push('-W'); - openArgs.push('--stdout', tmpfile); - openArgs.push('--stderr', tmpfileError); + openArgs.push('--stdout', tmpStdoutName); + openArgs.push('--stderr', tmpStderrName); } const argsArr: string[] = []; const isDev = env['NODE_ENV'] === 'development'; @@ -389,28 +390,9 @@ export async function main(argv: string[]): Promise { child = spawn('open', argsArr, options); if (requiresWait) { - const openPromise = (child: ChildProcess) => new Promise((c) => { - if (verbose) { - const stream = openSync(tmpfile, 'r'); - const bufferSize = 500; - const buffer = Buffer.alloc(bufferSize); - const interval = setInterval(() => { - const readAmount = readSync(stream, buffer, 0, bufferSize, null); - process.stdout.write(buffer.toString(undefined, 0, readAmount)); - }, 50); - child.on('exit', () => { - setTimeout(() => { - clearTimeout(interval); - closeSync(stream); - c(); - }, 500); - }); - } - }).then(() => { - unlinkSync(tmpfile); - unlinkSync(tmpfileError); - }); - processCallbacks.push(openPromise); + const stdoutPromise = tmpStdoutLogger!.track(); + const stderrPromise = tmpStderrLogger!.track(); + processCallbacks.push(stdoutPromise, stderrPromise); } } diff --git a/src/vs/code/node/cliVerboseLogger.ts b/src/vs/code/node/cliVerboseLogger.ts new file mode 100644 index 0000000000000..d9fb3ba4b3ecd --- /dev/null +++ b/src/vs/code/node/cliVerboseLogger.ts @@ -0,0 +1,48 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { ChildProcess } from 'child_process'; +import { watch, writeFileSync } from 'fs'; +import { Promises } from 'vs/base/node/pfs'; + +/** + * This class provides logging for verbose commands on Mac + * as part of #102975 + */ +export class CliVerboseLogger { + constructor(readonly filename: string) { + writeFileSync(filename, ''); + } + + public track(): (child: ChildProcess) => Promise { + return async (child: ChildProcess) => { + const watcher = watch(this.filename); + const fileHandle = await Promises.open(this.filename, 'r'); + const bufferSize = 512; + const buffer = Buffer.alloc(bufferSize); + return new Promise(async (c) => { + watcher.on('change', async () => { + while (true) { + const readObj = await Promises.read(fileHandle, buffer, 0, bufferSize, null); + if (!readObj.bytesRead) { + return; + } + process.stdout.write(readObj.buffer.toString(undefined, 0, readObj.bytesRead)); + } + }); + watcher.on('error', () => { + console.error('Verbose logger watcher encountered an error.'); + c(); + }); + child.on('close', () => { + c(); + }); + }).finally(() => { + Promises.close(fileHandle); + watcher.close(); + }); + }; + } +} From f3f84e52a6b0736f2392f25b375a7d1a2c2324be Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Tue, 31 Aug 2021 11:51:22 -0700 Subject: [PATCH 12/27] refactor to use better filenames --- src/vs/code/node/cli.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 45fb181d3288c..8b218dafbaad9 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -31,6 +31,10 @@ function shouldSpawnCliProcess(argv: NativeParsedArgs): boolean { || !!argv['telemetry']; } +function createFileName(dir: string, prefix: string): string { + return join(dir, prefix + '-' + Math.random().toString(16).slice(-4)); +} + interface IMainCli { main: (argv: NativeParsedArgs) => Promise; } @@ -233,7 +237,7 @@ export async function main(argv: string[]): Promise { throw new Error('Failed to find free ports for profiler. Make sure to shutdown all instances of the editor first.'); } - const filenamePrefix = join(homedir(), 'prof-' + Math.random().toString(16).slice(-4)); + const filenamePrefix = createFileName(homedir(), 'prof'); addArg(argv, `--inspect-brk=${portMain}`); addArg(argv, `--remote-debugging-port=${portRenderer}`); @@ -365,9 +369,8 @@ export async function main(argv: string[]): Promise { let tmpStdoutLogger: CliVerboseLogger | undefined; let tmpStderrLogger: CliVerboseLogger | undefined; if (requiresWait) { - const time = new Date().getTime(); - const tmpStdoutName = `${tmpdir()}/vscode-wait-transfer-${time}.log`; - const tmpStderrName = `${tmpdir()}/vscode-wait-transfer-error-${time}.log`; + const tmpStdoutName = createFileName(tmpdir(), 'code-stdout'); + const tmpStderrName = createFileName(tmpdir(), 'code-stderr'); tmpStdoutLogger = new CliVerboseLogger(tmpStdoutName); tmpStderrLogger = new CliVerboseLogger(tmpStderrName); openArgs.push('-W'); From 6c8d8a1ab9399b8b9064cd0f050fe9d41ef774c7 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Wed, 1 Sep 2021 10:37:19 -0700 Subject: [PATCH 13/27] Apply PR feedback --- src/vs/code/node/cli.ts | 2 +- src/vs/code/node/cliVerboseLogger.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 8b218dafbaad9..04ac1a857994e 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -378,7 +378,7 @@ export async function main(argv: string[]): Promise { openArgs.push('--stderr', tmpStderrName); } const argsArr: string[] = []; - const isDev = env['NODE_ENV'] === 'development'; + const isDev = env['VSCODE_DEV']; // When we're in development mode, call the OSS app rather than the OSS app's Electron const execPathToUse = isDev ? resolve(join(process.execPath, '../../..')) : process.execPath; argsArr.push('-a', execPathToUse); diff --git a/src/vs/code/node/cliVerboseLogger.ts b/src/vs/code/node/cliVerboseLogger.ts index d9fb3ba4b3ecd..beb514d59b6fb 100644 --- a/src/vs/code/node/cliVerboseLogger.ts +++ b/src/vs/code/node/cliVerboseLogger.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ChildProcess } from 'child_process'; -import { watch, writeFileSync } from 'fs'; +import { watch, writeFileSync, unlinkSync } from 'fs'; import { Promises } from 'vs/base/node/pfs'; /** @@ -42,6 +42,7 @@ export class CliVerboseLogger { }).finally(() => { Promises.close(fileHandle); watcher.close(); + unlinkSync(this.filename); }); }; } From 461c34a996013c7e1b90f7e995d4cd72eaf3fe4b Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Wed, 1 Sep 2021 19:12:05 -0700 Subject: [PATCH 14/27] Add polish --- src/vs/code/node/cli.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 04ac1a857994e..4f245f2639857 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -379,16 +379,16 @@ export async function main(argv: string[]): Promise { } const argsArr: string[] = []; const isDev = env['VSCODE_DEV']; - // When we're in development mode, call the OSS app rather than the OSS app's Electron - const execPathToUse = isDev ? resolve(join(process.execPath, '../../..')) : process.execPath; + const execPathToUse = process.execPath; argsArr.push('-a', execPathToUse); argsArr.push(...openArgs, '--args', ...argv.slice(2)); if (isDev) { // If we're in development mode, replace the . arg with the // vscode source arg. Because the OSS app isn't bundled, // it needs the vscode source arg to launch properly. - const launchDirIndex = argsArr.indexOf('.'); - argsArr[launchDirIndex] = resolve(join(execPathToUse, '../../..')); + const curdir = '.'; + const launchDirIndex = argsArr.indexOf(curdir); + argsArr[launchDirIndex] = resolve(curdir); } child = spawn('open', argsArr, options); From f0116b44bf6aafdb810dab6ec9cf4e38adc7cddd Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Thu, 2 Sep 2021 12:29:30 -0700 Subject: [PATCH 15/27] Apply PR feedback --- src/vs/code/node/cli.ts | 33 +++++++++++++------ src/vs/code/node/cliVerboseLogger.ts | 47 +++++++++++++--------------- 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 4f245f2639857..c46438dd91ab1 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -5,11 +5,11 @@ import { ChildProcess, spawn, SpawnOptions } from 'child_process'; import { chmodSync, existsSync, readFileSync, statSync, truncateSync, unlinkSync } from 'fs'; -import { homedir, platform, tmpdir } from 'os'; +import { homedir, tmpdir } from 'os'; import type { ProfilingSession, Target } from 'v8-inspect-profiler'; import { Event } from 'vs/base/common/event'; import { isAbsolute, join, resolve } from 'vs/base/common/path'; -import { IProcessEnvironment, isWindows } from 'vs/base/common/platform'; +import { IProcessEnvironment, isMacintosh, isWindows } from 'vs/base/common/platform'; import { randomPort } from 'vs/base/common/ports'; import { isString } from 'vs/base/common/types'; import { whenDeleted, writeFileSync } from 'vs/base/node/pfs'; @@ -343,7 +343,8 @@ export async function main(argv: string[]): Promise { } let child: ChildProcess; - if (platform() !== 'darwin') { + if (!isMacintosh) { + // We spawn process.execPath directly child = spawn(process.execPath, argv.slice(2), options); if (args.wait && waitMarkerFilePath) { @@ -355,7 +356,6 @@ export async function main(argv: string[]): Promise { // Complete when wait marker file is deleted whenDeleted(waitMarkerFilePath!).then(resolve, resolve); }).then(() => { - // Make sure to delete the tmp stdin file if we have any if (stdinFilePath) { unlinkSync(stdinFilePath); @@ -363,16 +363,23 @@ export async function main(argv: string[]): Promise { }); } } else { + // On mac, we spawn using the open command to obtain behavior + // similar to if the app was launched from the dock + // https://github.com/microsoft/vscode/issues/102975 const requiresWait = verbose || hasReadStdinArg; const openArgs: string[] = ['-n']; let tmpStdoutLogger: CliVerboseLogger | undefined; let tmpStderrLogger: CliVerboseLogger | undefined; + let tmpStdoutName: string | undefined; + let tmpStderrName: string | undefined; if (requiresWait) { - const tmpStdoutName = createFileName(tmpdir(), 'code-stdout'); - const tmpStderrName = createFileName(tmpdir(), 'code-stderr'); - tmpStdoutLogger = new CliVerboseLogger(tmpStdoutName); - tmpStderrLogger = new CliVerboseLogger(tmpStderrName); + tmpStdoutName = createFileName(tmpdir(), 'code-stdout'); + tmpStderrName = createFileName(tmpdir(), 'code-stderr'); + tmpStdoutLogger = new CliVerboseLogger(); + tmpStderrLogger = new CliVerboseLogger(); + writeFileSync(tmpStdoutName, ''); + writeFileSync(tmpStderrName, ''); openArgs.push('-W'); openArgs.push('--stdout', tmpStdoutName); openArgs.push('--stderr', tmpStderrName); @@ -393,8 +400,14 @@ export async function main(argv: string[]): Promise { child = spawn('open', argsArr, options); if (requiresWait) { - const stdoutPromise = tmpStdoutLogger!.track(); - const stderrPromise = tmpStderrLogger!.track(); + function createLoggerPromise(logger: CliVerboseLogger, filename: string): (child: ChildProcess) => Promise { + return async (child: ChildProcess) => { + await logger.track(child, filename); + unlinkSync(filename); + }; + } + const stdoutPromise = createLoggerPromise(tmpStdoutLogger!, tmpStdoutName!); + const stderrPromise = createLoggerPromise(tmpStderrLogger!, tmpStderrName!); processCallbacks.push(stdoutPromise, stderrPromise); } } diff --git a/src/vs/code/node/cliVerboseLogger.ts b/src/vs/code/node/cliVerboseLogger.ts index beb514d59b6fb..d2cf52ffddfcb 100644 --- a/src/vs/code/node/cliVerboseLogger.ts +++ b/src/vs/code/node/cliVerboseLogger.ts @@ -4,26 +4,25 @@ *--------------------------------------------------------------------------------------------*/ import { ChildProcess } from 'child_process'; -import { watch, writeFileSync, unlinkSync } from 'fs'; +import { IDisposable } from 'vs/base/common/lifecycle'; import { Promises } from 'vs/base/node/pfs'; +import { watchFile } from 'vs/base/node/watcher'; /** * This class provides logging for verbose commands on Mac * as part of #102975 */ export class CliVerboseLogger { - constructor(readonly filename: string) { - writeFileSync(filename, ''); - } + public async track(child: ChildProcess, logfile: string): Promise { + // Assume logfile already exists at this point + const fileHandle = await Promises.open(logfile, 'r'); + const bufferSize = 512; + const buffer = Buffer.allocUnsafe(bufferSize); + let watcher: IDisposable; - public track(): (child: ChildProcess) => Promise { - return async (child: ChildProcess) => { - const watcher = watch(this.filename); - const fileHandle = await Promises.open(this.filename, 'r'); - const bufferSize = 512; - const buffer = Buffer.alloc(bufferSize); - return new Promise(async (c) => { - watcher.on('change', async () => { + return new Promise(async (c) => { + watcher = watchFile(logfile, async (type) => { + if (type === 'changed') { while (true) { const readObj = await Promises.read(fileHandle, buffer, 0, bufferSize, null); if (!readObj.bytesRead) { @@ -31,19 +30,17 @@ export class CliVerboseLogger { } process.stdout.write(readObj.buffer.toString(undefined, 0, readObj.bytesRead)); } - }); - watcher.on('error', () => { - console.error('Verbose logger watcher encountered an error.'); - c(); - }); - child.on('close', () => { - c(); - }); - }).finally(() => { - Promises.close(fileHandle); - watcher.close(); - unlinkSync(this.filename); + } + }, (err) => { + console.error('Verbose logger file watcher encountered an error. ' + err); + c(); + }); + child.on('close', () => { + c(); }); - }; + }).finally(async () => { + watcher.dispose(); + await Promises.close(fileHandle); + }); } } From f503dfcf88f911a07be4d83696604b002f314486 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Thu, 2 Sep 2021 12:51:49 -0700 Subject: [PATCH 16/27] Handle wait on macOS, refactor --- src/vs/code/node/cli.ts | 76 +++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index c46438dd91ab1..9015423b6de11 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -346,70 +346,66 @@ export async function main(argv: string[]): Promise { if (!isMacintosh) { // We spawn process.execPath directly child = spawn(process.execPath, argv.slice(2), options); - - if (args.wait && waitMarkerFilePath) { - return new Promise(resolve => { - - // Complete when process exits - child.once('exit', () => resolve(undefined)); - - // Complete when wait marker file is deleted - whenDeleted(waitMarkerFilePath!).then(resolve, resolve); - }).then(() => { - // Make sure to delete the tmp stdin file if we have any - if (stdinFilePath) { - unlinkSync(stdinFilePath); - } - }); - } } else { // On mac, we spawn using the open command to obtain behavior // similar to if the app was launched from the dock // https://github.com/microsoft/vscode/issues/102975 - const requiresWait = verbose || hasReadStdinArg; const openArgs: string[] = ['-n']; - let tmpStdoutLogger: CliVerboseLogger | undefined; - let tmpStderrLogger: CliVerboseLogger | undefined; - let tmpStdoutName: string | undefined; - let tmpStderrName: string | undefined; - if (requiresWait) { - tmpStdoutName = createFileName(tmpdir(), 'code-stdout'); - tmpStderrName = createFileName(tmpdir(), 'code-stderr'); - tmpStdoutLogger = new CliVerboseLogger(); - tmpStderrLogger = new CliVerboseLogger(); + if (verbose || hasReadStdinArg || args.wait) { + openArgs.push('-W'); + } + + if (verbose) { + // Set up arguments to pass to open + const tmpStdoutName = createFileName(tmpdir(), 'code-stdout'); + const tmpStderrName = createFileName(tmpdir(), 'code-stderr'); + const tmpStdoutLogger = new CliVerboseLogger(); + const tmpStderrLogger = new CliVerboseLogger(); writeFileSync(tmpStdoutName, ''); writeFileSync(tmpStderrName, ''); - openArgs.push('-W'); openArgs.push('--stdout', tmpStdoutName); openArgs.push('--stderr', tmpStderrName); + + function createLoggerPromise(logger: CliVerboseLogger, filename: string): (child: ChildProcess) => Promise { + return async (child: ChildProcess) => { + await logger.track(child, filename); + unlinkSync(filename); + }; + } + const stdoutPromise = createLoggerPromise(tmpStdoutLogger!, tmpStdoutName!); + const stderrPromise = createLoggerPromise(tmpStderrLogger!, tmpStderrName!); + processCallbacks.push(stdoutPromise, stderrPromise); } const argsArr: string[] = []; - const isDev = env['VSCODE_DEV']; const execPathToUse = process.execPath; argsArr.push('-a', execPathToUse); argsArr.push(...openArgs, '--args', ...argv.slice(2)); - if (isDev) { + if (env['VSCODE_DEV']) { // If we're in development mode, replace the . arg with the // vscode source arg. Because the OSS app isn't bundled, - // it needs the vscode source arg to launch properly. + // it needs the full vscode source arg to launch properly. const curdir = '.'; const launchDirIndex = argsArr.indexOf(curdir); argsArr[launchDirIndex] = resolve(curdir); } child = spawn('open', argsArr, options); + } - if (requiresWait) { - function createLoggerPromise(logger: CliVerboseLogger, filename: string): (child: ChildProcess) => Promise { - return async (child: ChildProcess) => { - await logger.track(child, filename); - unlinkSync(filename); - }; + if (args.wait && waitMarkerFilePath) { + const waitPromise = (child: ChildProcess) => new Promise(resolve => { + // Complete when process exits + child.once('exit', () => resolve(undefined)); + + // Or, complete when wait marker file is deleted + whenDeleted(waitMarkerFilePath!).finally(resolve); + }).then(() => { + // Make sure to delete the tmp stdin file if we have any + if (stdinFilePath) { + unlinkSync(stdinFilePath); } - const stdoutPromise = createLoggerPromise(tmpStdoutLogger!, tmpStdoutName!); - const stderrPromise = createLoggerPromise(tmpStderrLogger!, tmpStderrName!); - processCallbacks.push(stdoutPromise, stderrPromise); - } + }); + processCallbacks.push(waitPromise); } return Promise.all(processCallbacks.map(callback => callback(child))); From 247fd453c8e9843cbd490376d389fb241f9d4e72 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Fri, 3 Sep 2021 08:43:23 -0700 Subject: [PATCH 17/27] Update src/vs/code/node/cli.ts Co-authored-by: Benjamin Pasero --- src/vs/code/node/cli.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 9015423b6de11..e9ecb2a4b293b 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -32,7 +32,7 @@ function shouldSpawnCliProcess(argv: NativeParsedArgs): boolean { } function createFileName(dir: string, prefix: string): string { - return join(dir, prefix + '-' + Math.random().toString(16).slice(-4)); + return join(dir, `${prefix}-${Math.random().toString(16).slice(-4)}`); } interface IMainCli { From 870f2383c2aba063df5aea8f22464b26ba17644f Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Fri, 3 Sep 2021 09:33:01 -0700 Subject: [PATCH 18/27] Move child dep out of logger --- src/vs/code/node/cli.ts | 35 ++++++++++++++++------------ src/vs/code/node/cliVerboseLogger.ts | 18 ++++++-------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index e9ecb2a4b293b..37a752fcf0d45 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -357,25 +357,30 @@ export async function main(argv: string[]): Promise { } if (verbose) { - // Set up arguments to pass to open - const tmpStdoutName = createFileName(tmpdir(), 'code-stdout'); - const tmpStderrName = createFileName(tmpdir(), 'code-stderr'); - const tmpStdoutLogger = new CliVerboseLogger(); - const tmpStderrLogger = new CliVerboseLogger(); - writeFileSync(tmpStdoutName, ''); - writeFileSync(tmpStderrName, ''); - openArgs.push('--stdout', tmpStdoutName); - openArgs.push('--stderr', tmpStderrName); - - function createLoggerPromise(logger: CliVerboseLogger, filename: string): (child: ChildProcess) => Promise { + // The open command only allows for redirecting stderr and stdout to files, + // so we make it redirect those to temp files, and then use a logger to + // redirect the file output to the console + function createLoggerPromise(logger: CliVerboseLogger, filename: string, stream: NodeJS.WriteStream): (child: ChildProcess) => Promise { return async (child: ChildProcess) => { - await logger.track(child, filename); + const childClosePromise = new Promise(resolve => { + child.on('close', () => { + resolve(); + }); + }); + await Promise.race([logger.streamFile(filename, stream), childClosePromise]); unlinkSync(filename); }; } - const stdoutPromise = createLoggerPromise(tmpStdoutLogger!, tmpStdoutName!); - const stderrPromise = createLoggerPromise(tmpStderrLogger!, tmpStderrName!); - processCallbacks.push(stdoutPromise, stderrPromise); + + for (const outputType of ['stdout', 'stderr']) { + const stream = outputType === 'stdout' ? process.stdout : process.stderr; + const tmpName = createFileName(tmpdir(), `code-${outputType}`); + const tmpLogger = new CliVerboseLogger(); + writeFileSync(tmpName, ''); + openArgs.push(`--${outputType}`, tmpName); + const outputPromise = createLoggerPromise(tmpLogger, tmpName, stream); + processCallbacks.push(outputPromise); + } } const argsArr: string[] = []; const execPathToUse = process.execPath; diff --git a/src/vs/code/node/cliVerboseLogger.ts b/src/vs/code/node/cliVerboseLogger.ts index d2cf52ffddfcb..f7ed89fb8fd08 100644 --- a/src/vs/code/node/cliVerboseLogger.ts +++ b/src/vs/code/node/cliVerboseLogger.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ChildProcess } from 'child_process'; import { IDisposable } from 'vs/base/common/lifecycle'; import { Promises } from 'vs/base/node/pfs'; import { watchFile } from 'vs/base/node/watcher'; @@ -13,29 +12,26 @@ import { watchFile } from 'vs/base/node/watcher'; * as part of #102975 */ export class CliVerboseLogger { - public async track(child: ChildProcess, logfile: string): Promise { - // Assume logfile already exists at this point - const fileHandle = await Promises.open(logfile, 'r'); + public async streamFile(path: string, stream: NodeJS.WriteStream): Promise { + // Assume path already exists at this point + const fileHandle = await Promises.open(path, 'r'); const bufferSize = 512; const buffer = Buffer.allocUnsafe(bufferSize); let watcher: IDisposable; - return new Promise(async (c) => { - watcher = watchFile(logfile, async (type) => { + return new Promise((c) => { + watcher = watchFile(path, async (type) => { if (type === 'changed') { while (true) { const readObj = await Promises.read(fileHandle, buffer, 0, bufferSize, null); if (!readObj.bytesRead) { return; } - process.stdout.write(readObj.buffer.toString(undefined, 0, readObj.bytesRead)); + stream.write(readObj.buffer.toString(undefined, 0, readObj.bytesRead)); } } }, (err) => { - console.error('Verbose logger file watcher encountered an error. ' + err); - c(); - }); - child.on('close', () => { + console.error('File watcher encountered an error. ' + err); c(); }); }).finally(async () => { From 36cebd9be60c6acddabdcf28ad0be7b8aae1ce77 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Tue, 7 Sep 2021 12:10:44 -0700 Subject: [PATCH 19/27] =?UTF-8?q?=F0=9F=92=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/vs/code/node/cli.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 37a752fcf0d45..c0b669b06be55 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -362,13 +362,12 @@ export async function main(argv: string[]): Promise { // redirect the file output to the console function createLoggerPromise(logger: CliVerboseLogger, filename: string, stream: NodeJS.WriteStream): (child: ChildProcess) => Promise { return async (child: ChildProcess) => { - const childClosePromise = new Promise(resolve => { - child.on('close', () => { - resolve(); - }); + await Promise.race([ + logger.streamFile(filename, stream), + Event.toPromise(Event.fromNodeEventEmitter(child, 'close')) + ]).finally(() => { + unlinkSync(filename); }); - await Promise.race([logger.streamFile(filename, stream), childClosePromise]); - unlinkSync(filename); }; } @@ -383,8 +382,7 @@ export async function main(argv: string[]): Promise { } } const argsArr: string[] = []; - const execPathToUse = process.execPath; - argsArr.push('-a', execPathToUse); + argsArr.push('-a', process.execPath); argsArr.push(...openArgs, '--args', ...argv.slice(2)); if (env['VSCODE_DEV']) { // If we're in development mode, replace the . arg with the From 102078943d34aea0116053f6e5c8989cd04c08c2 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 8 Sep 2021 06:22:54 +0200 Subject: [PATCH 20/27] introduce `watchFileContents` and adopt --- src/vs/base/node/watcher.ts | 67 ++++++++++++++++++++++++++++ src/vs/code/node/cli.ts | 58 +++++++++++++----------- src/vs/code/node/cliVerboseLogger.ts | 42 ----------------- 3 files changed, 100 insertions(+), 67 deletions(-) delete mode 100644 src/vs/code/node/cliVerboseLogger.ts diff --git a/src/vs/base/node/watcher.ts b/src/vs/base/node/watcher.ts index 0b6947dde41d2..169f2907e79b2 100644 --- a/src/vs/base/node/watcher.ts +++ b/src/vs/base/node/watcher.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { watch } from 'fs'; +import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; import { isEqualOrParent } from 'vs/base/common/extpath'; import { Disposable, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { normalizeNFC } from 'vs/base/common/normalization'; @@ -202,3 +203,69 @@ function doWatchNonRecursive(file: { path: string, isDirectory: boolean }, onCha watcherDisposables = dispose(watcherDisposables); }); } + +/** + * Watch the provided `path` for changes and return + * the data in chunks of `Uint8Array` for further use. + */ +export async function watchFileContents(path: string, onData: (chunk: Uint8Array) => void, token: CancellationToken, bufferSize = 512): Promise { + const handle = await Promises.open(path, 'r'); + const buffer = Buffer.allocUnsafe(bufferSize); + + const cts = new CancellationTokenSource(token); + + let error: Error | undefined = undefined; + let isReading = false; + + const watcher = watchFile(path, async type => { + if (type === 'changed') { + + if (isReading) { + return; // return early if we are already reading the output + } + + isReading = true; + + try { + // Consume the new contents of the file until finished + // everytime there is a change event signalling a change + while (!cts.token.isCancellationRequested) { + const { bytesRead } = await Promises.read(handle, buffer, 0, bufferSize, null); + if (!bytesRead || cts.token.isCancellationRequested) { + break; + } + + onData(buffer.slice(0, bytesRead)); + } + } catch (err) { + error = new Error(err); + cts.dispose(true); + } finally { + isReading = false; + } + } + }, err => { + error = new Error(err); + cts.dispose(true); + }); + + let resolveFn: Function; + let rejectFn: Function; + const promise = new Promise((resolve, reject) => { + resolveFn = resolve; + rejectFn = reject; + }); + + cts.token.onCancellationRequested(async () => { + watcher.dispose(); + await Promises.close(handle); + + if (error) { + rejectFn(error); + } else { + resolveFn(); + } + }); + + return promise; +} diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index c0b669b06be55..9e5134c37fa5b 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -14,13 +14,14 @@ import { randomPort } from 'vs/base/common/ports'; import { isString } from 'vs/base/common/types'; import { whenDeleted, writeFileSync } from 'vs/base/node/pfs'; import { findFreePort } from 'vs/base/node/ports'; -import { CliVerboseLogger } from 'vs/code/node/cliVerboseLogger'; +import { watchFileContents } from 'vs/base/node/watcher'; import { NativeParsedArgs } from 'vs/platform/environment/common/argv'; import { buildHelpMessage, buildVersionMessage, OPTIONS } from 'vs/platform/environment/node/argv'; import { addArg, parseCLIProcessArgv } from 'vs/platform/environment/node/argvHelper'; import { getStdinFilePath, hasStdinWithoutTty, readFromStdin, stdinDataListener } from 'vs/platform/environment/node/stdin'; import { createWaitMarkerFile } from 'vs/platform/environment/node/wait'; import product from 'vs/platform/product/common/product'; +import { CancellationTokenSource } from 'vs/base/common/cancellation'; function shouldSpawnCliProcess(argv: NativeParsedArgs): boolean { return !!argv['install-source'] @@ -350,59 +351,66 @@ export async function main(argv: string[]): Promise { // On mac, we spawn using the open command to obtain behavior // similar to if the app was launched from the dock // https://github.com/microsoft/vscode/issues/102975 - const openArgs: string[] = ['-n']; + + const spawnArgs = ['-n']; // -n: launches even when opened already + spawnArgs.push('-a', process.execPath); // -a: opens a specific application if (verbose || hasReadStdinArg || args.wait) { - openArgs.push('-W'); + spawnArgs.push('--wait-apps'); // `open --wait-apps`: blocks until the launched app is closed (even if they were already running) } if (verbose) { // The open command only allows for redirecting stderr and stdout to files, // so we make it redirect those to temp files, and then use a logger to // redirect the file output to the console - function createLoggerPromise(logger: CliVerboseLogger, filename: string, stream: NodeJS.WriteStream): (child: ChildProcess) => Promise { - return async (child: ChildProcess) => { - await Promise.race([ - logger.streamFile(filename, stream), - Event.toPromise(Event.fromNodeEventEmitter(child, 'close')) - ]).finally(() => { - unlinkSync(filename); - }); - }; - } - for (const outputType of ['stdout', 'stderr']) { - const stream = outputType === 'stdout' ? process.stdout : process.stderr; + + // Tmp file to target output to const tmpName = createFileName(tmpdir(), `code-${outputType}`); - const tmpLogger = new CliVerboseLogger(); writeFileSync(tmpName, ''); - openArgs.push(`--${outputType}`, tmpName); - const outputPromise = createLoggerPromise(tmpLogger, tmpName, stream); - processCallbacks.push(outputPromise); + spawnArgs.push(`--${outputType}`, tmpName); + + // Listener to redirect content to stdout/stderr + processCallbacks.push(async (child: ChildProcess) => { + try { + const stream = outputType === 'stdout' ? process.stdout : process.stderr; + + const cts = new CancellationTokenSource(); + await Promise.race([ + watchFileContents(tmpName, chunk => stream.write(chunk), cts.token), + Event.toPromise(Event.fromNodeEventEmitter(child, 'close')).then(() => cts.dispose(true)) + ]); + } finally { + unlinkSync(tmpName); + } + }); } } - const argsArr: string[] = []; - argsArr.push('-a', process.execPath); - argsArr.push(...openArgs, '--args', ...argv.slice(2)); + + spawnArgs.push('--args', ...argv.slice(2)); // pass on our arguments + if (env['VSCODE_DEV']) { // If we're in development mode, replace the . arg with the // vscode source arg. Because the OSS app isn't bundled, // it needs the full vscode source arg to launch properly. const curdir = '.'; - const launchDirIndex = argsArr.indexOf(curdir); - argsArr[launchDirIndex] = resolve(curdir); + const launchDirIndex = spawnArgs.indexOf(curdir); + spawnArgs[launchDirIndex] = resolve(curdir); } - child = spawn('open', argsArr, options); + + child = spawn('open', spawnArgs, options); } if (args.wait && waitMarkerFilePath) { const waitPromise = (child: ChildProcess) => new Promise(resolve => { + // Complete when process exits child.once('exit', () => resolve(undefined)); // Or, complete when wait marker file is deleted whenDeleted(waitMarkerFilePath!).finally(resolve); }).then(() => { + // Make sure to delete the tmp stdin file if we have any if (stdinFilePath) { unlinkSync(stdinFilePath); diff --git a/src/vs/code/node/cliVerboseLogger.ts b/src/vs/code/node/cliVerboseLogger.ts deleted file mode 100644 index f7ed89fb8fd08..0000000000000 --- a/src/vs/code/node/cliVerboseLogger.ts +++ /dev/null @@ -1,42 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { IDisposable } from 'vs/base/common/lifecycle'; -import { Promises } from 'vs/base/node/pfs'; -import { watchFile } from 'vs/base/node/watcher'; - -/** - * This class provides logging for verbose commands on Mac - * as part of #102975 - */ -export class CliVerboseLogger { - public async streamFile(path: string, stream: NodeJS.WriteStream): Promise { - // Assume path already exists at this point - const fileHandle = await Promises.open(path, 'r'); - const bufferSize = 512; - const buffer = Buffer.allocUnsafe(bufferSize); - let watcher: IDisposable; - - return new Promise((c) => { - watcher = watchFile(path, async (type) => { - if (type === 'changed') { - while (true) { - const readObj = await Promises.read(fileHandle, buffer, 0, bufferSize, null); - if (!readObj.bytesRead) { - return; - } - stream.write(readObj.buffer.toString(undefined, 0, readObj.bytesRead)); - } - } - }, (err) => { - console.error('File watcher encountered an error. ' + err); - c(); - }); - }).finally(async () => { - watcher.dispose(); - await Promises.close(fileHandle); - }); - } -} From 33e043f25ba98dcda7d780373a1233ecf21c38dc Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 8 Sep 2021 06:50:15 +0200 Subject: [PATCH 21/27] cli - move --wait support to respective place --- src/vs/code/node/cli.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 9e5134c37fa5b..a6e7cf39a8924 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -401,24 +401,6 @@ export async function main(argv: string[]): Promise { child = spawn('open', spawnArgs, options); } - if (args.wait && waitMarkerFilePath) { - const waitPromise = (child: ChildProcess) => new Promise(resolve => { - - // Complete when process exits - child.once('exit', () => resolve(undefined)); - - // Or, complete when wait marker file is deleted - whenDeleted(waitMarkerFilePath!).finally(resolve); - }).then(() => { - - // Make sure to delete the tmp stdin file if we have any - if (stdinFilePath) { - unlinkSync(stdinFilePath); - } - }); - processCallbacks.push(waitPromise); - } - return Promise.all(processCallbacks.map(callback => callback(child))); } } From b76ea265c2fa256ac8846617c303dc917fe197f4 Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Wed, 8 Sep 2021 10:29:07 -0700 Subject: [PATCH 22/27] Use -W only for verbose --- src/vs/code/node/cli.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index a6e7cf39a8924..7691a8932a28a 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -355,11 +355,9 @@ export async function main(argv: string[]): Promise { const spawnArgs = ['-n']; // -n: launches even when opened already spawnArgs.push('-a', process.execPath); // -a: opens a specific application - if (verbose || hasReadStdinArg || args.wait) { + if (verbose) { spawnArgs.push('--wait-apps'); // `open --wait-apps`: blocks until the launched app is closed (even if they were already running) - } - if (verbose) { // The open command only allows for redirecting stderr and stdout to files, // so we make it redirect those to temp files, and then use a logger to // redirect the file output to the console From 06c81f5b2f07d87c07a111ca476dcf559be4641c Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Wed, 8 Sep 2021 11:13:49 -0700 Subject: [PATCH 23/27] Change promise to get mac wait flag working --- src/vs/code/node/cli.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 7691a8932a28a..740b9c3900430 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -211,10 +211,22 @@ export async function main(argv: string[]): Promise { // - the wait marker file has been deleted (e.g. when closing the editor) // - the launched process terminates (e.g. due to a crash) processCallbacks.push(async child => { + let childExitPromise = Event.toPromise(Event.fromNodeEventEmitter(child, 'exit')); + if (isMacintosh) { + childExitPromise = new Promise((resolve) => { + // Only resolve this promise if the child (i.e. open) exited with an error + child.on('exit', (code, signal) => { + if (code !== 0 || signal) { + resolve(); + } + }); + }); + } try { await Promise.race([ whenDeleted(waitMarkerFilePath!), - Event.toPromise(Event.fromNodeEventEmitter(child, 'exit')) + Event.toPromise(Event.fromNodeEventEmitter(child, 'error')), + childExitPromise ]); } finally { if (stdinFilePath) { From f0833d3a62002602649e8b38de286f04dc16bd2d Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Thu, 9 Sep 2021 09:52:38 -0700 Subject: [PATCH 24/27] Clarify childExitPromise --- src/vs/code/node/cli.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index 740b9c3900430..eba35a2970f8a 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -211,8 +211,11 @@ export async function main(argv: string[]): Promise { // - the wait marker file has been deleted (e.g. when closing the editor) // - the launched process terminates (e.g. due to a crash) processCallbacks.push(async child => { - let childExitPromise = Event.toPromise(Event.fromNodeEventEmitter(child, 'exit')); + let childExitPromise; if (isMacintosh) { + // On macOS, we resolve the following promise only when the child, + // i.e. the open command, exited with a signal or error. Otherwise, we + // wait for the marker file to be deleted or for the child to error. childExitPromise = new Promise((resolve) => { // Only resolve this promise if the child (i.e. open) exited with an error child.on('exit', (code, signal) => { @@ -221,6 +224,10 @@ export async function main(argv: string[]): Promise { } }); }); + } else { + // On other platforms, we listen for exit in case the child exits before the + // marker file is deleted. + childExitPromise = Event.toPromise(Event.fromNodeEventEmitter(child, 'exit')); } try { await Promise.race([ From 12b3edd852609ddaf7ec1cc0636fa4262de24fc6 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 13 Sep 2021 16:57:12 +0200 Subject: [PATCH 25/27] Update src/vs/base/node/watcher.ts Co-authored-by: Raymond Zhao --- src/vs/base/node/watcher.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/vs/base/node/watcher.ts b/src/vs/base/node/watcher.ts index 169f2907e79b2..1bd4e2b0be026 100644 --- a/src/vs/base/node/watcher.ts +++ b/src/vs/base/node/watcher.ts @@ -251,7 +251,17 @@ export async function watchFileContents(path: string, onData: (chunk: Uint8Array let resolveFn: Function; let rejectFn: Function; - const promise = new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { + cts.token.onCancellationRequested(async () => { + watcher.dispose(); + await Promises.close(handle); + + if (error) { + reject(error); + } else { + resolve(); + } + }); resolveFn = resolve; rejectFn = reject; }); From f73d9d15a98d8f53254d69f4a95bac3d29322fec Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 13 Sep 2021 17:00:30 +0200 Subject: [PATCH 26/27] fix accidental change --- src/vs/base/node/watcher.ts | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/src/vs/base/node/watcher.ts b/src/vs/base/node/watcher.ts index 1bd4e2b0be026..58adf3749438d 100644 --- a/src/vs/base/node/watcher.ts +++ b/src/vs/base/node/watcher.ts @@ -249,33 +249,16 @@ export async function watchFileContents(path: string, onData: (chunk: Uint8Array cts.dispose(true); }); - let resolveFn: Function; - let rejectFn: Function; return new Promise((resolve, reject) => { - cts.token.onCancellationRequested(async () => { - watcher.dispose(); - await Promises.close(handle); - - if (error) { - reject(error); - } else { - resolve(); - } - }); - resolveFn = resolve; - rejectFn = reject; - }); - - cts.token.onCancellationRequested(async () => { - watcher.dispose(); - await Promises.close(handle); - - if (error) { - rejectFn(error); - } else { - resolveFn(); - } + cts.token.onCancellationRequested(async () => { + watcher.dispose(); + await Promises.close(handle); + + if (error) { + reject(error); + } else { + resolve(); + } + }); }); - - return promise; } From 7c7016e54878a81b2f7e9e05ddf0aaf7bbbfa73e Mon Sep 17 00:00:00 2001 From: Raymond Zhao Date: Mon, 13 Sep 2021 10:02:07 -0700 Subject: [PATCH 27/27] Refactor --- src/vs/code/node/cli.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/vs/code/node/cli.ts b/src/vs/code/node/cli.ts index eba35a2970f8a..5f6acd3639568 100644 --- a/src/vs/code/node/cli.ts +++ b/src/vs/code/node/cli.ts @@ -393,10 +393,8 @@ export async function main(argv: string[]): Promise { const stream = outputType === 'stdout' ? process.stdout : process.stderr; const cts = new CancellationTokenSource(); - await Promise.race([ - watchFileContents(tmpName, chunk => stream.write(chunk), cts.token), - Event.toPromise(Event.fromNodeEventEmitter(child, 'close')).then(() => cts.dispose(true)) - ]); + child.on('close', () => cts.dispose(true)); + await watchFileContents(tmpName, chunk => stream.write(chunk), cts.token); } finally { unlinkSync(tmpName); }