Skip to content
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.

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

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

Closed
Chuxel opened this issue Aug 5, 2022 · 7 comments · Fixed by #352
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Chuxel
Copy link
Member

Chuxel commented Aug 5, 2022

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 Chuxel added the bug Something isn't working label Aug 5, 2022
@joshspicer
Copy link
Member

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

@Chuxel
Copy link
Member Author

Chuxel commented Aug 5, 2022

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.

@juzuluag
Copy link
Contributor

juzuluag commented Aug 5, 2022

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.

@chrmarti
Copy link
Contributor

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.

@aaronadamsCA
Copy link

I just ran into this while trying out devcontainers/ci#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.

@Chuxel
Copy link
Member Author

Chuxel commented Jan 3, 2023

@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
Copy link
Member Author

Chuxel commented Jan 5, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants