Skip to content

Conversation

@Vipul-Cariappa
Copy link
Collaborator

No description provided.

endif()
else()
set(Python_INCLUDE_DIRS
"${CMAKE_PREFIX_PATH}/include/python3.11"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't want Python_INCLUDE_DIRS to be set for a particular python version. The ci has version 3.11 for non wasm jobs, but we will want cppyy to support more than just that version. I plan to make this as part of the ci soon. Also given wasm support is constantly change, its probably best to look at using the latest python version for this work.


if (NOT Python_Development_FOUND)
message(FATAL_ERROR "Python development package not found and python component required")
if(NOT EMSCRIPTEN)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rather have if (emscripten) then else as that is more clear

add_library(cppyy SHARED ${cppyy_src})

if (EMSCRIPTEN)
target_link_options(cppyy
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer set_target_properties and to use the link_flags option, and we can drop the CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS "-s SIDE_MODULE=1") and set(CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS "-s SIDE_MODULE=1") and put as compile_flags for the cppyy target.

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.

2 participants