Build/issue 1563 pyproject migration - #1569
Conversation
Migrate static metadata, dynamic versioning, and dependency management to resolve Issue Xilinx#1563. Signed-off-by: Andrew Hansraj <hansrajandrew@gmail.com>
Remove distutils and custom file operations, migrating static metadata to pyproject.toml and utilizing shutil for extension building. Signed-off-by: Andrew Hansraj <hansrajandrew@gmail.com>
Add recursive-include rules to correctly bundle nested python and data files for the core pynq module. Signed-off-by: Andrew Hansraj <hansrajandrew@gmail.com>
Add dist/ directory to prevent tracking of generated wheel and tarball archives. Signed-off-by: Andrew Hansraj <hansrajandrew@gmail.com>
|
Hi @and-hans. This is something that has been on my todo list for a while. I have to ask, in what context are you using Python 3.12? Is this for use with PYNQ.remote? |
|
Hey @jogomojo Honestly, the context is that I just wanted to contribute to the project. I recently graduated university, so I thought I'd spend some of my free time working on something I enjoyed while I search for a job. I like this project because it's a mix of hardware and software. Also I've seen areas of the code I could help clean up some technical debt, but since I don't have a physical board on hand right now, I'm mostly focusing on the software and tooling side of things. As to how I found this issue: I am currently working locally on an M1 MacBook, which defaults to newer Python environments (3.12). When I was initially setting up my environment and exploring the repository, the installation failed right away due to the legacy "distutils" configuration. I saw that there was an issue involving migrating the project, so I figured I would figure out the migration and send a PR. |
|
Hi @and-hans, I really appreciate this PR, and I'd like to get it merged if possible. At the moment, though, I'm in the middle of a fairly substantial refactoring of the PYNQ sdbuild backend that I'll be merging in over the next few weeks. Those changes are likely to cause conflicts and may leave this PR out of date quite quickly. Would you mind if we revisited this once the backend changes are in place? Unfortunately, I don't have the capacity right now to thoroughly validate a change of this size on hardware, and I'd prefer to review it against the updated codebase. Thanks again for the contribution, and I hope we can pick this up again soon. |
|
Hey @jogomojo Yeah no worries, that's definitely a bigger priority. Once your sdbuild changes are merged in, I'd be more than happy to pull the new main branch, resolve any merge conflicts on my end, and update this PR so it's ready for hardware validation. I was gonna work on issue 1558, but I imagine the dependency list will experience some changes with your refactor as well. Are there any other open issues that are safe to tackle right now? |
|
Feel free to tackle any issue you want. However, I would say that issues like #1558 require a lot of testing to get that right, so hardware would be required. |
Resolves #1563
Summary of Changes:
This PR tackles the build failures popping up on newer Python environments (3.12+) by moving away from the deprecated distutils library and modernizing the packaging setup.
Modifications:
pyproject.toml: Moved all the static project metadata, dynamic version tracking, and dependency lists here.setup.py:Nixed the old distutils configurations and custom file-crawling logic. The C-extension compilation and Jupyter notebook staging are now handled using shutil operations, which makes the script much lighter.MANIFEST.in: Added recursive-include rules so we don't miss any deeply nested Python scripts or hardware assets when building the distribution wheel..gitignore: Added the dist/ directory so generated build artifacts stop popping up in the working trees.How I tested this:
(I neither have an Xilinix FPGA board nor a Windows computer, so bare with me)
Hopefully this makes the installation process easier and kills those deprecation crashes. But there's still some additional things that need to be added and changed in the project to fully fix the deprecation issues, which should be addressed in a seperate issue.
Let me know if anything needs tweaking or explaining.