Skip to content
Open
Changes from all commits
Commits
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
56 changes: 46 additions & 10 deletions src/spec-node/devContainersSpecCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import fs from 'fs';
import * as path from 'path';
import yargs, { Argv } from 'yargs';

Expand Down Expand Up @@ -82,6 +83,16 @@ const mountRegex = /^type=(bind|volume),source=([^,]+),target=([^,]+)(?:,externa

})().catch(console.error);

function findWorkspaceFolder(): string | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be sufficient to just make . the default for --workspace-folder?

const workspacePath = path.join(process.cwd(), '.devcontainer');
if (fs.existsSync(workspacePath)) {
if (fs.lstatSync(workspacePath).isDirectory()) {
return process.cwd();
}
}
return undefined;
}

export type UnpackArgv<T> = T extends Argv<infer U> ? U : T;

function provisionOptions(y: Argv) {
Expand Down Expand Up @@ -124,14 +135,12 @@ function provisionOptions(y: Argv) {
})
.check(argv => {
const idLabels = (argv['id-label'] && (Array.isArray(argv['id-label']) ? argv['id-label'] : [argv['id-label']])) as string[] | undefined;
const isWorkspaceFound = argv['workspace-folder'] || findWorkspaceFolder();
if (idLabels?.some(idLabel => !/.+=.+/.test(idLabel))) {
throw new Error('Unmatched argument format: id-label must match <name>=<value>');
}
if (!(argv['workspace-folder'] || argv['id-label'])) {
throw new Error('Missing required argument: workspace-folder or id-label');
}
if (!(argv['workspace-folder'] || argv['override-config'])) {
throw new Error('Missing required argument: workspace-folder or override-config');
if (!(argv['id-label'] || argv['override-config'] || isWorkspaceFound)) {
throw new Error('Missing required argument: workspace-folder or id-label or override-config');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As workspace-folder is no longer a required parameter, this error message could be misleading. Instead can we give a similar error (eg. Dev container config not found.) 👇 if workspace is not found?

throw new ContainerError({ description: `Dev container config (${uriToFsPath(configFile, cliHost.platform)}) not found.` });

}
const mounts = (argv.mount && (Array.isArray(argv.mount) ? argv.mount : [argv.mount])) as string[] | undefined;
if (mounts?.some(mount => !mountRegex.test(mount))) {
Expand All @@ -142,6 +151,10 @@ function provisionOptions(y: Argv) {
throw new Error('Unmatched argument format: remote-env must match <name>=<value>');
}
return true;
}).middleware((argv) => {
if (!(argv['id-label'] || argv['override-config'] || argv['workspace-folder'])) {
argv['workspace-folder'] = findWorkspaceFolder();
}
});
}

Expand Down Expand Up @@ -188,7 +201,6 @@ async function provision({
'dotfiles-target-path': dotfilesTargetPath,
'container-session-data-folder': containerSessionDataFolder,
}: ProvisionArgs) {

const workspaceFolder = workspaceFolderArg ? path.resolve(process.cwd(), workspaceFolderArg) : undefined;
const addRemoteEnvs = addRemoteEnv ? (Array.isArray(addRemoteEnv) ? addRemoteEnv as string[] : [addRemoteEnv]) : [];
const addCacheFroms = addCacheFrom ? (Array.isArray(addCacheFrom) ? addCacheFrom as string[] : [addCacheFrom]) : [];
Expand Down Expand Up @@ -451,7 +463,7 @@ function buildOptions(y: Argv) {
'user-data-folder': { type: 'string', description: 'Host path to a directory that is intended to be persisted and share state between sessions.' },
'docker-path': { type: 'string', description: 'Docker CLI path.' },
'docker-compose-path': { type: 'string', description: 'Docker Compose CLI path.' },
'workspace-folder': { type: 'string', required: true, description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.' },
'workspace-folder': { type: 'string', default: '.', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.' },
'log-level': { choices: ['info' as 'info', 'debug' as 'debug', 'trace' as 'trace'], default: 'info' as 'info', description: 'Log level.' },
'log-format': { choices: ['text' as 'text', 'json' as 'json'], default: 'text' as 'text', description: 'Log format.' },
'no-cache': { type: 'boolean', default: false, description: 'Builds the image with `--no-cache`.' },
Expand All @@ -465,6 +477,14 @@ function buildOptions(y: Argv) {
'skip-feature-auto-mapping': { type: 'boolean', default: false, hidden: true, description: 'Temporary option for testing.' },
'experimental-image-metadata': { type: 'boolean', default: experimentalImageMetadataDefault, hidden: true, description: 'Temporary option for testing.' },
'skip-persisting-customizations-from-features': { type: 'boolean', default: false, hidden: true, description: 'Do not save customizations from referenced Features as image metadata' },
}).check(argv => {
if (argv['workspace-folder'] === '.') {
const workspaceFolder = findWorkspaceFolder();
if (!workspaceFolder) {
throw new Error('Unable to find Configuration File, provide workspace-folder argument');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new Error('Unable to find Configuration File, provide workspace-folder argument');
throw new Error('Unable to find the dev container config, provide workspace-folder argument');

}
}
return true;
});
}

Expand Down Expand Up @@ -696,17 +716,23 @@ function runUserCommandsOptions(y: Argv) {
})
.check(argv => {
const idLabels = (argv['id-label'] && (Array.isArray(argv['id-label']) ? argv['id-label'] : [argv['id-label']])) as string[] | undefined;
const isWorkspaceFound = argv['workspace-folder'] || findWorkspaceFolder();
console.debug(isWorkspaceFound);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should we remove this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the debug log? I just removed it.
The isWorkspaceFound is required in a lower check, but I adjusted the Error Message there as well.

if (idLabels?.some(idLabel => !/.+=.+/.test(idLabel))) {
throw new Error('Unmatched argument format: id-label must match <name>=<value>');
}
const remoteEnvs = (argv['remote-env'] && (Array.isArray(argv['remote-env']) ? argv['remote-env'] : [argv['remote-env']])) as string[] | undefined;
if (remoteEnvs?.some(remoteEnv => !/.+=.+/.test(remoteEnv))) {
throw new Error('Unmatched argument format: remote-env must match <name>=<value>');
}
if (!argv['container-id'] && !idLabels?.length && !argv['workspace-folder']) {
if (!(argv['container-id'] || idLabels?.length || isWorkspaceFound)) {
throw new Error('Missing required argument: One of --container-id, --id-label or --workspace-folder is required.');
}
return true;
}).middleware((argv) => {
if (!(argv['id-label'] || argv['override-config'] || argv['workspace-folder'])) {
argv['workspace-folder'] = findWorkspaceFolder();
}
});
}

Expand Down Expand Up @@ -882,13 +908,18 @@ function readConfigurationOptions(y: Argv) {
})
.check(argv => {
const idLabels = (argv['id-label'] && (Array.isArray(argv['id-label']) ? argv['id-label'] : [argv['id-label']])) as string[] | undefined;
const isWorkspaceFound = argv['workspace-folder'] || findWorkspaceFolder();
if (idLabels?.some(idLabel => !/.+=.+/.test(idLabel))) {
throw new Error('Unmatched argument format: id-label must match <name>=<value>');
}
if (!argv['container-id'] && !idLabels?.length && !argv['workspace-folder']) {
if (!(argv['container-id'] || idLabels?.length || isWorkspaceFound)) {
throw new Error('Missing required argument: One of --container-id, --id-label or --workspace-folder is required.');
}
return true;
}).middleware((argv) => {
if (!(argv['id-label'] || argv['override-config'] || argv['workspace-folder'])) {
argv['workspace-folder'] = findWorkspaceFolder();
}
});
}

Expand Down Expand Up @@ -1053,17 +1084,22 @@ function execOptions(y: Argv) {
})
.check(argv => {
const idLabels = (argv['id-label'] && (Array.isArray(argv['id-label']) ? argv['id-label'] : [argv['id-label']])) as string[] | undefined;
const isWorkspaceFound = argv['workspace-folder'] || findWorkspaceFolder();
if (idLabels?.some(idLabel => !/.+=.+/.test(idLabel))) {
throw new Error('Unmatched argument format: id-label must match <name>=<value>');
}
const remoteEnvs = (argv['remote-env'] && (Array.isArray(argv['remote-env']) ? argv['remote-env'] : [argv['remote-env']])) as string[] | undefined;
if (remoteEnvs?.some(remoteEnv => !/.+=.+/.test(remoteEnv))) {
throw new Error('Unmatched argument format: remote-env must match <name>=<value>');
}
if (!argv['container-id'] && !idLabels?.length && !argv['workspace-folder']) {
if (!(argv['container-id'] || idLabels?.length || isWorkspaceFound)) {
throw new Error('Missing required argument: One of --container-id, --id-label or --workspace-folder is required.');
}
return true;
}).middleware((argv) => {
if (!(argv['id-label'] || argv['override-config'] || argv['workspace-folder'])) {
argv['workspace-folder'] = findWorkspaceFolder();
}
});
}

Expand Down