Copilot commented on code in PR #1029:
URL: https://github.com/apache/iceberg-cpp/pull/1029#discussion_r4297304310
##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -233,14 +233,33 @@ function(resolve_arrow_dependency)
"https://archive.apache.org/dist/thrift/0.22.0/thrift-0.22.0.tar.gz";)
endif()
+ # FetchContent sets arrow_SOURCE_DIR to the tarball root (SOURCE_SUBDIR is
cpp)
+ # and older CMake (e.g. 3.28) does not override it in project(arrow), so
Arrow
+ # looks for thirdparty/versions.txt outside cpp/. Patch Arrow to use
+ # ARROW_SOURCE_DIR instead. See
https://github.com/apache/iceberg-cpp/issues/1028.
+ # TODO: remove once the upstream Arrow fix is released.
+ set(ARROW_PATCH_COMMAND)
+ find_program(PATCH patch)
+ if(PATCH)
+ set(ARROW_PATCH_COMMAND ${PATCH} -p1 -i)
+ else()
+ find_program(GIT git)
+ if(GIT)
+ set(ARROW_PATCH_COMMAND ${GIT} apply)
Review Comment:
When `patch` is unavailable and Arrow is extracted under the iceberg-cpp Git
checkout (for example, `build/_deps/arrow-src`), `git apply` discovers the
enclosing repository. Its `cpp/...` patch paths are outside the current
repository subdirectory, so Git can skip them and report success without
modifying Arrow. The older-CMake failure then remains. Make the fallback
operate independently of the enclosing Git repository, or replace it with a
CMake patch helper.
##########
cmake_modules/IcebergThirdpartyToolchain.cmake:
##########
@@ -233,14 +233,33 @@ function(resolve_arrow_dependency)
"https://archive.apache.org/dist/thrift/0.22.0/thrift-0.22.0.tar.gz";)
endif()
+ # FetchContent sets arrow_SOURCE_DIR to the tarball root (SOURCE_SUBDIR is
cpp)
+ # and older CMake (e.g. 3.28) does not override it in project(arrow), so
Arrow
+ # looks for thirdparty/versions.txt outside cpp/. Patch Arrow to use
+ # ARROW_SOURCE_DIR instead. See
https://github.com/apache/iceberg-cpp/issues/1028.
+ # TODO: remove once the upstream Arrow fix is released.
+ set(ARROW_PATCH_COMMAND)
+ find_program(PATCH patch)
+ if(PATCH)
+ set(ARROW_PATCH_COMMAND ${PATCH} -p1 -i)
+ else()
+ find_program(GIT git)
+ if(GIT)
+ set(ARROW_PATCH_COMMAND ${GIT} apply)
+ endif()
+ endif()
+ if(NOT ARROW_PATCH_COMMAND)
+ message(FATAL_ERROR "patch or git is required to build the vendored Arrow")
+ endif()
Review Comment:
This check runs before `FetchContent_MakeAvailable(Arrow)` can select an
installed Arrow through `FIND_PACKAGE_ARGS CONFIG`. Builds using installed
Arrow therefore fail when neither `patch` nor `git` is available, even though
no sources need patching. Move tool discovery and validation into a patch-time
helper so these tools are required only when vendoring Arrow.
--
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]
