devcontainer: Use new prebuilt devcontainer image.#43139
Conversation
Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
dad7b69 to
e6ba39a
Compare
| "dockerfile": "Ubuntu.Dockerfile", | ||
| "context": ".." | ||
| }, | ||
| "image": "ghcr.io/servo/servo/devcontainer-ubuntu:latest", |
There was a problem hiding this comment.
I think I understand the rationale for specifying this image (to make setting up the devcontainer more efficient), but it appears to have a big downside: the image may not match the contents of the Dockerfile. So devcontainer behaviour becomes non-reproducible (with all the downsides of a non-reproducible build).
There was a problem hiding this comment.
While this is technically true I think it's not really a problem in practice, because we have postCreateCommand that will make sure that image contains everything needed from current checkout. Also we do not expect many changes to Dockerfile. If one wants to change dockerfile it can also change devcontainer file.
Ideally docker would check if local dockerfile matches dockerfile from image, but this is not possible.
There was a problem hiding this comment.
Ok, sounds reasonable then. If problems occur, that will be the point at which to address them.
glyn
left a comment
There was a problem hiding this comment.
If my comment about a non-reproducible devcontainer is correct, I don't think this PR should be merged.
sagudev
left a comment
There was a problem hiding this comment.
IMO we should still do this for faster onboarding.
Follow-up to servo#43131. Testing: Tested locally. Note: This only works for x86_64 based systems. Arm Linux users will need to build from source, but building in the devcontainer on arm linux is anyway not working currently. Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com> Signed-off-by: Bruno Nicoletti <bjn@dummy_address.com>
Follow-up to #43131.
Testing: Tested locally. Note: This only works for x86_64 based systems. Arm Linux users will need to build from source, but building in the devcontainer on arm linux is anyway not working currently.