Skip to content

feat: Uses cwd as default workspace folder #956

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.

Sign up for GitHub

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

feat: Uses cwd as default workspace folder #956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
aae6c36
:test_tube: adds test for default workspace folder
ekwoka Feb 13, 2025
5828e43
:sparkles: Uses cwd as default workspace folder
ekwoka Feb 13, 2025
b512bdf
:coffin: Removes only test flags
ekwoka Feb 13, 2025
231757e
:test_tube: Adds Test for Build without workspace folder
ekwoka Feb 13, 2025
f43c050
:sparkles: Uses cwd as workspace for build
ekwoka Feb 13, 2025
5751450
:test_tube: Tests run-user-commands without workspace
ekwoka Feb 13, 2025
cdd73d8
:sparkles: Uses cwd as workspace in run-user-commands
ekwoka Feb 13, 2025
a5b0b34
:test_tube: Adds test for read-configuration
ekwoka Feb 13, 2025
5d0778f
:sparkles: Uses cwd as workspace for read-configuration
ekwoka Feb 13, 2025
8564f2e
:test_tube: Tests outdated
ekwoka Feb 13, 2025
0064a09
:sparkles: Uses cwd as the workspace for outdate
ekwoka Feb 13, 2025
28fc47f
:test_tube: Tests exec without workspace
ekwoka Feb 13, 2025
352ddba
:sparkles: Uses CWD as workspace for exec
ekwoka Feb 13, 2025
4c2a428
:construction: Clean up only test flags and test script
ekwoka Feb 13, 2025
426589a
:rewind: Reincludes id label validation
ekwoka Mar 3, 2025
71a212a
:recycle: Use Yargs default for Build
ekwoka Mar 3, 2025
5a7bf03
♻️ Uses Yargs Default for other commands.
ekwoka Mar 3, 2025
125b369
:bug: Fixes missing (
ekwoka Mar 3, 2025
b51a60e
:rotating_light: Fix Linting Issues
ekwoka Mar 12, 2025
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
37 changes: 11 additions & 26 deletions src/spec-node/devContainersSpecCLI.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function provisionOptions(y: Argv) {
'docker-compose-path': { type: 'string', description: 'Docker Compose CLI path.' },
'container-data-folder': { type: 'string', description: 'Container data folder where user data inside the container will be stored.' },
'container-system-data-folder': { type: 'string', description: 'Container system data folder where system data inside the container will be stored.' },
'workspace-folder': { type: 'string', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.' },
'workspace-folder': { type: 'string', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.', default: '.', defaultDescription: 'Current Working Directory' },
'workspace-mount-consistency': { choices: ['consistent' as 'consistent', 'cached' as 'cached', 'delegated' as 'delegated'], default: 'cached' as 'cached', description: 'Workspace mount consistency.' },
'gpu-availability': { choices: ['all' as 'all', 'detect' as 'detect', 'none' as 'none'], default: 'detect' as 'detect', description: 'Availability of GPUs in case the dev container requires any. `all` expects a GPU to be available.' },
'mount-workspace-git-root': { type: 'boolean', default: true, description: 'Mount the workspace using its Git root.' },
Expand Down Expand Up @@ -148,12 +148,6 @@ function provisionOptions(y: Argv) {
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');
}
const mounts = (argv.mount && (Array.isArray(argv.mount) ? argv.mount : [argv.mount])) as string[] | undefined;
if (mounts?.some(mount => !mountRegex.test(mount))) {
throw new Error('Unmatched argument format: mount must match type=<bind|volume>,source=<source>,target=<target>[,external=<true|false>]');
Expand Down Expand Up @@ -218,7 +212,7 @@ async function provision({
'include-merged-configuration': includeMergedConfig,
}: ProvisionArgs) {

const workspaceFolder = workspaceFolderArg ? path.resolve(process.cwd(), workspaceFolderArg) : undefined;
const workspaceFolder = path.resolve(process.cwd(), workspaceFolderArg);
const addRemoteEnvs = addRemoteEnv ? (Array.isArray(addRemoteEnv) ? addRemoteEnv as string[] : [addRemoteEnv]) : [];
const addCacheFroms = addCacheFrom ? (Array.isArray(addCacheFrom) ? addCacheFrom as string[] : [addCacheFrom]) : [];
const additionalFeatures = additionalFeaturesJson ? jsonc.parse(additionalFeaturesJson) as Record<string, string | boolean | Record<string, string | boolean>> : {};
Expand Down Expand Up @@ -507,7 +501,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', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.', default: '.', defaultDescription: 'Current Working Directory' },
'config': { type: 'string', description: 'devcontainer.json path. The default is to use .devcontainer/devcontainer.json or, if that does not exist, .devcontainer.json in the workspace folder.' },
'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.' },
Expand Down Expand Up @@ -680,7 +674,7 @@ async function doBuild({
if (envFile) {
composeGlobalArgs.push('--env-file', envFile);
}

const composeConfig = await readDockerComposeConfig(buildParams, composeFiles, envFile);
const projectName = await getProjectName(params, workspace, composeFiles, composeConfig);
const services = Object.keys(composeConfig.services || {});
Expand Down Expand Up @@ -752,7 +746,7 @@ function runUserCommandsOptions(y: Argv) {
'docker-compose-path': { type: 'string', description: 'Docker Compose CLI path.' },
'container-data-folder': { type: 'string', description: 'Container data folder where user data inside the container will be stored.' },
'container-system-data-folder': { type: 'string', description: 'Container system data folder where system data inside the container will be stored.' },
'workspace-folder': { type: 'string', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.' },
'workspace-folder': { type: 'string', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.', default: '.', defaultDescription: 'Current Working Directory' },
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, run-user-commands can be run without a workspace folder when attaching to an existing container that is not based on a workspace with a devcontainer.json.

For compatibility it might make sense to keep the existing behavior.

Copy link
Author

Choose a reason for hiding this comment

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

What is the behavior meant to be for when a container ID AND workspace folder are provided right now?

Would this impact them?

If they pass the tests still, does this indicate the tests for container-id (and others) alone are incomplete?

Does the path not already prefer those before the workspace folder?

Copy link
Contributor

Choose a reason for hiding this comment

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

When container id and workspace folder are given the container is looked up based on the container id. I think that was faster than the lookup by label.

The tests are incomplete, additions appreciated.

For consistency this change could assume the cwd as the workspace folder in all commands only when no container id and no id labels are given. Conveniently this case currently fails, so the change wouldn't break existing working setups. This should still cover most cases when run from a terminal because these rarely use the container id or id labels - I expect. Does this make sense?

'mount-workspace-git-root': { type: 'boolean', default: true, description: 'Mount the workspace using its Git root.' },
'container-id': { type: 'string', description: 'Id of the container to run the user commands for.' },
'id-label': { type: 'string', description: 'Id label(s) of the format name=value. If no --container-id is given the id labels will be used to look up the container. If no --id-label is given, one will be inferred from the --workspace-folder path.' },
Expand Down Expand Up @@ -784,9 +778,6 @@ function runUserCommandsOptions(y: Argv) {
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']) {
throw new Error('Missing required argument: One of --container-id, --id-label or --workspace-folder is required.');
}
return true;
});
}
Expand Down Expand Up @@ -840,7 +831,7 @@ async function doRunUserCommands({
await Promise.all(disposables.map(d => d()));
};
try {
const workspaceFolder = workspaceFolderArg ? path.resolve(process.cwd(), workspaceFolderArg) : undefined;
const workspaceFolder = path.resolve(process.cwd(), workspaceFolderArg);
const providedIdLabels = idLabel ? Array.isArray(idLabel) ? idLabel as string[] : [idLabel] : undefined;
const addRemoteEnvs = addRemoteEnv ? (Array.isArray(addRemoteEnv) ? addRemoteEnv as string[] : [addRemoteEnv]) : [];
const configFile = configParam ? URI.file(path.resolve(process.cwd(), configParam)) : undefined;
Expand Down Expand Up @@ -954,7 +945,7 @@ function readConfigurationOptions(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', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.' },
'workspace-folder': { type: 'string', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.', default: '.', defaultDescription: 'Current Working Directory' },
'mount-workspace-git-root': { type: 'boolean', default: true, description: 'Mount the workspace using its Git root.' },
'container-id': { type: 'string', description: 'Id of the container to run the user commands for.' },
'id-label': { type: 'string', description: 'Id label(s) of the format name=value. If no --container-id is given the id labels will be used to look up the container. If no --id-label is given, one will be inferred from the --workspace-folder path.' },
Expand All @@ -974,9 +965,6 @@ function readConfigurationOptions(y: Argv) {
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']) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as with run-user-commands, read-configuration can be run without workspace folder.

throw new Error('Missing required argument: One of --container-id, --id-label or --workspace-folder is required.');
}
return true;
});
}
Expand Down Expand Up @@ -1012,7 +1000,7 @@ async function readConfiguration({
};
let output: Log | undefined;
try {
const workspaceFolder = workspaceFolderArg ? path.resolve(process.cwd(), workspaceFolderArg) : undefined;
const workspaceFolder = path.resolve(process.cwd(), workspaceFolderArg);
const providedIdLabels = idLabel ? Array.isArray(idLabel) ? idLabel as string[] : [idLabel] : undefined;
const configFile = configParam ? URI.file(path.resolve(process.cwd(), configParam)) : undefined;
const overrideConfigFile = overrideConfig ? URI.file(path.resolve(process.cwd(), overrideConfig)) : undefined;
Expand Down Expand Up @@ -1107,7 +1095,7 @@ async function readConfiguration({
function outdatedOptions(y: Argv) {
return y.options({
'user-data-folder': { type: 'string', description: 'Host path to a directory that is intended to be persisted and share state between sessions.' },
'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', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.', default: '.', defaultDescription: 'Current Working Directory' },
'config': { type: 'string', description: 'devcontainer.json path. The default is to use .devcontainer/devcontainer.json or, if that does not exist, .devcontainer.json in the workspace folder.' },
'output-format': { choices: ['text' as 'text', 'json' as 'json'], default: 'text', description: 'Output format.' },
'log-level': { choices: ['info' as 'info', 'debug' as 'debug', 'trace' as 'trace'], default: 'info' as 'info', description: 'Log level for the --terminal-log-file. When set to trace, the log level for --log-file will also be set to trace.' },
Expand Down Expand Up @@ -1209,7 +1197,7 @@ function execOptions(y: Argv) {
'docker-compose-path': { type: 'string', description: 'Docker Compose CLI path.' },
'container-data-folder': { type: 'string', description: 'Container data folder where user data inside the container will be stored.' },
'container-system-data-folder': { type: 'string', description: 'Container system data folder where system data inside the container will be stored.' },
'workspace-folder': { type: 'string', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.' },
'workspace-folder': { type: 'string', description: 'Workspace folder path. The devcontainer.json will be looked up relative to this path.', default: '.', defaultDescription: 'Current Working Directory' },
'mount-workspace-git-root': { type: 'boolean', default: true, description: 'Mount the workspace using its Git root.' },
'container-id': { type: 'string', description: 'Id of the container to run the user commands for.' },
'id-label': { type: 'string', description: 'Id label(s) of the format name=value. If no --container-id is given the id labels will be used to look up the container. If no --id-label is given, one will be inferred from the --workspace-folder path.' },
Expand Down Expand Up @@ -1242,9 +1230,6 @@ function execOptions(y: Argv) {
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']) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

throw new Error('Missing required argument: One of --container-id, --id-label or --workspace-folder is required.');
}
return true;
});
}
Expand Down Expand Up @@ -1292,7 +1277,7 @@ export async function doExec({
let output: Log | undefined;
const isTTY = process.stdin.isTTY && process.stdout.isTTY || logFormat === 'json'; // If stdin or stdout is a pipe, we don't want to use a PTY.
try {
const workspaceFolder = workspaceFolderArg ? path.resolve(process.cwd(), workspaceFolderArg) : undefined;
const workspaceFolder = path.resolve(process.cwd(), workspaceFolderArg);
const providedIdLabels = idLabel ? Array.isArray(idLabel) ? idLabel as string[] : [idLabel] : undefined;
const addRemoteEnvs = addRemoteEnv ? (Array.isArray(addRemoteEnv) ? addRemoteEnv as string[] : [addRemoteEnv]) : [];
const configFile = configParam ? URI.file(path.resolve(process.cwd(), configParam)) : undefined;
Expand Down
14 changes: 12 additions & 2 deletions src/test/cli.build.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const pkg = require('../../package.json');
describe('Dev Containers CLI', function () {
this.timeout('120s');

const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp'));
const tmp = path.join(__dirname, 'tmp');
const cli = `npx --prefix ${tmp} devcontainer`;

before('Install', async () => {
Expand Down Expand Up @@ -53,7 +53,7 @@ describe('Dev Containers CLI', function () {
await shellExec(`${cli} build --workspace-folder ${testFolder} --image-name demo:v1`);
const tags = await shellExec(`docker images --format "{{.Tag}}" demo`);
const imageTags = tags.stdout.trim().split('\n').filter(tag => tag !== '<none>');
assert.equal(imageTags.length, 1, 'There should be only one tag for demo:v1');
assert.equal(imageTags.length, 1, 'There should be only one tag for demo:v1');
} catch (error) {
assert.equal(error.code, 'ERR_ASSERTION', 'Should fail with ERR_ASSERTION');
}
Expand Down Expand Up @@ -433,5 +433,15 @@ describe('Dev Containers CLI', function () {
const details = JSON.parse((await shellExec(`docker inspect ${response.imageName}`)).stdout)[0] as ImageDetails;
assert.strictEqual(details.Config.Labels?.test_build_options, 'success');
});

it('should build with default workspace folder', async () => {
const testFolder = `${__dirname}/configs/dockerfile-with-target`;
const res = await shellExec(`${cli} build`, { cwd: testFolder });
const response = JSON.parse(res.stdout);
assert.equal(response.outcome, 'success');
assert.ok(response.imageName);
const details = JSON.parse((await shellExec(`docker inspect ${response.imageName}`)).stdout)[0] as ImageDetails;
assert.strictEqual(details.Config.Labels?.test_build_options, 'success');
});
});
});
10 changes: 8 additions & 2 deletions src/test/cli.exec.base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function describeTests1({ text, options }: BuildKitOption) {
describe('Dev Containers CLI', function () {
this.timeout('360s');

const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp'));
const tmp = path.join(__dirname, 'tmp');
const cli = `npx --prefix ${tmp} devcontainer`;

before('Install', async () => {
Expand All @@ -36,6 +36,12 @@ export function describeTests1({ text, options }: BuildKitOption) {
assert.equal(res.signal, undefined);
assert.strictEqual(res.stdout.toString(), 'hi\n');
});
it('should execute without a workspace folder', async () => {
const res = await shellBufferExec(`${cli} exec echo hi`, { cwd: testFolder});
assert.strictEqual(res.code, 0);
assert.equal(res.signal, undefined);
assert.strictEqual(res.stdout.toString(), 'hi\n');
});
it('should not run in a terminal', async () => {
const res = await shellBufferExec(`${cli} exec --workspace-folder ${testFolder} [ ! -t 1 ]`);
assert.strictEqual(res.code, 0);
Expand Down Expand Up @@ -382,7 +388,7 @@ export function describeTests2({ text, options }: BuildKitOption) {
assert.match(res.stdout, /howdy, node/);
});
});

it('should fail with "not found" error when config is not found', async () => {
let success = false;
try {
Expand Down
14 changes: 13 additions & 1 deletion src/test/cli.set-up.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const pkg = require('../../package.json');
describe('Dev Containers CLI', function () {
this.timeout('120s');

const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp'));
const tmp = path.join(__dirname, 'tmp');
const cli = `npx --prefix ${tmp} devcontainer`;

before('Install', async () => {
Expand Down Expand Up @@ -103,6 +103,18 @@ describe('Dev Containers CLI', function () {

await shellExec(`docker rm -f ${containerId}`);
});

it('should succeed without a workspace folder', async () => {

await shellExec(`docker build -t devcontainer-set-up-test ${__dirname}/configs/set-up-with-metadata`);
const containerId = (await shellExec(`docker run -d devcontainer-set-up-test sleep inf`)).stdout.trim();

const res = await shellExec(`${cli} read-configuration --include-merged-configuration`, { cwd: `${__dirname}/configs/set-up-with-metadata` });
const response = JSON.parse(res.stdout);
assert.strictEqual(response.mergedConfiguration.postCreateCommands.length, 1);

await shellExec(`docker rm -f ${containerId}`);
});
});

describe('Command exec', () => {
Expand Down
9 changes: 7 additions & 2 deletions src/test/cli.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const pkg = require('../../package.json');
describe('Dev Containers CLI', function () {
this.timeout('120s');

const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp'));
const tmp = path.join(__dirname, 'tmp');
const cli = `npx --prefix ${tmp} devcontainer`;

before('Install', async () => {
Expand All @@ -37,6 +37,11 @@ describe('Dev Containers CLI', function () {
const response = JSON.parse(res.stdout);
assert.equal(response.outcome, 'success');
});
it('should execute successfully', async () => {
const res = await shellExec(`${cli} run-user-commands`, { cwd: testFolder });
const response = JSON.parse(res.stdout);
assert.equal(response.outcome, 'success');
});
});

it('should fail with "not found" error when config is not found', async () => {
Expand Down Expand Up @@ -125,4 +130,4 @@ describe('Dev Containers CLI', function () {
assert.strictEqual(response.configuration.remoteEnv.SUBFOLDER_CONFIG_REMOTE_ENV, 'true');
});
});
});
});
Loading
Loading