-
Notifications
You must be signed in to change notification settings - Fork 206
pyproject.toml for packaging #146
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
base: master
Are you sure you want to change the base?
Conversation
fschrempf
left a comment
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.
Thanks for your work! I'm not a Python packaging expert, so my feedback capabilities are limited, but I found some problems running this on Python 3.13.7 with setuptools 80.9.0.
|
Thanks for your comments! I'll take a look at it this weekend and do some further testing. |
ee74800 to
e4fb774
Compare
|
I've incorporated your requested changes and can confirm it can now be installed "from source" in a GitHub Codespace with the following tools/versions:
|
fschrempf
left a comment
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.
Thanks for keeping up the good work and sorry for the delay. Here are two more suggestions.
@doceme Do you have any general objections? Would you mind if I merge this?
| keywords = ["SPI", "Linux", "SPI device"] | ||
|
|
||
| [build-system] | ||
| requires = ["setuptools>=61.0"] |
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.
With the other changes, we are now depending on a more recent version of setuptools, so this should be lifted to v80.
| requires = ["setuptools>=61.0"] | |
| requires = ["setuptools>=80.0"] |
| dynamic = ["readme"] | ||
| version = "3.8" |
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.
Instead of duplicating the version here, we can still parse it from the C file with a little overhead. I would suggest to do:
| dynamic = ["readme"] | |
| version = "3.8" | |
| dynamic = ["readme", "version"] |
And then keep the setup.py script with the following content to parse the version string from the C file and pass it to the setup routine:
#!/usr/bin/env python
from setuptools import setup
version = "0.0"
lines = [x for x in open("spidev_module.c").read().split("\n") if "#define" in x and "_VERSION_" in x and "\"" in x]
if len(lines) > 0:
version = lines[0].split("\"")[1]
else:
raise Exception("Unable to find _VERSION_ in spidev_module.c")
setup(
version = version,
)
I'm not entirely sure if support for setup.py can be dropped, but I leave that up to the maintainers. This can replace the following files:
MANIFEST.in
setup.py
setup.cfg