Skip to content

Commit 4efb9a1

Browse files
committed
Enable ODBC build on MSVC CI
Code Clean up and enable ODBC tests in CI Still need to modify ODBCUtilEnvironment if we decide to use it. Still need to add ODBC V2 support so a different env and conn is used for ODBC 2 tests. Draft enable ODBC global setup/teardown Run ODBC test once outside of test script If ODBC test is run using MinGW Shell, segfault occurs. I am not getting any seg fault on my local MSVC Windows when I run the tests without the bash script. But if Windows CI breaks from running the standalone exe then I will look into this Prepend vcpkg to search vcpkg before `<prefix>/lib/cmake` Since we are installing dependencies on vcpkg, if `lib/cmake` is searched first, then cmake will look into that directory and use the wrong paths. Convert VCPKG Windows path to MSYS path Set `VCPKG_ROOT` in test phase Fix ODBC dll name Use `arrow_flight_sql_odbc.dll` instead of `libarrow_flight_sql_odbc.dll` which is the naming convention used on MinGW Windows Link ODBC library on all platforms On my local MSVC Windows machine, Visual Studio is used to build without needing to link ` ${ODBCINST}` explicitly, its behavior might be different from Ninja which is what CI uses. `${ODBCINST}` is likely needed by Linux as well, so adding it for all platforms. Attempt to resolve `arrow-compute-grouper-benchmark` build issue I think `if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static")` might be more appropriate here, as I am seeing build issues due to both dynamic and static linking occurring. I see in apache@59903d0, this check is used for `arrow-filesystem-s3fs-benchmark` Disable `UNITY_BUILD` for ODBC test due to conflict on `sqlite_sql_info.cc` On some workflows, unity build is set to ON to make the build faster. This is an attempt to resolve the build issue. Remove debug messages Fix lint Add `sql_info_undef.h` to sqlite_sql_info.cc Undefine duplicates in SQLGetInfo Add `#pragma once` to odbc_test_suite.h To avoid redefinition error Attempt to resolve conflict with sql/types.h Attempt to include Arrow headers before ODBC headers to avoid conflict with arrow/flight/sql/types.h [apacheGH-48084] Replace boost::optional with std::optional Addresses comment apache#40939 (comment) Replace boost::optional with std::optional in ODBC codebase apache#48084 Fix lint Remove `BOOST_SOURCE=BUNDLED` to use vcpkg's dynamic link to boost Since we need to use link to boost dynamically, we need to remove the bundled boost library flag. Add debug messages. Remove `ARROW_BOOST_USE_SHARED = OFF` Since we have a release build, can enable boost as shared. With `ARROW_BOOST_USE_SHARED = OFF`, I am getting error ``` D:\a\arrow\arrow\build\cpp\vcpkg_installed\x64-windows\include\boost/filesystem/config.hpp(96): fatal error C1189: #error: Must not define both BOOST_FILESYSTEM_DYN_LINK and BOOST_FILESYSTEM_STATIC_LINK ``` Also increased time limit, since 2 hours don't seem to be enough to finish the vcpkg build Change to release build Getting ``` orc.lib(Exceptions.cc.obj) : error LNK2038: mismatch detected for '_ITERATOR_DEBUG_LEVEL': value '0' doesn't match value '2' in unity_2_cxx.cxx.obj orc.lib(Exceptions.cc.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MD_DynamicRelease' doesn't match value 'MDd_DynamicDebug' in unity_2_cxx.cxx.obj ``` when building with debug and presumably ARROW_ORC Set ARROW_BOOST_USE_SHARED to OFF `ARROW_BOOST_USE_SHARED` is restored to `OFF`, which according to Windows doc should help with the debug build now. Retore value of `ARROW_BUILD_BENCHMARKS`, though noting it is set to `OFF` in GLib MSVC workflow. Add VCPKG set up Borrowed from the Glib workflow Add `/EHsc` flag * Add back `CMAKE_CXX_STANDARD: "17"` Remove `CMAKE_CXX_STANDARD` 17 * reason: gtest can't be used with C++17. Arrow project doesn't support C++ 17 yet. Extend run-time to 2hr windows-mingw also has timeout of 2hr Empty commit to trigger CI Specify `VCPKG_BINARY_SOURCES` and `VCPKG_DEFAULT_TRIPLET` Specify VCPKG_TARGET_TRIPLET as x64 windows Set ARROW_DEPENDENCY_SOURCE to VCPKG To make it easier to manage dependencies Enable static build on MSVC `ARROW_BUILD_BENCHMARKS` prevents Flight from being built Remove `ARROW_BOOST_USE_SHARED` Having static vs. shared issue Enable Flight & Flight SQL for ODBC Enable ODBC build on MSVC CI Enable regular ctest tests
1 parent 46b033f commit 4efb9a1

30 files changed

+418
-75
lines changed

.github/workflows/cpp_windows.yml

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,18 @@ on:
3636
jobs:
3737
windows:
3838
runs-on: ${{ inputs.os }}
39-
timeout-minutes: 60
39+
timeout-minutes: 240
4040
env:
41-
ARROW_BOOST_USE_SHARED: OFF
4241
ARROW_BUILD_BENCHMARKS: ON
4342
ARROW_BUILD_SHARED: ON
44-
ARROW_BUILD_STATIC: OFF
43+
ARROW_BUILD_STATIC: ON
4544
ARROW_BUILD_TESTS: ON
45+
ARROW_BUILD_TYPE: release
4646
ARROW_DATASET: ON
47-
ARROW_FLIGHT: OFF
47+
ARROW_DEPENDENCY_SOURCE: VCPKG
48+
ARROW_FLIGHT: ON
49+
ARROW_FLIGHT_SQL: ON
50+
ARROW_FLIGHT_SQL_ODBC: ON
4851
ARROW_HDFS: ON
4952
ARROW_HOME: /usr
5053
ARROW_JEMALLOC: OFF
@@ -62,11 +65,12 @@ jobs:
6265
ARROW_WITH_SNAPPY: ON
6366
ARROW_WITH_ZLIB: ON
6467
ARROW_WITH_ZSTD: ON
65-
BOOST_SOURCE: BUNDLED
6668
CMAKE_CXX_STANDARD: "17"
6769
CMAKE_GENERATOR: Ninja
6870
CMAKE_INSTALL_PREFIX: /usr
6971
CMAKE_UNITY_BUILD: ON
72+
VCPKG_BINARY_SOURCES: 'clear;nuget,GitHub,readwrite'
73+
VCPKG_DEFAULT_TRIPLET: x64-windows
7074
steps:
7175
- name: Disable Crash Dialogs
7276
run: |
@@ -110,15 +114,53 @@ jobs:
110114
path: ${{ steps.ccache-info.outputs.cache-dir }}
111115
key: cpp-ccache-windows-${{ inputs.arch }}-${{ hashFiles('cpp/**') }}
112116
restore-keys: cpp-ccache-windows-${{ inputs.arch }}-
117+
- name: Checkout vcpkg
118+
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
119+
with:
120+
fetch-depth: 0
121+
path: vcpkg
122+
repository: microsoft/vcpkg
123+
- name: Bootstrap vcpkg
124+
run: |
125+
vcpkg\bootstrap-vcpkg.bat
126+
$VCPKG_ROOT = $(Resolve-Path -LiteralPath "vcpkg").ToString()
127+
Write-Output ${VCPKG_ROOT} | `
128+
Out-File -FilePath ${Env:GITHUB_PATH} -Encoding utf8 -Append
129+
Write-Output "VCPKG_ROOT=${VCPKG_ROOT}" | `
130+
Out-File -FilePath ${Env:GITHUB_ENV} -Encoding utf8 -Append
131+
- name: Setup NuGet credentials for vcpkg caching
132+
shell: bash
133+
run: |
134+
$(vcpkg fetch nuget | tail -n 1) \
135+
sources add \
136+
-source "https://nuget.pkg.github.com/$GITHUB_REPOSITORY_OWNER/index.json" \
137+
-storepasswordincleartext \
138+
-name "GitHub" \
139+
-username "$GITHUB_REPOSITORY_OWNER" \
140+
-password "${{ secrets.GITHUB_TOKEN }}"
141+
$(vcpkg fetch nuget | tail -n 1) \
142+
setapikey "${{ secrets.GITHUB_TOKEN }}" \
143+
-source "https://nuget.pkg.github.com/$GITHUB_REPOSITORY_OWNER/index.json"
113144
- name: Build
114145
shell: cmd
115146
run: |
147+
set VCPKG_ROOT_KEEP=%VCPKG_ROOT%
116148
call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" ${{ inputs.arch }}
149+
set VCPKG_ROOT=%VCPKG_ROOT_KEEP%
117150
bash -c "ci/scripts/cpp_build.sh $(pwd) $(pwd)/build"
151+
- name: Register Flight SQL ODBC Driver
152+
shell: cmd
153+
run: |
154+
call "cpp\src\arrow\flight\sql\odbc\tests\install_odbc.cmd" ${{ github.workspace }}\build\cpp\%ARROW_BUILD_TYPE%\arrow_flight_sql_odbc.dll
118155
- name: Test
119156
shell: cmd
120157
run: |
158+
set VCPKG_ROOT_KEEP=%VCPKG_ROOT%
121159
call "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" ${{ inputs.arch }}
122160
# For ORC
123161
set TZDIR=${{ steps.setup-msys2.outputs.msys2-location }}\usr\share\zoneinfo
162+
163+
# Convert VCPKG Windows path to MSYS path
164+
for /f "usebackq delims=" %%I in (`bash -c "cygpath -u \"$VCPKG_ROOT_KEEP\""` ) do set VCPKG_ROOT=%%I
165+
124166
bash -c "ci/scripts/cpp_test.sh $(pwd) $(pwd)/build"

ci/scripts/cpp_test.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,9 @@ if [ "${ARROW_USE_MESON:-OFF}" = "OFF" ] && \
144144
CMAKE_PREFIX_PATH+="/lib/cmake/"
145145
;;
146146
esac
147+
# Search vcpkg before <prefix>/lib/cmake.
147148
if [ -n "${VCPKG_ROOT}" ] && [ -n "${VCPKG_DEFAULT_TRIPLET}" ]; then
148-
CMAKE_PREFIX_PATH+=";${VCPKG_ROOT}/installed/${VCPKG_DEFAULT_TRIPLET}"
149+
CMAKE_PREFIX_PATH="${VCPKG_ROOT}/installed/${VCPKG_DEFAULT_TRIPLET};${CMAKE_PREFIX_PATH}"
149150
fi
150151
cmake \
151152
-S "${source_dir}/examples/minimal_build" \

cpp/cmake_modules/SetupCxxFlags.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ if(WIN32)
186186
#
187187
# ARROW-2986: Without /EHsc we get C4530 warning
188188
set(CXX_COMMON_FLAGS "/W3 /EHsc")
189+
string(APPEND CMAKE_CXX_FLAGS " /EHsc")
189190
endif()
190191

191192
# Disable C5105 (macro expansion producing 'defined' has undefined

cpp/src/arrow/compute/registry.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ class FunctionRegistry::FunctionRegistryImpl {
127127

128128
const Function* cast_function() { return cast_function_; }
129129

130+
void ClearFunctioRegistry() { name_to_function_.clear(); }
131+
130132
private:
131133
// must not acquire mutex
132134
Status CanAddFunctionName(const std::string& name, bool allow_overwrite) {
@@ -277,6 +279,8 @@ int FunctionRegistry::num_functions() const { return impl_->num_functions(); }
277279

278280
const Function* FunctionRegistry::cast_function() const { return impl_->cast_function(); }
279281

282+
void FunctionRegistry::ClearFunctioRegistry() { impl_->ClearFunctioRegistry(); }
283+
280284
namespace internal {
281285

282286
static std::unique_ptr<FunctionRegistry> CreateBuiltInRegistry() {

cpp/src/arrow/compute/registry.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@ class ARROW_EXPORT FunctionRegistry {
112112
/// Helpful for get cast function as needed.
113113
const Function* cast_function() const;
114114

115+
/// \brief Clear function registry
116+
///
117+
/// Helpful to avoid segfault from race condition. Call this function before DLL unload.
118+
void ClearFunctioRegistry();
119+
115120
private:
116121
FunctionRegistry();
117122

cpp/src/arrow/compute/row/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ arrow_install_all_headers("arrow/compute/row")
2222

2323
if(ARROW_BUILD_BENCHMARKS AND ARROW_COMPUTE)
2424
add_arrow_benchmark(grouper_benchmark PREFIX "arrow-compute")
25-
if(ARROW_BUILD_STATIC)
25+
if(ARROW_TEST_LINKAGE STREQUAL "static")
2626
target_link_libraries(arrow-compute-grouper-benchmark PUBLIC arrow_compute_static)
2727
else()
2828
target_link_libraries(arrow-compute-grouper-benchmark PUBLIC arrow_compute_shared)

cpp/src/arrow/flight/sql/example/sqlite_sql_info.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include "arrow/flight/sql/types.h"
2121
#include "arrow/util/config.h"
2222

23+
#include "arrow/flight/sql/sql_info_undef.h"
24+
2325
namespace arrow {
2426
namespace flight {
2527
namespace sql {

cpp/src/arrow/flight/sql/odbc/odbc_impl/CMakeLists.txt

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,9 @@ if(WIN32)
129129
system_dsn.h)
130130
endif()
131131

132-
target_link_libraries(arrow_odbc_spi_impl PUBLIC arrow_flight_sql_shared
133-
arrow_compute_shared Boost::locale)
134-
135-
# Link libraries on MINGW64 and macOS
136-
if(MINGW OR APPLE)
137-
target_link_libraries(arrow_odbc_spi_impl PUBLIC ${ODBCINST})
138-
endif()
132+
target_link_libraries(arrow_odbc_spi_impl
133+
PUBLIC arrow_flight_sql_shared arrow_compute_shared Boost::locale
134+
${ODBCINST})
139135

140136
set_target_properties(arrow_odbc_spi_impl
141137
PROPERTIES ARCHIVE_OUTPUT_DIRECTORY

cpp/src/arrow/flight/sql/odbc/odbc_impl/blocking_queue.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
#pragma once
1919

2020
#include <atomic>
21-
#include <boost/optional.hpp>
2221
#include <condition_variable>
2322
#include <mutex>
23+
#include <optional>
2424
#include <thread>
2525
#include <vector>
2626

@@ -43,7 +43,7 @@ class BlockingQueue {
4343
std::atomic<bool> closed_{false};
4444

4545
public:
46-
typedef std::function<boost::optional<T>(void)> Supplier;
46+
typedef std::function<std::optional<T>(void)> Supplier;
4747

4848
explicit BlockingQueue(size_t capacity) : capacity_(capacity), buffer_(capacity) {}
4949

@@ -58,7 +58,7 @@ class BlockingQueue {
5858

5959
// Only one thread at a time be notified and call supplier
6060
auto item = supplier();
61-
if (!item) break;
61+
if (!item.has_value()) break;
6262

6363
Push(*item);
6464
}

cpp/src/arrow/flight/sql/odbc/odbc_impl/flight_sql_auth_method.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "arrow/result.h"
2525
#include "arrow/status.h"
2626

27+
#include <optional>
2728
#include <utility>
2829

2930
namespace arrow::flight::sql::odbc {
@@ -63,7 +64,7 @@ class UserPasswordAuthMethod : public FlightSqlAuthMethod {
6364
void Authenticate(FlightSqlConnection& connection,
6465
FlightCallOptions& call_options) override {
6566
FlightCallOptions auth_call_options;
66-
const boost::optional<Connection::Attribute>& login_timeout =
67+
const std::optional<Connection::Attribute>& login_timeout =
6768
connection.GetAttribute(Connection::LOGIN_TIMEOUT);
6869
if (login_timeout && boost::get<uint32_t>(*login_timeout) > 0) {
6970
// ODBC's LOGIN_TIMEOUT attribute and FlightCallOptions.timeout use

0 commit comments

Comments
 (0)