Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable test suite of libarrow #1058

Merged
merged 11 commits into from
Nov 25, 2024
Prev Previous commit
Next Next commit
update fix for glibc vs tz issue
h-vetinari committed Nov 23, 2024
commit e5b91bf798a573aef13b23fec6438762c17d9977
4 changes: 2 additions & 2 deletions recipe/meta.yaml
Original file line number Diff line number Diff line change
@@ -21,8 +21,8 @@ source:
- patches/0001-GH-44455-C-Update-vendored-date-to-3.0.3.patch
# backport https://github.com/apache/arrow/pull/44507
- patches/0002-GH-44448-C-Add-support-for-overriding-grpc_cpp_plugi.patch
# fix for https://github.com/apache/arrow/issues/43808
- patches/0003-GH-43808-C-set-kStrptimeSupportsZone-to-false-for-ol.patch
# backport https://github.com/apache/arrow/pull/44621
- patches/0003-GH-43808-C-skip-0117-in-StrptimeZoneOffset-for-old-g.patch
# skip gcsfs tests due to missing `storage-testbench`
- patches/0004-disable-gcsfs_test.patch
# testing-submodules not part of release tarball
Original file line number Diff line number Diff line change
@@ -25,7 +25,7 @@ date to parse it.
create mode 100755 cpp/src/arrow/vendored/datetime/update.sh

diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat
index 08a052e82f..084117f387 100644
index 08a052e82..084117f38 100644
--- a/ci/appveyor-cpp-build.bat
+++ b/ci/appveyor-cpp-build.bat
@@ -139,7 +139,7 @@ set PARQUET_HOME=%CONDA_PREFIX%\Library
@@ -38,7 +38,7 @@ index 08a052e82f..084117f387 100644
tar --extract --file tzdata.tar.gz --directory %USERPROFILE%\Downloads\test\tzdata
curl https://raw.githubusercontent.com/unicode-org/cldr/master/common/supplemental/windowsZones.xml ^
diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt
index c911f0f4e9..5f6b568460 100644
index c911f0f4e..5f6b56846 100644
--- a/cpp/src/arrow/CMakeLists.txt
+++ b/cpp/src/arrow/CMakeLists.txt
@@ -460,7 +460,7 @@ endif()
@@ -61,7 +61,7 @@ index c911f0f4e9..5f6b568460 100644
arrow_add_object_library(ARROW_VENDORED ${ARROW_VENDORED_SRCS})
diff --git a/cpp/src/arrow/vendored/datetime.cpp b/cpp/src/arrow/vendored/datetime.cpp
new file mode 100644
index 0000000000..0f0bd12c7e
index 000000000..0f0bd12c7
--- /dev/null
+++ b/cpp/src/arrow/vendored/datetime.cpp
@@ -0,0 +1,19 @@
@@ -85,7 +85,7 @@ index 0000000000..0f0bd12c7e
+#include "datetime/visibility.h"
+#include "datetime/tz.cpp"
diff --git a/cpp/src/arrow/vendored/datetime.h b/cpp/src/arrow/vendored/datetime.h
index e437cdcbc2..aea31ebe77 100644
index e437cdcbc..aea31ebe7 100644
--- a/cpp/src/arrow/vendored/datetime.h
+++ b/cpp/src/arrow/vendored/datetime.h
@@ -17,10 +17,11 @@
@@ -104,7 +104,7 @@ index e437cdcbc2..aea31ebe77 100644
+# undef NOEXCEPT
#endif
diff --git a/cpp/src/arrow/vendored/datetime/README.md b/cpp/src/arrow/vendored/datetime/README.md
index 5a0993b7b4..89132d9cba 100644
index 5a0993b7b..89132d9cb 100644
--- a/cpp/src/arrow/vendored/datetime/README.md
+++ b/cpp/src/arrow/vendored/datetime/README.md
@@ -17,12 +17,16 @@ copies or substantial portions of the Software.
@@ -129,7 +129,7 @@ index 5a0993b7b4..89132d9cba 100644
+$ ./update.sh 3.0.3
+```
diff --git a/cpp/src/arrow/vendored/datetime/date.h b/cpp/src/arrow/vendored/datetime/date.h
index 75e2624296..c17d6f3f7a 100644
index 75e262429..c17d6f3f7 100644
--- a/cpp/src/arrow/vendored/datetime/date.h
+++ b/cpp/src/arrow/vendored/datetime/date.h
@@ -84,9 +84,7 @@
@@ -154,7 +154,7 @@ index 75e2624296..c17d6f3f7a 100644
#ifdef _MSC_VER
# pragma warning(pop)
diff --git a/cpp/src/arrow/vendored/datetime/ios.h b/cpp/src/arrow/vendored/datetime/ios.h
index acad28d13b..d018e799a8 100644
index acad28d13..d018e799a 100644
--- a/cpp/src/arrow/vendored/datetime/ios.h
+++ b/cpp/src/arrow/vendored/datetime/ios.h
@@ -32,9 +32,7 @@
@@ -179,7 +179,7 @@ index acad28d13b..d018e799a8 100644
# endif // TARGET_OS_IPHONE
#else // !__APPLE__
diff --git a/cpp/src/arrow/vendored/datetime/ios.mm b/cpp/src/arrow/vendored/datetime/ios.mm
index 22b7ce6c30..70ba2adf0e 100644
index 22b7ce6c3..70ba2adf0 100644
--- a/cpp/src/arrow/vendored/datetime/ios.mm
+++ b/cpp/src/arrow/vendored/datetime/ios.mm
@@ -47,9 +47,7 @@
@@ -203,7 +203,7 @@ index 22b7ce6c30..70ba2adf0e 100644

#endif // TARGET_OS_IPHONE
diff --git a/cpp/src/arrow/vendored/datetime/tz.cpp b/cpp/src/arrow/vendored/datetime/tz.cpp
index 44c627775f..2cf6c62a84 100644
index 44c627775..2cf6c62a8 100644
--- a/cpp/src/arrow/vendored/datetime/tz.cpp
+++ b/cpp/src/arrow/vendored/datetime/tz.cpp
@@ -30,10 +30,6 @@
@@ -1009,7 +1009,7 @@ index 44c627775f..2cf6c62a84 100644
#if defined(__GNUC__) && __GNUC__ < 5
# pragma GCC diagnostic pop
diff --git a/cpp/src/arrow/vendored/datetime/tz.h b/cpp/src/arrow/vendored/datetime/tz.h
index df6d1a851a..61ab3df106 100644
index df6d1a851..61ab3df10 100644
--- a/cpp/src/arrow/vendored/datetime/tz.h
+++ b/cpp/src/arrow/vendored/datetime/tz.h
@@ -43,19 +43,13 @@
@@ -1124,7 +1124,7 @@ index df6d1a851a..61ab3df106 100644

#endif // TZ_H
diff --git a/cpp/src/arrow/vendored/datetime/tz_private.h b/cpp/src/arrow/vendored/datetime/tz_private.h
index a6bb8fd30a..1d7f858971 100644
index a6bb8fd30..1d7f85897 100644
--- a/cpp/src/arrow/vendored/datetime/tz_private.h
+++ b/cpp/src/arrow/vendored/datetime/tz_private.h
@@ -34,9 +34,7 @@
@@ -1150,7 +1150,7 @@ index a6bb8fd30a..1d7f858971 100644
#include "tz.h"
diff --git a/cpp/src/arrow/vendored/datetime/update.sh b/cpp/src/arrow/vendored/datetime/update.sh
new file mode 100755
index 0000000000..b4580c0426
index 000000000..b4580c042
--- /dev/null
+++ b/cpp/src/arrow/vendored/datetime/update.sh
@@ -0,0 +1,53 @@
@@ -1208,7 +1208,7 @@ index 0000000000..b4580c0426
+rm *.bak
+popd
diff --git a/cpp/src/arrow/vendored/datetime/visibility.h b/cpp/src/arrow/vendored/datetime/visibility.h
index ae031238d8..780c00d70b 100644
index ae031238d..780c00d70 100644
--- a/cpp/src/arrow/vendored/datetime/visibility.h
+++ b/cpp/src/arrow/vendored/datetime/visibility.h
@@ -17,10 +17,14 @@
@@ -1229,7 +1229,7 @@ index ae031238d8..780c00d70b 100644
+# define DATE_USE_DLL
#endif
diff --git a/cpp/src/gandiva/precompiled/CMakeLists.txt b/cpp/src/gandiva/precompiled/CMakeLists.txt
index c2bc7fc027..e1427e25fb 100644
index c2bc7fc02..e1427e25f 100644
--- a/cpp/src/gandiva/precompiled/CMakeLists.txt
+++ b/cpp/src/gandiva/precompiled/CMakeLists.txt
@@ -63,7 +63,7 @@ add_gandiva_test(precompiled-test
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ Subject: [PATCH 2/4] GH-44448: [C++] Add support for overriding
2 files changed, 13 insertions(+)

diff --git a/cpp/cmake_modules/DefineOptions.cmake b/cpp/cmake_modules/DefineOptions.cmake
index d823444cf7..a7bf9e59f8 100644
index d823444cf..a7bf9e59f 100644
--- a/cpp/cmake_modules/DefineOptions.cmake
+++ b/cpp/cmake_modules/DefineOptions.cmake
@@ -640,6 +640,11 @@ Always OFF if building binaries" OFF)
@@ -26,7 +26,7 @@ index d823444cf7..a7bf9e59f8 100644
set_option_category("Advanced developer")

diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index db151b4e0f..0b215b5b25 100644
index db151b4e0..0b215b5b2 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -4223,6 +4223,14 @@ if(ARROW_WITH_GRPC)

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
From acf75c664b813f49eff275e74c0026af5ba90e84 Mon Sep 17 00:00:00 2001
From: h-vetinari <[email protected]>
Date: Fri, 15 Nov 2024 19:38:40 +1100
Subject: [PATCH 3/4] GH-43808: [C++] skip `-0117` in StrptimeZoneOffset for
old glibc (#44621)

### Rationale for this change

Enable tests for libarrow in conda-forge: https://github.com/apache/arrow/issues/35587

### What changes are included in this PR?

old glibc does not actually support timezones like `-0117` (used in `StrptimeZoneOffset` test). The exact lower bound for glibc is hard for me to determine; I know that it passes with 2.28 and that it fails with 2.17. Anything in between is an open question. I went with the conservative option here.

### Are these changes tested?

Tested in https://github.com/conda-forge/arrow-cpp-feedstock/pull/1058

### Are there any user-facing changes?

* GitHub Issue: #43808

Lead-authored-by: H. Vetinari <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/arrow/util/value_parsing_test.cc | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/cpp/src/arrow/util/value_parsing_test.cc b/cpp/src/arrow/util/value_parsing_test.cc
index 7cd1ab1e2..a833d266a 100644
--- a/cpp/src/arrow/util/value_parsing_test.cc
+++ b/cpp/src/arrow/util/value_parsing_test.cc
@@ -838,12 +838,25 @@ TEST(TimestampParser, StrptimeZoneOffset) {
std::string format = "%Y-%d-%m %H:%M:%S%z";
auto parser = TimestampParser::MakeStrptime(format);

+ std::vector<std::string> values = {
+ "2018-01-01 00:00:00+0000",
+ "2018-01-01 00:00:00+0100",
+#if defined(__GLIBC__) && defined(__GLIBC_MINOR__)
+// glibc < 2.28 doesn't support "-0117" timezone offset.
+// See also: https://github.com/apache/arrow/issues/43808
+# if ((__GLIBC__ == 2) && (__GLIBC_MINOR__ >= 28)) || (__GLIBC__ >= 3)
+ "2018-01-01 00:00:00-0117",
+# endif
+#else
+ "2018-01-01 00:00:00-0117",
+#endif
+ "2018-01-01 00:00:00+0130"
+ };
+
// N.B. GNU %z supports ISO8601 format while BSD %z supports only
// +HHMM or -HHMM and POSIX doesn't appear to define %z at all
for (auto unit : TimeUnit::values()) {
- for (const std::string value :
- {"2018-01-01 00:00:00+0000", "2018-01-01 00:00:00+0100",
- "2018-01-01 00:00:00+0130", "2018-01-01 00:00:00-0117"}) {
+ for (const std::string& value : values) {
SCOPED_TRACE(value);
int64_t converted = 0;
int64_t expected = 0;
4 changes: 2 additions & 2 deletions recipe/patches/0004-disable-gcsfs_test.patch
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
From 43e073eb4821d1e940915add71553c84028f3358 Mon Sep 17 00:00:00 2001
From a828c478993de3369729011e9f433953ea823622 Mon Sep 17 00:00:00 2001
From: "H. Vetinari" <[email protected]>
Date: Sat, 2 Nov 2024 15:41:34 +1100
Subject: [PATCH 4/4] disable gcsfs_test
@@ -11,7 +11,7 @@ hard to fit this into our migration patterns
1 file changed, 8 deletions(-)

diff --git a/cpp/src/arrow/filesystem/CMakeLists.txt b/cpp/src/arrow/filesystem/CMakeLists.txt
index 7afdf566f2..b22a48a0c9 100644
index 7afdf566f..b22a48a0c 100644
--- a/cpp/src/arrow/filesystem/CMakeLists.txt
+++ b/cpp/src/arrow/filesystem/CMakeLists.txt
@@ -42,14 +42,6 @@ if(ARROW_BUILD_BENCHMARKS)