wgtmac commented on PR #44: URL: https://github.com/apache/iceberg-cpp/pull/44#issuecomment-2646093255
I have managed to add `sparrow` as a vendored thirdparty dependency to `libiceberg`. However, there are still two issues that break CI: 1. It cannot compile on Windows due to `int128_t` is not supported by MSVC: https://github.com/apache/iceberg-cpp/actions/runs/13217571682/job/36898649736?pr=44. ``` D:\a\iceberg-cpp\iceberg-cpp\build\_deps\sparrow-src\include\sparrow\details\3rdparty\large_integers\int128_t.hpp(92,23): error C2065: 'int128_t': undeclared identifier [D:\a\iceberg-cpp\iceberg-cpp\build\_deps\sparrow-build\sparrow.vcxproj] (compiling source file '../sparrow-src/src/array_factory.cpp') ``` 2. It cannot compile on MacOS with `AppleClang 15.0.0.15000309` since Sparrow README recommends `Apple Clang 16 or higher`: https://github.com/apache/iceberg-cpp/actions/runs/13217571682/job/36898649906?pr=44. ``` [ 0%] Building CXX object _deps/sparrow-build/CMakeFiles/sparrow.dir/src/array_factory.cpp.o In file included from /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/src/array_factory.cpp:17: In file included from /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/include/sparrow/layout/decimal_array.hpp:24: /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/include/sparrow/buffer/u8_buffer.hpp:114:19: error: out-of-line definition of 'u8_buffer<T>' does not match any declaration in 'u8_buffer<T>' u8_buffer<T>::u8_buffer(R&& range) ^~~~~~~~~ In file included from /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/src/array_factory.cpp:19: In file included from /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/include/sparrow/layout/fixed_width_binary_array.hpp:17: /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/include/sparrow/layout/fixed_width_binary_layout/fixed_width_binary_array.hpp:336:83: error: no member named 'join' in namespace 'std::ranges::views' auto data_buffer = u8_buffer<values_inner_value_type>(std::ranges::views::join(values)); ~~~~~~~~~~~~~~~~~~~~^ /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/include/sparrow/layout/fixed_width_binary_layout/fixed_width_binary_array.hpp:481:70: error: no member named 'join' in namespace 'std::ranges::views' const auto joined_repeated_value_range = std::ranges::views::join(my_repeat_view); ~~~~~~~~~~~~~~~~~~~~^ In file included from /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/src/array_factory.cpp:27: /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/include/sparrow/layout/variable_size_binary_layout/variable_size_binary_array.hpp:455:83: error: no member named 'join' in namespace 'std::ranges::views' auto data_buffer = u8_buffer<values_inner_value_type>(std::ranges::views::join(values)); ~~~~~~~~~~~~~~~~~~~~^ /Users/runner/work/iceberg-cpp/iceberg-cpp/build/_deps/sparrow-src/include/sparrow/layout/variable_size_binary_layout/variable_size_binary_array.hpp:683:70: error: no member named 'join' in namespace 'std::ranges::views' const auto joined_repeated_value_range = std::ranges::views::join(my_repeat_view); ~~~~~~~~~~~~~~~~~~~~^ 5 errors generated. ``` There are some other minor issues: 1. It should use `PREPEND` not `APPEND` at this line: https://github.com/man-group/sparrow/blob/532bf486243fc396f7a9392820fa7a77071782ad/CMakeLists.txt#L50. Otherwise I have to add `list(PREPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_BINARY_DIR}/_deps/sparrow-src/cmake)` on my side when using `FetchContent` to vendor sparrow to avoid error `include could not find requested file: sanitizers`. 2. It would be good to add a namespace `sparrow::` when installing the exported config file at https://github.com/man-group/sparrow/blob/532bf486243fc396f7a9392820fa7a77071782ad/CMakeLists.txt#L354. These are not blocking issues at the moment. I need more time to get familiar with its API and best practice. Just want to report what I have found so far. @JohanMabille @Alex-PLACET @pitrou -- 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
