Skip to content

Commit

Permalink
GH-41134: [GLib] Support building arrow-glib with MSVC (#41599)
Browse files Browse the repository at this point in the history
### Rationale for this change

Allow Windows users to more easily build the GLib libraries.

### What changes are included in this PR?

* Minor fixes to allow building with MSVC:
  * Changes some uses of variable length arrays to `std::vector`, because MSVC doesn't support variable length arrays
  * Moves some function definitions that use C++ types outside of `G_BEGIN_DECLS`/`G_END_DECLS` blocks (which expand to `extern C { }`), because this caused MSVC to error
* Fix libraries not having any exported symbols with MSVC, which defaults to hiding symbols
  * Add `visibility.h` which defines a new `GARROW_EXTERN` macro that adds `dllimport` or `dllexport` attributes when using MSVC.
  * Include the `GARROW_EXTERN` macro in the definitions of the `GARROW_AVAILABLE_IN_*` macros.
* Add a new CI job that builds the GLib libraries with MSVC on Windows, using vcpkg to install pkgconfig and glib.
* For now only `arrow-glib` is built, I can follow up with the other libraries after this PR. That will require introducing new per-library version macros.

### Are these changes tested?

The build will be tested in CI but I've only done some quick manual tests that the built library works correctly, I haven't got the ruby tests running against the build yet.

### Are there any user-facing changes?

No? Eventually some documentation should be updated when all the GLib libraries can be built with MSVC though
* GitHub Issue: #41134

Lead-authored-by: Adam Reeve <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
adamreeve and kou authored May 24, 2024
1 parent 522b097 commit 799021a
Show file tree
Hide file tree
Showing 82 changed files with 1,361 additions and 98 deletions.
96 changes: 94 additions & 2 deletions .github/workflows/ruby.yml
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ jobs:
shell: bash
run: ci/scripts/ruby_test.sh $(pwd) $(pwd)/build

windows:
windows-mingw:
name: AMD64 Windows MinGW ${{ matrix.mingw-n-bits }} GLib & Ruby
runs-on: windows-2019
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
Expand Down Expand Up @@ -267,7 +267,6 @@ jobs:
ridk exec bash ci\scripts\cpp_build.sh "${source_dir}" "${build_dir}"
- name: Build GLib
run: |
$Env:CMAKE_BUILD_PARALLEL_LEVEL = $Env:NUMBER_OF_PROCESSORS
$source_dir = "$(ridk exec cygpath --unix "$(Get-Location)")"
$build_dir = "$(ridk exec cygpath --unix "$(Get-Location)\build")"
$ErrorActionPreference = "Continue"
Expand Down Expand Up @@ -305,3 +304,96 @@ jobs:
$Env:MAKE = "ridk exec make"
$ErrorActionPreference = "Continue"
rake -f ruby\Rakefile
windows-msvc:
name: AMD64 Windows MSVC GLib
runs-on: windows-2019
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 90
strategy:
fail-fast: false
env:
ARROW_BOOST_USE_SHARED: OFF
ARROW_BUILD_BENCHMARKS: OFF
ARROW_BUILD_SHARED: ON
ARROW_BUILD_STATIC: OFF
ARROW_BUILD_TESTS: OFF
ARROW_ACERO: ON
ARROW_DATASET: ON
ARROW_FLIGHT: OFF
ARROW_FLIGHT_SQL: OFF
ARROW_GANDIVA: OFF
ARROW_HDFS: OFF
ARROW_HOME: "${{ github.workspace }}/dist"
ARROW_JEMALLOC: OFF
ARROW_MIMALLOC: ON
ARROW_ORC: OFF
ARROW_PARQUET: ON
ARROW_SUBSTRAIT: OFF
ARROW_USE_GLOG: OFF
ARROW_VERBOSE_THIRDPARTY_BUILD: OFF
ARROW_WITH_BROTLI: OFF
ARROW_WITH_BZ2: OFF
ARROW_WITH_LZ4: OFF
ARROW_WITH_OPENTELEMETRY: OFF
ARROW_WITH_SNAPPY: ON
ARROW_WITH_ZLIB: OFF
ARROW_WITH_ZSTD: ON
BOOST_SOURCE: BUNDLED
CMAKE_CXX_STANDARD: "17"
CMAKE_GENERATOR: Ninja
CMAKE_INSTALL_PREFIX: "${{ github.workspace }}/dist"
CMAKE_UNITY_BUILD: ON
steps:
- name: Disable Crash Dialogs
run: |
reg add `
"HKCU\SOFTWARE\Microsoft\Windows\Windows Error Reporting" `
/v DontShowUI `
/t REG_DWORD `
/d 1 `
/f
- name: Checkout Arrow
uses: actions/checkout@v4
with:
fetch-depth: 0
submodules: recursive
- name: Install vcpkg
shell: bash
run: |
ci/scripts/install_vcpkg.sh ./vcpkg
- name: Install meson
run: |
python -m pip install meson
- name: Install ccache
shell: bash
run: |
ci/scripts/install_ccache.sh 4.6.3 /usr
- name: Setup ccache
shell: bash
run: |
ci/scripts/ccache_setup.sh
- name: ccache info
id: ccache-info
shell: bash
run: |
echo "cache-dir=$(ccache --get-config cache_dir)" >> $GITHUB_OUTPUT
- name: Cache ccache
uses: actions/cache@v4
with:
path: ${{ steps.ccache-info.outputs.cache-dir }}
key: glib-ccache-msvc-${{ env.CACHE_VERSION }}-${{ hashFiles('cpp/**') }}
restore-keys: glib-ccache-msvc-${{ env.CACHE_VERSION }}-
env:
# We can invalidate the current cache by updating this.
CACHE_VERSION: "2024-05-09"
- name: Build C++
shell: cmd
run: |
call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
bash -c "ci/scripts/cpp_build.sh $(pwd) $(pwd)/build"
- name: Build GLib
shell: cmd
run: |
call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" x64
bash -c "VCPKG_ROOT=\"$(pwd)/vcpkg\" ci/scripts/c_glib_build.sh $(pwd) $(pwd)/build"
1 change: 1 addition & 0 deletions c_glib/arrow-cuda-glib/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ libarrow_cuda_glib = library('arrow-cuda-glib',
dependencies: dependencies,
implicit_include_directories: false,
include_directories: base_include_directories,
cpp_args: ['-DGARROW_CUDA_COMPILATION'],
soversion: so_version,
version: library_version)
arrow_cuda_glib = declare_dependency(link_with: libarrow_cuda_glib,
Expand Down
2 changes: 2 additions & 0 deletions c_glib/arrow-cuda-glib/version.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,6 @@
# define GARROW_CUDA_VERSION_MAX_ALLOWED GARROW_VERSION_MAX_ALLOWED
#endif

@VISIBILITY_MACROS@

@AVAILABILITY_MACROS@
1 change: 1 addition & 0 deletions c_glib/arrow-dataset-glib/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ libarrow_dataset_glib = library('arrow-dataset-glib',
dependencies: dependencies,
implicit_include_directories: false,
include_directories: base_include_directories,
cpp_args: ['-DGADATASET_COMPILATION'],
soversion: so_version,
version: library_version)
arrow_dataset_glib = declare_dependency(link_with: libarrow_dataset_glib,
Expand Down
2 changes: 2 additions & 0 deletions c_glib/arrow-dataset-glib/version.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,6 @@
# define GADATASET_VERSION_MAX_ALLOWED GARROW_VERSION_MAX_ALLOWED
#endif

@VISIBILITY_MACROS@

@AVAILABILITY_MACROS@
6 changes: 6 additions & 0 deletions c_glib/arrow-flight-glib/client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,23 @@

#include <arrow-flight-glib/client.h>

GAFLIGHT_EXTERN
GAFlightStreamReader *
gaflight_stream_reader_new_raw(arrow::flight::FlightStreamReader *flight_reader,
gboolean is_owner);

GAFLIGHT_EXTERN
arrow::flight::FlightCallOptions *
gaflight_call_options_get_raw(GAFlightCallOptions *options);

GAFLIGHT_EXTERN
arrow::flight::FlightClientOptions *
gaflight_client_options_get_raw(GAFlightClientOptions *options);

GAFLIGHT_EXTERN
std::shared_ptr<arrow::flight::FlightClient>
gaflight_client_get_raw(GAFlightClient *client);

GAFLIGHT_EXTERN
GAFlightClient *
gaflight_client_new_raw(std::shared_ptr<arrow::flight::FlightClient> *flight_client);
20 changes: 20 additions & 0 deletions c_glib/arrow-flight-glib/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,39 +23,59 @@

#include <arrow-flight-glib/common.h>

GAFLIGHT_EXTERN
GAFlightCriteria *
gaflight_criteria_new_raw(const arrow::flight::Criteria *flight_criteria);

GAFLIGHT_EXTERN
arrow::flight::Criteria *
gaflight_criteria_get_raw(GAFlightCriteria *criteria);

GAFLIGHT_EXTERN
arrow::flight::Location *
gaflight_location_get_raw(GAFlightLocation *location);

GAFLIGHT_EXTERN
GAFlightDescriptor *
gaflight_descriptor_new_raw(const arrow::flight::FlightDescriptor *flight_descriptor);

GAFLIGHT_EXTERN
arrow::flight::FlightDescriptor *
gaflight_descriptor_get_raw(GAFlightDescriptor *descriptor);

GAFLIGHT_EXTERN
GAFlightTicket *
gaflight_ticket_new_raw(const arrow::flight::Ticket *flight_ticket);

GAFLIGHT_EXTERN
arrow::flight::Ticket *
gaflight_ticket_get_raw(GAFlightTicket *ticket);

GAFLIGHT_EXTERN
GAFlightEndpoint *
gaflight_endpoint_new_raw(const arrow::flight::FlightEndpoint *flight_endpoint,
GAFlightTicket *ticket);

GAFLIGHT_EXTERN
arrow::flight::FlightEndpoint *
gaflight_endpoint_get_raw(GAFlightEndpoint *endpoint);

GAFLIGHT_EXTERN
GAFlightInfo *
gaflight_info_new_raw(arrow::flight::FlightInfo *flight_info);

GAFLIGHT_EXTERN
arrow::flight::FlightInfo *
gaflight_info_get_raw(GAFlightInfo *info);

GAFLIGHT_EXTERN
GAFlightStreamChunk *
gaflight_stream_chunk_new_raw(arrow::flight::FlightStreamChunk *flight_chunk);

GAFLIGHT_EXTERN
arrow::flight::FlightStreamChunk *
gaflight_stream_chunk_get_raw(GAFlightStreamChunk *chunk);

GAFLIGHT_EXTERN
arrow::flight::MetadataRecordBatchReader *
gaflight_record_batch_reader_get_raw(GAFlightRecordBatchReader *reader);
1 change: 1 addition & 0 deletions c_glib/arrow-flight-glib/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ libarrow_flight_glib = library('arrow-flight-glib',
dependencies: dependencies,
implicit_include_directories: false,
include_directories: base_include_directories,
cpp_args: ['-DGAFLIGHT_COMPILATION'],
soversion: so_version,
version: library_version)
arrow_flight_glib = declare_dependency(link_with: libarrow_flight_glib,
Expand Down
16 changes: 16 additions & 0 deletions c_glib/arrow-flight-glib/server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,34 +23,49 @@

#include <arrow-flight-glib/server.h>

GAFLIGHT_EXTERN
arrow::flight::FlightDataStream *
gaflight_data_stream_get_raw(GAFlightDataStream *stream);

GAFLIGHT_EXTERN
GAFlightMessageReader *
gaflight_message_reader_new_raw(arrow::flight::FlightMessageReader *flight_reader,
gboolean is_owner);

GAFLIGHT_EXTERN
arrow::flight::FlightMessageReader *
gaflight_message_reader_get_raw(GAFlightMessageReader *reader);

GAFLIGHT_EXTERN
GAFlightServerCallContext *
gaflight_server_call_context_new_raw(
const arrow::flight::ServerCallContext *flight_call_context);

GAFLIGHT_EXTERN
const arrow::flight::ServerCallContext *
gaflight_server_call_context_get_raw(GAFlightServerCallContext *call_context);

GAFLIGHT_EXTERN
GAFlightServerAuthSender *
gaflight_server_auth_sender_new_raw(arrow::flight::ServerAuthSender *flight_sender);

GAFLIGHT_EXTERN
arrow::flight::ServerAuthSender *
gaflight_server_auth_sender_get_raw(GAFlightServerAuthSender *sender);

GAFLIGHT_EXTERN
GAFlightServerAuthReader *
gaflight_server_auth_reader_new_raw(arrow::flight::ServerAuthReader *flight_reader);

GAFLIGHT_EXTERN
arrow::flight::ServerAuthReader *
gaflight_server_auth_reader_get_raw(GAFlightServerAuthReader *reader);

GAFLIGHT_EXTERN
std::shared_ptr<arrow::flight::ServerAuthHandler>
gaflight_server_auth_handler_get_raw(GAFlightServerAuthHandler *handler);

GAFLIGHT_EXTERN
arrow::flight::FlightServerOptions *
gaflight_server_options_get_raw(GAFlightServerOptions *options);

Expand All @@ -61,6 +76,7 @@ struct _GAFlightServableInterface
arrow::flight::FlightServerBase *(*get_raw)(GAFlightServable *servable);
};

GAFLIGHT_EXTERN
arrow::flight::FlightServerBase *
gaflight_servable_get_raw(GAFlightServable *servable);

Expand Down
2 changes: 2 additions & 0 deletions c_glib/arrow-flight-glib/version.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,6 @@
# define GAFLIGHT_VERSION_MAX_ALLOWED GARROW_VERSION_MAX_ALLOWED
#endif

@VISIBILITY_MACROS@

@AVAILABILITY_MACROS@
1 change: 1 addition & 0 deletions c_glib/arrow-flight-sql-glib/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ libarrow_flight_sql_glib = library('arrow-flight-sql-glib',
dependencies: dependencies,
implicit_include_directories: false,
include_directories: base_include_directories,
cpp_args: ['-DGAFLIGHTSQL_COMPILATION'],
soversion: so_version,
version: library_version)
arrow_flight_sql_glib = \
Expand Down
2 changes: 2 additions & 0 deletions c_glib/arrow-flight-sql-glib/version.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,6 @@
# define GAFLIGHTSQL_VERSION_MAX_ALLOWED GARROW_VERSION_MAX_ALLOWED
#endif

@VISIBILITY_MACROS@

@AVAILABILITY_MACROS@
10 changes: 5 additions & 5 deletions c_glib/arrow-glib/array-builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ garrow_array_builder_append_values(GArrowArrayBuilder *builder,
if (n_remains > 0) {
++n_loops;
}
std::vector<uint8_t> data(value_size * chunk_size);
for (gint64 i = 0; i < n_loops; ++i) {
uint8_t data[value_size * chunk_size];
uint8_t *valid_bytes = nullptr;
uint8_t valid_bytes_buffer[chunk_size];
if (is_valids_length > 0) {
Expand All @@ -255,7 +255,7 @@ garrow_array_builder_append_values(GArrowArrayBuilder *builder,
value = values[offset + j];
}
if (value) {
get_value_function(data + (value_size * j), value, value_size);
get_value_function(data.data() + (value_size * j), value, value_size);
} else {
is_valid = false;
if (!valid_bytes) {
Expand All @@ -267,7 +267,7 @@ garrow_array_builder_append_values(GArrowArrayBuilder *builder,
valid_bytes_buffer[j] = is_valid;
}
}
auto status = arrow_builder->AppendValues(data, n_values, valid_bytes);
auto status = arrow_builder->AppendValues(data.data(), n_values, valid_bytes);
if (!garrow_error_check(error, status, context)) {
return FALSE;
}
Expand Down Expand Up @@ -1035,13 +1035,13 @@ garrow_boolean_array_builder_append_values(GArrowBooleanArrayBuilder *builder,
gint64 is_valids_length,
GError **error)
{
guint8 arrow_values[values_length];
std::vector<guint8> arrow_values(values_length);
for (gint64 i = 0; i < values_length; ++i) {
arrow_values[i] = values[i];
}
return garrow_array_builder_append_values<arrow::BooleanBuilder>(
GARROW_ARRAY_BUILDER(builder),
arrow_values,
arrow_values.data(),
values_length,
is_valids,
is_valids_length,
Expand Down
Loading

0 comments on commit 799021a

Please sign in to comment.