-
Notifications
You must be signed in to change notification settings - Fork 913
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
Upgrade electron to version 22 #2444
Conversation
@bengotow I am trying to remove keytar, as I don't get Mailspring to build on Windows otherwise. Keytar is deprecated. See:
I tried to simply encrypt the credentials using the safeStorage method to encrypt the credentials nicely and then store them in the config. See: However, this does not work whatsoever. When creating a new mail account, I simply get the error message If you can give me pointers of what may work, I would be grateful. Trying to build with keytar included, I get the following error on Windows: Error Log
|
Hey, regarding the mentioned CVE above this gets more time-sensitive to us (NixOS maintainers) what are your plans on moving forward here or updating to a non affected version? :) |
7fad10b
to
c2ff2a2
Compare
@Phylu - May suggest that you update the title to something like: Upgrade electron to version 22.x.x or maybe just upgrade electron to version 22. So it's more clear that you try to upgrade to a recently patched version of Electron and not to 22.0.0. |
I think, I have found a solution on the keytar issue. The problem was, that I forgot about the Mailspring ID which could also be stored using keytar. However, to write a proper migration we need at least one version that contains the new logic using the safeStorage method as well as keytar to move all the credentials to the new storage. Therefore, I am now working on the migration step. We will need to have an intermediary Mailspring version then to prepare for this upgrade. |
@bengotow This should now be ready to go after we have the intermediary 1.12 version shipped. I successfully tested these changes on Linux (.deb & snap), Mac and Windows and could not find any further issues. |
@Phylu Thank you for this PR! |
@Tamriel You can find a fresh temporary build here: https://nextcloud.phynformatik.de/index.php/s/GbATAMr4dofL327 |
I tried it on Manjaro. My mail accounts cannot connect. I get |
Try to follow these steps: https://community.getmailspring.com/t/password-management-error-after-the-latest-upgrade/7298 Logging out of your Mailspring ID temporarily should fix the issue. |
I deleted the Mailspring folder
|
@Tamriel Can you try to start mailspring with the following parameter from the command line?
Which desktop environment is Manjaro using? Maybe we need to hardcode this parameter in the Mailspring startup if there are desktop environments that are not able to be properly recognized. I happened to run in this issue on Lubunt for testing now as well. Unfortunately, this might break (newly created) compatability on KDE environments... See also: https://www.electronjs.org/docs/latest/api/safe-storage#safestoragegetselectedstoragebackend-linux |
There are different flavors of Manjaro. I use the sway desktop environment. It worked with |
I just did some more testing. If you run Mailspring on a KDE5 based desktop, the safeStorage will use the correct I am not sure if there is a proper way to figure out if we are on a desktop environment that is unknown by Electron in order to automatically add the Otherwise, I think it needs to be put on the help page that for these (most likely less common) desktop environments the flag should be added manually to the .desktop file. However this is something that may not be kept on an upgrade. |
I installed Mailspring with an Arch User Repository package. In the package definition it is surely possible to add the flag to .desktop file file. I will add a comment the current Mailspring package, so the owner knows what to do when he updates to 1.12. |
I have added some more details to the corresponding help page: https://community.getmailspring.com/t/password-management-error/199/2 |
Hey @Phylu! All the changes here look great - I shipped the 1.12.0 release which is the stepping stone to this one, and I think we should go ahead and merge this! I'll start testing off master and using the Electron 22 one as my daily email client, and we can make smaller PRs to fix anything we run in to. Thanks for all the help with this! |
Hey folks, I merged this and fixed a few issues with the Travis / AppVeyor CI builds -- I've been testing the new Electron 22 version on Mac for the last day or so, and it's working great! Here are some new builds. It'd be great if you could give it a spin and let me know if there are any issues - Ben Windows: Mac:
Linux: |
I have the following issue: https://community.getmailspring.com/t/random-crash-with-1-12-on-manjaro/7336 |
When running the .deb version of Mailspring on one of my test systems, Zorin OS based on Ubuntu 22.04, I get the following error. It seems that the latest version of better-sqlite3 is linked against glibc 2.33 wheras the older Ubuntu version(s) are shipped with glibc 2.31.
|
On my Ubuntu 23.10 computer, I encounter the following issue after installing Mailspring 1.13 from the .deb file. Bildschirmaufzeichnung.vom.2023-11-18.16-03-30.webmThis seems to be spellcheck related, happening when the spellchecker wants to highlight a word or part of a sentence, deleting it instead. If I write in German as the spellchecker is configured, the text won't be touched. |
@bengotow: I could boil this one down to an issue in adding the decorators for spelling errors. If I uncomment the |
Hey hey! Taking a look at the spellcheck issue now -- I was able to fix the GLIBC compatibility problem (at least on my Ubuntu 18 machine) by using an older linux to build the app in CI. Want to try this file and see if it works for you as well? https://mailspring-builds.s3.amazonaws.com/client/fbb68340/linux/mailspring-1.13.0-amd64.deb |
Looking forward to seeing this fixed!
@bengotow Nice. I can confirm that this version runs on both my Ubuntu 23.10 as well as my 22.04 test system (which did not run the build that was linked against the newer glibc). |
I have updated electron to its latest version and bumped several connected dependencies as well.
Currently tested on Linux & Mac. I will try to test this on Windows within the next days and hope that everything there works as well.