Skip to content
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

Use PySide6 for non-windows + python >= 3.12 #645

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

luk1337
Copy link
Contributor

@luk1337 luk1337 commented Nov 9, 2023

PySide2 is dead and won't support Python 3.12 or newer.

@luk1337
Copy link
Contributor Author

luk1337 commented Nov 10, 2023

Looks like Windows build fails, should I do:

diff --git a/requirements_gui.txt b/requirements_gui.txt
index ba12db7..208cb3b 100644
--- a/requirements_gui.txt
+++ b/requirements_gui.txt
@@ -1,3 +1,3 @@
-pyside2>=5.11.0; sys_platform != 'darwin'
-pyside6; sys_platform == 'darwin'
+pyside2>=5.11.0; sys_platform == 'win32'
+pyside6; sys_platform != 'win32'
 requests>=2.20.0; sys_platform == 'darwin'

instead, or attempt to migrate win32 to PySide6?

@Et0h
Copy link
Contributor

Et0h commented Nov 13, 2023

In terms of which PySide to use, the constraints are as follows:

  • PySide6 doesn't work on Windows build of Syncplay (as we use the x86 version of Python 3.8 rather than the x64 version)
  • We want to continue to use PySide6 for macOS
  • Packaging of PySide6 on several different Linux distros is still not done

Taking this into account, for maximum compatibility the GUI requirements should be PySide6 for darwin (macOS) and for non-Darwin:

  • PySide2 for Python < 3.12
  • PySide6 for Python >= 3.12

We might move Windows to PySide6 and 64-bit in the future, but probably best not to make that switch without extensive testing.

@luk1337
Copy link
Contributor Author

luk1337 commented Nov 13, 2023

In terms of which PySide to use, the constraints are as follows:

  • PySide6 doesn't work on Windows build of Syncplay (as we use the x86 version of Python 3.8 rather than the x64 version)
  • We want to continue to use PySide6 for macOS
  • Packaging of PySide6 on several different Linux distros is still not done

Taking this into account, for maximum compatibility the GUI requirements should be PySide6 for darwin (macOS) and for non-Darwin:

  • PySide2 for Python < 3.12
  • PySide6 for Python >= 3.12

We might move Windows to PySide6 and 64-bit in the future, but probably best not to make that switch without extensive testing.

Does this look fine?

pyside2>=5.11.0; sys_platform != 'darwin' and (sys_platform == 'win32' or python_version < '3.12')
pyside6; sys_platform == 'darwin' or (sys_platform != 'win32' and python_version >= '3.12')
requests>=2.20.0; sys_platform == 'darwin'

@Et0h
Copy link
Contributor

Et0h commented Nov 15, 2023

Does this look fine?

From a visual inspection it looks good to me, although I am no requirements.txt expert. I guess the next thing to do is to make the commit on your branch for the build to update and then we test if it works in practice.

PySide2 is dead and won't support Python 3.12 or newer.
@luk1337
Copy link
Contributor Author

luk1337 commented Nov 15, 2023

Does this look fine?

From a visual inspection it looks good to me, although I am no requirements.txt expert. I guess the next thing to do is to make the commit on your branch for the build to update and then we test if it works in practice.

Ok, I force pushed my branch. Also I tried pip install . --force on Linux and it seemed fine.

@luk1337 luk1337 changed the title Require PySide6 by default Use PySide6 for non-windows + python >= 3.12 Nov 15, 2023
@Et0h
Copy link
Contributor

Et0h commented Nov 16, 2023

Runs fine for me on Windows 10.

@Et0h Et0h requested a review from daniel-123 November 16, 2023 17:09
@Et0h Et0h merged commit fc0d2c5 into Syncplay:master Nov 24, 2023
3 checks passed
@luk1337 luk1337 deleted the luk/pyside6 branch November 24, 2023 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants