Adds PySide 6 Support For Windows and MacOS

[deeplow]

I tested:

• MacOS: dev env + building and running a .app
• Windows: dev env + building and running .exe
• Linux: dev env (still on PySide2) -- see reasoning here

Fixes #177

Note: When building on Windows, CXFreeze apparently copies the entire poetry virtualenv into the .exe and when you run poetry install it only adds the new software -- it doesn't remove old one. So I moved from PySide2 to PySide6 and PySide2 was still installed but no longer the dependencies. However, I saw in the build log that it was getting included.

To ensure we don't ship PySide2 accidentally it's recommended to uninstall the virtualenv with poetry env remove [virtualenv]

[apyrgio]

Some minor comments:

1. In your first commit message, you write:

   in Linux distro's packages, whereas with Linux and macOS

   it should be "whereas on Windows and macOS".

2. I think we don't need this line anymore:

   dangerzone/install/windows/build-wxs.py

   Lines 25 to 27 in 84b8212

   	# Skip lib/Pyside2/Examples folder
   	if "\\build\\exe.win-amd64-3.10\\lib\\PySide2\\examples" in dirname:
   	continue

3. Have you tried out running mypy against our codebase on Windows / macOS, with this PR? I'm asking because we no longer use the Pyside2-stubs, and I see reports of mypy failing on PySide6 (example).

Other than that, the PR looks good.

[deeplow]

Have you tried out running mypy against our codebase on Windows / macOS, with this PR? I'm asking because we no longer use the Pyside2-stubs, and I see reports of mypy failing on PySide6 (python/mypy#10209).

Even on linux it shows multiple errors. How come CI didn't run on this branch?

I came across mypy and pyside2 issues before. And apparently this is still an upstream issue. I wonder if we'll need a PySide6-stubs or similar to fix it...

it should be "whereas on Windows and macOS".

Thanks! I'll fix this when I rebase.

I think we don't need this line anymore:

Done. I checked on windows and there is no examples lib either. So we don't have to exclude the pyside6 equivalent.

[deeplow]

Have you tried out running mypy against our codebase on Windows / macOS, with this PR? I'm asking because we no longer use the Pyside2-stubs, and I see reports of mypy failing on PySide6 (python/mypy#10209).

With the new fix it runs on Linux and macOS.

[deeplow]

Tested now on windows and it only has an unrelated error which we should fix in another PR.

[deeplow]

(rebased on main)

[apyrgio]

I took a look at your suggestion (7c6eab4) for handling mypy compatibility with PySide2/PySide6, but I'm afraid it has some issues. The underlying reason is a bit convoluted, but I'll try my best to explain:

Problematic code

The problematic part is this import chain and removing --warn-unused-ignores from the mypy flags:

try:
    from PySide2 import QtCore, QtGui, QtWidgets
except ImportError:
    from PySide6 import QtCore, QtGui, QtWidgets  # type: ignore [no-redef]

Here's what actually happens under the hood, as far as Mypy is concerned:

Under the hood

1. First, it will try to import import the Qt modules from PySide2. Whether it succeeds or not is inconsequential, because...

2. ... it will then try to import the same Qt modules from PySide6. Why? Because it doesn't understand the try ... except block (see mypy bug with try/except conditional imports python/mypy#1153).

   • Funnily enough, if we had written the same block using if sys.platform == "linux": ... else: ..., it would understand it, and would not proceed with re-importing the same modules.
   • Not that we can rely on this check though, because PySide6 breaks Mypy, but more on that below.

3. Irrespective of whether PySide6 is installed or not, Mypy assumes that the modules are redefined, which is why we have to use # type: ignore [no-redef].

   • Effectively, this means that in systems where PySide2 is installed, Mypy ignores its stubs.

4. If PySide6 is not installed, Mypy will treat the interface of these modules as Any, due to the --ignore-missing-imports flag.

   • Because the interface of these modules is Any, the type: ignore hints that we use in subsequent lines are not used and, due to the --warn-unused-ignores flag, Mypy complains loudly.
   • That's why in this commit we had to remove the --warn-unused-ignores flag.

5. If PySide6 is installed, Mypy will use the symbols from these modules to perform its static checks. However, this will not work, since the checks fail in practice. These are some of the errors I encounter:

   dangerzone/gui/logic.py:43: error: "Type[QFontDatabase]" has no attribute "FixedFont"  [attr-defined]                                                                                          
   dangerzone/gui/logic.py:136: error: "Type[Qt]" has no attribute "CustomizeWindowHint"  [attr-defined]                                                                                          
   dangerzone/gui/logic.py:137: error: "Type[Qt]" has no attribute "WindowTitleHint"  [attr-defined]                                                                                              
   dangerzone/gui/logic.py:138: error: "Type[Qt]" has no attribute "WindowSystemMenuHint"  [attr-defined]                                                                                         
   dangerzone/gui/logic.py:139: error: "Type[Qt]" has no attribute "WindowCloseButtonHint"  [attr-defined]                                                                                        
   dangerzone/gui/logic.py:140: error: "Type[Qt]" has no attribute "WindowStaysOnTopHint"  [attr-defined]
   

   • There is also an open PySide6 issue for that.
   • Effectively, this means that we can't run Mypy on dev environments with PySide6 installed. Do you see the same in your environment as well?

Why is this bad?

What we end up with if we do the above is:

• PySide6 is installed: The checks fail.
• PySide6 is not installed: Mypy does not check anything Qt related, and we miss unused 'type: ignore' hints in other parts of our code.

So, we are in a worse situation than before.

[apyrgio]

Proposal

The best way forward would be to use if sys.platform == "linux" ... else: ..., and move the imports there. However, we can't do this until PySide6 fixes its Mypy issues.

The next best thing is this:

if typing.TYPE_CHECKING:
    from PySide2 import QtCore, QtGui, QtWidgets
else:
    try:
        from PySide6 import QtCore, QtGui, QtWidgets
    except ImportError:
        from PySide2 import QtCore, QtGui, QtWidgets

and install PySide2 stubs in all platforms. The rationale is the following:

1. The PySide2 and PySide6 interfaces we are using happen to be the same, so the checks we do for PySide2 will apply to PySide6 as well.
2. Environments only with PySide2 remain unaffected.
3. The --warn-unused-ignores flag can be reinstated.

Check this out in the 117-apple-silicon-mypy branch (8dfec99).

[deeplow]

Wow. Thanks for digging deeper into this. Something did in fact feel old about my "hack" but since it was just a linter, it didn't seem much consequential. I totally agree with your assessment. I will attempt to implement it now.

[deeplow]

3.

Check this out in the 117-apple-silicon-mypy branch (8dfec99).

This may work on windows but I it leads to the initial problem on macOS (#177). And on a fresh virtualenv on fedora-36, it's getting stuck on installing shiboken2 as well:

  • Installing shiboken2 (5.15.2.1): Pending...

It just hangs there...

[apyrgio]

Current state of this PR:

I had made some recent comments and research regarding Mypy, so we decided to take over this part, and implement the fixes myself. Here's what I've done in general on this PR:

1. I squashed all the fixes we have agreed on and tidied up the commits.
2. I switched PySide2-stubs with types-PySide2. These stubs are more complete (we even remove some type: ignore statements) and don't bring PySide2 as a dependency. This means that we can use them in all platforms, even M1 MacOS.
3. I've updated the original issue with some decisions we took along the way on this PR (see Support building on M1 macs #177 (comment)).
4. I've pushed the above to 117-apple-silicon-mypy. I couldn't push to the original branch, because it's in your forked repo.

My suggestion is to:

1. Change the author in the commits of this branch, and sign the commits with your key.
2. Force push the *-mypy branch to the original branch, and then merge it.
   • This way, we will retain a pointer to the topics we discussed here.

[deeplow]

I like how the classify the PySide original stubs as abysmal 🤣 on the types-PySide2 project page:

[deeplow]

The changes LGTM. And I tested on Mac and Windows and it checks out.

My suggestion is to:
1. Change the author in the commits of this branch, and sign the commits with your key.
2. Force push the *-mypy branch to the original branch, and then merge it.

I have done so now and I've rebased it on top of main.

[deeplow]

I guess types-PySide2 introduced some recursion in the Windows / MacOS tests. They are failing now.