cli icon indicating copy to clipboard operation
cli copied to clipboard

--platform and --push flags force the use of a Dockerfile even if features are referenced

Open Chuxel opened this issue 3 years ago • 4 comments

The --platform and --push flags are important enablers for pre-building images - particularly those that take advantage of Dev Container Features (devcontainers/spec#61).

Unfortunately, right now there's an error if only an image is referenced along with a set of features in devcontainer.json. Given just being able to just start from an image is one of the primary goals of Dev Container Features, this seems to be pretty clearly a bug.

This occurred in version 0.9.1 of the CLI.

Error: --platform or --push require dockerfilePath.
    at doBuild (/usr/local/share/npm-global/lib/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:342:23)
    at async build (/usr/local/share/npm-global/lib/node_modules/@devcontainers/cli/dist/spec-node/devContainersSpecCLI.js:233:20)
{"outcome":"error","message":"--platform or --push require dockerfilePath.","description":"--platform or --push require dockerfilePath."}

//cc @chrmarti @joshspicer @alexdima @bamurtaugh

Chuxel avatar Aug 05 '22 16:08 Chuxel

This seems intentional - @juzuluag do you recall what scenario this was attempting to prevent? https://github.com/devcontainers/cli/blob/main/src/spec-node/devContainersSpecCLI.ts#L396

joshspicer avatar Aug 05 '22 17:08 joshspicer

Before features was implemented, this would make sense. Post-features, it doesn't - so I'm assuming this was just a good check that no longer applies.

Chuxel avatar Aug 05 '22 18:08 Chuxel

This seems intentional - @juzuluag do you recall what scenario this was attempting to prevent? https://github.com/devcontainers/cli/blob/main/src/spec-node/devContainersSpecCLI.ts#L396

Hi, @joshspicer. As far I recall, it was to prevent an error when there was no dockerfile file specified as part of .devcontainer/devcontainer.json. Doing unit test, it was failing using the feature test folders. Not sure whether Features merged first before this functionality merged.

juzuluag avatar Aug 05 '22 20:08 juzuluag

I missed that case when we brought the multi-arch changes in on-top of then recent work on features. I see buildAndExtendDockerCompose in dockerCompose.ts handles the image case, so that could serve as a guide.

chrmarti avatar Aug 11 '22 09:08 chrmarti

I just ran into this while trying out https://github.com/devcontainers/ci/pull/175.

Hopefully this can be resolved soon, because we'd love to migrate our repos to use devcontainer features, but I'm not sure it's worth it until we can get ARM64 prebuilds too.

aaronadamsCA avatar Dec 30 '22 12:12 aaronadamsCA

@joshspicer @chrmarti @jkeech Given the templates now use an image by default, I think the priority of this one is pretty high as I think about it. It sounds like a minor fix.

Chuxel avatar Jan 03 '23 15:01 Chuxel

This should now be working with yesterday's release (0.27.1). 🎉 The GitHub Action should be grabbing the latest verison of the CLI automatically and should now work too.

Chuxel avatar Jan 05 '23 15:01 Chuxel