cmusphinx / pocketsphinx

A small speech recognizer
Other
3.87k stars 713 forks source link

Build installs Python module in wrong place #341

Open flyn-org opened 1 year ago

flyn-org commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.