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

Build installs Python module in wrong place #341

Open
flyn-org opened this issue May 15, 2023 · 2 comments
Open

Build installs Python module in wrong place #341

flyn-org opened this issue May 15, 2023 · 2 comments

Comments

@flyn-org
Copy link
Contributor

I maintain the pocketsphinx package for Fedora, and I am presently trying to package the recent 5.0.0. release. I cannot figure out how to cause cmake to install the Python module in the correct place. I wrote the following patch, which aims to fix this:

diff -u --recursive pocketsphinx-5.0.0-vanilla/cython/CMakeLists.txt pocketsphinx-5.0.0/cython/CMakeLists.txt
--- pocketsphinx-5.0.0-vanilla/cython/CMakeLists.txt	2022-10-05 08:52:38.000000000 -0500
+++ pocketsphinx-5.0.0/cython/CMakeLists.txt	2023-05-15 17:00:54.298758625 -0500
@@ -19,7 +19,7 @@
   _pocketsphinx INTERFACE ${CMAKE_BINARY_DIR}/include
   )
 python_extension_module(_pocketsphinx)
-install(TARGETS _pocketsphinx LIBRARY DESTINATION cython/pocketsphinx)
+install(TARGETS _pocketsphinx LIBRARY DESTINATION ${CMAKE_INSTALL_PYDIR}/pocketsphinx)
 if(NOT USE_INSTALLED_POCKETSPHINX)
-  install(DIRECTORY ${PROJECT_SOURCE_DIR}/model DESTINATION cython/pocketsphinx)
+  install(DIRECTORY ${PROJECT_SOURCE_DIR}/model DESTINATION ${CMAKE_INSTALL_PYDIR}/pocketsphinx)
 endif()

With this patch applied, I can build pocketsphinx by providiing cmake with this argument: -DCMAKE_INSTALL_PYDIR="/usr/lib64/python3.11/site-packages/". Does this make sense? I would be happy to establish a pull request, but I wasn't sure I was going about this right.

@dhdaines
Copy link
Contributor

This doesn't seem like the right solution as there is probably a predefined variable from scikit-build that should be used. If I were to accept this patch then the build would always fail unless CMAKE_INSTALL_PYDIR is defined.

The priority is always to support python packaging, in my opinion distribution packages can simply move the files to the right place in their package building scripts, at least this is how Debian packaging works.

@flyn-org
Copy link
Contributor Author

It looks like PYTHON_SITE_PACKAGES_DIR might help (see "6.3 PythonExtensions" in https://readthedocs.org/projects/scikit-build/downloads/pdf/latest/). However, scikit-build expands this to /usr/lib/... rather than /usr/lib64/.... The latter is used on Fedora for native shared libraries. For now, I took your suggested approach. My package definition moves things around after the fact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants