search

eclipse-theia / theia-ide

The Eclipse IDE is a modern and open IDE for cloud and desktop. The Theia IDE is based on the Theia platform. The Theia IDE is available as a downloadable desktop application. You can also try the latest version of the Theia IDE online. For more details, see the Readme below.
https://theia-ide.org/#theiaide
MIT License
343 stars 129 forks source link

Updated docker base image to node:18-bullseye in browser.Dockerfile #346

Closed dannaf closed 6 months ago

dannaf commented 7 months ago

What it does

Resolves https://github.com/eclipse-theia/theia-blueprint/issues/345

node:16-bullseye seems outdated and resulted in the bug documented in the above issue. It is not currently supported on the official list of node docker images: https://hub.docker.com/_/node

The issue at https://github.com/eclipse-theia/theia-blueprint/issues/345 was resolved even by changing to node:18-bullseye only in the build-stage, but presumably it should be already upgraded also in the production-stage, so this commit/PR also updates the production stage based image.

How to test

Following/repeating the instructions here: https://github.com/eclipse-theia/theia-blueprint/issues/345

  1. git clone https://github.com/eclipse-theia/theia-blueprint
  2. cd theia-blueprint
  3. docker build -t theia-ide -f browser.Dockerfile . (like instructed here)
  4. Look for side-effects in the usual testing manner for this project, or otherwise decide if to make this upgrade. I am merely submitting the 'quick-fix' here for the docker build command failure documented in issue #345. But I am relying on the core development team to consider how and whether/when such an upgrade fits with the rest of the development process.

Review checklist

Reminder for reviewers

dannaf commented 7 months ago

Note that the Theia platform seems to require only node>=16 in its package.json here, but in practice this did not work on the node:16-bullseye docker image.

See also this comment https://github.com/eclipse-theia/theia-website/pull/524#issuecomment-2026322030 regarding the broader context of the interplay between the Theia platform and Theia IDE with the concept of Theia IDE serving as a product/integration test for the Theia platform.

dannaf commented 6 months ago

Actually on a quick glance it appears that such a simple upgrade did not work, as there are numerous immediately visible side-effects, like that all the button graphics are a square. (When I opened the PR I only tested that the docker image builds, not any of the Theia functionality, as noted in Step 4.)

jfaltermeier commented 6 months ago

Thank you very much. I could not see any issues with this PR locally. Also all graphics seemed to work for me, so I think we can go ahead and merge this. The failing license check may be ignored, because the yarn.lock file is unchanged. This is an internal error of the check. I have to apply one additional fix afterwards regarding Theia 1.48.0, but this is unrelated to the required node update.