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

broken wheels #5

Closed
lgarrison opened this issue Feb 11, 2025 · 1 comment · Fixed by #7
Closed

broken wheels #5

lgarrison opened this issue Feb 11, 2025 · 1 comment · Fixed by #7

Comments

@lgarrison
Copy link
Member

Installing from source doesn't seem to be putting the .so Python extension in the right place. Similarly the wheels on PyPI have the extension in the wrong dir. The error is:

uv run --with simple-ans python -c 'import simple_ans'
Installed 2 packages in 1.19s
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import simple_ans
  File "/mnt/home/lgarrison/.cache/uv/archive-v0/G9TokNRD90kFwghGNt2tU/lib/python3.13/site-packages/simple_ans/__init__.py", line 1, in <module>
    from .encode_decode import ans_encode
  File "/mnt/home/lgarrison/.cache/uv/archive-v0/G9TokNRD90kFwghGNt2tU/lib/python3.13/site-packages/simple_ans/encode_decode.py", line 5, in <module>
    from ._simple_ans import (
    ...<15 lines>...
    )
ModuleNotFoundError: No module named 'simple_ans._simple_ans'

scikit-build-core installs relative to platlib, so the modification I would recommend to CMakeLists.txt would be:

❯ git diff
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0177ca5..13d8316 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -61,18 +61,4 @@ endif()
 
 
 # Install rules
-include(GNUInstallDirs)
-install(TARGETS _simple_ans
-    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
-    ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
-    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
-)
-install(FILES ${HEADERS}
-    DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/simple_ans
-)
-
-# For editable installs, copy the extension module to the source directory
-add_custom_command(TARGET _simple_ans POST_BUILD
-    COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:_simple_ans> ${CMAKE_SOURCE_DIR}/simple_ans/
-    COMMENT "Copying extension module to source directory for editable install"
-)
+install(TARGETS _simple_ans LIBRARY DESTINATION simple_ans)

If the intention is for CMake to build and install a library separate from the Python extension, then you might want some of the other install commands. But for just installing a Python extension, I think that's all you need. Happy to make a PR if you agree with this fix.

By the way, cibuildwheel has a nice built-in mechanism (https://cibuildwheel.pypa.io/en/stable/options/#test-command) to test the wheels after build, which is very useful for catching issues like this.

@magland
Copy link
Collaborator

magland commented Feb 11, 2025

Hi @lgarrison

I have very little experience packaging Python with C++, so I was hoping someone would help with this. You are very welcome to submit a PR!

@lgarrison lgarrison mentioned this issue Feb 12, 2025
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 a pull request may close this issue.

2 participants