Skip to content

Commit ddb0c0b

Browse files
gauravsaini04gauravsaini04
authored
Merge branch 'main' into cli_issue_#904_solution
2 parents 39028a3 + 7178ceb commit ddb0c0b

File tree

5 files changed

+96
-60
lines changed

5 files changed

+96
-60
lines changed

‎src/spec-common/injectHeadless.ts‎

Lines changed: 46 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -487,38 +487,48 @@ async function runLifecycleCommand({ lifecycleHook }: ResolverParameters, contai
487487
},
488488
onDidChangeDimensions: lifecycleHook.output.onDidChangeDimensions,
489489
}, LogLevel.Info);
490-
try {
491-
const remoteCwd = containerProperties.remoteWorkspaceFolder || containerProperties.homeFolder;
492-
async function runSingleCommand(postCommand: string | string[], name?: string) {
493-
const progressDetail = typeof postCommand === 'string' ? postCommand : postCommand.join(' ');
494-
infoOutput.event({
495-
type: 'progress',
496-
name: progressName,
497-
status: 'running',
498-
stepDetail: progressDetail
499-
});
500-
501-
// If we have a command name then the command is running in parallel and
502-
// we need to hold output until the command is done so that the output
503-
// doesn't get interleaved with the output of other commands.
504-
const printMode = name ? 'off' : 'continuous';
505-
const env = { ...(await remoteEnv), ...(await secrets) };
490+
const remoteCwd = containerProperties.remoteWorkspaceFolder || containerProperties.homeFolder;
491+
async function runSingleCommand(postCommand: string | string[], name?: string) {
492+
const progressDetails = typeof postCommand === 'string' ? postCommand : postCommand.join(' ');
493+
infoOutput.event({
494+
type: 'progress',
495+
name: progressName,
496+
status: 'running',
497+
stepDetail: progressDetails
498+
});
499+
// If we have a command name then the command is running in parallel and
500+
// we need to hold output until the command is done so that the output
501+
// doesn't get interleaved with the output of other commands.
502+
const printMode = name ? 'off' : 'continuous';
503+
const env = { ...(await remoteEnv), ...(await secrets) };
504+
try {
506505
const { cmdOutput } = await runRemoteCommand({ ...lifecycleHook, output: infoOutput }, containerProperties, typeof postCommand === 'string' ? ['/bin/sh', '-c', postCommand] : postCommand, remoteCwd, { remoteEnv: env, pty: true, print: printMode });
507506

508507
// 'name' is set when parallel execution syntax is used.
509508
if (name) {
510-
infoOutput.raw(`\x1b[1mRunning ${name} from ${userCommandOrigin}...\x1b[0m\r\n${cmdOutput}\r\n`);
509+
infoOutput.raw(`\x1b[1mRunning ${name} of ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n${cmdOutput}\r\n`);
510+
}
511+
} catch (err) {
512+
if (printMode === 'off' && err?.cmdOutput) {
513+
infoOutput.raw(`\r\n\x1b[1m${err.cmdOutput}\x1b[0m\r\n\r\n`);
514+
}
515+
if (err && (err.code === 130 || err.signal === 2)) { // SIGINT seen on darwin as code === 130, would also make sense as signal === 2.
516+
infoOutput.raw(`\r\n\x1b[1m${name ? `${name} of ${lifecycleHookName}` : lifecycleHookName} from ${userCommandOrigin} interrupted.\x1b[0m\r\n\r\n`);
517+
} else {
518+
if (err?.code) {
519+
infoOutput.write(toErrorText(`${name ? `${name} of ${lifecycleHookName}` : lifecycleHookName} from ${userCommandOrigin} failed with exit code ${err.code}. Skipping any further user-provided commands.`));
520+
}
521+
throw new ContainerError({
522+
description: `${name ? `${name} of ${lifecycleHookName}` : lifecycleHookName} from ${userCommandOrigin} failed.`,
523+
originalError: err
524+
});
511525
}
512-
513-
infoOutput.event({
514-
type: 'progress',
515-
name: progressName,
516-
status: 'succeeded',
517-
});
518526
}
527+
}
519528

520-
infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n\r\n`);
529+
infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n\r\n`);
521530

531+
try {
522532
let commands;
523533
if (typeof userCommand === 'string' || Array.isArray(userCommand)) {
524534
commands = [runSingleCommand(userCommand)];
@@ -528,24 +538,24 @@ async function runLifecycleCommand({ lifecycleHook }: ResolverParameters, contai
528538
return runSingleCommand(command, name);
529539
});
530540
}
531-
await Promise.all(commands);
541+
542+
const results = await Promise.allSettled(commands); // Wait for all commands to finish (successfully or not) before continuing.
543+
const rejection = results.find(p => p.status === 'rejected');
544+
if (rejection) {
545+
throw (rejection as PromiseRejectedResult).reason;
546+
}
547+
infoOutput.event({
548+
type: 'progress',
549+
name: progressName,
550+
status: 'succeeded',
551+
});
532552
} catch (err) {
533553
infoOutput.event({
534554
type: 'progress',
535555
name: progressName,
536556
status: 'failed',
537557
});
538-
if (err && (err.code === 130 || err.signal === 2)) { // SIGINT seen on darwin as code === 130, would also make sense as signal === 2.
539-
infoOutput.raw(`\r\n\x1b[1m${lifecycleHookName} interrupted.\x1b[0m\r\n\r\n`);
540-
} else {
541-
if (err?.code) {
542-
infoOutput.write(toErrorText(`${lifecycleHookName} failed with exit code ${err.code}. Skipping any further user-provided commands.`));
543-
}
544-
throw new ContainerError({
545-
description: `The ${lifecycleHookName} in the ${userCommandOrigin} failed.`,
546-
originalError: err,
547-
});
548-
}
558+
throw err;
549559
}
550560
}
551561
}

‎src/spec-node/featuresCLI/resolveDependencies.ts‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,23 @@
11
import * as path from 'path';
2-
import * as jsonc from 'jsonc-parser';
32
import { Argv } from 'yargs';
43
import { LogLevel, mapLogLevel } from '../../spec-utils/log';
54
import { getPackageConfig } from '../../spec-utils/product';
65
import { createLog } from '../devContainers';
76
import { UnpackArgv } from '../devContainersSpecCLI';
8-
import { isLocalFile, readLocalFile } from '../../spec-utils/pfs';
9-
import { DevContainerConfig, DevContainerFeature } from '../../spec-configuration/configuration';
7+
import { isLocalFile } from '../../spec-utils/pfs';
8+
import { DevContainerFeature } from '../../spec-configuration/configuration';
109
import { buildDependencyGraph, computeDependsOnInstallationOrder, generateMermaidDiagram } from '../../spec-configuration/containerFeaturesOrder';
1110
import { OCISourceInformation, processFeatureIdentifier, userFeaturesToArray } from '../../spec-configuration/containerFeaturesConfiguration';
1211
import { readLockfile } from '../../spec-configuration/lockfile';
1312
import { runAsyncHandler } from '../utils';
13+
import { loadNativeModule } from '../../spec-common/commonUtils';
14+
import { getCLIHost } from '../../spec-common/cliHost';
15+
import { ContainerError } from '../../spec-common/errors';
16+
import { uriToFsPath } from '../../spec-configuration/configurationCommonUtils';
17+
import { workspaceFromPath } from '../../spec-utils/workspaces';
18+
import { readDevContainerConfigFile } from '../configContainer';
19+
import { URI } from 'vscode-uri';
20+
1421

1522
interface JsonOutput {
1623
installOrder?: {
@@ -61,28 +68,28 @@ async function featuresResolveDependencies({
6168
configPath = path.join(workspaceFolder, '.devcontainer', 'devcontainer.json');
6269
}
6370

64-
// Load dev container config
65-
const buffer = await readLocalFile(configPath);
66-
if (!buffer) {
67-
output.write(`Could not load devcontainer.json file from path ${configPath}`, LogLevel.Error);
68-
process.exit(1);
69-
}
71+
const params = {
72+
output,
73+
env: process.env,
74+
};
7075

71-
// Parse dev container config
72-
const config: DevContainerConfig = jsonc.parse(buffer.toString());
73-
if (!config || !config.features) {
74-
output.write(`No Features object in configuration '${configPath}'`, LogLevel.Error);
75-
process.exit(1);
76+
const cwd = workspaceFolder || process.cwd();
77+
const cliHost = await getCLIHost(cwd, loadNativeModule, true);
78+
const workspace = workspaceFromPath(cliHost.path, workspaceFolder);
79+
const configFile: URI = URI.file(path.resolve(process.cwd(), configPath));
80+
const configs = await readDevContainerConfigFile(cliHost, workspace, configFile, false, output, undefined, undefined);
81+
82+
if (configFile && !configs) {
83+
throw new ContainerError({ description: `Dev container config (${uriToFsPath(configFile, cliHost.platform)}) not found.` });
7684
}
85+
const configWithRaw = configs!.config;
86+
const { config } = configWithRaw;
87+
7788
const userFeaturesConfig = userFeaturesToArray(config);
7889
if (!userFeaturesConfig) {
7990
output.write(`Could not parse features object in configuration '${configPath}'`, LogLevel.Error);
8091
process.exit(1);
8192
}
82-
const params = {
83-
output,
84-
env: process.env,
85-
};
8693

8794
const { lockfile } = await readLockfile(config);
8895
const processFeature = async (_userFeature: DevContainerFeature) => {

‎src/test/cli.up.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ describe('Dev Containers CLI', function () {
2525
});
2626

2727
describe('Command up', () => {
28+
2829
it('should execute successfully with valid config', async () => {
2930
const res = await shellExec(`${cli} up --workspace-folder ${__dirname}/configs/image --include-configuration --include-merged-configuration`);
3031
const response = JSON.parse(res.stdout);

‎src/test/configs/poetry-example/.devcontainer.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Example devcontainer.json configuration,
2+
// wired into the vscode launch task (.vscode/launch.json)
3+
{
4+
"image": "mcr.microsoft.com/devcontainers/base:latest",
5+
"features": {
6+
"ghcr.io/devcontainers/features/python:1": {
7+
"version": "latest"
8+
}
9+
},
10+
"postStartCommand": {
11+
"poetry setup": [
12+
"/bin/bash",
13+
"-i",
14+
"-c",
15+
"python3 -m venv $HOME/.local && source $HOME/.local/bin/activate && poetry install"
16+
]
17+
}
18+
}

‎src/test/container-features/lifecycleHooks.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -406,10 +406,10 @@ describe('Feature lifecycle hooks', function () {
406406
assert.match(containerUpStandardError, /Running the postAttachCommand from devcontainer.json/);
407407

408408
assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_1.testMarker/);
409-
assert.match(containerUpStandardError, /Running parallel1 from devcontainer.json.../);
409+
assert.match(containerUpStandardError, /Running parallel1 of postCreateCommand from devcontainer.json.../);
410410

411411
assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_2.testMarker/);
412-
assert.match(containerUpStandardError, /Running parallel2 from devcontainer.json.../);
412+
assert.match(containerUpStandardError, /Running parallel2 of postCreateCommand from devcontainer.json.../);
413413

414414
// Since lifecycle scripts are executed relative to the workspace folder,
415415
// to run a script bundled with the Feature, the Feature author needs to copy that script to a persistent directory.
@@ -429,10 +429,10 @@ describe('Feature lifecycle hooks', function () {
429429
assert.match(containerUpStandardError, /Running the postAttachCommand from Feature '\.\/rabbit'/);
430430

431431
assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_1.testMarker/);
432-
assert.match(containerUpStandardError, /Running parallel1 from Feature '\.\/rabbit'/);
432+
assert.match(containerUpStandardError, /Running parallel1 of postCreateCommand from Feature '\.\/rabbit'/);
433433

434434
assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_2.testMarker/);
435-
assert.match(containerUpStandardError, /Running parallel2 from Feature '\.\/rabbit'/);
435+
assert.match(containerUpStandardError, /Running parallel2 of postCreateCommand from Feature '\.\/rabbit'/);
436436

437437

438438
// -- 'Otter' Feature
@@ -449,10 +449,10 @@ describe('Feature lifecycle hooks', function () {
449449
assert.match(containerUpStandardError, /Running the postAttachCommand from Feature '\.\/otter'/);
450450

451451
assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_1.testMarker/);
452-
assert.match(containerUpStandardError, /Running parallel1 from Feature '\.\/otter'/);
452+
assert.match(containerUpStandardError, /Running parallel1 of postCreateCommand from Feature '\.\/otter'/);
453453

454454
assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_2.testMarker/);
455-
assert.match(containerUpStandardError, /Running parallel2 from Feature '\.\/otter'/);
455+
assert.match(containerUpStandardError, /Running parallel2 of postCreateCommand from Feature '\.\/otter'/);
456456

457457
// -- Assert that at no point did logging the lifecycle hook fail.
458458
assert.notMatch(containerUpStandardError, /Running the (.*) from \?\?\?/);

0 commit comments

Comments
 (0)