project: Switch to pyproject #715
Conversation
|
Could you merge |
Are you sure we want to add it as shown here: https://tox.wiki/en/latest/config.html#pyproject-toml ? |
Didn't know this, I agree. |
There was a problem hiding this comment.
I wish I could review this but I really don't have the time to take a python packaging course right now. I hope the approvers know a bit more than me?
My lack of python packaging knowledge aside (!), I see nothing wrong here.
The relevant PEP is dated 2020 so I had a concern about Python 3.8 compatibility but CI is apparently testing that - great!
|
It does seem that the $ west twister -T samples/hello_world -p native_sim
Traceback (most recent call last):
File "/home/pdgendt/zephyrproject/.venv/bin/west", line 8, in <module>
sys.exit(main())
^^^^^^
File "/home/pdgendt/zephyrproject/.venv/lib/python3.11/site-packages/west/app/main.py", line 1087, in main
app.run(argv or sys.argv[1:])
File "/home/pdgendt/zephyrproject/.venv/lib/python3.11/site-packages/west/app/main.py", line 246, in run
self.run_command(argv, early_args)
File "/home/pdgendt/zephyrproject/.venv/lib/python3.11/site-packages/west/app/main.py", line 507, in run_command
self.run_extension(args.command, argv)
File "/home/pdgendt/zephyrproject/.venv/lib/python3.11/site-packages/west/app/main.py", line 631, in run_extension
self.cmd.add_parser(subparser_gen)
File "/home/pdgendt/zephyrproject/.venv/lib/python3.11/site-packages/west/commands.py", line 204, in add_parser
parser = self.do_add_parser(parser_adder)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/pdgendt/zephyrproject/zephyr/scripts/west_commands/twister_cmd.py", line 50, in do_add_parser
parser = add_parse_arguments(parser)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/pdgendt/zephyrproject/zephyr/scripts/pylib/twister/twisterlib/environment.py", line 261, in add_parse_arguments
modules = zephyr_module.parse_modules(ZEPHYR_BASE)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/pdgendt/zephyrproject/zephyr/scripts/zephyr_module.py", line 703, in parse_modules
west_projs = west_projs or west_projects(manifest)
^^^^^^^^^^^^^^^^^^^^^^^
File "/home/pdgendt/zephyrproject/zephyr/scripts/zephyr_module.py", line 675, in west_projects
from west.version import __version__ as WestVersion
ModuleNotFoundError: No module named 'west.version' |
|
I am testing the package from start to finish with upstream Zephyr once. Will update once that is done. Update: Yup, everything including twister seems to be working. |
I have update the instructions to build as described here: https://packaging.python.org/en/latest/tutorials/packaging-projects/#generating-distribution-archives |
There was a problem hiding this comment.
I have update the instructions to build as described here: https://packaging.python.org/en/latest/tutorials/packaging-projects/#generating-distribution-archives
This looks like a great page! Can you please add it somewhere in MAINTAINERS.rst so no one wastes time on some inferior stackoverflow or AI-generated duplicate?
Did you re-test MAINTAINERS.rst all the way now, including the twine upload to the test PyPI server?
MAINTAINERS.rst
Outdated
| @@ -88,7 +88,8 @@ Building and uploading the release wheels | |||
| You need the zephyr-project PyPI credentials for the 'twine upload' command. :: | |||
|
|
|||
| git clean -ffdx | |||
| python3 setup.py sdist bdist_wheel | |||
| python3 -m pip install --upgrade build twine | |||
There was a problem hiding this comment.
-
Why
python3 -m pip install...instead of the usual, shorter and clearerpip3 install ...? -
Rather than blindly upgrading
buildto "the latest", how about addingbuild>=1.somethingto[dependencies]inpyproject.toml?
twineis different because it is not built-in and not needed for local installations.
So I think this script would be more consistent with the English text:
git clean -fdx
python3 -m build
pip3 install --upgrade twine
twine upload ...
There was a problem hiding this comment.
pyproject-build is also not needed for local installations. I can do pip install . without having pyproject-build installed.
There was a problem hiding this comment.
Indeed, thanks for correcting me. It's not "built-in" either: it just happened to be already installed on my system for some reason.
Starting with PEP 621, `pyproject.toml` is the standard way of specifying project metadata. Also switch to using the version from `pyproject.toml` instead of having a west.version python module. Adjust documentation for the same. Update MAINTAINERS.rst to use build for dist building. Signed-off-by: Ayush Singh <ayush@beagleboard.org>
|
@marc-hb any open issues with this PR? |
| @@ -88,12 +88,15 @@ Building and uploading the release wheels | |||
| You need the zephyr-project PyPI credentials for the 'twine upload' command. :: | |||
|
|
|||
| git clean -ffdx | |||
| python3 setup.py sdist bdist_wheel | |||
| pip3 install --upgrade build twine | |||
There was a problem hiding this comment.
I'm still not a fan of using the "random version of the day" but these instructions seem to work now and this file is only for MAINTAINERS who are supposed to know what they're doing.
Starting with PEP 621,
pyproject.tomlis the standard way of specifying project metadata.Also switch to using the version from
pyproject.tomlinstead of having a west.version python module. Adjust documentation for the same.