wgtmac commented on code in PR #6:
URL: https://github.com/apache/iceberg-cpp/pull/6#discussion_r1904813642
##########
cmake_modules/BuildUtils.cmake:
##########
@@ -201,17 +202,26 @@ function(ADD_ICEBERG_LIB LIB_NAME)
PUBLIC
"$<BUILD_INTERFACE:${ARG_STATIC_LINK_LIBS}>")
endif()
- install(TARGETS ${LIB_NAME}_static ${INSTALL_IS_OPTIONAL}
- EXPORT ${LIB_NAME}_targets
+ install(TARGETS ${LIB_NAME}_static
+ EXPORT iceberg_targets
ARCHIVE DESTINATION ${INSTALL_ARCHIVE_DIR}
LIBRARY DESTINATION ${INSTALL_LIBRARY_DIR}
RUNTIME DESTINATION ${INSTALL_RUNTIME_DIR}
INCLUDES
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
endif()
- if(ARG_CMAKE_PACKAGE_NAME)
- iceberg_install_cmake_package(${ARG_CMAKE_PACKAGE_NAME}
${LIB_NAME}_targets)
+ # generate export header file
+ string(TOUPPER ${LIB_NAME} LIB_NAME_UPPER)
+ if(BUILD_SHARED)
+ generate_export_header(${LIB_NAME}_shared BASE_NAME ${LIB_NAME_UPPER})
+ target_compile_definitions(${LIB_NAME}_shared PUBLIC
${LIB_NAME}_shared_EXPORTS)
Review Comment:
Thanks for the explanation! @kou
> is it OK that iceberg::Puffin exists in libiceberg_core.dll not
libiceberg_puffin.dll?
I think this depends on how we implement the puffin interfaces. It seems to
me there are at least two different interpretations of `libiceberg_puffin`:
1. `libiceberg_core` provides only the interface of `iceberg::Puffin` reader
and writer. `libiceberg_puffin` provides a concrete implementation of
`iceberg::Puffin` together with roaring bitmap serialization and
deserialization. This means that `libiceberg_puffin` cannot be used alone and
should always depend on `libiceberg_core`.
2. `libiceberg_puffin` provide the minimal building block of the puffin file
format and does not depend on anything. In this case, `libiceberg_core` depends
on `libiceberg_puffin` to provide a concrete implementation.
As the goal of `libiceberg_core` is to provide a lightweight library with
minimal dependency to implement the core logic of Apache Iceberg and puffin is
a required component (same as Parquet and Avro), I feel a little bit suspicious
about the point of splitting an individual `libiceberg_puffin` library at this
moment. As this was a request from @gaborkaszab, could you please chime in to
provide more context about your perspective? Thanks!
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
