Skip to content

Fix find_package case for pre-installed Ginkgo #100

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.

Sign up for GitHub

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

Open
rho-novatron wants to merge 3 commits into
base: main
Choose a base branch
from
+20 −16
Open

Fix find_package case for pre-installed Ginkgo #100

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6c63f2
Fix find_package case for pre-installed Ginkgo
rho-novatron Mar 27, 2026
fa97fe9
Fix cmake install destination for py-build-cmake wheel packaging
rho-novatron Mar 27, 2026
818e2c5
Update CMakeLists.txt
rho-novatron Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ if(NOT nlohmann_json_FOUND)
FetchContent_MakeAvailable(nlohmann_json)
endif()

# Find Ginkgo find_package(Ginkgo 1.7.0 QUIET)
find_package(ginkgo QUIET)
# Find Ginkgo (quietly; no minimum version specified)
find_package(Ginkgo QUIET)

# Fetch Ginkgo if not found
if(NOT ginkgo_FOUND)
if(NOT Ginkgo_FOUND)
# If not found, fetch Ginkgo
include(FetchContent)
if(NOT DEFINED GINKGO_BUILD_TESTS)
Expand Down Expand Up @@ -182,16 +182,20 @@ endif()
set(CMAKE_INSTALL_RPATH ${Python_SITELIB}/${PROJECT_NAME})

Comment on lines 182 to 183
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

CMAKE_INSTALL_RPATH is still set using the absolute ${Python_SITELIB}/${PROJECT_NAME} path, even though installation is now intended to go through a staging prefix for wheel builds. This can embed a build-environment-specific absolute RPATH into the extension, which is problematic for wheel relocatability. Consider setting an RPATH relative to the installed module location (e.g., $ORIGIN) or deriving it from the actual install destination instead of Python_SITELIB.

Copilot uses AI. Check for mistakes.
message(STATUS "Install path: ${Python_SITELIB}")
install(DIRECTORY ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}
DESTINATION ${Python_SITELIB})
install(
IMPORTED_RUNTIME_ARTIFACTS
ginkgo
ginkgo_device
ginkgo_hip
ginkgo_cuda
ginkgo_omp
ginkgo_dpcpp
ginkgo_reference
DESTINATION
${Python_SITELIB}/${PROJECT_NAME})
install(DIRECTORY ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}/
Comment on lines 184 to +185
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

The status message still prints Install path: ${Python_SITELIB}, but the install destination was changed to ${PY_BUILD_CMAKE_MODULE_NAME}. This is now misleading when diagnosing packaging/install issues; it should report the actual computed install destination (including any fallback logic) rather than the old Python_SITELIB value.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@rho-novatron rho-novatron Apr 18, 2026

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

DESTINATION ${PY_BUILD_CMAKE_MODULE_NAME})
Comment on lines +185 to +186
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

PY_BUILD_CMAKE_MODULE_NAME is used as the install() destination, but it isn’t defined anywhere in this project (only provided by py-build-cmake). This breaks the documented plain-CMake workflow (cmake --install .) because the install destination becomes empty and files get installed into the install prefix root. Consider adding a fallback (e.g., default to ${PROJECT_NAME} or ${Python_SITELIB}/${PROJECT_NAME} when PY_BUILD_CMAKE_MODULE_NAME is not defined), or make the destination configurable via an option/cache variable.

Copilot uses AI. Check for mistakes.
if(NOT Ginkgo_FOUND)
# Only install Ginkgo shared libraries when built from source (FetchContent).
# When using a pre-installed Ginkgo (e.g. conda), its libraries are already available.
install(
IMPORTED_RUNTIME_ARTIFACTS
ginkgo
ginkgo_device
ginkgo_hip
ginkgo_cuda
ginkgo_omp
ginkgo_dpcpp
ginkgo_reference
DESTINATION
${PY_BUILD_CMAKE_MODULE_NAME})
endif()
Loading