From 36196207580a7738763dd8925ab72d0697c79853 Mon Sep 17 00:00:00 2001 From: Prathamesh Zarkar <159782310+prathameshzarkar9@users.noreply.github.com> Date: Wed, 5 Jun 2024 11:26:29 +0000 Subject: [PATCH 1/4] obtuse and confusing error with local feature under WSL #834 --- .../containerFeaturesConfiguration.ts | 4 ++-- src/test/cli.build.test.ts | 11 +++++++++++ .../image-with-local-feature/.devcontainer.json | 6 ++++++ .../myfeature/devcontainer-feature.json | 0 4 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 src/test/configs/image-with-local-feature/.devcontainer.json create mode 100644 src/test/configs/image-with-local-feature/myfeature/devcontainer-feature.json diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 8b5968f32..3257f8d5b 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -698,8 +698,8 @@ export async function getFeatureIdType(params: CommonParams, userFeatureId: stri // Legacy feature-set ID if (!userFeatureId.includes('/') && !userFeatureId.includes('\\')) { - output.write(`Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements.`, LogLevel.Error); - throw new ContainerError({ description: `Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements.` }); + output.write(`Legacy feature '${userFeatureId}' forgot to prepend './' at the begining of the feature. Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for replacements.`, LogLevel.Error); + throw new ContainerError({ description: `Legacy feature '${userFeatureId}' forgot to prepend './' at the begining of the feature. Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for replacements.` }); } // Direct tarball reference diff --git a/src/test/cli.build.test.ts b/src/test/cli.build.test.ts index dbec52704..134639184 100644 --- a/src/test/cli.build.test.ts +++ b/src/test/cli.build.test.ts @@ -27,6 +27,17 @@ describe('Dev Containers CLI', function () { describe('Command build', () => { + it('should fail to build with correct error message for local feature', async () => { + const testFolder = `${__dirname}/configs/image-with-local-feature`; + try { + await shellExec(`${cli} build --workspace-folder ${testFolder} --image-name demo:v1`); + } catch (error) { + const res = JSON.parse(error.stdout); + assert.equal(res.outcome, 'error'); + assert.match(res.message, /forgot to prepend '.\/'/); + } + }); + it('should correctly configure the image name to push from --image-name with --push true', async () => { const testFolder = `${__dirname}/configs/example`; try { diff --git a/src/test/configs/image-with-local-feature/.devcontainer.json b/src/test/configs/image-with-local-feature/.devcontainer.json new file mode 100644 index 000000000..b206dc646 --- /dev/null +++ b/src/test/configs/image-with-local-feature/.devcontainer.json @@ -0,0 +1,6 @@ +{ + "image": "ubuntu:latest", + "features": { + "myfeature": {} + } +} \ No newline at end of file diff --git a/src/test/configs/image-with-local-feature/myfeature/devcontainer-feature.json b/src/test/configs/image-with-local-feature/myfeature/devcontainer-feature.json new file mode 100644 index 000000000..e69de29bb From 71ff12a448ccc18aef500c7f71ccddaf6ffd99eb Mon Sep 17 00:00:00 2001 From: Prathamesh Zarkar <159782310+prathameshzarkar9@users.noreply.github.com> Date: Thu, 6 Jun 2024 20:53:50 +0000 Subject: [PATCH 2/4] addressed review comments and modified error message accordingly --- src/spec-configuration/containerFeaturesConfiguration.ts | 7 +++++-- src/test/cli.build.test.ts | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 3257f8d5b..79a355685 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -698,8 +698,11 @@ export async function getFeatureIdType(params: CommonParams, userFeatureId: stri // Legacy feature-set ID if (!userFeatureId.includes('/') && !userFeatureId.includes('\\')) { - output.write(`Legacy feature '${userFeatureId}' forgot to prepend './' at the begining of the feature. Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for replacements.`, LogLevel.Error); - throw new ContainerError({ description: `Legacy feature '${userFeatureId}' forgot to prepend './' at the begining of the feature. Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for replacements.` }); + output.write(`Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements. \n + If you were hoping to use local Features, remember to prepend your Feature name with "./". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information.`, LogLevel.Error); + throw new ContainerError({ + description: `Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements. \n + If you were hoping to use local Features, remember to prepend your Feature name with "./". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information.` }); } // Direct tarball reference diff --git a/src/test/cli.build.test.ts b/src/test/cli.build.test.ts index 134639184..96d2cd2ce 100644 --- a/src/test/cli.build.test.ts +++ b/src/test/cli.build.test.ts @@ -34,7 +34,7 @@ describe('Dev Containers CLI', function () { } catch (error) { const res = JSON.parse(error.stdout); assert.equal(res.outcome, 'error'); - assert.match(res.message, /forgot to prepend '.\/'/); + assert.match(res.message, /prepend your Feature name with/); } }); From e89755b86ea43dd3912f7699eefdd1e9350892d9 Mon Sep 17 00:00:00 2001 From: Prathamesh Zarkar <159782310+prathameshzarkar9@users.noreply.github.com> Date: Mon, 10 Jun 2024 06:46:30 +0000 Subject: [PATCH 3/4] review comments addressed and error message changed as needed --- src/spec-configuration/containerFeaturesConfiguration.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 79a355685..98f35a7e6 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -686,6 +686,8 @@ export function updateDeprecatedFeaturesIntoOptions(userFeatures: DevContainerFe export async function getFeatureIdType(params: CommonParams, userFeatureId: string, lockfile: Lockfile | undefined) { const { output } = params; + const error_message = `Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements. + If you were hoping to use local Features, remember to prepend your Feature name with "./". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information.`; // See the specification for valid feature identifiers: // > https://github.com/devcontainers/spec/blob/main/proposals/devcontainer-features.md#referencing-a-feature // @@ -698,11 +700,10 @@ export async function getFeatureIdType(params: CommonParams, userFeatureId: stri // Legacy feature-set ID if (!userFeatureId.includes('/') && !userFeatureId.includes('\\')) { - output.write(`Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements. \n - If you were hoping to use local Features, remember to prepend your Feature name with "./". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information.`, LogLevel.Error); + output.write(error_message, LogLevel.Error); throw new ContainerError({ - description: `Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements. \n - If you were hoping to use local Features, remember to prepend your Feature name with "./". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information.` }); + description: error_message + }); } // Direct tarball reference From 73af144abd17923bd513ed53def657df2e97780d Mon Sep 17 00:00:00 2001 From: Prathamesh Zarkar <159782310+prathameshzarkar9@users.noreply.github.com> Date: Wed, 12 Jun 2024 06:54:39 +0000 Subject: [PATCH 4/4] error message tab removed and variable moved to if condition --- src/spec-configuration/containerFeaturesConfiguration.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 98f35a7e6..18c20309d 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -686,8 +686,6 @@ export function updateDeprecatedFeaturesIntoOptions(userFeatures: DevContainerFe export async function getFeatureIdType(params: CommonParams, userFeatureId: string, lockfile: Lockfile | undefined) { const { output } = params; - const error_message = `Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements. - If you were hoping to use local Features, remember to prepend your Feature name with "./". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information.`; // See the specification for valid feature identifiers: // > https://github.com/devcontainers/spec/blob/main/proposals/devcontainer-features.md#referencing-a-feature // @@ -700,9 +698,11 @@ export async function getFeatureIdType(params: CommonParams, userFeatureId: stri // Legacy feature-set ID if (!userFeatureId.includes('/') && !userFeatureId.includes('\\')) { - output.write(error_message, LogLevel.Error); + const errorMessage = `Legacy feature '${userFeatureId}' not supported. Please check https://containers.dev/features for replacements. +If you were hoping to use local Features, remember to prepend your Feature name with "./". Please check https://containers.dev/implementors/features-distribution/#addendum-locally-referenced for more information.`; + output.write(errorMessage, LogLevel.Error); throw new ContainerError({ - description: error_message + description: errorMessage }); }