Copilot commented on code in PR #51315:
URL: https://github.com/apache/arrow/pull/51315#discussion_r3996755968
##########
cpp/src/arrow/util/config.h.cmake:
##########
@@ -54,6 +54,7 @@
#cmakedefine ARROW_HDFS
#cmakedefine ARROW_S3
#cmakedefine ARROW_USE_GLOG
+#cmakedefine01 ARROW_USE_STD_CHRONO
Review Comment:
When Arrow is added as a CMake subproject, `ARROW_DEFINE_OPTIONS` defaults
to OFF, so `ARROW_USE_STD_CHRONO` is undefined and `CheckStdChrono.cmake`
intentionally skips resolution. However, `#cmakedefine01` still generates
`#define ARROW_USE_STD_CHRONO 0`, preventing `chrono_internal.h` from using its
existing Windows feature-detection fallback and changing those builds from the
previous std::chrono backend to vendored datetime. Preserve the undefined state
for an unset option (while retaining `0` for an explicit OFF) or resolve the
subproject default explicitly.
##########
cpp/src/arrow/config.cc:
##########
@@ -91,7 +103,11 @@ RuntimeInfo GetRuntimeInfo() {
Status Initialize(const GlobalOptions& options) noexcept {
ARROW_SUPPRESS_DEPRECATION_WARNING
if (options.timezone_db_path.has_value()) {
-#if !USE_OS_TZDB
+#if defined(ARROW_USE_STD_CHRONO) && ARROW_USE_STD_CHRONO
+ return Status::Invalid(
+ "Arrow was built with C++20 std::chrono and uses the OS timezone
database, "
+ "so a downloaded database cannot be provided at runtime.");
Review Comment:
This new error path is also reached by the R package's startup
`configure_tzdb()`, which unconditionally calls
`set_timezone_database(tzdb::tzdb_path("text"))` when the `tzdb` package is
installed. In a std::chrono build the OS tzdb is the intended backend, so R
will report a false startup failure that timezones are unavailable; make the R
initialization backend-aware or otherwise avoid passing a vendored database
path for this backend.
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -549,9 +549,16 @@ set(ARROW_VENDORED_SRCS
vendored/uriparser/UriRecompose.c
vendored/uriparser/UriResolve.c
vendored/uriparser/UriShorten.c)
-if(APPLE)
+if(APPLE AND NOT ARROW_USE_STD_CHRONO)
list(APPEND ARROW_VENDORED_SRCS vendored/datetime/ios.mm)
endif()
+if(ARROW_USE_STD_CHRONO)
Review Comment:
When `ARROW_GANDIVA` is enabled, `gandiva/cast_time.cc` and
`gdv_function_stubs.cc` still call
`arrow_vendored::date::locate_zone`/`zoned_time`. Those definitions come from
`vendored/datetime.cpp`, and Gandiva links `arrow_shared`/`arrow_static`;
removing this translation unit here leaves undefined symbols (or a runtime load
failure). Keep the TU for Gandiva or migrate those call sites to the standard
backend.
--
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]
