Copilot commented on code in PR #631:
URL: https://github.com/apache/iceberg-cpp/pull/631#discussion_r3173683504
##########
src/iceberg/CMakeLists.txt:
##########
@@ -169,6 +157,60 @@ add_iceberg_lib(iceberg
OUTPUTS
ICEBERG_LIBRARIES)
+set(ICEBERG_DATA_SOURCES
+ data/data_writer.cc
+ data/delete_loader.cc
+ data/equality_delete_writer.cc
+ data/position_delete_writer.cc
+ data/writer.cc
+ deletes/position_delete_index.cc
+ deletes/roaring_position_bitmap.cc
+ puffin/file_metadata.cc
+ puffin/json_serde.cc
+ puffin/puffin_format.cc)
+
+set(ICEBERG_DATA_STATIC_BUILD_INTERFACE_LIBS)
+set(ICEBERG_DATA_SHARED_BUILD_INTERFACE_LIBS)
+set(ICEBERG_DATA_STATIC_INSTALL_INTERFACE_LIBS)
+set(ICEBERG_DATA_SHARED_INSTALL_INTERFACE_LIBS)
+
+list(APPEND ICEBERG_DATA_STATIC_BUILD_INTERFACE_LIBS
+ "$<IF:$<TARGET_EXISTS:iceberg_static>,iceberg_static,iceberg_shared>"
+ roaring::roaring)
+list(APPEND ICEBERG_DATA_SHARED_BUILD_INTERFACE_LIBS
+ "$<IF:$<TARGET_EXISTS:iceberg_shared>,iceberg_shared,iceberg_static>"
+ roaring::roaring)
+list(APPEND
+ ICEBERG_DATA_STATIC_INSTALL_INTERFACE_LIBS
+
"$<IF:$<TARGET_EXISTS:iceberg::iceberg_static>,iceberg::iceberg_static,iceberg::iceberg_shared>"
+ "$<IF:$<BOOL:${CROARING_VENDORED}>,iceberg::roaring,roaring::roaring>")
+list(APPEND
+ ICEBERG_DATA_SHARED_INSTALL_INTERFACE_LIBS
+
"$<IF:$<TARGET_EXISTS:iceberg::iceberg_shared>,iceberg::iceberg_shared,iceberg::iceberg_static>"
+ "$<IF:$<BOOL:${CROARING_VENDORED}>,iceberg::roaring,roaring::roaring>")
+
+add_iceberg_lib(iceberg_data
+ SOURCES
+ ${ICEBERG_DATA_SOURCES}
+ EXTRA_INCLUDES
+ ${ICEBERG_INCLUDES}
+ SHARED_LINK_LIBS
+ ${ICEBERG_DATA_SHARED_BUILD_INTERFACE_LIBS}
+ STATIC_LINK_LIBS
+ ${ICEBERG_DATA_STATIC_BUILD_INTERFACE_LIBS}
+ STATIC_INSTALL_INTERFACE_LIBS
+ ${ICEBERG_DATA_STATIC_INSTALL_INTERFACE_LIBS}
+ SHARED_INSTALL_INTERFACE_LIBS
+ ${ICEBERG_DATA_SHARED_INSTALL_INTERFACE_LIBS})
+
+if(TARGET iceberg_data_shared)
+ target_compile_definitions(iceberg_data_shared PRIVATE ICEBERG_EXPORTING)
+endif()
+
+if(TARGET iceberg_data_static)
+ target_compile_definitions(iceberg_data_static PRIVATE ICEBERG_STATIC)
Review Comment:
`iceberg_data_static` sets `ICEBERG_STATIC` as a PRIVATE compile definition,
but consumers of the static library also need `ICEBERG_STATIC` so that
`ICEBERG_EXPORT` in public headers (e.g., `iceberg/data/writer.h`) doesn’t
expand to `dllimport` on Windows. As-is, linking an external target against
`iceberg::iceberg_data_static` can fail due to incorrect import/export
annotations. Consider making `ICEBERG_STATIC` an INTERFACE/PUBLIC compile
definition on `iceberg_data_static` (potentially gated to WIN32), or switching
the data headers to a dedicated `ICEBERG_DATA_*` export macro that
`add_iceberg_lib()` already defines.
```suggestion
target_compile_definitions(iceberg_data_static
PUBLIC $<$<PLATFORM_ID:Windows>:ICEBERG_STATIC>)
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
