PyQt6 Upgrade #1685

i1sm3ky · 2023-04-08T18:52:36Z

Description

This PR aims to upgrade Vorta from using PyQt5 to PyQt6.

Related Issue

Issue: #765 PyQt6 Upgrade by @m3nu.

Motivation and Context

This change upgrades Vorta's GUI library (PyQt5) to its latest version at the time (PyQt6) which natively supports ARM architecture. Due to an update on PyQt5, it is no longer supported on ARM architecture which makes us unable to install and use Vorta on ARM based operating systems. By upgrading Vorta from PyQt5 to PyQt6, it'll add support for ARM architecture allowing us to use Vorta on ARM based systems too.

How Has This Been Tested?

This has been tested using the pre-written unit tests.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have read the CONTRIBUTING guide.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

m3nu · 2023-04-08T18:59:22Z

Oh, I think you accidentally pushed all your icon work too. The PyQt6 commit should be based on the current master branch, not your other work. Now it's all mixed up. Best to force-push this again.

m3nu · 2023-04-08T19:04:48Z

You can just cherry-pick your one PyQt commit, as tested here: #1686

Commands:

git checkout master (also be sure you pull the latest changes
git cherry-pick c97f3e970e606d379a7828f2bed8e606bb9dac01 (hash of the one commit you want)
(then fix a minor conflict)
done

i1sm3ky · 2023-04-08T19:10:25Z

Oh, I think you accidentally pushed all your icon work too. The PyQt6 commit should be based on the current master branch, not your other work. Now it's all mixed up. Best to force-push this again.

I initially made the changes on my fork which contained the changes for the icons too and the icons also took some changes to work with PyQt6 so I thought those could also be added here. I'll make those changes on the icons PR and keep this to PyQt6 upgrade only.

m3nu · 2023-04-08T19:13:38Z

Great, thanks! You should also change pyqt5 in setup.cfg. After that it works without issue.

I'm surprised it doesn't need more changes than this. Was expecting to update the .UI files too.

If this is really the case, we should quickly start a new 0.9 branch with this.

i1sm3ky · 2023-04-08T19:29:25Z

Great, thanks! You should also change pyqt5 in setup.cfg. After that it works without issue.

Ya, I forgot to change that.

I'm surprised it doesn't need more changes than this. Was expecting to update the .UI files too.

The only major changes it took were the full enums and the imports, for which I wrote a script that took care of those. Other than that it required some minor changes here and there including change in the import of QAction and QShortcut, removal of High DPI scaling, some flag changes, data type changes in SetChecked, change in some deprecated methods and some more.

If this is really the case, we should quickly start a new 0.9 branch with this.

Yes, after fixing those failing tests which I mentioned I think we might be good with the upgrade.

m3nu · 2023-04-08T20:17:08Z

Yeah, I see the changes. It's a bit more than search & replace, but not a crazy change. And no change to the UI XML files.

For your commits, you added one new commit that undoes previous commits. Would be better to just have the actual changes. You see all the commits of this PR here: https://github.com/borgbase/vorta/pull/1685/commits

If you like I can also force-push just your one commit to this repo and we take it from there. But I'm sure you can also figure it out.

m3nu · 2023-04-08T20:24:17Z

A system package may need adding in .github/workflows/test.yml

https://packages.ubuntu.com/bionic/libegl1

i1sm3ky · 2023-04-08T20:29:07Z

If you like I can also force-push just your one commit to this repo and we take it from there. But I'm sure you can also figure it out.

You can force-push my commit to this repo if you want. My bad I've committed my icons work on my master branch so when I do checkout master it still keeps those changes.

m3nu · 2023-04-08T20:37:55Z

Done. As you can see we only have 2 commits now and the history is much clearer than first adding and then removing stuff.

But don't worry, this Git stuff can be confusing in the beginning.

i1sm3ky · 2023-04-08T20:43:08Z

Done. As you can see we only have 2 commits now and the history is much clearer than first adding and then removing stuff.

But don't worry, this Git stuff can be confusing in the beginning.

Yeah, Thanks for the help! But it still contains some of my code from the icon issue, is that fine?

m3nu · 2023-04-08T20:43:50Z

With this Ubuntu package added, test properly run on all platforms, but have a few minor errors. Can look into those tomorrow.

i1sm3ky · 2023-04-08T20:46:21Z

With this Ubuntu package added, test properly run on all platforms, but have a few minor errors. Can look into those tomorrow.

Yeah Sure!

m3nu · 2023-04-08T20:46:31Z

Packaging with PyInstaller for macOS also works well. The resulting app is actually 30% or so smaller than Qt5. 🤷‍♂️

i1sm3ky · 2023-04-08T20:48:58Z

Packaging with PyInstaller for macOS also works well. The resulting app is actually 30% or so smaller than Qt5. 🤷‍♂️

Oh great! I think the upgrade will be complete sooner then expected.

src/vorta/views/extract_dialog.py

tests/test_extract.py

m3nu · 2023-04-09T13:05:36Z

Fixed the tests. Nothing major to change. There were some subtle changes to dBus notifications. Would be good if someone can test those on Linux. 🙏

i1sm3ky · 2023-04-09T13:13:10Z

Should I mark this PR as ready for review?

m3nu · 2023-04-09T13:16:33Z

Should I mark this PR as ready for review?

Yes, please!

i1sm3ky · 2023-04-09T13:23:02Z

What should I do/work on next?

The signature of `QVariant.convert` has changed breaking this keyring provider. * src/vorta/keyring/kwallet.py (VortaKWallet5Keyring.try_unlock)

real-yfprojects

This has to be updated too, I think:

vorta/setup.cfg

Line 103 in 6edd409

extension-pkg-whitelist=PyQt5

I couldn't test notifications because they no longer work on my testing machine for some reason.

src/vorta/views/extract_dialog.py

src/vorta/tray_menu.py

tests/conftest.py

Co-authored-by: yfprojects <62463991+real-yfprojects@users.noreply.github.com>

m3nu · 2023-04-16T10:04:48Z

This has to be updated too, I think:

vorta/setup.cfg

Line 103 in 6edd409

extension-pkg-whitelist=PyQt5

Changed. Do we still use pylint anywhere?

m3nu · 2023-04-16T10:13:27Z

I couldn't test notifications because they no longer work on my testing machine for some reason.

Notifications should work. I fixed and tested them on Ubuntu 22.04.

m3nu · 2023-04-16T10:21:49Z

I've pinned pyobjc, since today's update causes a segfault in our autostart code. This will be a separate PR.

* src/vorta/borg/extract.py (BorgExtractJob.prepare): Compare `item.data.checkstate` to type `Qt.Checkstate` instead of the value of the enum

real-yfprojects · 2023-04-16T16:18:29Z

Changed. Do we still use pylint anywhere?

I use it locally. But it isn't used in the repository.

Fixes type hints and type handling. * src/vorta/views/extract_dialog.py

m3nu · 2023-04-17T10:18:15Z

Merged this and will soon do a v0.9.0 beta release so macOS users can easily test the compiled version.

Thanks to @i1sm3ky for getting the ball rolling on this and @real-yfprojects for reviewing and adding valuable fixes and input!

i1sm3ky · 2023-04-17T10:22:46Z

@m3nu @real-yfprojects What else should I get on doing next?

m3nu · 2023-04-17T10:28:03Z

You can look for stale PRs that may only need a small push or change to be useable. E.g. #1639 should be nearly ready, but was abandoned by the person or something.

Careful, since some PRs may also include features that are either too specific or add too much maintenance effort in the long term. So we don't want to merge those.

i1sm3ky · 2023-04-17T10:31:30Z

You can look for stale PRs that may only need a small push or change to be useable. E.g. #1639 should be nearly ready, but was abandoned by the person or something.

Ok thanks! I had seen this issue before, I'll work on it and get it fixed.

Careful, since some PRs may also include features that are either too specific or add too much maintenance effort in the long term. So we don't want to merge those.

Ok gotcha.

m3nu · 2023-04-17T10:49:32Z

These docs may need updating too. I'm on an Intel mac and can't test Arm stuff.

https://vorta.borgbase.com/contributing/#local-development-setup

Would be good to build an Arm binary or universal binary somehow. GH Actions is also Intel.

i1sm3ky · 2023-04-17T11:31:38Z

These docs may need updating too. I'm on an Intel mac and can't test Arm stuff.

https://vorta.borgbase.com/contributing/#local-development-setup

Would be good to build an Arm binary or universal binary somehow. GH Actions is also Intel.

I'll look more into it and get back to you.

i1sm3ky · 2023-04-18T01:05:27Z

Would be good to build an Arm binary or universal binary somehow.

pyinstaller --clean --noconfirm --target-arch universal2 package/vorta.spec It should build a universal binary which could run on both -x86_64 as well as arm64 architecture.

real-yfprojects · 2023-04-18T06:49:59Z

I think you need universal2 python binaries for that to work.

m3nu · 2023-04-18T07:07:47Z

Not impossible to build universal2 Python bins:

pyenv/pyenv#2262 (comment)

Maybe an afternoon to get this working on Github Actions with macOS.

i1sm3ky · 2023-04-18T07:56:54Z

Not impossible to build universal2 Python bins:

pyenv/pyenv#2262 (comment)

Maybe an afternoon to get this working on Github Actions with macOS.

The Apple dev docs suggest a similar approach using lipo to merge the individual binaries.

i1sm3ky · 2023-04-18T08:00:23Z

I think you need universal2 python binaries for that to work.

Thanks, I think you're right, I might have to look further into it.

This puts Vorta on PyQt6 and starts a new main branch 0.9. --------- Co-authored-by: real-yfprojects <real-yfprojects@users.noreply.github.com> Co-authored-by: Manu <3916435+m3nu@users.noreply.github.com> Co-authored-by: yfprojects <62463991+real-yfprojects@users.noreply.github.com>

PyQt6 Upgrade(Pre-1)

d36dea4

Dependency in setup.cfg

510ffec

m3nu force-pushed the PyQt6-Upgrade branch from 87391cd to 510ffec Compare April 8, 2023 20:36

Add missing OS dependency

50d2db3

real-yfprojects reviewed Apr 9, 2023

View reviewed changes

src/vorta/views/extract_dialog.py Outdated Show resolved Hide resolved

tests/test_extract.py Outdated Show resolved Hide resolved

Changed a deprecated method to obtain child

0f9f8e3

m3nu force-pushed the PyQt6-Upgrade branch 3 times, most recently from 982378b to 3f6a46d Compare April 9, 2023 12:37

Fix tests and linting

0b7ba74

m3nu force-pushed the PyQt6-Upgrade branch from 3f6a46d to 0b7ba74 Compare April 9, 2023 13:01

i1sm3ky marked this pull request as ready for review April 9, 2023 13:16

real-yfprojects linked an issue Apr 16, 2023 that may be closed by this pull request

PyQt6 Upgrade #765

Closed

Fix type convert of a QVariant in kwallet code.

aa0a948

The signature of `QVariant.convert` has changed breaking this keyring provider. * src/vorta/keyring/kwallet.py (VortaKWallet5Keyring.try_unlock)

real-yfprojects reviewed Apr 16, 2023

View reviewed changes

src/vorta/views/extract_dialog.py Show resolved Hide resolved

src/vorta/views/extract_dialog.py Outdated Show resolved Hide resolved

src/vorta/tray_menu.py Outdated Show resolved Hide resolved

tests/conftest.py Outdated Show resolved Hide resolved

Better variable name

1df611c

Co-authored-by: yfprojects <62463991+real-yfprojects@users.noreply.github.com>

Implement feedback

c1197bc

m3nu force-pushed the PyQt6-Upgrade branch from f016b83 to c1197bc Compare April 16, 2023 10:06

Pin newly released pyobjc

69b5bfd

Fix qt enum comparison in vorta.borg.extract.

b851bae

* src/vorta/borg/extract.py (BorgExtractJob.prepare): Compare `item.data.checkstate` to type `Qt.Checkstate` instead of the value of the enum

Expand ExtractTree type hints

76494bb

m3nu force-pushed the PyQt6-Upgrade branch from 3d1460a to 76494bb Compare April 16, 2023 18:42

Fix ExtractTree.setData typing.

f61ab00

Fixes type hints and type handling. * src/vorta/views/extract_dialog.py

real-yfprojects approved these changes Apr 17, 2023

View reviewed changes

m3nu merged commit 7535f92 into borgbase:master Apr 17, 2023
11 checks passed

real-yfprojects mentioned this pull request Apr 29, 2023

feat: settings to allow new networks and show notif when a network is disallowed #1658

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PyQt6 Upgrade #1685

PyQt6 Upgrade #1685

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 9, 2023

i1sm3ky commented Apr 9, 2023

m3nu commented Apr 9, 2023

i1sm3ky commented Apr 9, 2023

real-yfprojects left a comment

m3nu commented Apr 16, 2023 •

edited

m3nu commented Apr 16, 2023

m3nu commented Apr 16, 2023

real-yfprojects commented Apr 16, 2023

m3nu commented Apr 17, 2023

i1sm3ky commented Apr 17, 2023

m3nu commented Apr 17, 2023

i1sm3ky commented Apr 17, 2023

m3nu commented Apr 17, 2023

i1sm3ky commented Apr 17, 2023

i1sm3ky commented Apr 18, 2023

real-yfprojects commented Apr 18, 2023

m3nu commented Apr 18, 2023

i1sm3ky commented Apr 18, 2023

i1sm3ky commented Apr 18, 2023

PyQt6 Upgrade #1685

PyQt6 Upgrade #1685

Conversation

i1sm3ky commented Apr 8, 2023

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

m3nu commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 8, 2023

i1sm3ky commented Apr 8, 2023

m3nu commented Apr 9, 2023

i1sm3ky commented Apr 9, 2023

m3nu commented Apr 9, 2023

i1sm3ky commented Apr 9, 2023

real-yfprojects left a comment

Choose a reason for hiding this comment

m3nu commented Apr 16, 2023 • edited

m3nu commented Apr 16, 2023

m3nu commented Apr 16, 2023

real-yfprojects commented Apr 16, 2023

m3nu commented Apr 17, 2023

i1sm3ky commented Apr 17, 2023

m3nu commented Apr 17, 2023

i1sm3ky commented Apr 17, 2023

m3nu commented Apr 17, 2023

i1sm3ky commented Apr 17, 2023

i1sm3ky commented Apr 18, 2023

real-yfprojects commented Apr 18, 2023

m3nu commented Apr 18, 2023

i1sm3ky commented Apr 18, 2023

i1sm3ky commented Apr 18, 2023

m3nu commented Apr 16, 2023 •

edited