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.

Sign up for GitHub

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

Jump to bottom

update repo for Electron 9 / node 12 #107

Merged
merged 1 commit into from
Aug 3, 2020
Merged

update repo for Electron 9 / node 12 #107

merged 1 commit into from
Aug 3, 2020

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented Jul 30, 2020 •
edited
Loading

Fixes #106

A few little tweaks are necessary to accommodate the new Electron and node versions. ATM the Electron example app fails at runtime when running on Gitpod, because of a missing a dynamic library:

/workspace/theia-trace-extension/node_modules/electron/dist/electron: error while loading shared libraries: libgbm.so.1: cannot

We need to update the Gitpod config so the dockerfile used for this repoi will contain a new Ubuntu package, required for Electron 9. This config file originally came from the main Theia repo, and the updated version is the same. See LICENSE.gitpod.md for more details.

Here's where the --no-sandbox idea came from - it permits the Electron version of the example app to start in Gitpod:
eclipse-theia/theia#7968 (comment)

Signed-off-by: Marc Dumais [email protected]

@marcdumais-work marcdumais-work mentioned this pull request Jul 30, 2020
Merged
@marcdumais-work
Copy link
Contributor Author

We'll need to update a bit more - stay tuned:

yarn run v1.22.4
$ yarn rebuild:electron ; yarn --cwd electron-app start ../TraceCompassTutorialTraces/ --hostname=0.0.0.0
$ theia rebuild:electron
/bin/sh: 1: theia: not found
error Command failed with exit code 127.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error [email protected]: The engine "node" is incompatible with this module. Expected version ">=10.2.0 < 12". Got "12.14.1"

@marcdumais-work marcdumais-work changed the title update repo's gitpod docker config for Electron 9 / node 12 update repo for Electron 9 / node 12 Jul 30, 2020
@marcdumais-work marcdumais-work force-pushed the update-gp-config branch 3 times, most recently from 194dd34 to 275d23e Compare July 30, 2020 22:54
@marcdumais-work
Copy link
Contributor Author

Looks like this did the trick: it's now again possible to test with the Electron example app in Gitpod. The pre-build seems to have worked too.

image

@marcdumais-work
Copy link
Contributor Author

marcdumais-work commented Jul 30, 2020 •
edited
Loading

@bhufmann FYI I based myself on your PR so I could test using the latest stable Theia. You'll see this PR contains 3 commits, 2 being yours from your PR. If you merge, GitHub will automatically adjust and show just my 1 commit I think. If you make adjustments before merging I'll compensate.

PR updated to sync with parent PR.

@marcdumais-work marcdumais-work force-pushed the update-gp-config branch 2 times, most recently from 849b921 to 06ea2e0 Compare July 31, 2020 14:33
tahini
tahini reviewed Jul 31, 2020
View reviewed changes
Copy link
Contributor

@tahini tahini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works well! Just wait for Bernd's PR to be merged and rebase on master

@marcdumais-work
Copy link
Contributor Author

works well! Just wait for Bernd's PR to be merged and rebase on master

Looks like this is ready to go in, but I will need an approval before I can merge.

tahini
tahini approved these changes Aug 3, 2020
View reviewed changes
Copy link
Contributor

@tahini tahini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There you go! Thanks!

@bhufmann
Copy link
Collaborator

bhufmann commented Aug 3, 2020

There you go! Thanks!

It works for me on my laptop but I get errors on gitpod and no visualization graphs open successfully due the errors. Does GitPod work for you?

@marcdumais-work
Copy link
Contributor Author

It works for me on my laptop but I get errors on gitpod and no visualization graphs open successfully due the errors. Does GitPod work for you?

I just rebased the PR - Gitpod is building the workspace - should be ready in a few minutes to test fresh.

@marcdumais-work
Copy link
Contributor Author

It works for me on my laptop but I get errors on gitpod and no visualization graphs open successfully due the errors. Does GitPod work for you?

I was able to reproduce this problem. It looks like Pixi uses WebGL and that's disabled by default . There is a switch that helps - I have added it to the Gitpod config so it's used to start our example application. New version of PR pushed. Fingers crossed.

@marcdumais-work
Copy link
Contributor Author

For whatever reason WebGL is disabled in Gitpod by default for our Electron app.

root ERROR Error: WebGL unsupported in this browser, use "pixi.js-legacy" for fallback canvas2d support.
    at Function.push.../node_modules/@pixi/core/lib/core.es.js.Renderer.create (file:///workspace/theia-trace-extension/electron-app/lib/44.bundle.js:21081:15)

The same does not seem to happen locally, so this may not be an Electron Default. More likely it has to do with some container-related weirdness. It might have been expected, in such a case, that Pixi automatically falls-back to the legacy mode that does not require WebGL, but it does not seem-to?

@marcdumais-work marcdumais-work force-pushed the update-gp-config branch 2 times, most recently from c06569b to 052d5e3 Compare August 3, 2020 16:10
@marcdumais-work
Copy link
Contributor Author

marcdumais-work commented Aug 3, 2020 •
edited
Loading

Ok, I think everything should be working again. I will update the commit message to clarify the security trade-off involved in making the Electron example app work in Docker (Gitpod). The tradeoff is limited to .gitpod.yml, where the security-disabling switches are present.

@marcdumais-work
update repo's configs for Electron 9 / node 12
c2ee835 
Fixes #106

A few little tweaks are necessary to accommodate the new Electron and node versions. ATM the Electron example app fails at runtime when running on Gitpod, because of a missing a dynamic library:

/workspace/theia-trace-extension/node_modules/electron/dist/electron: error while loading shared libraries: libgbm.so.1: cannot

We need to update the Gitpod config so the dockerfile used for this repoi will contain a new Ubuntu package, required for Electron 9. This config file originally came from the main Theia repo, and the updated version is the same. See LICENSE.gitpod.md for more details.

Security note: in order for the Electron version of the example application to work in Gitpod (probably Docker in general), two command-line arguments are required. These are not to be used in production since they decrease security. However they should only be needed momentarily for tests in Gitpod, until we can use the browser version in there and in any other future Cloud-based applications that will use the "trace" extension:

  --no-sandbox
  --ignore-gpu-blacklist

These two arguments are used only in Gitpod, from `.gitpod.yml`

Signed-off-by: Marc Dumais <[email protected]>
bhufmann
bhufmann approved these changes Aug 3, 2020
View reviewed changes
Copy link
Collaborator

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. It works in GitPod and on my local machine.

@marcdumais-work
Copy link
Contributor Author

Ok, merging!

@marcdumais-work marcdumais-work merged commit a2f67ea into master Aug 3, 2020
@marcdumais-work marcdumais-work deleted the update-gp-config branch August 3, 2020 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tahini tahini tahini approved these changes

@bhufmann bhufmann bhufmann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Gitpod] update configs for Electron 9 / node 12
3 participants
@marcdumais-work @bhufmann @tahini