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

Add prototype of mac open command #131213

Merged
merged 27 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
60e05a0
Get working prototype
rzhao271 Aug 17, 2021
de25b0d
Ignore underscore env var
rzhao271 Aug 17, 2021
e2d4dcf
Draft: Handle --status and --verbose
rzhao271 Aug 18, 2021
e0a81a3
Add draft for telemetry, add --exec-path
rzhao271 Aug 19, 2021
bf31afe
Get --telemetry working with --exec-path
rzhao271 Aug 19, 2021
9fb94dd
Add support for stdin arg
rzhao271 Aug 19, 2021
9459ab6
Apply PR feedback
rzhao271 Aug 23, 2021
ae84da5
Take away exec path, use code-cli.sh to debug
rzhao271 Aug 24, 2021
f32a171
Remove vscode wait flag for verbose
rzhao271 Aug 31, 2021
551082a
Apply more PR feedback
rzhao271 Aug 31, 2021
9c681c6
Add draft of file watcher impl
rzhao271 Aug 31, 2021
f3f84e5
refactor to use better filenames
rzhao271 Aug 31, 2021
6c8d8a1
Apply PR feedback
rzhao271 Sep 1, 2021
461c34a
Add polish
rzhao271 Sep 2, 2021
f0116b4
Apply PR feedback
rzhao271 Sep 2, 2021
f503dfc
Handle wait on macOS, refactor
rzhao271 Sep 2, 2021
247fd45
Update src/vs/code/node/cli.ts
rzhao271 Sep 3, 2021
870f238
Move child dep out of logger
rzhao271 Sep 3, 2021
36cebd9
💄
rzhao271 Sep 7, 2021
1020789
introduce `watchFileContents` and adopt
bpasero Sep 8, 2021
33e043f
cli - move --wait support to respective place
bpasero Sep 8, 2021
b76ea26
Use -W only for verbose
rzhao271 Sep 8, 2021
06c81f5
Change promise to get mac wait flag working
rzhao271 Sep 8, 2021
f0833d3
Clarify childExitPromise
rzhao271 Sep 9, 2021
12b3edd
Update src/vs/base/node/watcher.ts
bpasero Sep 13, 2021
f73d9d1
fix accidental change
bpasero Sep 13, 2021
7c7016e
Refactor
rzhao271 Sep 13, 2021
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
60 changes: 60 additions & 0 deletions src/vs/base/node/watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -202,3 +203,62 @@ 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<void> {
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);
});

return new Promise<void>((resolve, reject) => {
cts.token.onCancellationRequested(async () => {
watcher.dispose();
await Promises.close(handle);

if (error) {
reject(error);
} else {
resolve();
}
});
});
}
89 changes: 83 additions & 6 deletions src/vs/code/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,23 @@

import { ChildProcess, spawn, SpawnOptions } from 'child_process';
import { chmodSync, existsSync, readFileSync, statSync, truncateSync, unlinkSync } from 'fs';
import { homedir } 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 } from 'vs/base/common/path';
import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
import { isAbsolute, join, resolve } from 'vs/base/common/path';
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';
import { findFreePort } from 'vs/base/node/ports';
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']
Expand All @@ -30,6 +32,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<void>;
}
Expand Down Expand Up @@ -205,10 +211,29 @@ export async function main(argv: string[]): Promise<any> {
// - 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;
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<void>((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();
}
});
});
} 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([
whenDeleted(waitMarkerFilePath!),
Event.toPromise(Event.fromNodeEventEmitter(child, 'exit'))
Event.toPromise(Event.fromNodeEventEmitter(child, 'error')),
childExitPromise
]);
} finally {
if (stdinFilePath) {
Expand All @@ -232,7 +257,7 @@ export async function main(argv: string[]): Promise<any> {
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}`);
Expand Down Expand Up @@ -337,7 +362,59 @@ export async function main(argv: string[]): Promise<any> {
options['stdio'] = 'ignore';
}

const child = spawn(process.execPath, argv.slice(2), options);
let child: ChildProcess;
rzhao271 marked this conversation as resolved.
Show resolved Hide resolved
if (!isMacintosh) {
// We spawn process.execPath directly
child = spawn(process.execPath, argv.slice(2), options);
} 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 spawnArgs = ['-n']; // -n: launches even when opened already
spawnArgs.push('-a', process.execPath); // -a: opens a specific application

if (verbose) {
spawnArgs.push('--wait-apps'); // `open --wait-apps`: blocks until the launched app is closed (even if they were already running)

// 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
for (const outputType of ['stdout', 'stderr']) {

// Tmp file to target output to
const tmpName = createFileName(tmpdir(), `code-${outputType}`);
writeFileSync(tmpName, '');
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();
child.on('close', () => cts.dispose(true));
await watchFileContents(tmpName, chunk => stream.write(chunk), cts.token);
} finally {
unlinkSync(tmpName);
}
});
}
}

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 = spawnArgs.indexOf(curdir);
spawnArgs[launchDirIndex] = resolve(curdir);
}

child = spawn('open', spawnArgs, options);
}

return Promise.all(processCallbacks.map(callback => callback(child)));
}
Expand Down