From 957d8f962f20fab1f6da43eae1b7a862040aec4b Mon Sep 17 00:00:00 2001 From: elaihau Date: Thu, 20 Jun 2019 10:31:56 -0400 Subject: [PATCH] define Task CommandOptions and move "cwd" into it - "cwd" (i.e., the current working directory) is currently defined directly under Task, which is inconsistent from the VS Code Task schema. In this pull request "cwd" is moved into the "options" as part of the CommandOptions. - part of #5516 Signed-off-by: elaihau --- CHANGELOG.md | 1 + .../browser/process/process-task-resolver.ts | 16 ++++--- .../task/src/common/process/task-protocol.ts | 43 ++++++++++++++----- .../src/node/process/process-task-runner.ts | 19 ++++---- .../task/src/node/task-server.slow-spec.ts | 10 ++--- .../task/test-resources/.theia/tasks.json | 16 ++++--- 6 files changed, 69 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69a5f934715c3..800b0af652cf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Breaking changes: - [debug] align commands with VS Code [#5102](https://github.com/theia-ide/theia/issues/5102) - `debug.restart` renamed to `workbench.action.debug.restart` - [preferences] renamed overridenPreferenceName to overriddenPreferenceName +- [task] `cwd`, which used to be defined directly under `Task`, is moved into `Task.options` object ## v0.7.0 diff --git a/packages/task/src/browser/process/process-task-resolver.ts b/packages/task/src/browser/process/process-task-resolver.ts index c4d37c8505858..6cf1cc53a6bc3 100644 --- a/packages/task/src/browser/process/process-task-resolver.ts +++ b/packages/task/src/browser/process/process-task-resolver.ts @@ -38,18 +38,22 @@ export class ProcessTaskResolver implements TaskResolver { throw new Error('Unsupported task configuration type.'); } - const options = { context: new URI(taskConfig._source).withScheme('file') }; + const variableResolverOptions = { context: new URI(taskConfig._source).withScheme('file') }; const processTaskConfig = taskConfig as ProcessTaskConfiguration; const result: ProcessTaskConfiguration = { ...processTaskConfig, - command: await this.variableResolverService.resolve(processTaskConfig.command, options), - args: processTaskConfig.args ? await this.variableResolverService.resolveArray(processTaskConfig.args, options) : undefined, + command: await this.variableResolverService.resolve(processTaskConfig.command, variableResolverOptions), + args: processTaskConfig.args ? await this.variableResolverService.resolveArray(processTaskConfig.args, variableResolverOptions) : undefined, windows: processTaskConfig.windows ? { - command: await this.variableResolverService.resolve(processTaskConfig.windows.command, options), - args: processTaskConfig.windows.args ? await this.variableResolverService.resolveArray(processTaskConfig.windows.args, options) : undefined, + command: await this.variableResolverService.resolve(processTaskConfig.windows.command, variableResolverOptions), + args: processTaskConfig.windows.args ? await this.variableResolverService.resolveArray(processTaskConfig.windows.args, variableResolverOptions) : undefined, options: processTaskConfig.windows.options } : undefined, - cwd: await this.variableResolverService.resolve(processTaskConfig.cwd || '${workspaceFolder}', options) + options: { + cwd: await this.variableResolverService.resolve(processTaskConfig.options && processTaskConfig.options.cwd || '${workspaceFolder}', variableResolverOptions), + env: processTaskConfig.options && processTaskConfig.options.env, + shell: processTaskConfig.options && processTaskConfig.options.shell + } }; return result; } diff --git a/packages/task/src/common/process/task-protocol.ts b/packages/task/src/common/process/task-protocol.ts index 9893570c261d5..23110af34dff7 100644 --- a/packages/task/src/common/process/task-protocol.ts +++ b/packages/task/src/common/process/task-protocol.ts @@ -19,10 +19,42 @@ import { ApplicationError } from '@theia/core/lib/common/application-error'; export type ProcessType = 'shell' | 'process'; +export interface CommandOptions { + /** + * The 'current working directory' the task will run in. Can be a uri-as-string + * or plain string path. If the cwd is meant to be somewhere under the workspace, + * one can use the variable `${workspaceFolder}`, which will be replaced by its path, + * at runtime. If not specified, defaults to the workspace root. + * ex: cwd: '${workspaceFolder}/foo' + */ + cwd?: string; + + /** + * The environment of the executed program or shell. If omitted the parent process' environment is used. + */ + env?: { [key: string]: string | undefined; }; + + /** + * Configuration of the shell when task type is `shell` + */ + shell?: { + /** + * The shell to use. + */ + executable: string; + + /** + * The arguments to be passed to the shell executable to run in command mode + * (e.g ['-c'] for bash or ['/S', '/C'] for cmd.exe). + */ + args?: string[]; + }; +} + export interface CommandProperties { readonly command: string; readonly args?: T[]; - readonly options?: object; + readonly options?: CommandOptions; } /** Configuration of a Task that may be run as a process or a command inside a shell. */ @@ -33,15 +65,6 @@ export interface ProcessTaskConfiguration extends TaskConfiguration, * Windows version of CommandProperties. Used in preference on Windows, if defined. */ readonly windows?: CommandProperties; - - /** - * The 'current working directory' the task will run in. Can be a uri-as-string - * or plain string path. If the cwd is meant to be somewhere under the workspace, - * one can use the variable `${workspaceFolder}`, which will be replaced by its path, - * at runtime. If not specified, defaults to the workspace root. - * ex: cwd: '${workspaceFolder}/foo' - */ - readonly cwd?: string; } export interface ProcessTaskInfo extends TaskInfo { diff --git a/packages/task/src/node/process/process-task-runner.ts b/packages/task/src/node/process/process-task-runner.ts index 845d8f825e9b0..4d8f7c9908791 100644 --- a/packages/task/src/node/process/process-task-runner.ts +++ b/packages/task/src/node/process/process-task-runner.ts @@ -29,7 +29,7 @@ import { TaskFactory } from './process-task'; import { TaskRunner } from '../task-runner'; import { Task } from '../task'; import { TaskConfiguration } from '../../common/task-protocol'; -import { ProcessTaskError } from '../../common/process/task-protocol'; +import { ProcessTaskError, CommandOptions } from '../../common/process/task-protocol'; import * as fs from 'fs'; /** @@ -61,9 +61,9 @@ export class ProcessTaskRunner implements TaskRunner { let command: string | undefined; let args: Array | undefined; - let options: any; // tslint:disable-line:no-any + let options: CommandOptions; - // on windows, prefer windows-specific options, if available + // on windows, windows-specific options, if available, takes precedence if (isWindows && taskConfig.windows !== undefined) { command = taskConfig.windows.command; args = taskConfig.windows.args; @@ -76,17 +76,18 @@ export class ProcessTaskRunner implements TaskRunner { // sanity checks: // - we expect the cwd to be set by the client. - if (!taskConfig.cwd) { + if (!options || !options.cwd) { throw new Error("Can't run a task when 'cwd' is not provided by the client"); } - const cwd = this.asFsPath(taskConfig.cwd); // Use task's cwd with spawned process and pass node env object to // new process, so e.g. we can re-use the system path - options = { - cwd: cwd, - env: process.env - }; + if (options) { + options.env = { + ...process.env, + ...(options.env || {}) + }; + } try { const processType = taskConfig.type === 'process' ? 'process' : 'shell'; diff --git a/packages/task/src/node/task-server.slow-spec.ts b/packages/task/src/node/task-server.slow-spec.ts index 8894e8f2514ca..1c8218fee25eb 100644 --- a/packages/task/src/node/task-server.slow-spec.ts +++ b/packages/task/src/node/task-server.slow-spec.ts @@ -363,7 +363,7 @@ function createTaskConfig(taskType: string, command: string, args: string[]): Ta _scope: '/source/folder', command, args, - cwd: wsRoot, + options: { cwd: wsRoot } }; return options; } @@ -376,7 +376,7 @@ function createProcessTaskConfig(processType: ProcessType, command: string, args _scope: '/source/folder', command, args, - cwd, + options: { cwd: wsRoot }, }; } @@ -386,7 +386,7 @@ function createProcessTaskConfig2(processType: ProcessType, command: string, arg type: processType, command, args, - cwd: wsRoot, + options: { cwd: wsRoot }, }; } @@ -396,7 +396,7 @@ function createTaskConfigTaskLongRunning(processType: ProcessType): TaskConfigur type: processType, _source: '/source/folder', _scope: '/source/folder', - cwd: wsRoot, + options: { cwd: wsRoot }, command: commandLongRunning, args: [], windows: { @@ -406,7 +406,7 @@ function createTaskConfigTaskLongRunning(processType: ProcessType): TaskConfigur }; } -function checkSuccessfullProcessExit(taskInfo: TaskInfo, taskWatcher: TaskWatcher): Promise { +function checkSuccessfullProcessExit(taskInfo: TaskInfo, taskWatcher: TaskWatcher): Promise { const p = new Promise((resolve, reject) => { const toDispose = taskWatcher.onTaskExit((event: TaskExitedEvent) => { if (event.taskId === taskInfo.taskId && event.code === 0) { diff --git a/packages/task/test-resources/.theia/tasks.json b/packages/task/test-resources/.theia/tasks.json index f55fd67ddb14b..5cf9e38ea28d0 100644 --- a/packages/task/test-resources/.theia/tasks.json +++ b/packages/task/test-resources/.theia/tasks.json @@ -4,12 +4,13 @@ { "label": "test task", "type": "shell", - "cwd": "${workspaceFolder}", "command": "./task", "args": [ "test" ], - "options": {}, + "options": { + "cwd": "${workspaceFolder}" + }, "windows": { "command": "cmd.exe", "args": [ @@ -22,10 +23,11 @@ { "label": "long running test task", "type": "shell", - "cwd": "${workspaceFolder}", "command": "./task-long-running", "args": [], - "options": {}, + "options": { + "cwd": "${workspaceFolder}" + }, "windows": { "command": "cmd.exe", "args": [ @@ -38,12 +40,14 @@ { "label": "list all files", "type": "shell", - "cwd": "${workspaceFolder}", "command": "ls", "args": [ "-alR", "/" ], + "options": { + "cwd": "${workspaceFolder}" + }, "windows": { "command": "cmd.exe", "args": [ @@ -53,4 +57,4 @@ } } ] -} \ No newline at end of file +}