You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "zhjwpku (via GitHub)" <gi...@apache.org> on 2025/01/18 08:09:28 UTC
[PR] add libavrocpp_s [iceberg-cpp]
zhjwpku opened a new pull request, #34:
URL: https://github.com/apache/iceberg-cpp/pull/34
This closes #17
The avro_unittest is added for testing if libavrocpp works, once we supported Manifest, this should be removed.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923078620
##########
.github/workflows/test.yml:
##########
@@ -80,6 +80,10 @@ jobs:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
+ - name: Install ZLIB
+ shell: cmd
+ run: |
+ powershell -Command "(Invoke-WebRequest -Uri https://git.io/JnHTY -OutFile install_zlib.bat)"; ./install_zlib.bat; del install_zlib.bat
Review Comment:
I'm not sure if it is reliable, this is just a away I found that can walk around the zlib requirement. I will try your way later.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923958345
##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -126,3 +126,68 @@ endfunction()
if(ICEBERG_ARROW)
resolve_arrow_dependency()
endif()
+
+# ----------------------------------------------------------------------
+# Apache Avro
+
+function(resolve_avro_dependency)
+ prepare_fetchcontent()
+
+ set(AVRO_USE_BOOST
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_EXECUTABLES
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_TESTS
+ OFF
+ CACHE BOOL "" FORCE)
+
+ fetchcontent_declare(Avro
+ ${FC_DECLARE_COMMON_OPTIONS}
+ GIT_REPOSITORY https://github.com/apache/avro.git
+ GIT_TAG 1144cb7322bab4cd1c8bf330a9c504a0d4252b56
+ SOURCE_SUBDIR
+ lang/c++
+ FIND_PACKAGE_ARGS
+ NAMES
+ Avro
+ CONFIG)
Review Comment:
That would be great, I had some issue with `fmt::fmt-header-only` target, which is an interface link of avrocpp.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923919166
##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -126,3 +126,68 @@ endfunction()
if(ICEBERG_ARROW)
resolve_arrow_dependency()
endif()
+
+# ----------------------------------------------------------------------
+# Apache Avro
+
+function(resolve_avro_dependency)
+ prepare_fetchcontent()
+
+ set(AVRO_USE_BOOST
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_EXECUTABLES
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_TESTS
+ OFF
+ CACHE BOOL "" FORCE)
+
+ fetchcontent_declare(Avro
+ ${FC_DECLARE_COMMON_OPTIONS}
+ GIT_REPOSITORY https://github.com/apache/avro.git
+ GIT_TAG 1144cb7322bab4cd1c8bf330a9c504a0d4252b56
+ SOURCE_SUBDIR
+ lang/c++
+ FIND_PACKAGE_ARGS
+ NAMES
+ Avro
+ CONFIG)
Review Comment:
I cloned your commits and tried to make it work on my end. However, it seems that the Avro C++ library does not export CMake config file: https://issues.apache.org/jira/browse/AVRO-3088. It means that we have to maintain a `FindAvro.cmake` file to manually find the header file and libraries from the installed location. I will try to fix this on the Avro side.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#issuecomment-2612108480
I think this is ready to merge @Xuanwo @Fokko
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1922577230
##########
src/iceberg/CMakeLists.txt:
##########
@@ -17,11 +17,16 @@
set(ICEBERG_SOURCES demo_table.cc)
+set(ICEBERG_AVRO_STATIC_BUILD_LIBS)
+list(APPEND ICEBERG_AVRO_STATIC_BUILD_LIBS Avro::avro_static)
Review Comment:
To fully support reading (and writing) Avro files, we may need to add `zlib` and `snappy` as dependencies. Should we add a `libiceberg-avro` instead and only define the Avro reader/writer interfaces in the `libiceberg` to make it depend on less libraries?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#issuecomment-2611302409
> I have created a PR against your repo: [zhjwpku#3](https://github.com/zhjwpku/iceberg-cpp/pull/3). The CIs are all green on my side: [wgtmac#2](https://github.com/wgtmac/iceberg-cpp/pull/2). Please take a look and merge if it makes sense. @zhjwpku
Appreciate for your impressive work, I'll merge it soon.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#issuecomment-2611262437
I have created a PR against your repo: https://github.com/zhjwpku/iceberg-cpp/pull/3. The CIs are all green on my side: https://github.com/wgtmac/iceberg-cpp/pull/2. Please take a look and merge if it makes sense. @zhjwpku
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#issuecomment-2599750119
Thanks @zhjwpku for working on this! I will have a try early next week.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923958345
##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -126,3 +126,68 @@ endfunction()
if(ICEBERG_ARROW)
resolve_arrow_dependency()
endif()
+
+# ----------------------------------------------------------------------
+# Apache Avro
+
+function(resolve_avro_dependency)
+ prepare_fetchcontent()
+
+ set(AVRO_USE_BOOST
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_EXECUTABLES
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_TESTS
+ OFF
+ CACHE BOOL "" FORCE)
+
+ fetchcontent_declare(Avro
+ ${FC_DECLARE_COMMON_OPTIONS}
+ GIT_REPOSITORY https://github.com/apache/avro.git
+ GIT_TAG 1144cb7322bab4cd1c8bf330a9c504a0d4252b56
+ SOURCE_SUBDIR
+ lang/c++
+ FIND_PACKAGE_ARGS
+ NAMES
+ Avro
+ CONFIG)
Review Comment:
That would be great, I had some issue with `fmt::fmt-header-only` target, which interface link of avrocpp.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923076221
##########
.github/workflows/test.yml:
##########
@@ -80,6 +80,10 @@ jobs:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
+ - name: Install ZLIB
Review Comment:
Yes, from the cmake log, zlib is available on MacOS and Ubuntu runner.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1928502982
##########
ci/scripts/build_example.sh:
##########
@@ -25,10 +25,27 @@ build_dir=${1}/build
mkdir ${build_dir}
pushd ${build_dir}
-cmake \
- -DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX:-${ICEBERG_HOME}} \
- ${source_dir}
-cmake --build .
+is_windows() {
+ [[ "${OSTYPE}" == "msys" || "${OSTYPE}" == "win32" ]]
+}
+
+CMAKE_ARGS=(
+ "-DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX:-${ICEBERG_HOME}}"
+)
+
+if is_windows; then
+ CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake")
+ CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Release")
Review Comment:
I have filed https://github.com/apache/iceberg-cpp/issues/39.
> Also, if you still want to have debug builds, probably better to choose RelWithDebInfo rather than Release.
Good suggestion! I'll try it in a separate PR.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1922579415
##########
src/iceberg/CMakeLists.txt:
##########
@@ -17,11 +17,16 @@
set(ICEBERG_SOURCES demo_table.cc)
+set(ICEBERG_AVRO_STATIC_BUILD_LIBS)
+list(APPEND ICEBERG_AVRO_STATIC_BUILD_LIBS Avro::avro_static)
+
add_iceberg_lib(iceberg
SOURCES
${ICEBERG_SOURCES}
PRIVATE_INCLUDES
- ${ICEBERG_INCLUDES})
+ ${ICEBERG_INCLUDES}
+ STATIC_LINK_LIBS
Review Comment:
Should we follow `libiceberg-arrow` to prefer `libavrocpp_s` for static linking and `libavrocpp` for dynamic linking?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923724860
##########
src/iceberg/CMakeLists.txt:
##########
@@ -17,11 +17,16 @@
set(ICEBERG_SOURCES demo_table.cc)
+set(ICEBERG_AVRO_STATIC_BUILD_LIBS)
+list(APPEND ICEBERG_AVRO_STATIC_BUILD_LIBS Avro::avro_static)
Review Comment:
@wgtmac I have implemented as you suggested, using iceberg_arrow as a template, pls give it a look when you are available, 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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923610607
##########
.github/workflows/test.yml:
##########
@@ -80,6 +80,10 @@ jobs:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
+ - name: Install ZLIB
+ shell: cmd
+ run: |
+ powershell -Command "(Invoke-WebRequest -Uri https://git.io/JnHTY -OutFile install_zlib.bat)"; ./install_zlib.bat; del install_zlib.bat
Review Comment:
Let's go this way, good news is that github action windows runner has vcpkg installed [1]. I also changed the windows runner image to Windows2022, which resolved the `fatal error C1001: Internal compiler error`. We are using the newest ubuntu runner version gh-actions supported, so I guess we can use a slight newer version of windows(the newest is Windows2025).
[1] https://github.com/actions/runner-images/blob/main/images/windows/Windows2022-Readme.md
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923776627
##########
src/iceberg/CMakeLists.txt:
##########
@@ -17,11 +17,16 @@
set(ICEBERG_SOURCES demo_table.cc)
+set(ICEBERG_AVRO_STATIC_BUILD_LIBS)
+list(APPEND ICEBERG_AVRO_STATIC_BUILD_LIBS Avro::avro_static)
Review Comment:
Great job! Are all CIs green on your fork?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1925539978
##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -126,3 +126,68 @@ endfunction()
if(ICEBERG_ARROW)
resolve_arrow_dependency()
endif()
+
+# ----------------------------------------------------------------------
+# Apache Avro
+
+function(resolve_avro_dependency)
+ prepare_fetchcontent()
+
+ set(AVRO_USE_BOOST
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_EXECUTABLES
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_TESTS
+ OFF
+ CACHE BOOL "" FORCE)
+
+ fetchcontent_declare(Avro
+ ${FC_DECLARE_COMMON_OPTIONS}
+ GIT_REPOSITORY https://github.com/apache/avro.git
+ GIT_TAG 1144cb7322bab4cd1c8bf330a9c504a0d4252b56
+ SOURCE_SUBDIR
+ lang/c++
+ FIND_PACKAGE_ARGS
+ NAMES
+ Avro
+ CONFIG)
Review Comment:
I have created https://github.com/apache/avro/pull/3299
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "raulcd (via GitHub)" <gi...@apache.org>.
raulcd commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1928409957
##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -126,3 +126,77 @@ endfunction()
if(ICEBERG_ARROW)
resolve_arrow_dependency()
endif()
+
+# ----------------------------------------------------------------------
+# Apache Avro
+
+function(resolve_avro_dependency)
+ prepare_fetchcontent()
+
+ set(AVRO_USE_BOOST
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_EXECUTABLES
+ OFF
+ CACHE BOOL "" FORCE)
+
+ set(AVRO_BUILD_TESTS
+ OFF
+ CACHE BOOL "" FORCE)
+
+ fetchcontent_declare(Avro
+ ${FC_DECLARE_COMMON_OPTIONS}
+ # TODO: switch to upstream once the PR below is merged
+ # https://github.com/apache/avro/pull/3299
+ # Eventually, we should switch to Apache Avro 1.3.0.
+ GIT_REPOSITORY https://github.com/wgtmac/avro.git
Review Comment:
Great work @wgtmac
##########
ci/scripts/build_example.sh:
##########
@@ -25,10 +25,27 @@ build_dir=${1}/build
mkdir ${build_dir}
pushd ${build_dir}
-cmake \
- -DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX:-${ICEBERG_HOME}} \
- ${source_dir}
-cmake --build .
+is_windows() {
+ [[ "${OSTYPE}" == "msys" || "${OSTYPE}" == "win32" ]]
+}
+
+CMAKE_ARGS=(
+ "-DCMAKE_PREFIX_PATH=${CMAKE_INSTALL_PREFIX:-${ICEBERG_HOME}}"
+)
+
+if is_windows; then
+ CMAKE_ARGS+=("-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake")
+ CMAKE_ARGS+=("-DCMAKE_BUILD_TYPE=Release")
Review Comment:
From my understanding there was an issue with debug builds on windows based on the other PRs, right? Do we want to add a follow up issue to track building with debug on Windows?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923400782
##########
.github/workflows/test.yml:
##########
@@ -80,6 +80,10 @@ jobs:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
+ - name: Install ZLIB
+ shell: cmd
+ run: |
+ powershell -Command "(Invoke-WebRequest -Uri https://git.io/JnHTY -OutFile install_zlib.bat)"; ./install_zlib.bat; del install_zlib.bat
Review Comment:
What about using `vcpkg`? I believe we may need to add more libraries to the Windows CI. Using `vcpkg` may be easier. Below is the suggestion from deepseek.
```
name: CI
on: [push]
jobs:
build:
runs-on: windows-latest
steps:
- name: Checkout code
uses: actions/checkout@v2
- name: Set up vcpkg
run: |
git clone https://github.com/microsoft/vcpkg.git
.\vcpkg\bootstrap-vcpkg.bat
- name: Install zlib
run: .\vcpkg\vcpkg install zlib:x64-windows
- name: Build your project
run: |
mkdir build
cd build
cmake .. -DCMAKE_TOOLCHAIN_FILE=../vcpkg/scripts/buildsystems/vcpkg.cmake
cmake --build .
```
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1922569711
##########
.github/workflows/test.yml:
##########
@@ -80,6 +80,10 @@ jobs:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
+ - name: Install ZLIB
+ shell: cmd
+ run: |
+ powershell -Command "(Invoke-WebRequest -Uri https://git.io/JnHTY -OutFile install_zlib.bat)"; ./install_zlib.bat; del install_zlib.bat
Review Comment:
Is this script reliable?
Basically I did the following steps to install the zlib library on Windows:
```
git clone https://github.com/madler/zlib.git
cd zlib
cmake -S . -B build -DCMAKE_INSTALL_PREFIX=/tmp/zlib -DZLIB_BUILD_EXAMPLES=OFF
cmake --build build --config Release
cmake --install build
```
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#issuecomment-2599658206
@wgtmac Sadly I came across a Windows ci build issue, I don't have a Windows PC, it would be great if you could help to resolve this problem. The problem is two folds:
1. Windows 2019 does not have zlib installed, I installed it in a way that I'm not sure if it's the right way for gh actions
2. it has compile error on avro side, I searched the avro ci directory, I did not see any windows build example
The compile error is as below, see [1] for details, [2] says it is probably an optimization error, I could not go further because I don't have a windows PC ;(
```
D:\a\iceberg-cpp\iceberg-cpp\build\_deps\avro-src\lang\c++\include\avro\NodeImpl.hh(280): fatal error C1001: Internal compiler error. [D:\a\iceberg-cpp\iceberg-cpp\build\_deps\avro-build\avrocpp_s.vcxproj]
(compiler file 'D:\a\_work\1\s\src\vctools\Compiler\Utc\src\p2\main.c', line 213)
To work around this problem, try simplifying or changing the program near the locations listed above.
```
[1] https://github.com/zhjwpku/iceberg-cpp/actions/runs/12842821805/job/35814127788
[2] https://learn.microsoft.com/en-us/cpp/error-messages/compiler-errors-1/fatal-error-c1001?view=msvc-170
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1922562686
##########
.github/workflows/test.yml:
##########
@@ -80,6 +80,10 @@ jobs:
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
fetch-depth: 0
+ - name: Install ZLIB
Review Comment:
The CIs for MacOS and Linux do not require manual installation of ZLIB because it is already available?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add iceberg_avro interface [iceberg-cpp]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] add libavrocpp_s [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #34:
URL: https://github.com/apache/iceberg-cpp/pull/34#discussion_r1923082582
##########
src/iceberg/CMakeLists.txt:
##########
@@ -17,11 +17,16 @@
set(ICEBERG_SOURCES demo_table.cc)
+set(ICEBERG_AVRO_STATIC_BUILD_LIBS)
+list(APPEND ICEBERG_AVRO_STATIC_BUILD_LIBS Avro::avro_static)
Review Comment:
Reasonable, will work on this.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org