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

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

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

dannaf
Copy link
Contributor

@dannaf dannaf commented Mar 28, 2024

What it does

Resolves #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 #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: #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

Resolves eclipse-theia#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 eclipse-theia#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.
@dannaf
Copy link
Contributor Author

dannaf commented Mar 29, 2024

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 eclipse-theia/theia-website#524 (comment) 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
Copy link
Contributor Author

dannaf commented Mar 29, 2024

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
Copy link
Contributor

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.

@jfaltermeier jfaltermeier merged commit f69faa7 into eclipse-theia:master Apr 2, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theia IDE browser.Dockerfile not working
2 participants