kou commented on code in PR #6:
URL: https://github.com/apache/iceberg-cpp/pull/6#discussion_r1902525180
##########
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:
> 79bba23
Thanks.
We should not define `XXX_STATIC_DEFINE` for a shared library:
```diff
diff --git a/cmake_modules/BuildUtils.cmake b/cmake_modules/BuildUtils.cmake
index bc848bf..d1bc7ca 100644
--- a/cmake_modules/BuildUtils.cmake
+++ b/cmake_modules/BuildUtils.cmake
@@ -216,8 +216,7 @@ function(add_iceberg_lib LIB_NAME)
if(BUILD_SHARED)
generate_export_header(${LIB_NAME}_shared BASE_NAME ${LIB_NAME_UPPER})
target_compile_definitions(${LIB_NAME}_shared
- PRIVATE ${LIB_NAME}_shared_EXPORTS
- PUBLIC ${LIB_NAME_UPPER}_STATIC_DEFINE)
+ PRIVATE ${LIB_NAME}_shared_EXPORTS)
if(BUILD_STATIC)
target_compile_definitions(${LIB_NAME}_static
PUBLIC ${LIB_NAME_UPPER}_STATIC_DEFINE)
```
> However I cannot make it work :(
Could you try this?
```diff
diff --git a/cmake_modules/BuildUtils.cmake b/cmake_modules/BuildUtils.cmake
index c915b3e..8a30753 100644
--- a/cmake_modules/BuildUtils.cmake
+++ b/cmake_modules/BuildUtils.cmake
@@ -214,7 +214,6 @@ function(add_iceberg_lib LIB_NAME)
# generate export header file
if(BUILD_SHARED)
generate_export_header(${LIB_NAME}_shared BASE_NAME ${LIB_NAME})
- target_compile_definitions(${LIB_NAME}_shared PUBLIC
${LIB_NAME}_shared_EXPORTS)
if(BUILD_STATIC)
string(TOUPPER ${LIB_NAME} LIB_NAME_UPPER)
target_compile_definitions(${LIB_NAME}_static
diff --git a/src/iceberg/demo_table.cc b/src/iceberg/demo_table.cc
index 9e46d7a..d960f1b 100644
--- a/src/iceberg/demo_table.cc
+++ b/src/iceberg/demo_table.cc
@@ -18,6 +18,7 @@
*/
#include "iceberg/demo_table.h"
+#include "iceberg/puffin.h"
namespace iceberg {
```
--
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
