Skip to content

Commit

Permalink
Use CustomExecution instead of ShellExecution (#1360)
Browse files Browse the repository at this point in the history
* Use CustomExecution instead of ShellExecution

* Rev to 0.9.0

* Change to use child_process.spawn

* Refresh dockerode before trees

* Aggregate Docker build/run task labels with VS Code defaults (#1396)
  • Loading branch information
bwateratmsft authored Nov 6, 2019
1 parent b58b338 commit 836a765
Show file tree
Hide file tree
Showing 29 changed files with 569 additions and 239 deletions.
1 change: 1 addition & 0 deletions .azure-pipelines/common/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ steps:
SERVICE_PRINCIPAL_DOMAIN: $(SERVICE_PRINCIPAL_DOMAIN)
DISPLAY: :10 # Only necessary for linux tests
DOCKER_UNAVAILABLE: ${{ parameters.dockerUnavailable }}
CODE_VERSION: insiders # TODO : remove this line

- task: PublishTestResults@2
displayName: 'Publish Test Results'
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 16 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "vscode-docker",
"version": "0.8.3-alpha",
"version": "0.9.0-alpha",
"preview": true,
"publisher": "ms-azuretools",
"displayName": "Docker",
Expand Down Expand Up @@ -833,6 +833,13 @@
"labels": {
"type": "object",
"description": "Labels applied to the Docker image used for debugging.",
"properties": {
"includeDefaults": {
"type": "boolean",
"description": "Whether to include the default set of labels defined by the Docker extension",
"default": true
}
},
"additionalProperties": {
"type": "string"
}
Expand Down Expand Up @@ -917,6 +924,13 @@
"labels": {
"type": "object",
"description": "Labels applied to the Docker container used for debugging.",
"properties": {
"includeDefaults": {
"type": "boolean",
"description": "Whether to include the default set of labels defined by the Docker extension",
"default": true
}
},
"additionalProperties": {
"type": "string"
}
Expand Down Expand Up @@ -1920,7 +1934,7 @@
}
},
"engines": {
"vscode": "^1.37.0"
"vscode": "^1.40.0"
},
"scripts": {
"vscode:prepublish": "npm run webpack-prod",
Expand Down
7 changes: 7 additions & 0 deletions src/configureWorkspace/configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as path from "path";
import * as vscode from "vscode";
import { IActionContext, TelemetryProperties } from 'vscode-azureextensionui';
import * as xml2js from 'xml2js';
import { DockerOrchestration } from '../constants';
import { ext } from '../extensionVariables';
import { extractRegExGroups } from '../utils/extractRegExGroups';
import { globAsync } from '../utils/globAsync';
Expand Down Expand Up @@ -385,6 +386,7 @@ async function configureCore(context: IActionContext, options: ConfigureApiOptio
os = await quickPickOS();
}
properties.configureOs = os;
properties.orchestration = 'single' as DockerOrchestration;

let generateComposeFiles = true;

Expand Down Expand Up @@ -448,6 +450,11 @@ async function configureCore(context: IActionContext, options: ConfigureApiOptio
let filesWritten: string[] = [];
await Promise.all(Object.keys(DOCKER_FILE_TYPES).map(async (fileName) => {
const dockerFileType = DOCKER_FILE_TYPES[fileName];

if (dockerFileType.isComposeGenerator && generateComposeFiles) {
properties.orchestration = 'docker-compose' as DockerOrchestration;
}

return dockerFileType.isComposeGenerator !== true || generateComposeFiles
? createWorkspaceFileIfNotExists(fileName, dockerFileType.generator)
: Promise.resolve();
Expand Down
2 changes: 2 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ export const FILE_SEARCH_MAX_RESULT = 1000;
export const dockerHubUrl: string = 'https://hub.docker.com/';

export const extensionId: string = 'ms-azuretools.vscode-docker';

export type DockerOrchestration = 'single' | 'docker-compose';
3 changes: 2 additions & 1 deletion src/debugging/DockerDebugConfigurationProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import { CancellationToken, debug, DebugConfiguration, DebugConfigurationProvider, ProviderResult, WorkspaceFolder } from 'vscode';
import { callWithTelemetryAndErrorHandling, IActionContext } from 'vscode-azureextensionui';
import { DockerOrchestration } from '../constants';
import { getAssociatedDockerRunTask } from '../tasks/TaskHelper';
import { DockerClient } from './coreclr/CliDockerClient';
import { DebugHelper, DockerDebugContext, ResolvedDebugConfiguration } from './DebugHelper';
Expand Down Expand Up @@ -36,6 +37,7 @@ export class DockerDebugConfigurationProvider implements DebugConfigurationProvi

const debugPlatform = getPlatform(debugConfiguration);
actionContext.telemetry.properties.platform = debugPlatform;
actionContext.telemetry.properties.orchestration = 'single' as DockerOrchestration; // TODO: docker-compose, when support is added

return await this.resolveDebugConfigurationInternal(
{
Expand All @@ -61,7 +63,6 @@ export class DockerDebugConfigurationProvider implements DebugConfigurationProvi
await this.registerRemoveContainerAfterDebugging(resolvedConfiguration);
}

// TODO: addDockerSettingsToEnv?
return resolvedConfiguration;
}

Expand Down
2 changes: 1 addition & 1 deletion src/debugging/coreclr/ChildProcessProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import * as cp from 'child_process';
import * as process from 'process';
import { execAsync } from '../../utils/execAsync';
import { execAsync } from '../../utils/spawnAsync';

export type ProcessProviderExecOptions = cp.ExecOptions & { progress?(content: string, process: cp.ChildProcess): void };

Expand Down
5 changes: 4 additions & 1 deletion src/debugging/coreclr/CliDockerClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*--------------------------------------------------------*/

import { CommandLineBuilder } from "../../utils/commandLineBuilder";
import { spawnAsync } from "../../utils/spawnAsync";
import { ProcessProvider } from "./ChildProcessProvider";
import { LineSplitter } from "./lineSplitter";

Expand Down Expand Up @@ -131,7 +132,9 @@ export class CliDockerClient implements DockerClient {
lineSplitter.write(content);
};

await this.processProvider.exec(command, { progress: buildProgress });
// Use spawnAsync instead of the usual childProcessProvider, since build output can be long
// This unfortunately precludes effectively unit testing this method
await spawnAsync(command, {}, buildProgress);

lineSplitter.close();

Expand Down
5 changes: 2 additions & 3 deletions src/debugging/coreclr/LocalOSProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import * as os from 'os';
import * as path from 'path';
import { pathNormalize } from '../../utils/pathNormalize';
import { PlatformOS } from '../../utils/platform';

export interface OSProvider {
Expand Down Expand Up @@ -38,9 +39,7 @@ export class LocalOSProvider implements OSProvider {
}

public pathNormalize(pathOS: PlatformOS, rawPath: string): string {
return rawPath.replace(
pathOS === 'Windows' ? /\//g : /\\/g,
pathOS === 'Windows' ? '\\' : '/');
return pathNormalize(rawPath, pathOS);
}

public pathParse(pathOS: PlatformOS, rawPath: string): path.ParsedPath {
Expand Down
14 changes: 7 additions & 7 deletions src/debugging/coreclr/dockerNetCoreDebugConfigurationProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as path from 'path';
import { CancellationToken, DebugConfiguration, DebugConfigurationProvider, ProviderResult, WorkspaceFolder } from 'vscode';
import { callWithTelemetryAndErrorHandling } from 'vscode-azureextensionui';
import { PlatformOS } from '../../utils/platform';
import { resolveFilePath } from '../../utils/resolveFilePath';
import { resolveVariables } from '../../utils/resolveVariables';
import { DockerContainerExtraHost, DockerContainerPort, DockerContainerVolume } from './CliDockerClient';
import { UserSecretsRegex } from './CommandLineDotNetClient';
import { DebugSessionManager } from './debugSessionManager';
Expand Down Expand Up @@ -159,7 +159,7 @@ export class DockerNetCoreDebugConfigurationProvider implements DebugConfigurati
debugConfiguration.dockerRun = debugConfiguration.dockerRun || {};

const envFiles = debugConfiguration.dockerRun.envFiles
? debugConfiguration.dockerRun.envFiles.map(file => resolveFilePath(file, folder))
? debugConfiguration.dockerRun.envFiles.map(file => resolveVariables(file, folder))
: undefined;

if (ssl || userSecrets) {
Expand Down Expand Up @@ -190,7 +190,7 @@ export class DockerNetCoreDebugConfigurationProvider implements DebugConfigurati

private static inferVolumes(folder: WorkspaceFolder, debugConfiguration: DockerDebugConfiguration): DockerContainerVolume[] {
return debugConfiguration && debugConfiguration.dockerRun && debugConfiguration.dockerRun.volumes
? debugConfiguration.dockerRun.volumes.map(volume => ({ ...volume, localPath: resolveFilePath(volume.localPath, folder) }))
? debugConfiguration.dockerRun.volumes.map(volume => ({ ...volume, localPath: resolveVariables(volume.localPath, folder) }))
: [];
}

Expand All @@ -211,7 +211,7 @@ export class DockerNetCoreDebugConfigurationProvider implements DebugConfigurati

const folders = {
appFolder,
resolvedAppFolder: resolveFilePath(appFolder, folder)
resolvedAppFolder: resolveVariables(appFolder, folder)
};

if (!await this.fsProvider.dirExists(folders.resolvedAppFolder)) {
Expand Down Expand Up @@ -255,7 +255,7 @@ export class DockerNetCoreDebugConfigurationProvider implements DebugConfigurati

const projects = {
appProject,
resolvedAppProject: resolveFilePath(appProject, folder)
resolvedAppProject: resolveVariables(appProject, folder)
};

if (!await this.fsProvider.fileExists(projects.resolvedAppProject)) {
Expand All @@ -272,7 +272,7 @@ export class DockerNetCoreDebugConfigurationProvider implements DebugConfigurati
? resolvedAppFolder // The context defaults to the application folder if it's the same as the workspace folder (i.e. there's no solution folder).
: path.dirname(resolvedAppFolder); // The context defaults to the application's parent (i.e. solution) folder.

const resolvedContext = resolveFilePath(context, folder);
const resolvedContext = resolveVariables(context, folder);

if (!await this.fsProvider.dirExists(resolvedContext)) {
throw new Error(`The context folder '${resolvedContext}' does not exist. Ensure that the 'context' property is set correctly in the Docker debug configuration.`);
Expand All @@ -286,7 +286,7 @@ export class DockerNetCoreDebugConfigurationProvider implements DebugConfigurati
? configuration.dockerBuild.dockerfile
: path.join(resolvedAppFolder, 'Dockerfile'); // CONSIDER: Omit dockerfile argument if not specified or possibly infer from context.

dockerfile = resolveFilePath(dockerfile, folder);
dockerfile = resolveVariables(dockerfile, folder);

if (!await this.fsProvider.fileExists(dockerfile)) {
throw new Error(`The Dockerfile '${dockerfile}' does not exist. Ensure that the 'dockerfile' property is set correctly in the Docker debug configuration.`);
Expand Down
9 changes: 5 additions & 4 deletions src/debugging/netcore/NetCoreDebugHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import * as path from 'path';
import { DebugConfiguration } from 'vscode';
import { ext } from '../../extensionVariables';
import { NetCoreTaskHelper, NetCoreTaskOptions } from '../../tasks/netcore/NetCoreTaskHelper';
import { pathNormalize } from '../../utils/pathNormalize';
import { PlatformOS } from '../../utils/platform';
import { unresolveFilePath } from '../../utils/resolveFilePath';
import { unresolveWorkspaceFolder } from '../../utils/resolveVariables';
import { ChildProcessProvider } from '../coreclr/ChildProcessProvider';
import { CommandLineDotNetClient } from '../coreclr/CommandLineDotNetClient';
import { LocalFileSystemProvider } from '../coreclr/fsProvider';
Expand Down Expand Up @@ -91,7 +92,7 @@ export class NetCoreDebugHelper implements DebugHelper {
request: 'launch',
preLaunchTask: 'docker-run: debug',
netCore: {
appProject: unresolveFilePath(options.appProject, context.folder)
appProject: unresolveWorkspaceFolder(options.appProject, context.folder)
}
}
];
Expand Down Expand Up @@ -228,8 +229,8 @@ export class NetCoreDebugHelper implements DebugHelper {
const relativePath = path.relative(path.dirname(debugConfiguration.netCore.appProject), appOutput);

return platformOS === 'Windows' ?
path.win32.normalize(path.win32.join('C:\\app', relativePath)).replace(/\//g, '\\') :
path.posix.normalize(path.posix.join('/app', relativePath)).replace(/\\/g, '/');
pathNormalize(path.win32.join('C:\\app', relativePath)) :
pathNormalize(path.posix.join('/app', relativePath));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ export async function activateInternal(ctx: vscode.ExtensionContext, perfStats:
)
);

await refreshDockerode();

registerTrees();
registerCommands();

Expand All @@ -118,8 +120,6 @@ export async function activateInternal(ctx: vscode.ExtensionContext, perfStats:
registerDebugProvider(ctx);
registerTaskProviders(ctx);

await refreshDockerode();

await consolidateDefaultRegistrySettings();
activateLanguageClient(ctx);

Expand Down
4 changes: 2 additions & 2 deletions src/tasks/DockerBuildTaskDefinitionBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { TaskDefinitionBase } from './TaskDefinitionBase';
import { DockerLabels, TaskDefinitionBase } from './TaskDefinitionBase';

export interface DockerBuildOptions {
buildArgs?: { [key: string]: string };
context?: string;
dockerfile?: string;
labels?: { [key: string]: string };
labels?: DockerLabels;
tag?: string;
target?: string;
pull?: boolean;
Expand Down
Loading

0 comments on commit 836a765

Please sign in to comment.