-
Notifications
You must be signed in to change notification settings - Fork 278
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
[WIP] Qt6 Conversion #1601
base: main
Are you sure you want to change the base?
[WIP] Qt6 Conversion #1601
Conversation
Moved to Qt6 scoped enums Enabled PyQt6 pyuic
Thank you so much for working on this! This migration is incredibly important for the maintainability of plover. I'm happy to test this the next days on Windows, Linux, and macOS. Let me know if there's anything else I can do to help. |
@mkrnr Any testing you can do would be very helpful. Post any error messages or not-working features here. If there are any plugins you use that you can test that would be useful, although I expect only non-graphical plugins have a chance of working without updates. The packaging systems need fixing too (see all the red in the CI right now...) . I'll take a look into that over the next few days/weeks but if anyone knows how all the packaging scripts work then help would be appreciated. |
Launches successfully on Apple Silicon Macbook Air (M2 chip). Qt6 takes system theme (Dark) too, neat! |
Here are some tests on Windows 11 64 bit using a Georgi: tox r -e launchall built-in functionality works flawlessly:
Warnings during start, maybe the one regarding
tox r -e testonly failures are in:
This fails on the master as well and is likely fixed by #1599. tox r -e plugins_installExecuted successfully. Is there a way to test this at this point? Because when I then run tox r -e plugins_install -- C:\Users\mkoer\git\plover_regenpfeiferExecuted successfully. Don't know how to test this, see above. tox r -e packaging_checks
tox r -e release_prepare
|
Thanks for running the tests, that's great to know this works for other people.
Don't think these warnings happened for Qt5 - I'll look at what's causing these messages
You might be able to launch plover directly using the environment created by tox. Enter the environment (something like I'm going to look at packaging this, then hopefully we'll be able to share this and get some beta testers for a Qt6 Plover release. |
I'll give that a try next days, thanks! We'll also have to look into updating plover_plugins_manager to Qt6 since that's such an integral part of plover. Did you look into that already by any chance? |
bed0d2b
to
d7065ba
Compare
Switch builds to PyQt6 Include resources folder in distribution Remove extra MANIFEST.in entries to silence warnings Pin PyQt6-Qt6 - googlefonts/fontra-pak#27 Add pyqt6rc to test/build requirements
d7065ba
to
c428339
Compare
I've had a quick look at the plugin manager - there are a few small changes needed there for Qt6 but shouldn't be anything too major. We'll need to update both this project and the plugin manager together though as they're mutually dependent. |
edec04b
to
327a818
Compare
Awesome to see your progress on this, now with the plugin manager migration! I'd like to help you testing this but when I launch with tox ( |
Most of my recent testing has been through the CI system in GitHub, just fixing errors and getting the builds working. The CI versions have the plugin manager bundled (which was causing build failures using a non-patched Qt5 plugin manager version) - you can download these as build artefacts. Seems to work for the Linux AppImage, but the Windows exe doesn't yet run. I've yet to sort out how to use the plugin manager successfully when launching locally from tox. Getting local builds to work is not easy - you have to have almost exactly the same setup (including library versions) that the build scripts need else you'll get a bunch of unhelpful error messages from pip. I've also run into issues where some of the download sources for build scripts are rate limited so stop working after a few attempts. Honestly the build system needs work to be simplified, it's a little unmanageable right now. |
That makes a lot of sense to focus on getting the Github CI build system to work. The macOS DMG installs and starts perfectly fine (macOS 13.4.1, M1 chip) but the Plugins Manager is not showing up. The CI build looked fine to me but I'll continue looking into that one. On Win11 64bit I get Let me know if I can do more tests. I'll also try to work on solutions. Thanks again for working on this, I would love to buy you a coffee! |
I've sorted out the bundling of assets now, and I think mostly everything works (well, it's likely there's some bugs hiding somewhere). All the CI versions seem to run ok for me as far as I can test. The plugin manager works using openstenoproject/plover_plugins_manager#22 , but no plugins will work as they're all PyQt5 - I'm not aware of any non-graphical plugins. I'm sure some work is still needed (reviews are welcome) but I'm going to mark this as ready. |
This is really exciting! Here are the results of my first tests: Win11 64bit opened via ZIP
Win11 64bit via installer
MacOS m1 (Ventura 13.4.1) via DMG
Ubuntu 64bit via AppImage
It would be awesome to have more people testing this. |
Thanks for the help testing, especially across multiple systems.
This is a PyQt api change, I think it's meant to be The Windows installer error looks like a missing file in the bundle. Maybe it's not extracting correctly? This is literally just an icon failing to load however, so maybe we can catch + log instead of aborting. Not sure why the plugin manager isn't working on MacOS though. Do you have multiple plover installs at all? Plover uses the default (global) python system import system so can get confused quite easily. Will take a look at the DMG contents as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UserRole fix works, no more errors around that 👍
Also I found out the reason for the plugins manager not showing up on MacOS (M1): There's a plugins folder where the plover configs are stored. This plugins folder also contains the Plugins Manager Plugin #inception
Deleting this folder fixed the issue and now the manager shows up.
Test summary:
- MacOS M1 ✅
- Windows 11 64 bit ✅
- Ubuntu 64 bit ✅
Amazing work Greg!
.github/workflows/ci.yml
Outdated
@@ -36,15 +36,15 @@ jobs: | |||
id: set_cache | |||
run: | | |||
job_cache_extra_deps=(); job_id=test_linux; job_name='Test (Linux)'; job_needs=(); job_os=Linux; job_platform=ubuntu-22.04; job_python=3.9; job_reqs=(reqs/dist.txt reqs/test.txt); job_skip_cache_name=skip_test_linux_py-3.9_ubuntu-22.04; job_skip_cache_path=.skip_cache_test_linux; job_skiplists=(job_test os_linux); job_test_args='-p no:pytest-qt --ignore=test/gui_qt'; job_type=test; job_variant=Linux; analyze_set_job_skip_cache_key | |||
job_cache_extra_deps=(osx/deps.sh); job_id=test_macos; job_name='Test (macOS)'; job_needs=(); job_os=macOS; job_platform=macos-10.15; job_python=3.9; job_reqs=(reqs/dist.txt reqs/test.txt); job_skip_cache_name=skip_test_macos_py-3.9_macos-10.15; job_skip_cache_path=.skip_cache_test_macos; job_skiplists=(job_test os_macos); job_test_args='-p no:pytest-qt --ignore=test/gui_qt'; job_type=test; job_variant=macOS; analyze_set_job_skip_cache_key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll get merge conflicts when rebasing/merging master into this branch because I bumped that version. I didn't go for latest though to prevent "random" issues in the future with new macOS versions. In my view freezing this version makes the build more robust. (Same for the other places in this file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pinning the version is a good idea. I bumped it up to latest because there was some issue with old macOS versions in the CI, and was too lazy to check which versions actually worked. I'll fix the version to match what you had in your patch.
One more thing I noticed now: when installing via DMG on mac (M1), the dark mode doesn't work (😱).
In terms of general functionality it works though. |
@mkrnr It's just a warning though, so I don't think it's causing the issue of plover not picking up default theme. Does it work as expected on Linux/Windows machines? |
@Robberduckzilla makes sense. Another thought: maybe it's because the DMG is compiled for intel Macs and Rosetta doesn't care about theming? Anyhow, I wouldn't consider this a show stopper at all. Definitely something that we can take care of in a follow-up PR. |
…nto pyqt6-migration
I don't have write access to your repo, so a quick note on a small change you should make: - __version__ = '4.0.0.dev12'
+ __version__ = '5.0.0' This will show version 5 in the about window, as well as (I think) install it into a plover 5.0.0 folder, rather than the misleading 4.0.0dev12 |
Summary of changes
Updates Plover to use Qt6 instead of Qt5
This is work-in-progress, see below for details
Pull Request Checklist
Progress
Release plan