-
-
Notifications
You must be signed in to change notification settings - Fork 674
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
Switch to PEP621 compliant project metadata. #2229
Conversation
Hold off on merging this; I've got a lead on how we can permanently fix the dynamic dependency issue. |
7839190
to
4a32684
Compare
@mhsmith I've been able to completely remove setup.py using the tip that came from @raphaelm There's some unfortunate code duplication, but I don't think we can avoid that without actually publishing a custom build backend... and I don't think PyPI needs that noise. The duplicated code isn't that complicated, and is unlikely to need to change that often, so I think I can hold my nose and live with the code smell. |
FWIW: A custom build backend might not be the worst idea, as long as it's not Toga specific. From a little more investigation, it should be relatively straightforward, using the same entry point to setuptools that setuptools_scm uses ( |
Even if we don't make an independent build backend, I think it would be better to actually use This could be done as follows:
|
ad97a01
to
156030b
Compare
156030b
to
67f5572
Compare
@mhsmith Ok - I've got this working with a setuptools plugin, and no setup.py or setup.cfg files. |
4472f41
to
ab2e45d
Compare
pyproject.toml
Outdated
[tool.setuptools_scm] | ||
# We're not packaging at the root repo, but we need this declaration | ||
# to prevent warnings in some contexts. |
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.
Better record what those contexts are before you forget, otherwise we'll never know when it's safe to remove this.
toga/pyproject.toml
Outdated
[tool.setuptools_dynamic_dependencies.optional-dependencies] | ||
':sys_platform=="win32"' = ["toga-winforms == {version}"] | ||
':sys_platform=="linux"' = ["toga-gtk == {version}"] | ||
':"freebsd" in sys_platform' = ["toga-gtk == {version}"] | ||
':sys_platform=="darwin"' = ["toga-cocoa == {version}"] |
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've never seen this method of listing platform-specific dependencies. Does it actually work? Even if it does, it might be better to switch to the more conventional semicolon syntax.
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.
It definitely works - it's how the toga-demo
install works.
It's based on an edge case of optional (nee - extra) dependency handling. If I remember correctly, back the early days, you could install toga[cocoa]
to install a specific backend, and this syntax was there so that the default behavior was to install the right backend if you didn't specify an explicit extra.
However, I agree that doing this as a "base" dependency with conditional install syntax will be less confusing.
"toga-cocoa == {version}; sys_platform=='darwin'", | ||
# Mobile platforms | ||
"toga-iOS == {version}; sys_platform=='ios'", | ||
"toga-android == {version}; sys_platform=='android'", |
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.
We're not actually using this for mobile platforms at present, but I think it would work for the iOS case as it currently stands. android
won't be returned on any platform at present, but in the event it ever is, we can be ahead of the situation. FWIW, toga.platform
already supports sys.platform == 'android'
.
Replace the use of setup.py and setup.cfg with PEP621 project metadata.
Dependabot has issues when a setup.py is used, but doesn't contain the dependency definition. We historically used setup.py beacuse we needed a dynamic dependency on toga-core for the backends; but we can work around that by providing a custom setuptools plugin.
setuptools_scm
will populate the source tarball with all version controlled files.Fixes #2226.
PR Checklist: