PyQt6 compatibility #1495
Open
PyQt6 compatibility #1495
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Qt5 is not installed by default in dom0 under Qubes 4.3 (fedora 41), whereas PyQt6 is.
Use Qt6 to regenerate the compiled UI. This should NOT break
compatibility with PyQt5. To achieve this I installed pyqt6-dev-tools
and ran it with:
pyuic6 sdw_updater.ui > UpdaterAppUiQt6.py
NOTES:
- version mismatch between version used for compilation (6.4.2) and
the one available in dom0 (6.8.1) should not make a difference since
syntax changes should be constant in the same Qt/PyQt version
- Ruff lint was applied on top of the result.
Fixes #1494. Other than changing the imports to support both versions, this commit updates the code that was not forward-compatible, namely: - replace "exec_()" with exec() (Qt5 targetted also python2, which meant that it had "exec" reserved. This is no longer the case, so we can use the more aptly named "exec()" - Expand enums like QMessageBox.Ok to have its full name (QMessageBox.StandardButtons.Ok) None of these changes should break compatibility with PyQt5. A consolidated overview of the upgrade notes can be found at https://www.pythonguis.com/faq/pyqt5-vs-pyqt6/
In oreder to support both PyQt5 and 6 we need an try...except logic.
Mypy doesn't seem to like this and complains about a re-definition:
[...] already defined (by an import) [no-redef]
Instructions no longer made sense since the project moved from virtualenv to poetry. It made specific references to PyQt versions that are current set explicitly in pyproject.toml (which poetry uses).
Launcher tests were failing on 'poetry install' due to:
The currently activated Python version 3.11.6 is not supported
by the project (>=3.13).
Trying to find and use a compatible version.
Poetry was unable to find a compatible version. If you have one,
you can explicitly use it via the "env use" command.
(test on: openqa)
The test system package requirements were being explcitly stated in the Makefile. This would require creating a conditional for installing Qt5 in Qubes 4.2 and Qt6 in 4.3. Rather than adding it, this PR istead searches for the system python packages required by the SPEC file and installs those. The important bit is that they bring along other system dependencies (e.g. Qt6) that would otherwise need to be explicitly installed. In case tests use poetry instead, it should also work, given that poetry's packages should override system ones. But critically, the non-python system packages that they would depend on, are now automatically managed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1494. Updates code to use be compatible with both PyQt5 and 6.
This is necessary for #1373 if we don't want to add pyqt6 as a dom0 dependency.
Test plan
FEDORA_VERSION=37(Qubes 4.2) and runningrpm -qR path/to/rpmshows PyQt5 as depdencyFEDORA_VERSION=41(Qubes 4.3) and runningrpm -qR path/to/rpmshows PyQt6 as depdencyChecklist
This change accounts for:
MANIFEST.inandrpm-build/SPECS/securedrop-workstation-dom0-config.spec)