Skip to content
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

WIP: Fix python #135

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

WIP: Fix python #135

wants to merge 23 commits into from

Conversation

reynoldsnlp
Copy link

No description provided.

Copy link
Author

@reynoldsnlp reynoldsnlp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TinoDidriksen you should address/remove the TODO comment in python/CMakeLists.txt before merging.

Also, I highly recommend Squashing the commits because I have many useless commits from trying to debug the Github Action when it was working locally.

@@ -29,8 +30,8 @@ add_custom_target(wrapper ALL
)

if(NOT PYTHON_INSTALL_PARAMS)
set(PYTHON_INSTALL_PARAMS "--prefix=${CMAKE_INSTALL_PREFIX} --root=\$ENV{DESTDIR}/")
set(PYTHON_INSTALL_PARAMS "--no-index --find-links=dist/") # TODO removing --prefix=${CMAKE_INSTALL_PREFIX} --root=\$ENV{DESTDIR}/ probably has downstream effects
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that I removed the arguments for --prefix and --root to simplify the Github Action test of the python module. I'm guessing this affects downstream scripts for publishing debian or nightly builds. I would recommend basing downstream scripts on the wheel file created in cg3/python/dist/, but another option is to expand python's search path in the GH Action to find the install.

@TinoDidriksen
Copy link
Member

Finally had time to try this out. It only builds a wheel. But I don't understand what changes cause that. Wheels and distro packages are two orthogonal worlds. Wheels belong to pip and PyPi, neither of which are allowed in distro packages. The build won't even have network access.

So, the changes are ok for wheel building, but unfortunately wrong for distro building. This repo must continue using setup.py until Debian supports something else (https://bugs.debian.org/1001459, https://bugs.debian.org/1027864).

But it might make sense to split it off as a cg3-python repo.

TinoDidriksen added a commit that referenced this pull request Dec 27, 2023
… shared library instead of rebuilding; Consolidate source file list
TinoDidriksen added a commit that referenced this pull request Dec 27, 2023
… shared library instead of rebuilding; Consolidate source file list
TinoDidriksen added a commit that referenced this pull request Dec 27, 2023
… shared library instead of rebuilding; Consolidate source file list
TinoDidriksen added a commit that referenced this pull request Dec 27, 2023
… shared library instead of rebuilding; Consolidate source file list
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants