-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
electron downlevel on Raspian Buster #1800
Comments
So, what version do we need to bump to? Feel free to send a PR. |
working on that. 6.0.12 seems good.. |
As discussed, the version change of Electron was reverted because of Travis issues. Electron definitely needs to be updated, so we need to find a way to make it work with Travis. @roramirez Any idea? |
For anyone running Buster, if you want to switch to a working Electron version, use the following command:
Please report if this solves your issue. Thanks! |
concerning the electron issue, you are talking about the failed tests here? I did tests with electron 6.0.12, 6.1.7 and 7.1.7. Tests with version 6 are failing, but with 7.1.7 everything is fine. |
Does 7.1.7 work on a Raspberry? Haven't tried it yet. If so, feel free to send a PR, and let's check if Travis works. |
no.. 7.1.7 will not install on PI4 buster
|
Sam is correct, pi doesn't run with 7. Is 5.0.13 an option? Tests passes and pi is working. |
6.0.12 looks good, but Travis env needs fixed |
I did the tests without Travis, they fail with 6.0.12 or 6.1.7. Same tests are o.k. with 7.1.7 and 5.0.13. So I'm no electron expert, no idea whats going wrong with 6. So if noone has an idea to get the tests running with 6 we have the choice between
|
the tests work locally if u export DISPLAY=:0 u also have to update spectron to match the electron version. Spectron is used in the testcases themselves SOME testcases fail for timeouts loading the electron browser. and the timeout needs to be increased |
What version of Electron we'll expect to set for the project? I'll try to get a free window time this week to working on in this and find a something can work. |
i think we a have to go to 6, thats the 1st version that supports the current node 10, and electron 7 doesn't work. I tested with 6.0.12, which seemed stable and reliable for all the modules I tested with. there were breaking changes to go to 6, but the MM core has already adjusted. there is a bug somewhere in the vendor tests, which launch MM using config of port 8080, but actually use 9515. i posted info in some issue. change the urls in testcase to use port 9515 and they work |
can confirm that (with updated spectron) the tests passes with electron v6, may we need adjust the 10 sec. in the weather tests, they fail sometimes because of timeout. I tested with v6.1.7, this is the latest v6, see Version History here I'm not shure if 6.0.12 is still getting updates:
|
we were on 3.0.13, LONG LONG since any updates.. i suspect we will update maybe once a year.. |
Do you have the # for this issue? |
Did some investigation in the weather tests (with electron v6.x). This seems to be a bug in v6.x, they fail whatever is coded as timeout. So I decided to investigate in the electron v7 problem, where Tested this fix on a raspberry, and with v7 the weather tests are o.k. So if someone can confirm these results, this could be the solution for the electron problem. Here some logs: |
Did some updates of the dependencies in the So a new
where
So this could be the content of a pull request solving the electron problem and updating the dependencies. |
what will we have to do each quarter to retest that latest is ok? |
same as now. You have already 1/3 "latest" in the current Its a discussion how getting updates without breaking the app. May one approach is to work with a "latest-package.json" on the |
I'm not a fan of using |
Also note that the install.sh script will be removed in the near future. |
so how to continue with electron? Update to 7.x with spectron 9.x? This works only with install.sh or a similar construction. |
is |
this is necessary on the raspberry only. |
Bummer, otherwise it could be part of the travis script. Hopefully there will be a more elegant solution available before 2019/4/1. |
One solution is to put this extra line into
This installs electron before the "main" install, only for electron the arch-param is needed. |
doesn't this run the risk of having some modules at arvm7l (required by electron) and others not? I tried the --arch=armv7l to get around the pi 0 (armv6l) lack of running versions, but that really didn't work out well. |
@MichMich >before 2019/4/1. u meant 2020/4/1! |
no, building electron v7 with the arch param is a workaround for this problem.
the future of pi 0 is already answered by @MichMich here. Another "solution" instead of using preinstall in package.json could be only updating the install docs with
|
I already have that in the automated script. Just have to invert the test for armv6l |
This change has references MagicMirrorOrg#1800
New version of Electron has enable by default sandbox http://www.atom.pe/docs/api/sandbox-option/ There was some issues to migrate a new version of Electron for MagicMirror. Using the new version in Travis CI was failing at this time. The problem is because the testing runner is a Docker enviroment The issue experimented is the same topic mentioned here: - electron/electron#17972 - electron-userland/spectron#443 The fix for to all of this is to set the `--no-sandbox` mode in CI testing https://electronjs.org/docs/all#--no-sandbox This change use the feature to set and disable Sandbox using by enviroment variable `ELECTRON_DISABLE_SANDBOX=1` electron/electron#16576 This change has reference MagicMirrorOrg#1800
I pushed the Pull Request #1892 using a new version of Electron and the testing for e2e are passing except the weather module. I will take a look after this PR if is merged because is not a related issue here. I think to move the 7 version of Electron is too rush. Also I'm not a big fan of the newest versions |
as said before, weather tests are not running with electron v6, but e.g. with v7. |
I think we'll should keep in the 6 version and fix the weather test, after that may be will move to 7 version of Electron. How I see it with the 6 version there are doesn't troubles with Raspberry model, but really really not sure |
@roramirez the vendor tests also did not correctly execute as per #1842
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
waiting on release 2.11 for proposed fix |
2.11 ships 6.1.7 |
Platform: Place your platform here... give us your web browser/Electron version and your hardware (Raspberry Pi 2/3, Windows, Mac, Linux, System V UNIX).
Raspian Buster
Node Version: Make sure it's version 0.12.13 or later.
MagicMirror Version: Now that the versions have split, tell us if you are using the PHP version (v1) or the newer JavaScript version (v2).
2.9.0
Description: Provide a detailed description about the issue and include specific details to help us understand the problem. Adding screenshots will help describing the problem.
electron cannot start
Steps to Reproduce: List the step by step process to reproduce the issue.
Expected Results: Describe what you expected to see.
Actual Results: Describe what you actually saw.
Configuration: What does the used config.js file look like? Don't forget to remove any sensitive information!
Additional Notes: Provide any other relevant notes not previously mentioned. This is optional.
Electron project says upgrade electron is only fix.
The text was updated successfully, but these errors were encountered: