joemarshall commented on code in PR #37822:
URL: https://github.com/apache/arrow/pull/37822#discussion_r1574508173
##########
python/CMakeLists.txt:
##########
@@ -763,8 +813,12 @@ foreach(module ${CYTHON_EXTENSIONS})
set_target_properties(${module_name} PROPERTIES COMPILE_DEFINITIONS
"CYTHON_TRACE=1;CYTHON_TRACE_NOGIL=1")
endif()
-
- target_link_libraries(${module_name} PRIVATE ${LINK_LIBS})
+ # user this rather roundabout way of including, otherwise the
+ # in static linking mode the target will depend on libarrow and
+ # build it into every cython extension
+ target_include_directories(${module_name} PUBLIC
$<TARGET_PROPERTY:arrow_python,INTERFACE_INCLUDE_DIRECTORIES>)
+ target_link_libraries(${module_name} PUBLIC $<TARGET_FILE:arrow_python>)
+ add_dependencies(${module_name} arrow_python)
Review Comment:
The reason for this is:
1) libarrow.a is a static library
2) arrow_python links to libarrow.a
3) If we depend on the arrow_python target, we also inherit the libarrow.a
dependencies, and libarrow.a gets linked into every cython module, as well as
in arrow_python shared library.
4) Two bad things happen - (a) the extension size is massive, (b) there are
multiple copies of libarrow in the source, which means two copies of things
that are supposed to be unique, which breaks some of compute.
I tried messing with private / public / interface targets, but I couldn't
get past the fact that we need the include directories from arrow_python
dependencies, but we need to not link the static libraries that it links in. If
I make the static library targets private dependencies for arrow_python, then
we lose them from the linker command line to cython modules, but we also lose
the include directories, so cython can't call anything in arrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
