-
Notifications
You must be signed in to change notification settings - Fork 214
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
modernised python packaging #493
Open
batmanfeynman
wants to merge
2
commits into
Syncplay:master
Choose a base branch
from
batmanfeynman:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| [build-system] | ||
| requires = [ | ||
| "setuptools>=51.0.0", | ||
| "wheel" | ||
| ] | ||
| build-backend = "setuptools.build_meta" |
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| [metadata] | ||
| name = syncplay | ||
| version = attr: syncplay.version | ||
| author = Syncplay | ||
| author_email = dev@syncplay.pl | ||
| description = Client/server to synchronize media playback on mpv/VLC/MPC-HC/MPC-BE across many computers | ||
| long_description = file: README.md | ||
| long_description_content_type = text/markdown | ||
| url = https://syncplay.pl/ | ||
| download_url = https://syncplay.pl/download/ | ||
| project_urls = | ||
| Source Code = https://github.com/Syncplay/syncplay | ||
| Bug Tracker = https://github.com/Syncplay/syncplay/issues | ||
| license_file = LICENSE | ||
| classifiers = | ||
| Development Status :: 5 - Production/Stable | ||
| Environment :: MacOS X :: Cocoa | ||
| Environment :: Win32 (MS Windows) | ||
| Environment :: X11 Applications :: Qt | ||
| Framework :: Twisted | ||
| Intended Audience :: End Users/Desktop | ||
| License :: OSI Approved :: Apache Software License | ||
| Operating System :: MacOS :: MacOS X | ||
| Operating System :: Microsoft :: Windows | ||
| Operating System :: POSIX :: Linux | ||
| Natural Language :: English | ||
| Natural Language :: German | ||
| Natural Language :: Italian | ||
| Natural Language :: Russian | ||
| Natural Language :: Spanish | ||
| Programming Language :: Python :: 3 | ||
| Programming Language :: Python :: 3.6 | ||
| Programming Language :: Python :: 3.7 | ||
| Programming Language :: Python :: 3.8 | ||
| Programming Language :: Python :: 3.9 | ||
| Programming Language :: Python :: 3.10 | ||
| Programming Language :: Python :: 3 :: Only | ||
| Topic :: Internet | ||
| Topic :: Multimedia :: Video | ||
|
|
||
| [options] | ||
| include_package_data = True | ||
| packages = find: | ||
| install_requires = | ||
| appnope>=0.1.0; sys_platform == 'darwin' | ||
| certifi>=2018.11.29 | ||
| pypiwin32>=223; sys_platform == 'win32' | ||
| pyside2>=5.11.0 | ||
| requests>=2.20.0; sys_platform == 'darwin' | ||
| twisted[tls]>=16.4.0 | ||
| zope.interface>=4.4.0; sys_platform == 'win32' | ||
| python_requires = >=3.6 | ||
|
|
||
| [options.entry_points] | ||
| console_scripts = | ||
| syncplay-server = syncplay.ep_server:main | ||
| gui_scripts = | ||
| syncplay = syncplay.ep_client:main | ||
|
|
||
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,65 +1,6 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| import os | ||
| import setuptools | ||
| from setuptools import setup | ||
| setup() | ||
|
|
||
| from syncplay import projectURL, version as syncplay_version | ||
|
|
||
| def read(fname): | ||
| with open(fname, 'r') as f: | ||
| return f.read() | ||
|
|
||
| installRequirements = read('requirements.txt').splitlines() +\ | ||
| read('requirements_gui.txt').splitlines() | ||
|
|
||
| setuptools.setup( | ||
| name="syncplay", | ||
| version=syncplay_version, | ||
| author="Syncplay", | ||
| author_email="dev@syncplay.pl", | ||
| description=' '.join([ | ||
| 'Client/server to synchronize media playback', | ||
| 'on mpv/VLC/MPC-HC/MPC-BE on many computers' | ||
| ]), | ||
| long_description=read('README.md'), | ||
| long_description_content_type="text/markdown", | ||
| url=projectURL, | ||
| download_url=projectURL + 'download/', | ||
| packages=setuptools.find_packages(), | ||
| install_requires=installRequirements, | ||
| python_requires=">=3.4", | ||
| entry_points={ | ||
| 'console_scripts': [ | ||
| 'syncplay-server = syncplay.ep_server:main', | ||
| ], | ||
| 'gui_scripts': [ | ||
| 'syncplay = syncplay.ep_client:main', | ||
| ] | ||
| }, | ||
| include_package_data=True, | ||
| classifiers=[ | ||
| "Development Status :: 5 - Production/Stable", | ||
| "Environment :: MacOS X :: Cocoa", | ||
| "Environment :: Win32 (MS Windows)", | ||
| "Environment :: X11 Applications :: Qt", | ||
| "Framework :: Twisted", | ||
| "Intended Audience :: End Users/Desktop", | ||
| "License :: OSI Approved :: Apache Software License", | ||
| "Operating System :: MacOS :: MacOS X", | ||
| "Operating System :: Microsoft :: Windows", | ||
| "Operating System :: POSIX :: Linux", | ||
| "Natural Language :: English", | ||
| "Natural Language :: German", | ||
| "Natural Language :: Italian", | ||
| "Natural Language :: Russian", | ||
| "Natural Language :: Spanish", | ||
| "Programming Language :: Python :: 3", | ||
| "Programming Language :: Python :: 3.4", | ||
| "Programming Language :: Python :: 3.5", | ||
| "Programming Language :: Python :: 3.6", | ||
| "Programming Language :: Python :: 3.7", | ||
| "Programming Language :: Python :: 3.8", | ||
| "Topic :: Internet", | ||
| "Topic :: Multimedia :: Video" | ||
| ], | ||
| ) |
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.
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.
I am not a fan of duplicated information, and switching to PEP 621 without touching the
requirements*.txtfiles does exactly that.I would be in favor of deleting the requirement files, but currently the Windows build system relies on them. This could be circumvented by using
pip-compilee.g. viain the CI workflows before the file is actually needed.
On the other hand, we could retain the requirements files and start using them in the proper way: to pin dependencies for CI/release builds. We actually need to pin dependencies on macOS but, for the moment I did that in the workflow code (very bad, but I did not have a proper alternative up to now).
@Et0h @daniel-123 what is your opinion here?
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.
As someone who is a Windows user I don't have any strong opinions on packaging beyond the general desire for things to work and be low-maintenance.
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.
Personally I'd prefer to avoid pinning dependencies to specific version unless strictly as rollback resulting from a bug in latest version of given dependency. At least as far as AppImage goes.
For OS packages - they just use what's on the OS, so pinning isn't really a useful concept in first place.
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.
Hi @daniel-123 @albertosottile
I just wanted to remind you of this!
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.
@batmanfeynman Sorry if I neglected this. The gist of this issue is that in both Windows and macOS workflows we currently have:
and I do not see a clear way to reproduce the same behavior after this PR is merged UNLESS the information about dependencies is duplicated in
setup.cfgand in the two requirements file. Am I missing something here? Is there a command to just install the dependencies of a package?